Skip to content

Commit

Permalink
Endpointslices reflection fix
Browse files Browse the repository at this point in the history
  • Loading branch information
fra98 committed Sep 25, 2023
1 parent f1b63fe commit daf93c5
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 15 deletions.
1 change: 0 additions & 1 deletion cmd/virtual-kubelet/root/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion cmd/virtual-kubelet/root/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion deployments/liqo/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,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. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 0 additions & 2 deletions deployments/liqo/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 44 additions & 8 deletions pkg/virtualKubelet/reflection/exposition/endpointslice.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"reflect"
"strings"
"sync"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/virtualKubelet/reflection/generic/reflector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit daf93c5

Please sign in to comment.