add pod metadata and annotations as part of the service discovery response #1254
Conversation
Configure Renovate
|
|
||
| } | ||
|
|
||
| public Metadata(@DefaultValue("true") boolean addLabels, String labelsPrefix, |
There was a problem hiding this comment.
add a new constructor not to break the public existing one
| this(instanceId, serviceId, host, port, metadata, secure, null, null, Map.of(), Map.of()); | ||
| } | ||
|
|
||
| public DefaultKubernetesServiceInstance(String instanceId, String serviceId, String host, int port, |
There was a problem hiding this comment.
keep the previous constructor intact and introduce a new one
|
@ryanjbaxter ready to look at. thank you. |
| } | ||
|
|
||
| @Override | ||
| public Map<String, String> podLabels() { |
There was a problem hiding this comment.
Is there a reason to not just put this in the metadata?
There was a problem hiding this comment.
I was afraid that service labels and pod labels could create a mess if put together, like what if labels colide? i thought that separating them would be cleaner.
There was a problem hiding this comment.
Yeah that is kind of what I was saying here....I suppose there is no other way, we either run the risk of colliding or we separate things.
Since we have some freedom here with this new property could we make it Map<String, Map<String, String> call it podMetadata and put both labels and annotations in that map?
There was a problem hiding this comment.
this is where I was hesitant too, I thought not everyone is interested in one or the other... I don't know. I'll listen to your advice here. I mean what if you want to separate your logic on labels only, for example?
There was a problem hiding this comment.
if you enable one or the other only that one would be added to the map
There was a problem hiding this comment.
my turn to miss-read!
Map<String, Map<String, String>
so something like : labels = {a = b} and annotations = {c = d}, right?
3 participants
No description provided.