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

Service Metadata differs in fabric8 and k8s client in some corner case #1405

Closed
wind57 opened this issue Aug 8, 2023 · 2 comments · Fixed by #1406
Closed

Service Metadata differs in fabric8 and k8s client in some corner case #1405

wind57 opened this issue Aug 8, 2023 · 2 comments · Fixed by #1406
Labels
Milestone

Comments

@wind57
Copy link
Contributor

wind57 commented Aug 8, 2023

As part of the ServiceInstance that a discovery client returns, there is a field called metadata, which is some meta information about the service. As part of it, we return a so called port name and port number, it really does not even matter what they mean or where they come from.

The relevant part for this issue is that sometimes port name might not be present. In that case:

  • fabric8 ignores that field
  • k8s takes that field into account

For example, here is how fabric8 would represent a response:

Map.of("k8s_namespace", "namespace1", "type", "ClusterIP")

while k8s:

Map.of("<unset>", "8080", "k8s_namespace", "namespace1", "type", "ClusterIP")

Notice the "<unset>", "8080".


imo, k8s is not doing the correct thing, for a few reasons:

  • in general, we do not care about this metadata too much. We care about the "primary port" of the service, that is returned elsewhere and is computed correctly for both clients.
  • if there are multiple ports without a name, k8s will override each other, because it does:
Map<String, String> ports = ...

for (CoreV1EndpointPorts onePort: allOfServicesPorts) { 
     if (onePort.getName == null) {
           ports.put("<unset>", onePort.getNumber())
     }
}

you can see that if there are more then one port without a name, one will override the other.

I am open to any suggestions, cause this is rather confusing for me too.

@ryanjbaxter
Copy link
Contributor

The k8s client behavior was changed as a result of this issue

#938

I think we should recreate that with fabric8 as well

@wind57
Copy link
Contributor Author

wind57 commented Aug 8, 2023

good memory! thank you for explaining, I'll fix it in fabric8. want to convert this to a bug then?

@ryanjbaxter ryanjbaxter added bug and removed question labels Aug 8, 2023
@ryanjbaxter ryanjbaxter linked a pull request Aug 10, 2023 that will close this issue
@ryanjbaxter ryanjbaxter mentioned this issue Aug 10, 2023
@ryanjbaxter ryanjbaxter added this to the 3.0.5 milestone Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants