fix: IPFamilyPolicy not synced for default vcluster service #1592
Conversation
✅ Deploy Preview for vcluster-docs canceled.Built without sensitive environment variables
|
| @@ -53,14 +53,18 @@ func SyncKubernetesService( | |||
| return err | |||
| } | |||
|
|
|||
| // detect if cluster ips changed | |||
| clusterIPsChanged := vObj.Spec.ClusterIP != pObj.Spec.ClusterIP || !equality.Semantic.DeepEqual(vObj.Spec.ClusterIPs, pObj.Spec.ClusterIPs) | |||
There was a problem hiding this comment.
could we use slices.Equal here instead maybe since clusterIPs is just []string ?
We don't need to check the family policy here because it's a 1 to 1 relationship with the clusterIPs slice right?
There was a problem hiding this comment.
Actually, do you know what would happen if we go from PreferDualStack to RequireDualStack ? That may be worth to include it in the condition
There was a problem hiding this comment.
Makes sense :) pushed!
There was a problem hiding this comment.
IMO not important to sync as we copy the ClusterIPs, only the ClusterIPs depend on this. May be best to future proof anyway depending if anything else starts to depend on this, hmm. WDYT?
There was a problem hiding this comment.
Also, it currently seems to update. I'm not sure why...
There was a problem hiding this comment.
Seems like it's because something resets the ports before we update it here when the syncer starts. I think think the IPFamilyPolicy only really needs to sync if the ClusterIPs change so I think this is fine
There was a problem hiding this comment.
That something is probably the controller-manager that also owns this service, but does not update it
Signed-off-by: Rohan CJ <rohantmp@gmail.com>
What issue type does this pull request address? (keep at least one, remove the others)
/kind bugfix
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>if possible)resolves ENG-3024
fixes #1587
Please provide a short message that should be published in the vcluster release notes
Fixed an issue where vcluster has incorrectly
What else do we need to know?