spring-cloud / spring-cloud-kubernetes

Kubernetes integration with Spring Cloud Discovery Client, Configuration, etc...
Apache License 2.0
3.46k stars 1.03k forks source link

Kubernetes service discovery doesn't work along with simple discovery service #1286

Closed rechandler12 closed 1 year ago

rechandler12 commented 1 year ago

Describe the bug I want to have 2 layers of discovery service: Kubernetes discovery client and simple discovery client. It's working with eureka (local dev env) and it was working before 3.0 spring-cloud-kubernetes.

Sample Here's my configs:

spring:
  config:
    import: "kubernetes:"
  cloud:
    kubernetes:
      discovery:
        service-labels:
          "[app.kubernetes.io/part-of]": asdf
      loadbalancer:
        mode: SERVICE
spring:
  cloud:
    discovery:
      client:
        simple:
          instances:
            xyz:
              - uri: http://xyz/
            xyz-custom:
              - uri: http://xyz/

and I've got: Load balancer does not contain an instance for the service xyz-custom

ryanjbaxter commented 1 year ago

Can you provide a complete, minimal, verifiable sample that reproduces the problem? It should be available as a GitHub (or similar) project or attached to this issue as a zip file.

rechandler12 commented 1 year ago

I'm attaching spring boot project. I don't attach K8s yaml?

When you run with profile local it should connect to eureka and it's working. When you deploy to K8s it doesn't work.

Endpoint: GET app_address/weather

mvp.zip

wind57 commented 1 year ago

I just imported your sample, and what am I supposed to see there? How do I understand what problem are you facing? You need to tell us, how to deploy this (yes, k8s yaml files) and requests you make, what you expect, etc.

I assume you build an image out of that project, have a deployment. So after it ends up in k8s cluster (again, show us how you do that), what are you doing with it? You make some requests? How is eureka related to this and where is it in your project set-up?

rechandler12 commented 1 year ago

Local without K8s (everything working fine)

  1. download zip
  2. unzip
  3. run: mvn spring-boot:run
  4. make GET request to: http://localhost:8080/weather
  5. get response, something like this: {"current_weather":{"time":"2023-04-13T09:00:00","temperature":9.8,"weathercode":3,"windspeed":11.1,"winddirection":205.0}}
  6. everything works fine

On K8s (not working)

  1. download example.txt file and change name to example.yaml (attached to this post)
  2. install app on K8s (I have pushed already image to docker hub)
  3. run kubectl port-forward YOUR_POD_NAME 8080:8080
  4. make GET request to: http://localhost:8080/weather
  5. you've got an error:
    feign.FeignException$ServiceUnavailable: [503] during [GET] to [http://openmeteo/v1/forecast?latitude=52.52&longitude=13.41&current_weather=true&hourly=temperature_2m&hourly=relativehumidity_2m&hourly=windspeed_10m] [OpenMeteoFeignClient#getWeather()]: [Load balancer does not contain an instance for the service openmeteo]

It isn't working when I'm using simple discovery client combined with K8s service discovery. It was working on version 2.x. Also when i combine simple discovery client with eureka server (without any K8s) it's also working.

example.txt

wind57 commented 1 year ago

Much, much better now! Thank you.

I can re-produce and I think I know where the problem is... I'll update this post once I have all the proper conclusions.

wind57 commented 1 year ago

This is the code to be blamed:

List<V1Service> services = properties.allNamespaces()
                ? serviceLister.list().stream().filter(svc -> serviceId.equals(svc.getMetadata().getName())).toList()
                : List.of(serviceLister.namespace(namespace).get(serviceId));

Specifically this part: List.of(serviceLister.namespace(namespace).get(serviceId)). Because CompositeDiscoveryClient kicks in, it will call KubernetesInformerDiscoveryClient::getInstances with a value of openmeteo. In turn this leads to serviceLister.namespace(namespace).get(serviceId) being null, and a null in an ImmutableCollection triggers a NPE.

That is a bug.

The good part, is that because of recent simplifications, there is no fix needed here (I will still add a minor integration test), that code was removed.


What I did is test this against our current 3.0.3-SNAPSHOT:

        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-kubernetes-client-all</artifactId>
            <version>3.0.3-SNAPSHOT</version>
        </dependency>

in your dependency, deploy locally in k8s and :

root@kind-control-plane:/# curl 10.244.0.5:8080/weather
{"current_weather":{"time":"2023-04-13T17:00:00","temperature":12.7,"weathercode":3,"windspeed":7.5,"winddirection":235.0}}

@ryanjbaxter I'll add a minor integration test case here, so please let's keep it open until then. Thank you.

rechandler12 commented 1 year ago

perfect, thank you. Have you any estimate time when you release next version?

ryanjbaxter commented 1 year ago

Tentative release dates can be found here https://github.com/spring-cloud/spring-cloud-release/milestones

wind57 commented 1 year ago

I've added some test cases for this issue in this PR