Add support for IP Family Policy and IP Families #4076
Conversation
Add IP Family Policy and IP Families to CRD for Services
upstream changes
upstream changes
update to enum for IP Families
| // --- | ||
| // +optional | ||
| // +kubebuilder:validation:Enum=SingleStack;PreferDualStack;RequireDualStack | ||
| IPFamilyPolicy string `json:"ipFamilyPolicy,omitempty"` |
There was a problem hiding this comment.
❓ I think we can avoid the type conversions in the controller if we use the upstream type here. Similar regarding pointer. Does this generate the same YAML?
| IPFamilyPolicy string `json:"ipFamilyPolicy,omitempty"` | |
| IPFamilyPolicy *corev1.IPFamilyPolicy `json:"ipFamilyPolicy,omitempty"` |
There was a problem hiding this comment.
yes it does - thanks - refactoring
| // +optional | ||
| IPFamilies []IPFamily `json:"ipFamilies,omitempty"` |
There was a problem hiding this comment.
❓ We should be able to use the upstream type and add the enum validation from here. Does this generate the same YAML?
| // +optional | |
| IPFamilies []IPFamily `json:"ipFamilies,omitempty"` | |
| // +optional | |
| // +kubebuilder:validation:items:Enum={IPv4,IPv6} | |
| IPFamilies []corev1.IPFamily `json:"ipFamilies,omitempty"` |
There was a problem hiding this comment.
yes it does - thanks - refactoring
| service.Spec.IPFamilyPolicy = &policy | ||
| } | ||
| if len(spec.IPFamilies) > 0 { | ||
| service.Spec.IPFamilies = []corev1.IPFamily{} |
There was a problem hiding this comment.
📝 If the Go types in the CRD structs can change, these assignments might reduce to service.Spec... = spec....
There was a problem hiding this comment.
yes - this one simplified also - thanks for the suggestion
|
The code to pass through these values looks good 👍 Have you looked for tests where we are checking that ServiceSpec values are being passed into the service? |
There was a problem hiding this comment.
I don't think tests for this change would be doing much. It would basically be making sure that anything that shows up in the CR ends up on the service spec.
Since we already don't have tests for TrafficPolicy, I feel ok merging this without updating the tests
| if spec.IPFamilyPolicy != nil { | ||
| service.Spec.IPFamilyPolicy = spec.IPFamilyPolicy | ||
| } | ||
| if len(spec.IPFamilies) > 0 { | ||
| service.Spec.IPFamilies = spec.IPFamilies | ||
| } |
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
Currently the CRD does not manage the
ipFamilyPolicyoripFamiliesfields on the Services managed by CPK. This makes it more difficult for users in IPv6 and DualStack environments.What is the new behavior (if this is a feature change)?
Other Information: