Skip to content

Make InetUtils optional for the simple discovery client - #1758

Open
Akhil-1527 wants to merge 1 commit into
spring-cloud:5.0.xfrom
Akhil-1527:fix-util-disabled-startup
Open

Akhil-1527 wants to merge 1 commit into
spring-cloud:5.0.xfrom
Akhil-1527:fix-util-disabled-startup

Conversation

@Akhil-1527

Copy link
Copy Markdown
Contributor

Setting spring.cloud.util.enabled=false stops the application from starting. The blocking and reactive simple discovery client configurations take InetUtils in their constructors, and that bean only exists while the util auto-configuration is enabled.

Both configurations now take an ObjectProvider<InetUtils> and use it only when the bean is available, to set the local instance host. Without it, the local instance keeps the host bound from spring.cloud.discovery.client.simple.local.host and still gets the web server port.

UtilDisabledSimpleDiscoveryPropertiesAutoConfigurationTests starts a web application with util disabled and checks the blocking and reactive local instances. It fails without the change.

Fixes #631

Setting spring.cloud.util.enabled=false stopped the application from
starting, because the blocking and reactive simple discovery client
configurations required an InetUtils bean. They now use it only when
it is available, to set the local instance host.

Fixes spring-cloudgh-631

Signed-off-by: Akhil CH <200716675+Akhil-1527@users.noreply.github.com>

@kdelay kdelay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right, and the new test passes locally. One note: the host assertion holds only because InetUtils is absent. Same test without spring.cloud.util.enabled=false (keeping local.host=myhost):

expected: "myhost"
 but was: "10.10.11.209"

onApplicationEvent(WebServerInitializedEvent) re-sets the host whenever InetUtils exists, so a configured local.host is kept only when util is disabled. That predates this PR, so maybe a separate issue.

@Akhil-1527

Copy link
Copy Markdown
Contributor Author

Thanks for running it. You're right, that overwrite is already on 5.0.x: onApplicationEvent replaces the host with the InetUtils one when the web server starts, so a configured local.host only survives when InetUtils is missing. The new test covers the spring.cloud.util.enabled=false case from gh-631, which is why it passes.

I'd keep this PR to gh-631. The fix would be a small check in both onApplicationEvent methods (set the host only when local.host is empty). Happy to add it here or open a separate issue, whichever the maintainers prefer.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants