From 8b9fdda51ff9401b2d7e2820a1841de36888f662 Mon Sep 17 00:00:00 2001 From: Francesco Torta <62566275+fra98@users.noreply.github.com> Date: Fri, 22 Sep 2023 15:48:49 +0200 Subject: [PATCH] Endpointslices reflection fix --- cmd/virtual-kubelet/root/opts.go | 1 - cmd/virtual-kubelet/root/root.go | 6 ++- deployments/liqo/README.md | 1 - .../liqo-controller-manager-deployment.yaml | 1 - deployments/liqo/values.yaml | 2 - .../reflection/exposition/endpointslice.go | 52 ++++++++++++++++--- .../reflection/generic/reflector.go | 2 +- 7 files changed, 50 insertions(+), 15 deletions(-) diff --git a/cmd/virtual-kubelet/root/opts.go b/cmd/virtual-kubelet/root/opts.go index 5a023322fe..55c0a449b8 100644 --- a/cmd/virtual-kubelet/root/opts.go +++ b/cmd/virtual-kubelet/root/opts.go @@ -63,7 +63,6 @@ var DefaultReflectorsWorkers = map[generic.ResourceReflected]uint{ var DefaultReflectorsTypes = map[generic.ResourceReflected]consts.ReflectionType{ generic.Pod: consts.CustomLiqo, generic.Service: consts.DenyList, - generic.EndpointSlice: consts.DenyList, generic.Ingress: consts.DenyList, generic.ConfigMap: consts.DenyList, generic.Secret: consts.DenyList, diff --git a/cmd/virtual-kubelet/root/root.go b/cmd/virtual-kubelet/root/root.go index 5c79165beb..b8a6ac6ceb 100644 --- a/cmd/virtual-kubelet/root/root.go +++ b/cmd/virtual-kubelet/root/root.go @@ -283,7 +283,11 @@ func getReflectorsConfigs(c *Opts) (map[generic.ResourceReflected]*generic.Refle if isReflectionTypeNotCustomizable(*resource) { reflectionType = DefaultReflectorsTypes[*resource] } else { - reflectionType = consts.ReflectionType(*c.ReflectorsType[string(*resource)]) + if *resource == generic.EndpointSlice { + reflectionType = consts.ReflectionType(*c.ReflectorsType[string(generic.Service)]) + } else { + reflectionType = consts.ReflectionType(*c.ReflectorsType[string(*resource)]) + } if reflectionType != consts.DenyList && reflectionType != consts.AllowList { return nil, fmt.Errorf("reflection type %q is not valid for resource %s. Ammitted values: %q, %q", reflectionType, *resource, consts.DenyList, consts.AllowList) diff --git a/deployments/liqo/README.md b/deployments/liqo/README.md index 353e684e4b..943c106292 100644 --- a/deployments/liqo/README.md +++ b/deployments/liqo/README.md @@ -125,7 +125,6 @@ | pullPolicy | string | `"IfNotPresent"` | The pullPolicy for liqo pods. | | reflection.configmap.type | string | `"DenyList"` | The type of reflection used for the configmaps reflector. Ammitted values: "DenyList", "AllowList". | | reflection.configmap.workers | int | `3` | The number of workers used for the configmaps reflector. Set 0 to disable the reflection of configmaps. | -| reflection.endpointslice.type | string | `"DenyList"` | The type of reflection used for the endpointslices reflector Ammitted values: "DenyList", "AllowList". | | reflection.endpointslice.workers | int | `10` | The number of workers used for the endpointslices reflector. Set 0 to disable the reflection of endpointslices. | | reflection.event.type | string | `"DenyList"` | The type of reflection used for the events reflector. Ammitted values: "DenyList", "AllowList". | | reflection.event.workers | int | `3` | The number of workers used for the events reflector. Set 0 to disable the reflection of events. | diff --git a/deployments/liqo/templates/liqo-controller-manager-deployment.yaml b/deployments/liqo/templates/liqo-controller-manager-deployment.yaml index 830438df8c..65e2d11cfc 100644 --- a/deployments/liqo/templates/liqo-controller-manager-deployment.yaml +++ b/deployments/liqo/templates/liqo-controller-manager-deployment.yaml @@ -87,7 +87,6 @@ spec: - --persistentvolumeclaim-reflection-workers={{ .Values.reflection.persistentvolumeclaim.workers }} - --event-reflection-workers={{ .Values.reflection.event.workers }} - --service-reflection-type={{ .Values.reflection.service.type }} - - --endpointslice-reflection-type={{ .Values.reflection.endpointslice.type }} - --ingress-reflection-type={{ .Values.reflection.ingress.type }} - --configmap-reflection-type={{ .Values.reflection.configmap.type }} - --secret-reflection-type={{ .Values.reflection.secret.type }} diff --git a/deployments/liqo/values.yaml b/deployments/liqo/values.yaml index 3f1c38d26f..13b40aeede 100644 --- a/deployments/liqo/values.yaml +++ b/deployments/liqo/values.yaml @@ -66,8 +66,6 @@ reflection: endpointslice: # -- The number of workers used for the endpointslices reflector. Set 0 to disable the reflection of endpointslices. workers: 10 - # -- The type of reflection used for the endpointslices reflector Ammitted values: "DenyList", "AllowList". - type: DenyList ingress: # -- The number of workers used for the ingresses reflector. Set 0 to disable the reflection of ingresses. workers: 3 diff --git a/pkg/virtualKubelet/reflection/exposition/endpointslice.go b/pkg/virtualKubelet/reflection/exposition/endpointslice.go index d8c8255dd1..7b7c70184c 100644 --- a/pkg/virtualKubelet/reflection/exposition/endpointslice.go +++ b/pkg/virtualKubelet/reflection/exposition/endpointslice.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "reflect" + "strings" "sync" corev1 "k8s.io/api/core/v1" @@ -327,30 +328,65 @@ func (ner *NamespacedEndpointSliceReflector) UnmapEndpointIPs(ctx context.Contex return nil } +// forcedAllowOrSkip checks whether the given object is explicitly marked to be allowed or skipped. +// If so, it returns whether the object should be skipped, or an error if unable to determine it. +func forcedAllowOrSkip(obj metav1.Object) (forced, shouldSkip bool, err error) { + allowAnnot, skipAnnot := false, false + + value, ok := obj.GetAnnotations()[consts.AllowReflectionAnnotationKey] + if ok && !strings.EqualFold(value, "false") { + allowAnnot = true + } + value, ok = obj.GetAnnotations()[consts.SkipReflectionAnnotationKey] + if ok && !strings.EqualFold(value, "false") { + skipAnnot = true + } + + if allowAnnot && skipAnnot { + return false, false, fmt.Errorf("endpointslice %q can't have both the allow and deny annotations set", klog.KObj(obj)) + } + + switch { + case allowAnnot: + forced = true + shouldSkip = false + case skipAnnot: + forced = true + shouldSkip = true + default: + forced = false + } + + return forced, shouldSkip, nil +} + // ShouldSkipReflection returns whether the reflection of the given object should be skipped. func (ner *NamespacedEndpointSliceReflector) ShouldSkipReflection(obj metav1.Object) (bool, error) { - // Check if the endpointslice is marked to be skipped. - skipReflection, err := ner.NamespacedReflector.ShouldSkipReflection(obj) + // Check if the endpointslice is explicitly marked to be skipped or allowed. + forced, shouldSkip, err := forcedAllowOrSkip(obj) if err != nil { return true, err - } - if skipReflection { - return true, nil + } else if forced { + return shouldSkip, nil } // Check if a service is associated to the EndpointSlice, and whether it is marked to be skipped. svcname, ok := obj.GetLabels()[discoveryv1.LabelServiceName] if !ok { - return false, nil + // If the endpointslice is not associated to a service, use standard logic to determine whetever or not + // to reflect the endpointslice (note: reflection type is inherited from service). + return ner.NamespacedReflector.ShouldSkipReflection(obj) } svc, err := ner.localServices.Get(svcname) - // Continue with the reflection in case the service is not found, as this is likely due to a race conditions + // Continue with the standard logic in case the service is not found, as this is likely due to a race conditions // (i.e., the service has not yet been cached). If necessary, the informer will trigger a re-enqueue, // thus performing once more this check. if err != nil { - return false, nil + return ner.NamespacedReflector.ShouldSkipReflection(obj) } + + // The associated service exists, hence check if it is marked to be skipped. return ner.NamespacedReflector.ShouldSkipReflection(svc) } diff --git a/pkg/virtualKubelet/reflection/generic/reflector.go b/pkg/virtualKubelet/reflection/generic/reflector.go index 2a865f2e95..3fa7476c42 100644 --- a/pkg/virtualKubelet/reflection/generic/reflector.go +++ b/pkg/virtualKubelet/reflection/generic/reflector.go @@ -105,7 +105,7 @@ const ( var Reflectors = []ResourceReflected{Pod, Service, EndpointSlice, Ingress, ConfigMap, Secret, ServiceAccount, PersistentVolumeClaim, Event} // ReflectorsCustomizableType is the list of resources for which the reflection type can be customized. -var ReflectorsCustomizableType = []ResourceReflected{Service, EndpointSlice, Ingress, ConfigMap, Secret, Event} +var ReflectorsCustomizableType = []ResourceReflected{Service, Ingress, ConfigMap, Secret, Event} // ReflectorConfig contains configuration parameters of the reflector. type ReflectorConfig struct {