Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port is set to null - NewService.Check #40

Closed
jmirc opened this issue Jun 3, 2015 · 4 comments
Closed

Port is set to null - NewService.Check #40

jmirc opened this issue Jun 3, 2015 · 4 comments

Comments

@jmirc
Copy link
Contributor

jmirc commented Jun 3, 2015

The port when a new instance of the NewService.Check is set to null when the healthCheckUrl is not set in the application.yml or in the bootstrap.yml.

In this case, the http method is set using the following code and the port is retrieved from the service. I think it should be retrieved from the properties.

/Users/jmirc/.m2/repository/org/springframework/cloud/spring-cloud-consul-discovery/1.0.0.M1/spring-cloud-consul-discovery-1.0.0.M1-sources.jar!/org/springframework/cloud/consul/discovery/ConsulLifecycle.java:77

check.setHttp(String.format("http://%s:%s%s", properties.getHostname(),
                    service.getPort(), properties.getHealthCheckPath()));

You can check the consul log to see the problem

2015/06/02 20:39:35 [WARN] agent: http request failed 'http://localhost:null/health': Get http://localhost:null/health: dial tcp: unknown port tcp/null

If I check the healthCheckUrl property, everything works perfectly like

spring:
  cloud:
    consul:
      discovery:
        healthCheckUrl: http://localhost:8080
@jmirc jmirc changed the title Wrong port - NewService.Check Port is set to null - NewService.Check Jun 3, 2015
@spencergibb
Copy link
Member

So M1 depends on a snapshot of s-c-commons. AbstractDiscoveryLifecycle sets the port first so we can support when you want a random port and set server.port=0. So we wait to get the port from the container. Make sure you have the latest s-c-commons. If not, you'll need to show how to reproduce it.

@jmirc
Copy link
Contributor Author

jmirc commented Jun 3, 2015

Ok. I see.
I am using the latest s-c-commons which is 1.0.1.RELEASE and this release doesn't contain the code you describe and the port is not set. Did you release a new version?

@spencergibb
Copy link
Member

@jmirc no, it would be in a snapshot. Will need to do a release soon. See https://github.com/spring-cloud/spring-cloud-consul/blob/master/pom.xml#L61

@jmirc
Copy link
Contributor Author

jmirc commented Jun 17, 2015

Tested and it works. I am closing this bug.

@jmirc jmirc closed this as completed Jun 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants