Add namespace information to service instance metadata

[m-kay]

As mentioned in issue #473 the namespace should be reflected in the serviceId when using discovery with all-namespaces set to true.

[Haybu]

@m-kay - I wonder why to consider a namespace as long as it's still one service doing the same thing, but happens to be across different namespaces? why not to discover and balance across regardless of the namespace as long as endpoints are reachable using registered IP/port?

[spencergibb]

I'll have to rely on @Haybu and @ryanjbaxter for implementation checks

[ryanjbaxter]

I agree with @Haybu, I don't think the service id should be different. Maybe we should add namespace information to the metadata?

[m-kay]

Correct me if I'm wrong, but I think it's not possible to access the service by only its service id from outside its own namespace. See: https://kubernetes.io/docs/concepts/services-networking/service/#dns

[Haybu]

@m-kay - that is true when a service name/id is used directly and you depend on k8s platform to resolve. But the idea here is that the discovery client uses a service id to interrogate the platform server for the service endpoints, hence providing a list of pods ip:port to pick from and hit.

[m-kay]

But how can you be sure that two services are really the same? It's totally valid to have two different services with the same name in different namespaces. In my case I have different applications which are structured the same way in different namespaces. Each application has the same service for example "gateway". However each gateway does something different because it's specific for the application in that namespace.

[spencergibb]

We'll need to have a discussion. What you've described and what Haytham and Ryan have said are both valid, but at odds with each other. We'll need to see if we can reconcile the two. I don't know if it's in discovery client or in loadbalancer or both.

[GianniGiglio]

Hi am looking for a similar solution where services can be deployed on multiple namespaces (1 namespace eq customer instance).

I still have a multitenant application using the kuberentesDiscoveryClient and I would need to route the request to multiple namespaces.

I was wondering why not add the namespace to kubernetesServiceInstance instead of in the serviceId?

[ryanjbaxter]

After discussing this with the team today we think the best solution at this time is to only take the portion of this PR which puts the namespace in the service metadata
https://github.com/spring-cloud/spring-cloud-kubernetes/pull/503/files#diff-bf6bb75a365ae3173c5d845358dfdc85R116

From there clients should be able to determine which instance of the service they want to target.

@m-kay if you are interested in modifying the PR to just include those changes please let me know

[GianniGiglio]

After discussing this with the team today we think the best solution at this time is to only take the portion of this PR which puts the namespace in the service metadata
https://github.com/spring-cloud/spring-cloud-kubernetes/pull/503/files#diff-bf6bb75a365ae3173c5d845358dfdc85R116

From there clients should be able to determine which instance of the service they want to target.

@m-kay if you are interested in modifying the PR to just include those changes please let me know

If the service instance would contain the namespace as a field, Spring cloud gateway could for instance forward to the correct namespace when using kubernetes services dns.

example config:

spring: cloud: gateway: discovery: locator: url-expression: "'http://'+serviceId+' + 'namespace'+''.svc.cluster-domain.example'

[m-kay]

@ryanjbaxter yes I would like to change the pull requests. However the code you mentioned in the diff does not include the namespace in the service metadata but was to filter service instances in the k8s client. As I understood the idea is to include the namespace information in the ServiceInstance, right?

@GianniGiglio I could include a getter method in the KubernetesServiceInstance which returns the information from the metadata, that should also do the trick. However you will still have the problem that the service which is used can be from any namespace because the discovery client will return an unfiltered list of all instances of the service from all namespaces.

[GianniGiglio]

@ryanjbaxter yes I would like to change the pull requests. However the code you mentioned in the diff does not include the namespace in the service metadata but was to filter service instances in the k8s client. As I understood the idea is to include the namespace information in the ServiceInstance, right?

@GianniGiglio I could include a getter method in the KubernetesServiceInstance which returns the information from the metadata, that should also do the trick. However, you will still have the problem that the service which is used can be from any namespace because the discovery client will return an unfiltered list of all instances of the service from all namespaces.

That depends on how the Kubernetes client is configured I think. At least your proposed solution is how I worked around it by over-writing the kubernetesServiceInstance and populate the namespace based on the metadata.

this.kubernetesClient.endpoints().inNamespace("default") --> will only return the endpoints for the default namespace
this.kubernetesClient.endpoints().inAnyNamespace() --> will return all enpoints in all namespaces.

endpoints.getMetadata().getNamespace() should give you the namespace.

[ryanjbaxter]

Looks good, just one small tweak

[m-kay]

Can we get this in the next 1.1.x release by any chance or will it only make it in the next major release?

[spencergibb]

Can you retarget to 1.1.x branch?

[spencergibb]

NAMESPACE_METADAT_KEY -> NAMESPACE_METADATA_KEY (metadata is missspelled).