Expose the new metrics port in the all-pods service #489
Conversation
| }, | ||
| }, | ||
| dcServicePorts: []int32{8080, 9000, 9042, 9103, 9142, 9160}, | ||
| allPodsServicePorts: []int32{8080, 9000, 9042, 9103}, |
There was a problem hiding this comment.
I wonder: shouldn't all-pods service expose the same ports as the dc service? But I didn't want to change that in this PR.
There was a problem hiding this comment.
it depends what the all-ports service is used for.
If no one expressed any requirement around that, I agree we shouldn't change it.
There was a problem hiding this comment.
From what I understand the all-pods service is identical to the dc service, but also includes pods not ready. So it's not suitable to be used as a contact point for the drivers, but it is useful for metrics for instance. Maybe that's the reason why it doesn't expose so many ports. That said, following this logic it shouldn't expose 9042 either. Anyways 🤷♂️
There was a problem hiding this comment.
cass-operator sets allPods as the ServiceName for all pods to ensure that they get DNS working correctly. Using only the dcPods would make the DNS names fail in certain cases:
| }, | ||
| }, | ||
| }, | ||
| // FIXME: 9004 should be in the list of open ports |
There was a problem hiding this comment.
This is interesting: I copied this test from TestPorts, but I realized that if the user changes a managed port at container level, the change is not reflected at service level.
| }, | ||
| }, | ||
| dcServicePorts: []int32{8080, 9000, 9042, 9103, 9142, 9160}, | ||
| allPodsServicePorts: []int32{8080, 9000, 9042, 9103}, |
There was a problem hiding this comment.
it depends what the all-ports service is used for.
If no one expressed any requirement around that, I agree we shouldn't change it.
What this PR does:
Exposes the 9000 port in 2 services: DC and all-pods.
Which issue(s) this PR fixes:
Fixes #488
Checklist