diff --git a/Makefile b/Makefile index 03deac46..2e9ea4b8 100644 --- a/Makefile +++ b/Makefile @@ -317,7 +317,7 @@ local-redeploy: ## re-deploy operator in local kind cluster $(MAKE) docker-build @echo "Deploying Limitador control plane" $(KIND) load docker-image ${IMG} --name ${KIND_CLUSTER_NAME} - make deploy-develmode + $(MAKE) deploy-develmode kubectl rollout restart deployment -n limitador-operator-system limitador-operator-controller-manager @echo "Wait for all deployments to be up" kubectl -n limitador-operator-system wait --timeout=300s --for=condition=Available deployments --all diff --git a/api/v1alpha1/limitador_types.go b/api/v1alpha1/limitador_types.go index 5dd5c2bc..28b60472 100644 --- a/api/v1alpha1/limitador_types.go +++ b/api/v1alpha1/limitador_types.go @@ -23,8 +23,8 @@ import ( "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "github.com/kuadrant/limitador-operator/pkg/helpers" ) @@ -64,7 +64,7 @@ type LimitadorSpec struct { Limits []RateLimit `json:"limits,omitempty"` // +optional - PodDisruptionBudget *policyv1.PodDisruptionBudgetSpec `json:"pdb,omitempty"` + PodDisruptionBudget *PodDisruptionBudgetType `json:"pdb,omitempty"` } //+kubebuilder:object:root=true @@ -292,3 +292,18 @@ func (s *LimitadorStatus) Equals(other *LimitadorStatus, logger logr.Logger) boo func init() { SchemeBuilder.Register(&Limitador{}, &LimitadorList{}) } + +type PodDisruptionBudgetType struct { + // An eviction is allowed if at most "maxUnavailable" pods selected by + // "selector" are unavailable after the eviction, i.e. even in absence of + // the evicted pod. For example, one can prevent all voluntary evictions + // by specifying 0. This is a mutually exclusive setting with "minAvailable". + // +optional + MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"` + // An eviction is allowed if at least "minAvailable" pods selected by + // "selector" will still be available after the eviction, i.e. even in the + // absence of the evicted pod. So for example you can prevent all voluntary + // evictions by specifying "100%". + // +optional + MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"` +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 8ad17129..449d5f96 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,10 +22,10 @@ limitations under the License. package v1alpha1 import ( - corev1 "k8s.io/api/core/v1" - "k8s.io/api/policy/v1" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -140,7 +140,7 @@ func (in *LimitadorSpec) DeepCopyInto(out *LimitadorSpec) { } if in.PodDisruptionBudget != nil { in, out := &in.PodDisruptionBudget, &out.PodDisruptionBudget - *out = new(v1.PodDisruptionBudgetSpec) + *out = new(PodDisruptionBudgetType) (*in).DeepCopyInto(*out) } } @@ -207,6 +207,31 @@ func (in *Listener) DeepCopy() *Listener { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PodDisruptionBudgetType) DeepCopyInto(out *PodDisruptionBudgetType) { + *out = *in + if in.MaxUnavailable != nil { + in, out := &in.MaxUnavailable, &out.MaxUnavailable + *out = new(intstr.IntOrString) + **out = **in + } + if in.MinAvailable != nil { + in, out := &in.MinAvailable, &out.MinAvailable + *out = new(intstr.IntOrString) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodDisruptionBudgetType. +func (in *PodDisruptionBudgetType) DeepCopy() *PodDisruptionBudgetType { + if in == nil { + return nil + } + out := new(PodDisruptionBudgetType) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Ports) DeepCopyInto(out *Ports) { *out = *in @@ -252,7 +277,7 @@ func (in *Redis) DeepCopyInto(out *Redis) { *out = *in if in.ConfigSecretRef != nil { in, out := &in.ConfigSecretRef, &out.ConfigSecretRef - *out = new(corev1.ObjectReference) + *out = new(v1.ObjectReference) **out = **in } } @@ -272,7 +297,7 @@ func (in *RedisCached) DeepCopyInto(out *RedisCached) { *out = *in if in.ConfigSecretRef != nil { in, out := &in.ConfigSecretRef, &out.ConfigSecretRef - *out = new(corev1.ObjectReference) + *out = new(v1.ObjectReference) **out = **in } if in.Options != nil { diff --git a/bundle/manifests/limitador.kuadrant.io_limitadors.yaml b/bundle/manifests/limitador.kuadrant.io_limitadors.yaml index 808ac45e..11b3e3ad 100644 --- a/bundle/manifests/limitador.kuadrant.io_limitadors.yaml +++ b/bundle/manifests/limitador.kuadrant.io_limitadors.yaml @@ -76,7 +76,6 @@ spec: type: object type: object pdb: - description: PodDisruptionBudgetSpec is a description of a PodDisruptionBudget. properties: maxUnavailable: anyOf: @@ -98,54 +97,6 @@ spec: example you can prevent all voluntary evictions by specifying "100%". x-kubernetes-int-or-string: true - selector: - description: Label query over pods whose evictions are managed - by the disruption budget. A null selector will match no pods, - while an empty ({}) selector will select all pods within the - namespace. - properties: - matchExpressions: - description: matchExpressions is a list of label selector - requirements. The requirements are ANDed. - items: - description: A label selector requirement is a selector - that contains values, a key, and an operator that relates - the key and values. - properties: - key: - description: key is the label key that the selector - applies to. - type: string - operator: - description: operator represents a key's relationship - to a set of values. Valid operators are In, NotIn, - Exists and DoesNotExist. - type: string - values: - description: values is an array of string values. If - the operator is In or NotIn, the values array must - be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced - during a strategic merge patch. - items: - type: string - type: array - required: - - key - - operator - type: object - type: array - matchLabels: - additionalProperties: - type: string - description: matchLabels is a map of {key,value} pairs. A - single {key,value} in the matchLabels map is equivalent - to an element of matchExpressions, whose key field is "key", - the operator is "In", and the values array contains only - "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic type: object rateLimitHeaders: description: RateLimitHeadersType defines the valid options for the diff --git a/config/crd/bases/limitador.kuadrant.io_limitadors.yaml b/config/crd/bases/limitador.kuadrant.io_limitadors.yaml index 874ad1c7..bee21170 100644 --- a/config/crd/bases/limitador.kuadrant.io_limitadors.yaml +++ b/config/crd/bases/limitador.kuadrant.io_limitadors.yaml @@ -77,7 +77,6 @@ spec: type: object type: object pdb: - description: PodDisruptionBudgetSpec is a description of a PodDisruptionBudget. properties: maxUnavailable: anyOf: @@ -99,54 +98,6 @@ spec: example you can prevent all voluntary evictions by specifying "100%". x-kubernetes-int-or-string: true - selector: - description: Label query over pods whose evictions are managed - by the disruption budget. A null selector will match no pods, - while an empty ({}) selector will select all pods within the - namespace. - properties: - matchExpressions: - description: matchExpressions is a list of label selector - requirements. The requirements are ANDed. - items: - description: A label selector requirement is a selector - that contains values, a key, and an operator that relates - the key and values. - properties: - key: - description: key is the label key that the selector - applies to. - type: string - operator: - description: operator represents a key's relationship - to a set of values. Valid operators are In, NotIn, - Exists and DoesNotExist. - type: string - values: - description: values is an array of string values. If - the operator is In or NotIn, the values array must - be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced - during a strategic merge patch. - items: - type: string - type: array - required: - - key - - operator - type: object - type: array - matchLabels: - additionalProperties: - type: string - description: matchLabels is a map of {key,value} pairs. A - single {key,value} in the matchLabels map is equivalent - to an element of matchExpressions, whose key field is "key", - the operator is "In", and the values array contains only - "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic type: object rateLimitHeaders: description: RateLimitHeadersType defines the valid options for the diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go index c9eaca52..d7d4acab 100644 --- a/controllers/limitador_controller.go +++ b/controllers/limitador_controller.go @@ -27,7 +27,6 @@ import ( v1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -135,23 +134,11 @@ func (r *LimitadorReconciler) reconcilePdb(ctx context.Context, limitadorObj *li return nil } - limitadorPdb := limitadorObj.Spec.PodDisruptionBudget - if err := limitador.ValidatePDB(limitadorPdb); err != nil { + pdb := limitador.PodDisruptionBudget(limitadorObj) + if err := limitador.ValidatePDB(pdb); err != nil { return err } - limitadorPdb.Selector = &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "limitador"}, - } - - pdb := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{ - Name: limitador.PodDisruptionBudgetName(limitadorObj), - Namespace: limitadorObj.ObjectMeta.Namespace, - }, - Spec: *limitadorPdb, - } - // controller reference if err := r.SetOwnerReference(limitadorObj, pdb); err != nil { return err diff --git a/controllers/limitador_controller_test.go b/controllers/limitador_controller_test.go index fbe7d77e..99818f08 100644 --- a/controllers/limitador_controller_test.go +++ b/controllers/limitador_controller_test.go @@ -92,7 +92,7 @@ var _ = Describe("Limitador controller", func() { GRPC: grpcPort, }, Limits: limits, - PodDisruptionBudget: &policyv1.PodDisruptionBudgetSpec{ + PodDisruptionBudget: &limitadorv1alpha1.PodDisruptionBudgetType{ MaxUnavailable: maxUnavailable, }, }, @@ -259,7 +259,7 @@ var _ = Describe("Limitador controller", func() { return err == nil }, timeout, interval).Should(BeTrue()) Expect(createdPdb.Spec.MaxUnavailable).To(Equal(maxUnavailable)) - Expect(createdPdb.Spec.Selector.MatchLabels).To(Equal(map[string]string{"app": "limitador"})) + Expect(createdPdb.Spec.Selector.MatchLabels).To(Equal(limitador.Labels())) }) }) diff --git a/pkg/limitador/k8s_objects.go b/pkg/limitador/k8s_objects.go index 828babe1..437b9295 100644 --- a/pkg/limitador/k8s_objects.go +++ b/pkg/limitador/k8s_objects.go @@ -31,7 +31,7 @@ func Service(limitador *limitadorv1alpha1.Limitador) *v1.Service { ObjectMeta: metav1.ObjectMeta{ Name: ServiceName(limitador), Namespace: limitador.ObjectMeta.Namespace, // TODO: revisit later. For now assume same. - Labels: labels(), + Labels: Labels(), }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ @@ -48,7 +48,7 @@ func Service(limitador *limitadorv1alpha1.Limitador) *v1.Service { TargetPort: intstr.FromString("grpc"), }, }, - Selector: labels(), + Selector: Labels(), ClusterIP: v1.ClusterIPNone, Type: v1.ServiceTypeClusterIP, }, @@ -74,16 +74,16 @@ func Deployment(limitador *limitadorv1alpha1.Limitador, storageConfigSecret *v1. ObjectMeta: metav1.ObjectMeta{ Name: limitador.ObjectMeta.Name, // TODO: revisit later. For now assume same. Namespace: limitador.ObjectMeta.Namespace, // TODO: revisit later. For now assume same. - Labels: labels(), + Labels: Labels(), }, Spec: appsv1.DeploymentSpec{ Replicas: &replicas, Selector: &metav1.LabelSelector{ - MatchLabels: labels(), + MatchLabels: Labels(), }, Template: v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ - Labels: labels(), + Labels: Labels(), }, Spec: v1.PodSpec{ Containers: []v1.Container{ @@ -171,7 +171,7 @@ func LimitsConfigMap(limitador *limitadorv1alpha1.Limitador) (*v1.ConfigMap, err ObjectMeta: metav1.ObjectMeta{ Name: LimitsCMNamePrefix + limitador.Name, Namespace: limitador.Namespace, - Labels: map[string]string{"app": "limitador"}, + Labels: Labels(), }, }, nil } @@ -180,18 +180,35 @@ func ServiceName(limitadorObj *limitadorv1alpha1.Limitador) string { return fmt.Sprintf("limitador-%s", limitadorObj.Name) } +func PodDisruptionBudget(limitadorObj *limitadorv1alpha1.Limitador) *policyv1.PodDisruptionBudget { + return &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: PodDisruptionBudgetName(limitadorObj), + Namespace: limitadorObj.ObjectMeta.Namespace, + Labels: Labels(), + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MaxUnavailable: limitadorObj.Spec.PodDisruptionBudget.MaxUnavailable, + MinAvailable: limitadorObj.Spec.PodDisruptionBudget.MinAvailable, + Selector: &metav1.LabelSelector{ + MatchLabels: Labels(), + }, + }, + } +} + func PodDisruptionBudgetName(limitadorObj *limitadorv1alpha1.Limitador) string { return fmt.Sprintf("limitador-%s", limitadorObj.Name) } -func ValidatePDB(pdb *policyv1.PodDisruptionBudgetSpec) error { - if pdb.MaxUnavailable != nil && pdb.MinAvailable != nil { +func ValidatePDB(pdb *policyv1.PodDisruptionBudget) error { + if pdb.Spec.MaxUnavailable != nil && pdb.Spec.MinAvailable != nil { return fmt.Errorf("pdb spec invalid, maxunavailable and minavailable are mutually exclusive") } return nil } -func labels() map[string]string { +func Labels() map[string]string { return map[string]string{"app": "limitador"} } diff --git a/pkg/limitador/k8s_objects_test.go b/pkg/limitador/k8s_objects_test.go index 3647fe77..5477573c 100644 --- a/pkg/limitador/k8s_objects_test.go +++ b/pkg/limitador/k8s_objects_test.go @@ -10,6 +10,11 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" ) +var intStrOne = &intstr.IntOrString{ + Type: 0, + IntVal: 1, +} + func TestConstants(t *testing.T) { assert.Check(t, DefaultReplicas == 1) assert.Check(t, LimitadorRepository == "quay.io/kuadrant/limitador") @@ -44,11 +49,8 @@ func newTestLimitadorObj(name, namespace string, limits []limitadorv1alpha1.Rate GRPC: &limitadorv1alpha1.TransportProtocol{Port: &grpcPort}, }, Limits: limits, - PodDisruptionBudget: &policyv1.PodDisruptionBudgetSpec{ - MaxUnavailable: &intstr.IntOrString{ - Type: 0, - IntVal: 1, - }, + PodDisruptionBudget: &limitadorv1alpha1.PodDisruptionBudgetType{ + MaxUnavailable: intStrOne, }, }, } @@ -103,14 +105,19 @@ func TestPodDisruptionBudgetName(t *testing.T) { } func TestValidatePdb(t *testing.T) { - intStrOne := &intstr.IntOrString{ - Type: 0, - IntVal: 1, - } - limitadorPdb := &policyv1.PodDisruptionBudgetSpec{ - MaxUnavailable: intStrOne, - MinAvailable: intStrOne, + limitadorPdb := &policyv1.PodDisruptionBudget{ + Spec: policyv1.PodDisruptionBudgetSpec{ + MaxUnavailable: intStrOne, + MinAvailable: intStrOne, + }, } err := ValidatePDB(limitadorPdb) assert.Error(t, err, "pdb spec invalid, maxunavailable and minavailable are mutually exclusive") } + +func TestPodDisruptionBudget(t *testing.T) { + limitadorObj := newTestLimitadorObj("my-limitador-instance", "default", nil) + pdb := PodDisruptionBudget(limitadorObj) + assert.DeepEqual(t, pdb.Spec.MaxUnavailable, intStrOne) + assert.DeepEqual(t, pdb.Spec.Selector.MatchLabels, Labels()) +}