k8s: Consolidate check for EndpointSlice support #15561
Conversation
christarazi
added
sig/k8s
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
|
test-me-please |
| @@ -363,7 +363,7 @@ func SupportsEndpointSlice() bool { | |||
| // SupportsEndpointSliceV1 returns true if cilium-operator or cilium-agent should | |||
| // watch and process endpoint slices V1. | |||
| func SupportsEndpointSliceV1() bool { | |||
| return version.Capabilities().EndpointSliceV1 && option.Config.K8sEnableK8sEndpointSlice | |||
| return SupportsEndpointSlice() && version.Capabilities().EndpointSliceV1 | |||
There was a problem hiding this comment.
My reading is that SupportsEndpointSlice() checks for < k8s 1.21 EndpointSlice support. Is it guaranteed that on >= k8s 1.21 not only Capabilities().EndpointSliceV1, but also Capabilities().EndpointSlice will be set to true too?
There was a problem hiding this comment.
The idea was to have the option option.Config.K8sEnableK8sEndpointSlice used to disable EndpointSlice support for both v1beta1 and v1.
There was a problem hiding this comment.
Yes @brb, and yes it still will @aanm:
func SupportsEndpointSlice() bool {
return version.Capabilities().EndpointSlice && option.Config.K8sEnableK8sEndpointSlice
}
It checks the option and now this PR simply combines the two functions by calling the above (SupportsEndpointSlice()). As mentioned, one variant cannot return true and the other false, and vice versa.
| @@ -363,7 +363,7 @@ func SupportsEndpointSlice() bool { | |||
| // SupportsEndpointSliceV1 returns true if cilium-operator or cilium-agent should | |||
| // watch and process endpoint slices V1. | |||
| func SupportsEndpointSliceV1() bool { | |||
| return version.Capabilities().EndpointSliceV1 && option.Config.K8sEnableK8sEndpointSlice | |||
| return SupportsEndpointSlice() && version.Capabilities().EndpointSliceV1 | |||
There was a problem hiding this comment.
The idea was to have the option option.Config.K8sEnableK8sEndpointSlice used to disable EndpointSlice support for both v1beta1 and v1.
When v1 support for EndpointSlice is checked, the capability for the base EndpointSlice support is also toggled, therefore we can consolidate the logic inside the support helpers. It's not possible for one variant to return true and the other false, and vice versa. Fixes: 5ee4c6f ("add support for EndpointSlice V1") Signed-off-by: Chris Tarazi <chris@isovalent.com>
christarazi
force-pushed
the
pr/christarazi/consolidate-epslices-check
branch
from
3334930
to
1ead6a8
Compare
|
test-me-please |
|
Only failures are FQDN-related (link), seems orthogonal to the endpoint slices changes being made here. Otherwise passing CI and has required codeowner review. Merging. |
When v1 support for EndpointSlice is checked, the capability for the
base EndpointSlice support is also toggled, therefore we can consolidate
the logic inside the support helpers. It's not possible for one variant
to return true and the other false, and vice versa.
Fixes: 5ee4c6f ("add support for EndpointSlice V1")
Signed-off-by: Chris Tarazi chris@isovalent.com