Merge pull request #708 from fabriziosestito/feat/limits-requests

feat: add limits and requests to PolicyServer

config/crd/bases/policies.kubewarden.io_policyservers.yaml

@@ -1143,6 +1143,16 @@ spec:
  items:
  type: string
  type: array
+ limits:
+ additionalProperties:
+ anyOf:
+ - type: integer
+ - type: string
+ pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+ x-kubernetes-int-or-string: true
+ description: Limits describes the maximum amount of compute resources
+ allowed.
+ type: object
  maxUnavailable:
  anyOf:
  - type: integer
@@ -1161,6 +1171,18 @@ spec:
  description: Replicas is the number of desired replicas.
  format: int32
  type: integer
+ requests:
+ additionalProperties:
+ anyOf:
+ - type: integer
+ - type: string
+ pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+ x-kubernetes-int-or-string: true
+ description: Requests describes the minimum amount of compute resources
+ required. If Request is omitted for, it defaults to Limits if that
+ is explicitly specified, otherwise to an implementation-defined
+ value
+ type: object
  securityContexts:
  description: Security configuration to be used in the Policy Server
  workload. The field allows different configurations for the pod

controllers/policyserver_controller_test.go

@@ -22,6 +22,8 @@ import (
 
  . "github.com/onsi/ginkgo/v2" //nolint:revive
  . "github.com/onsi/gomega" //nolint:revive
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/apimachinery/pkg/api/resource"
  "k8s.io/apimachinery/pkg/util/intstr"
  "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -73,7 +75,6 @@ var _ = Describe("PolicyServer controller", func() {
  return err
  }, timeout, pollInterval).ShouldNot(Succeed())
  })
-
  })
 
  Context("with assigned policies", Serial, func() {
@@ -173,7 +174,6 @@ var _ = Describe("PolicyServer controller", func() {
  pdb, _ := getPolicyServerPodDisruptionBudget(policyServerName)
  return pdb
  }, timeout, pollInterval).Should(policyServerPodDisruptionBudgetMatcher(policyServer, &minAvailable, nil))
-
  })
 
  It("with MaxUnavailable PodDisruptionBudget configuration should create PDB", func() {
@@ -261,7 +261,6 @@ var _ = Describe("PolicyServer controller", func() {
  pdb, _ := getPolicyServerPodDisruptionBudget(policyServerName)
  return pdb
  }, timeout, pollInterval).Should(policyServerPodDisruptionBudgetMatcher(policyServer, nil, &maxUnavailable))
-
  })
 
  AfterEach(func() {
@@ -283,4 +282,78 @@ var _ = Describe("PolicyServer controller", func() {
  })
  })
 
+ When("creating a PolicyServer with requests and no limits", func() {
+ policyServerName := newName("policy-server")
+ policyServer := policyServerFactory(policyServerName)
+ policyServer.Spec.Limits = corev1.ResourceList{
+ "cpu": resource.MustParse("100m"),
+ "memory": resource.MustParse("1Gi"),
+ }
+
+ It("should create the PolicyServer pod with the limits and the requests", func() {
+ By("creating the PolicyServer")
+ Expect(
+ k8sClient.Create(ctx, policyServer),
+ ).To(haveSucceededOrAlreadyExisted())
+
+ By("creating a deployment with limits and requests set")
+ Eventually(func(g Gomega) error {
+ deployment, err := getTestPolicyServerDeployment(policyServerName)
+ if err != nil {
+ return err
+ }
+
+ Expect(deployment.Spec.Template.Spec.Containers[0].Resources.Limits).To(Equal(policyServer.Spec.Limits))
+ By("setting the requests to the same value as the limits")
+ Expect(deployment.Spec.Template.Spec.Containers[0].Resources.Requests).To(Equal(policyServer.Spec.Limits))
+
+ return nil
+ })
+
+ By("creating a pod with limit and request set")
+ Eventually(func(g Gomega) error {
+ pod, err := getTestPolicyServerPod(policyServerName)
+ if err != nil {
+ return err
+ }
+
+ Expect(pod.Spec.Containers[0].Resources.Limits).To(Equal(policyServer.Spec.Limits))
+ Expect(pod.Spec.Containers[0].Resources.Requests).To(Equal(policyServer.Spec.Limits))
+
+ return nil
+ }, timeout, pollInterval).Should(Succeed())
+ })
+
+ When("the requests are updated", func() {
+ It("should update the PolicyServer pod with the new requests", func() {
+ By("updating the PolicyServer requests")
+ policyServer, err := getTestPolicyServer(policyServerName)
+ Expect(err).ToNot(HaveOccurred())
+
+ policyServer.Spec.Requests = corev1.ResourceList{
+ "cpu": resource.MustParse("50m"),
+ "memory": resource.MustParse("500Mi"),
+ }
+
+ Expect(
+ k8sClient.Update(ctx, policyServer),
+ ).To(Succeed())
+
+ By("updating the pod with the new requests")
+ Eventually(func(g Gomega) (*corev1.Container, error) {
+ pod, err := getTestPolicyServerPod(policyServerName)
+ if err != nil {
+ return nil, err
+ }
+
+ return &pod.Spec.Containers[0], nil
+ }, timeout, pollInterval).Should(
+ And(
+ HaveField("Resources.Requests", Equal(policyServer.Spec.Requests)),
+ HaveField("Resources.Limits", Equal(policyServer.Spec.Limits)),
+ ),
+ )
+ })
+ })
+ })
 })

controllers/utils_test.go

@@ -31,6 +31,7 @@ import (
  "github.com/onsi/gomega/types"
 
  admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+ appsv1 "k8s.io/api/apps/v1"
  corev1 "k8s.io/api/core/v1"
  k8spoliciesv1 "k8s.io/api/policy/v1"
  apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -168,6 +169,36 @@ func getTestPolicyServerService(policyServerName string) (*corev1.Service, error
  return &service, nil
 }
 
+func getTestPolicyServerDeployment(policyServerName string) (*appsv1.Deployment, error) {
+ policyServer := policiesv1.PolicyServer{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: policyServerName,
+ },
+ }
+ deploymentName := policyServer.NameWithPrefix()
+
+ deployment := appsv1.Deployment{}
+ if err := reconciler.APIReader.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: DeploymentsNamespace}, &deployment); err != nil {
+ return nil, errors.Join(errors.New("could not find Deployment owned by PolicyServer"), err)
+ }
+ return &deployment, nil
+}
+
+func getTestPolicyServerPod(policyServerName string) (*corev1.Pod, error) {
+ podList := corev1.PodList{}
+ if err := reconciler.APIReader.List(ctx, &podList, client.MatchingLabels{
+ constants.PolicyServerLabelKey: policyServerName,
+ }); err != nil {
+ return nil, errors.Join(errors.New("could not list Pods owned by PolicyServer"), err)
+ }
+
+ if len(podList.Items) == 0 {
+ return nil, errors.New("could not find Pod owned by PolicyServer")
+ }
+
+ return &podList.Items[0], nil
+}
+
 func getTestValidatingWebhookConfiguration(name string) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
  validatingWebhookConfiguration := admissionregistrationv1.ValidatingWebhookConfiguration{}
  if err := reconciler.APIReader.Get(ctx, client.ObjectKey{Name: name}, &validatingWebhookConfiguration); err != nil {
@@ -266,7 +297,8 @@ func policyServerPodDisruptionBudgetMatcher(policyServer *policiesv1.PolicyServe
  }),
  "MatchExpressions": Ignore(),
  })),
- })}),
+ }),
+ }),
  ),
  )
 }

docs/crds/CRD-docs-for-docs-repo.md

@@ -241,6 +241,8 @@ _Appears in:_
 | --- | --- |
 | `image` _string_ | Docker image name. |
 | `replicas` _integer_ | Replicas is the number of desired replicas. |
+| `minAvailable` _IntOrString_ | Number of policy server replicas that must be still available after the eviction |
+| `maxUnavailable` _IntOrString_ | Number of policy server replicas that can be unavailable after the eviction |
 | `annotations` _object (keys:string, values:string)_ | Annotations is an unstructured key value map stored with a resource that may be set by external tools to store and retrieve arbitrary metadata. They are not queryable and should be preserved when modifying objects. More info: http://kubernetes.io/docs/user-guide/annotations |
 | `env` _[EnvVar](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#envvar-v1-core) array_ | List of environment variables to set in the container. |
 | `serviceAccountName` _string_ | Name of the service account associated with the policy server. Namespace service account will be used if not specified. |
@@ -249,6 +251,9 @@ _Appears in:_
 | `sourceAuthorities` _object (keys:string, values:string array)_ | Key value map of registry URIs endpoints to a list of their associated PEM encoded certificate authorities that have to be used to verify the certificate used by the endpoint. The `sourceAuthorities` content format corresponds with the contents of the `source_authorities` key in `sources.yaml`. Reference for `sources.yaml` is found in the Kubewarden documentation in the reference section. |
 | `verificationConfig` _string_ | Name of VerificationConfig configmap in the same namespace, containing Sigstore verification configuration. The configuration must be under a key named verification-config in the Configmap. |
 | `securityContexts` _[PolicyServerSecurity](#policyserversecurity)_ | Security configuration to be used in the Policy Server workload. The field allows different configurations for the pod and containers. If set for the containers, this configuration will not be used in containers added by other controllers (e.g. telemetry sidecars) |
+| `affinity` _[Affinity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#affinity-v1-core)_ | Affinity rules for the associated Policy Server pods. |
+| `limits` _object (keys:[ResourceName](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#resourcename-v1-core), values:Quantity)_ | Limits describes the maximum amount of compute resources allowed. |
+| `requests` _object (keys:[ResourceName](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#resourcename-v1-core), values:Quantity)_ | Requests describes the minimum amount of compute resources required. If Request is omitted for, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value |
 
 
 

internal/pkg/admission/policy-server-deployment.go

@@ -125,6 +125,7 @@ func shouldUpdatePolicyServerDeployment(policyServer *policiesv1.PolicyServer, o
  originalDeployment.Spec.Template.Spec.SecurityContext != newDeployment.Spec.Template.Spec.SecurityContext ||
  originalDeployment.Annotations[constants.PolicyServerDeploymentConfigVersionAnnotation] != newDeployment.Annotations[constants.PolicyServerDeploymentConfigVersionAnnotation] ||
  !reflect.DeepEqual(originalDeployment.Spec.Template.Spec.Containers[0].Env, newDeployment.Spec.Template.Spec.Containers[0].Env) ||
+ !reflect.DeepEqual(originalDeployment.Spec.Template.Spec.Containers[0].Resources, newDeployment.Spec.Template.Spec.Containers[0].Resources) ||
  !reflect.DeepEqual(originalDeployment.Spec.Template.Spec.Affinity, newDeployment.Spec.Template.Spec.Affinity) ||
  !haveEqualAnnotationsWithoutRestart(originalDeployment, newDeployment), nil
 }
@@ -267,6 +268,10 @@ func (r *Reconciler) deployment(configMapVersion string, policyServer *policiesv
  },
  },
  },
+ Resources: corev1.ResourceRequirements{
+ Requests: policyServer.Spec.Requests,
+ Limits: policyServer.Spec.Limits,
+ },
  }
  if r.AlwaysAcceptAdmissionReviewsInDeploymentsNamespace {
  admissionContainer.Env = append(admissionContainer.Env, corev1.EnvVar{

pkg/apis/policies/v1/policyserver_types.go

@@ -99,6 +99,16 @@ type PolicyServerSpec struct {
  // Affinity rules for the associated Policy Server pods.
  // +optional
  Affinity corev1.Affinity `json:"affinity,omitempty"`
+
+ // Limits describes the maximum amount of compute resources allowed.
+ // +optional
+ Limits corev1.ResourceList `json:"limits,omitempty"`
+
+ // Requests describes the minimum amount of compute resources required.
+ // If Request is omitted for, it defaults to Limits if that is explicitly specified,
+ // otherwise to an implementation-defined value
+ // +optional
+ Requests corev1.ResourceList `json:"requests,omitempty"`
 }
 
 type ReconciliationTransitionReason string

pkg/apis/policies/v1/policyserver_webhook.go

@@ -18,10 +18,13 @@ package v1
 
 import (
  "context"
+ "errors"
  "fmt"
 
  "github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
  "github.com/kubewarden/kubewarden-controller/internal/pkg/policyserver"
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/apimachinery/pkg/api/resource"
  "k8s.io/apimachinery/pkg/runtime"
  validationutils "k8s.io/apimachinery/pkg/util/validation"
  ctrl "sigs.k8s.io/controller-runtime"
@@ -56,6 +59,16 @@ func (ps *PolicyServer) Default() {
  if ps.ObjectMeta.DeletionTimestamp == nil {
  controllerutil.AddFinalizer(ps, constants.KubewardenFinalizer)
  }
+
+ // Default the requests to the limits if not set
+ for limitName, limitQuantity := range ps.Spec.Limits {
+ if _, found := ps.Spec.Requests[limitName]; !found {
+ if ps.Spec.Requests == nil {
+ ps.Spec.Requests = make(corev1.ResourceList)
+ }
+ ps.Spec.Requests[limitName] = limitQuantity
+ }
+ }
 }
 
 // +kubebuilder:webhook:path=/validate-policies-kubewarden-io-v1-policyserver,mutating=false,failurePolicy=fail,sideEffects=None,groups=policies.kubewarden.io,resources=policyservers,verbs=create;update,versions=v1,name=vpolicyserver.kb.io,admissionReviewVersions=v1
@@ -86,7 +99,12 @@ func (v *policyServerValidator) validate(ctx context.Context, obj runtime.Object
 
  // Kubernetes does not allow to set both MinAvailable and MaxUnavailable at the same time
  if policyServer.Spec.MinAvailable != nil && policyServer.Spec.MaxUnavailable != nil {
- return fmt.Errorf("minAvailable and maxUnavailable cannot be both set")
+ return errors.New("minAvailable and maxUnavailable cannot be both set")
+ }
+
+ err := validateLimitsAndRequests(policyServer.Spec.Limits, policyServer.Spec.Requests)
+ if err != nil {
+ return err
  }
 
  return nil
@@ -103,3 +121,31 @@ func (v *policyServerValidator) ValidateUpdate(ctx context.Context, _, obj runti
 func (v *policyServerValidator) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
  return nil, nil
 }
+
+func validateLimitsAndRequests(limits, requests corev1.ResourceList) error {
+ if requests == nil || limits == nil {
+ return nil
+ }
+
+ for limitName, limitQuantity := range limits {
+ if limitQuantity.Cmp(resource.Quantity{}) < 0 {
+ return fmt.Errorf("%s limit must be greater than or equal to 0", limitName)
+ }
+ }
+
+ for requestName, requestQuantity := range requests {
+ if requestQuantity.Cmp(resource.Quantity{}) < 0 {
+ return fmt.Errorf("%s request must be greater than or equal to 0", requestName)
+ }
+
+ limitQuantity, ok := limits[requestName]
+ if !ok {
+ continue
+ }
+
+ if requestQuantity.Cmp(limitQuantity) > 0 {
+ return fmt.Errorf("request must be less than or equal to %s limit", requestName)
+ }
+ }
+ return nil
+}