From 407576a2a848a1596d9e89f3260590dfcc007ea0 Mon Sep 17 00:00:00 2001 From: Simone Tiraboschi Date: Fri, 19 May 2023 06:40:43 +0200 Subject: [PATCH] [release-1.9] Revert PR #2206 as it's not needed anymore (#2341) (#2347) Remove the support of the kubevirt.io/cpu-limit-to-request-ratio and the kubevirt.io/memory-limit-to-request-ratio annotations, as this workaround does now work as expected. Revert PR #2206 as it's not needed anymore This is a manual cherry-pick of: #2341 Signed-off-by: Nahshon Unna Tsameret <60659093+nunnatsa@users.noreply.github.com> Co-authored-by: Nahshon Unna Tsameret <60659093+nunnatsa@users.noreply.github.com> --- deploy/cluster_role.yaml | 1 - ...operator.v1.9.0.clusterserviceversion.yaml | 24 -- ...operator.v1.9.0.clusterserviceversion.yaml | 26 +- deploy/webhooks.yaml | 28 -- docs/cluster-configuration.md | 73 ---- pkg/components/components.go | 30 +- pkg/util/consts.go | 24 +- pkg/webhooks/mutator/virt-launcher-mutator.go | 230 ------------ .../mutator/virt-launcher-mutator_test.go | 339 ------------------ pkg/webhooks/setup.go | 2 - 10 files changed, 13 insertions(+), 764 deletions(-) delete mode 100644 pkg/webhooks/mutator/virt-launcher-mutator.go delete mode 100644 pkg/webhooks/mutator/virt-launcher-mutator_test.go diff --git a/deploy/cluster_role.yaml b/deploy/cluster_role.yaml index bacfe7dda6..472218c1e4 100644 --- a/deploy/cluster_role.yaml +++ b/deploy/cluster_role.yaml @@ -521,7 +521,6 @@ rules: - "" resources: - pods - - resourcequotas verbs: - get - list diff --git a/deploy/index-image/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml b/deploy/index-image/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml index e58c46a9ec..78d7d74c57 100644 --- a/deploy/index-image/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml +++ b/deploy/index-image/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml @@ -295,7 +295,6 @@ spec: - "" resources: - pods - - resourcequotas verbs: - get - list @@ -3928,29 +3927,6 @@ spec: timeoutSeconds: 10 type: MutatingAdmissionWebhook webhookPath: /mutate-ns-hco-kubevirt-io - - admissionReviewVersions: - - v1beta1 - - v1 - containerPort: 4343 - deploymentName: hco-webhook - failurePolicy: Fail - generateName: mutate-virt-launcher-hco.kubevirt.io - objectSelector: - matchLabels: - kubevirt.io: virt-launcher - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - resources: - - pods - sideEffects: NoneOnDryRun - timeoutSeconds: 10 - type: MutatingAdmissionWebhook - webhookPath: /mutate-virt-launcher-hco-kubevirt-io - admissionReviewVersions: - v1beta1 - v1 diff --git a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml index 9bd094f478..276c61fb43 100644 --- a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml +++ b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.9.0/manifests/kubevirt-hyperconverged-operator.v1.9.0.clusterserviceversion.yaml @@ -9,7 +9,7 @@ metadata: certified: "false" console.openshift.io/disable-operand-delete: "true" containerImage: quay.io/kubevirt/hyperconverged-cluster-operator:1.9.0-unstable - createdAt: "2023-04-29 05:12:12" + createdAt: "2023-05-18 15:22:36" description: A unified operator deploying and controlling KubeVirt and its supporting operators with opinionated defaults operatorframework.io/initialization-resource: '{"apiVersion":"hco.kubevirt.io/v1beta1","kind":"HyperConverged","metadata":{"annotations":{"deployOVS":"false"},"name":"kubevirt-hyperconverged","namespace":"kubevirt-hyperconverged"},"spec":{}}' @@ -295,7 +295,6 @@ spec: - "" resources: - pods - - resourcequotas verbs: - get - list @@ -3928,29 +3927,6 @@ spec: timeoutSeconds: 10 type: MutatingAdmissionWebhook webhookPath: /mutate-ns-hco-kubevirt-io - - admissionReviewVersions: - - v1beta1 - - v1 - containerPort: 4343 - deploymentName: hco-webhook - failurePolicy: Fail - generateName: mutate-virt-launcher-hco.kubevirt.io - objectSelector: - matchLabels: - kubevirt.io: virt-launcher - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - resources: - - pods - sideEffects: NoneOnDryRun - timeoutSeconds: 10 - type: MutatingAdmissionWebhook - webhookPath: /mutate-virt-launcher-hco-kubevirt-io - admissionReviewVersions: - v1beta1 - v1 diff --git a/deploy/webhooks.yaml b/deploy/webhooks.yaml index 8df47ed9bb..527405a7d1 100644 --- a/deploy/webhooks.yaml +++ b/deploy/webhooks.yaml @@ -213,34 +213,6 @@ metadata: labels: name: hyperconverged-cluster-webhook webhooks: -- name: mutate-virt-launcher-hco.kubevirt.io - admissionReviewVersions: - - v1beta1 - - v1 - clientConfig: - # caBundle: WILL BE INJECTED BY CERT-MANAGER BECAUSE OF THE ANNOTATION - service: - name: hyperconverged-cluster-webhook-service - namespace: kubevirt-hyperconverged - path: /mutate-virt-launcher-hco-kubevirt-io - port: 4343 - failurePolicy: Fail - matchPolicy: Equivalent - objectSelector: - matchLabels: - kubevirt.io: virt-launcher - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - resources: - - pods - scope: '*' - sideEffects: None - timeoutSeconds: 30 - name: mutate-hyperconverged-hco.kubevirt.io admissionReviewVersions: - v1beta1 diff --git a/docs/cluster-configuration.md b/docs/cluster-configuration.md index 1b99e29dde..649314dd07 100644 --- a/docs/cluster-configuration.md +++ b/docs/cluster-configuration.md @@ -1094,76 +1094,3 @@ The `tuningPolicy` feature can be enabled using the following patch: ```bash kubectl patch -n kubevirt-hyperconverged hco kubevirt-hyperconverged --type=json -p='[{"op": "add", "path": "/spec/tuningPolicy", "value": "annotation"}]' ``` - -## Enforce CPU/memory limits on namespaces with ResourceQuotas -This mechanism allows to align Kubevirt `virt-launcher` Pods with ResourceQuotas that are applied to the -namespace. - -As an example, let's say that we are creating Kubevirt's `examples/vmi-fedora.yaml`. - -When this VMI is created, the following `virt-launcher` pod is created (some details are omitted for simplicity): -```yaml -kind: Pod -metadata: - name: virt-launcher-vmi-fedora-lzzn6 - namespace: kubevirt-hyperconverged -spec: - containers: - name: compute - resources: - limits: - devices.kubevirt.io/kvm: "1" - devices.kubevirt.io/tun: "1" - devices.kubevirt.io/vhost-net: "1" - requests: - cpu: 100m - devices.kubevirt.io/kvm: "1" - devices.kubevirt.io/tun: "1" - devices.kubevirt.io/vhost-net: "1" - ephemeral-storage: 50M - memory: "1279755392" -``` - -As can be seen, this `virt-launcher` has only CPU and memory requests - but not limits. This means that if this VMI is being created in a namespace that has a ResourceQuota defined in it - the virt-launcher Pod won't be able to start. This now can be solved using this feature. - -To enable this mechanism, first a ratio between memory/CPU limits to request needs to be defined as an annotation in HCO object: -```yaml -kind: HyperConverged -metadata: - annotations: - kubevirt.io/cpu-limit-to-request-ratio: "2" - kubevirt.io/memory-limit-to-request-ratio: "1.5" -``` - -In addition, a ResourceQuota needs to exist on the relevant namespace. As an example, it's possible to create the following object: -```yaml -apiVersion: v1 -kind: ResourceQuota -metadata: - name: test-rq -spec: - hard: - limits.cpu: "200" - limits.memory: "2000G" -``` -Please take into account that if a ResourceQuota only sets a limit on `limits.cpu` or `limits.memory`, then CPU/memory limits will be set accordingly. If multiple ResourceQuota exist within the relevant namespace, it takes only one of then to limit CPU/memory limits in order to enforce these limits. - -When these annotations are enabled along with a ResourceQuota object, a mutating webhook that's targeted to virt-launcher pods will enforce limits on the pod. It would now look like the following: -```yaml - resources: - limits: - cpu: 200m - devices.kubevirt.io/kvm: "1" - devices.kubevirt.io/tun: "1" - devices.kubevirt.io/vhost-net: "1" - memory: "1919633088" - requests: - cpu: 100m - devices.kubevirt.io/kvm: "1" - devices.kubevirt.io/tun: "1" - devices.kubevirt.io/vhost-net: "1" - ephemeral-storage: 50M - memory: "1279755392" -``` - -Bear in mind that if the limit is already set, HCO will not fix it according to the annotations. diff --git a/pkg/components/components.go b/pkg/components/components.go index 6586f1c702..62a9eabc8f 100644 --- a/pkg/components/components.go +++ b/pkg/components/components.go @@ -471,7 +471,7 @@ func GetClusterPermissions() []rbacv1.PolicyRule { roleWithAllPermissions("", stringListToSlice("services")), { APIGroups: emptyAPIGroup, - Resources: stringListToSlice("pods", "resourcequotas"), + Resources: stringListToSlice("pods"), Verbs: stringListToSlice("get", "list", "watch"), }, { @@ -809,33 +809,6 @@ func GetCSVBase(params *CSVBaseParams) *csvv1alpha1.ClusterServiceVersion { WebhookPath: pointer.String(util.HCONSWebhookPath), } - mutatingVirtLauncherWebhook := csvv1alpha1.WebhookDescription{ - GenerateName: util.HcoMutatingWebhookVirtLauncher, - Type: csvv1alpha1.MutatingAdmissionWebhook, - DeploymentName: hcoWhDeploymentName, - ContainerPort: util.WebhookPort, - AdmissionReviewVersions: stringListToSlice("v1beta1", "v1"), - SideEffects: &mutatingWebhookSideEffects, - FailurePolicy: &failurePolicy, - TimeoutSeconds: &webhookTimeout, - ObjectSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"kubevirt.io": "virt-launcher"}, - }, - Rules: []admissionregistrationv1.RuleWithOperations{ - { - Operations: []admissionregistrationv1.OperationType{ - admissionregistrationv1.Create, - }, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{""}, - APIVersions: stringListToSlice("v1"), - Resources: stringListToSlice("pods"), - }, - }, - }, - WebhookPath: pointer.String(util.HCOVirtLauncherWebhookPath), - } - mutatingHyperConvergedWebhook := csvv1alpha1.WebhookDescription{ GenerateName: util.HcoMutatingWebhookHyperConverged, Type: csvv1alpha1.MutatingAdmissionWebhook, @@ -953,7 +926,6 @@ func GetCSVBase(params *CSVBaseParams) *csvv1alpha1.ClusterServiceVersion { WebhookDefinitions: []csvv1alpha1.WebhookDescription{ validatingWebhook, mutatingNamespaceWebhook, - mutatingVirtLauncherWebhook, mutatingHyperConvergedWebhook, }, CustomResourceDefinitions: csvv1alpha1.CustomResourceDefinitions{ diff --git a/pkg/util/consts.go b/pkg/util/consts.go index 539b495d06..ae1abc8acf 100644 --- a/pkg/util/consts.go +++ b/pkg/util/consts.go @@ -17,7 +17,6 @@ const ( KvUiPluginImageEnvV = "KV_CONSOLE_PLUGIN_IMAGE" HcoValidatingWebhook = "validate-hco.kubevirt.io" HcoMutatingWebhookNS = "mutate-ns-hco.kubevirt.io" - HcoMutatingWebhookVirtLauncher = "mutate-virt-launcher-hco.kubevirt.io" HcoMutatingWebhookHyperConverged = "mutate-hyperconverged-hco.kubevirt.io" AppLabel = "app" UndefinedNamespace = "" @@ -48,18 +47,17 @@ const ( PrometheusNSLabel = "openshift.io/cluster-monitoring" // HyperConvergedName is the name of the HyperConverged resource that will be reconciled - HyperConvergedName = "kubevirt-hyperconverged" - MetricsHost = "0.0.0.0" - MetricsPort int32 = 8383 - HealthProbeHost = "0.0.0.0" - HealthProbePort int32 = 6060 - ReadinessEndpointName = "/readyz" - LivenessEndpointName = "/livez" - HCOWebhookPath = "/validate-hco-kubevirt-io-v1beta1-hyperconverged" - HCOMutatingWebhookPath = "/mutate-hco-kubevirt-io-v1beta1-hyperconverged" - HCONSWebhookPath = "/mutate-ns-hco-kubevirt-io" - HCOVirtLauncherWebhookPath = "/mutate-virt-launcher-hco-kubevirt-io" - WebhookPort = 4343 + HyperConvergedName = "kubevirt-hyperconverged" + MetricsHost = "0.0.0.0" + MetricsPort int32 = 8383 + HealthProbeHost = "0.0.0.0" + HealthProbePort int32 = 6060 + ReadinessEndpointName = "/readyz" + LivenessEndpointName = "/livez" + HCOWebhookPath = "/validate-hco-kubevirt-io-v1beta1-hyperconverged" + HCOMutatingWebhookPath = "/mutate-hco-kubevirt-io-v1beta1-hyperconverged" + HCONSWebhookPath = "/mutate-ns-hco-kubevirt-io" + WebhookPort = 4343 WebhookCertName = "apiserver.crt" WebhookKeyName = "apiserver.key" diff --git a/pkg/webhooks/mutator/virt-launcher-mutator.go b/pkg/webhooks/mutator/virt-launcher-mutator.go deleted file mode 100644 index cd0d3f6616..0000000000 --- a/pkg/webhooks/mutator/virt-launcher-mutator.go +++ /dev/null @@ -1,230 +0,0 @@ -package mutator - -import ( - "context" - "fmt" - "net/http" - "strconv" - - "k8s.io/apimachinery/pkg/api/resource" - - "github.com/go-logr/logr" - "gomodules.xyz/jsonpatch/v2" - admissionv1 "k8s.io/api/admission/v1" - k8sv1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1" -) - -var _ admission.Handler = &VirtLauncherMutator{} - -const ( - cpuLimitToRequestRatioAnnotation = "kubevirt.io/cpu-limit-to-request-ratio" - memoryLimitToRequestRatioAnnotation = "kubevirt.io/memory-limit-to-request-ratio" - - launcherMutatorStr = "virtLauncherMutator" -) - -type VirtLauncherMutator struct { - cli client.Client - hcoNamespace string - decoder *admission.Decoder - logger logr.Logger -} - -func NewVirtLauncherMutator(cli client.Client, hcoNamespace string) *VirtLauncherMutator { - return &VirtLauncherMutator{ - cli: cli, - hcoNamespace: hcoNamespace, - logger: log.Log.WithName("virt-launcher mutator"), - } -} - -func (m *VirtLauncherMutator) Handle(ctx context.Context, req admission.Request) admission.Response { - m.logInfo("Starting virt-launcher mutator handling") - - if req.Operation != admissionv1.Create { - m.logInfo("not a pod creation - ignoring") - return admission.Allowed(ignoreOperationMessage) - } - - launcherPod := &k8sv1.Pod{} - err := m.decoder.Decode(req, launcherPod) - if err != nil { - m.logErr(err, "cannot decode virt-launcher pod") - return admission.Errored(http.StatusBadRequest, err) - } - originalPod := launcherPod.DeepCopy() - - hco, err := getHcoObject(ctx, m.cli, m.hcoNamespace) - if err != nil { - m.logErr(err, "cannot get the HyperConverged object") - return admission.Errored(http.StatusBadRequest, err) - } - - enforceCpuLimits, enforceMemoryLimits, err := m.getResourcesToEnforce(ctx, launcherPod.Namespace, hco) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - if !enforceCpuLimits && !enforceMemoryLimits { - return admission.Allowed(ignoreOperationMessage) - } - - if err := m.handleVirtLauncherCreation(launcherPod, hco, enforceCpuLimits, enforceMemoryLimits); err != nil { - m.logErr(err, "failed handling launcher pod %s", launcherPod.Name) - return admission.Errored(http.StatusBadRequest, err) - } - - allowResponse := m.getAllowedResponseWithPatches(launcherPod, originalPod) - m.logInfo("mutation completed successfully for pod %s", launcherPod.Name) - return allowResponse -} - -func (m *VirtLauncherMutator) handleVirtLauncherCreation(launcherPod *k8sv1.Pod, hco *v1beta1.HyperConverged, enforceCpuLimits, enforceMemoryLimits bool) error { - var cpuRatioStr, memRatioStr string - - if enforceCpuLimits { - cpuRatioStr = hco.Annotations[cpuLimitToRequestRatioAnnotation] - err := m.setResourceRatio(launcherPod, cpuRatioStr, cpuLimitToRequestRatioAnnotation, k8sv1.ResourceCPU) - if err != nil { - return err - } - } - if enforceMemoryLimits { - memRatioStr = hco.Annotations[memoryLimitToRequestRatioAnnotation] - err := m.setResourceRatio(launcherPod, memRatioStr, memoryLimitToRequestRatioAnnotation, k8sv1.ResourceMemory) - if err != nil { - return err - } - } - - return nil -} - -func (m *VirtLauncherMutator) setResourceRatio(launcherPod *k8sv1.Pod, ratioStr, annotationKey string, resourceName k8sv1.ResourceName) error { - ratio, err := strconv.ParseFloat(ratioStr, 64) - if err != nil { - return fmt.Errorf("%s can't parse ratio %s to float: %w. The ratio is the value of annotation key %s", launcherMutatorStr, ratioStr, err, annotationKey) - } - - if ratio < 1 { - return fmt.Errorf("%s doesn't support negative ratio: %v. The ratio is the value of annotation key %s", launcherMutatorStr, ratio, annotationKey) - } - - for i, container := range launcherPod.Spec.Containers { - request, requestExists := container.Resources.Requests[resourceName] - _, limitExists := container.Resources.Limits[resourceName] - - if requestExists && !limitExists { - newQuantity := m.multiplyResource(request, ratio) - m.logInfo("Replacing %s old quantity (%s) with new quantity (%s) for pod %s/%s, UID: %s, accodring to ratio: %v", - resourceName, request.String(), newQuantity.String(), launcherPod.Namespace, launcherPod.Name, launcherPod.UID, ratio) - - launcherPod.Spec.Containers[i].Resources.Limits[resourceName] = newQuantity - } - } - - return nil -} - -func (m *VirtLauncherMutator) multiplyResource(quantity resource.Quantity, ratio float64) resource.Quantity { - oldValue := quantity.ScaledValue(resource.Milli) - newValue := ratio * float64(oldValue) - newQuantity := *resource.NewScaledQuantity(int64(newValue), resource.Milli) - - return newQuantity -} - -// InjectDecoder injects the decoder. -// WebhookHandler implements admission.DecoderInjector so a decoder will be automatically injected. -func (m *VirtLauncherMutator) InjectDecoder(d *admission.Decoder) error { - m.decoder = d - return nil -} - -func (m *VirtLauncherMutator) logInfo(format string, a ...any) { - m.logger.Info(fmt.Sprintf(format, a...)) -} - -func (m *VirtLauncherMutator) logErr(err error, format string, a ...any) { - m.logger.Error(err, fmt.Sprintf(format, a...)) -} - -func (m *VirtLauncherMutator) getAllowedResponseWithPatches(launcherPod, originalPod *k8sv1.Pod) admission.Response { - const patchReplaceOp = "replace" - allowedResponse := admission.Allowed("") - - if !equality.Semantic.DeepEqual(launcherPod.Spec, originalPod.Spec) { - m.logInfo("generating spec replace patch for pod %s", launcherPod.Name) - allowedResponse.Patches = append(allowedResponse.Patches, - jsonpatch.Operation{ - Operation: patchReplaceOp, - Path: "/spec", - Value: launcherPod.Spec, - }, - ) - } - - if !equality.Semantic.DeepEqual(launcherPod.ObjectMeta, originalPod.ObjectMeta) { - m.logInfo("generating metadata replace patch for pod %s", launcherPod.Name) - allowedResponse.Patches = append(allowedResponse.Patches, - jsonpatch.Operation{ - Operation: patchReplaceOp, - Path: "/metadata", - Value: launcherPod.ObjectMeta, - }, - ) - } - - return allowedResponse -} - -func (m *VirtLauncherMutator) listResourceQuotas(ctx context.Context, namespace string) ([]k8sv1.ResourceQuota, error) { - quotaList := &k8sv1.ResourceQuotaList{} - err := m.cli.List(ctx, quotaList, &client.ListOptions{Namespace: namespace}) - if err != nil { - return nil, err - } - - return quotaList.Items, nil -} - -func (m *VirtLauncherMutator) getResourcesToEnforce(ctx context.Context, namespace string, hco *v1beta1.HyperConverged) (enforceCpuLimits, enforceMemoryLimits bool, err error) { - _, cpuRatioExists := hco.Annotations[cpuLimitToRequestRatioAnnotation] - _, memRatioExists := hco.Annotations[memoryLimitToRequestRatioAnnotation] - - if !cpuRatioExists && !memRatioExists { - return false, false, nil - } - - resourceQuotaList, err := m.listResourceQuotas(ctx, namespace) - if err != nil { - m.logErr(err, "could not list resource quotas") - return - } - - isQuotaEnforcingResource := func(resourceQuota k8sv1.ResourceQuota, resource k8sv1.ResourceName) bool { - _, exists := resourceQuota.Spec.Hard[resource] - return exists - } - - for _, resourceQuota := range resourceQuotaList { - if cpuRatioExists && isQuotaEnforcingResource(resourceQuota, "limits.cpu") { - enforceCpuLimits = true - } - if memRatioExists && isQuotaEnforcingResource(resourceQuota, "limits.memory") { - enforceMemoryLimits = true - } - - if enforceCpuLimits && enforceMemoryLimits { - break - } - } - - return -} diff --git a/pkg/webhooks/mutator/virt-launcher-mutator_test.go b/pkg/webhooks/mutator/virt-launcher-mutator_test.go deleted file mode 100644 index fc2f7a7859..0000000000 --- a/pkg/webhooks/mutator/virt-launcher-mutator_test.go +++ /dev/null @@ -1,339 +0,0 @@ -package mutator - -import ( - "context" - "fmt" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - admissionv1 "k8s.io/api/admission/v1" - k8sv1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" - "k8s.io/apimachinery/pkg/util/rand" - "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1" - "github.com/kubevirt/hyperconverged-cluster-operator/controllers/commonTestUtils" - "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" -) - -const ( - hcoNamespace = HcoValidNamespace - podNamespace = "fake-namespace" -) - -var _ = Describe("virt-launcher webhook mutator", func() { - - Describe("resource multiplier", func() { - DescribeTable("produces correct results", func(inputQuantityStr, expectedOutputQuantityStr string, ratio float64) { - inputQuantity := resource.MustParse(inputQuantityStr) - expectedOutputQuantity := resource.MustParse(expectedOutputQuantityStr) - - mutator := getVirtLauncherMutator() - actualOutput := mutator.multiplyResource(inputQuantity, ratio) - Expect(actualOutput.Equal(expectedOutputQuantity)).To(BeTrue(), fmt.Sprintf("expected %s to equal %s", actualOutput.String(), expectedOutputQuantity.String())) - }, - Entry("CPU: 100m with ratio 2", "100m", "200m", 2.0), - Entry("CPU: 700m with ratio 2", "700m", "1400m", 2.0), - Entry("CPU: 2.4 with ratio 2", "2.4", "4800m", 2.0), - Entry("CPU: 0.4 with ratio 2", "0.4", "800m", 2.0), - Entry("CPU: 200m with ratio 0.5", "200m", "100m", 0.5), - Entry("CPU: 1 with ratio 0.5", "1", "500m", 0.5), - - Entry("Memory: 256 with ratio 3.0", "256", "768", 3.0), - Entry("Memory: 256M with ratio 3.0", "256M", "768M", 3.0), - Entry("Memory: 256Mi with ratio 3.0", "256Mi", "768Mi", 3.0), - Entry("Memory: 256Gi with ratio 3.0", "256Gi", "768Gi", 3.0), - Entry("Memory: 700M with ratio 3.0", "700M", "2100M", 3.0), - Entry("Memory: 256M with ratio 3.0", "260M", "52M", 0.2), - ) - - }) - - DescribeTable("set resource ratio", func(memRatio, cpuRatio string, podResources, expectedResources k8sv1.ResourceRequirements) { - mutator := getVirtLauncherMutator() - launcherPod := getFakeLauncherPod() - hco := getHco() - hco.Annotations = map[string]string{ - cpuLimitToRequestRatioAnnotation: cpuRatio, - memoryLimitToRequestRatioAnnotation: memRatio, - } - - launcherPod.Spec.Containers[0].Resources = podResources - Expect(mutator.handleVirtLauncherCreation(launcherPod, hco, true, true)).To(Succeed()) - - resources := launcherPod.Spec.Containers[0].Resources - Expect(resources.Limits[k8sv1.ResourceCPU].Equal(expectedResources.Limits[k8sv1.ResourceCPU])).To(BeTrue()) - Expect(resources.Requests[k8sv1.ResourceCPU].Equal(expectedResources.Requests[k8sv1.ResourceCPU])).To(BeTrue()) - Expect(resources.Limits[k8sv1.ResourceMemory].Equal(expectedResources.Limits[k8sv1.ResourceMemory])).To(BeTrue()) - Expect(resources.Requests[k8sv1.ResourceMemory].Equal(expectedResources.Requests[k8sv1.ResourceMemory])).To(BeTrue()) - }, - Entry("200m cpu with ratio 2", "1.0", "2.0", - getResources(withCpuRequest("200m")), - getResources(withCpuRequest("200m"), withCpuLimit("400m")), - ), - Entry("100M memory with ratio 1.5", "1.5", "1.0", - getResources(withMemRequest("100M")), - getResources(withMemRequest("100M"), withMemLimit("150M")), - ), - Entry("200m cpu with ratio 2, 100M memory with ratio 1.5", "1.5", "2.0", - getResources(withCpuRequest("200m"), withMemRequest("100M")), - getResources(withCpuRequest("200m"), withCpuLimit("400m"), withMemRequest("100M"), withMemLimit("150M")), - ), - Entry("requests and limits are already set", "1.5", "2.0", - getResources(withCpuRequest("200m"), withCpuLimit("400m"), withMemRequest("100M"), withMemLimit("150M")), - getResources(withCpuRequest("200m"), withCpuLimit("400m"), withMemRequest("100M"), withMemLimit("150M")), - ), - Entry("requests and limits aren't set - nothing should be done", "1.5", "2.0", - getResources(), - getResources(), - ), - ) - - Context("resources to enforce", func() { - const ( - setRatio, dontSetRatio = true, false - setLimit, dontSetLimit = true, false - shouldEnforce, shouldNotEnforce = true, false - ) - - DescribeTable("should behave as expected", func(resourceName k8sv1.ResourceName, setRatio, setResourceQuotaLimit, shouldEnforce bool) { - Expect(resourceName).To(Or(Equal(k8sv1.ResourceMemory), Equal(k8sv1.ResourceCPU))) - - hco := getHco() - if setRatio { - if resourceName == k8sv1.ResourceCPU { - hco.Annotations[cpuLimitToRequestRatioAnnotation] = "1.2" - } else { - hco.Annotations[memoryLimitToRequestRatioAnnotation] = "3.4" - } - } - - mutator := getVirtLauncherMutatorWithoutResourceQuotas(true, true) - if setResourceQuotaLimit { - if resourceName == k8sv1.ResourceCPU { - mutator = getVirtLauncherMutatorWithoutResourceQuotas(false, true) - } else { - mutator = getVirtLauncherMutatorWithoutResourceQuotas(true, false) - } - } - - enforceCpuLimits, enforceMemoryLimits, err := mutator.getResourcesToEnforce(context.TODO(), podNamespace, hco) - Expect(err).ToNot(HaveOccurred()) - - if resourceName == k8sv1.ResourceCPU { - Expect(enforceCpuLimits).To(Equal(shouldEnforce)) - } else { - Expect(enforceMemoryLimits).To(Equal(shouldEnforce)) - } - }, - Entry("memory: setRatio, setLimit - shouldEnforce", k8sv1.ResourceMemory, setRatio, setLimit, shouldEnforce), - Entry("memory: setRatio, dontSetLimit - shouldNotEnforce", k8sv1.ResourceMemory, setRatio, dontSetLimit, shouldNotEnforce), - Entry("memory: dontSetRatio, setLimit - shouldNotEnforce", k8sv1.ResourceMemory, dontSetRatio, setLimit, shouldNotEnforce), - Entry("memory: dontSetRatio, dontSetLimit - shouldNotEnforce", k8sv1.ResourceMemory, dontSetRatio, dontSetLimit, shouldNotEnforce), - - Entry("cpu: setRatio, setLimit - shouldEnforce", k8sv1.ResourceCPU, setRatio, setLimit, shouldEnforce), - Entry("cpu: setRatio, dontSetLimit - shouldNotEnforce", k8sv1.ResourceCPU, setRatio, dontSetLimit, shouldNotEnforce), - Entry("cpu: dontSetRatio, setLimit - shouldNotEnforce", k8sv1.ResourceCPU, dontSetRatio, setLimit, shouldNotEnforce), - Entry("cpu: dontSetRatio, dontSetLimit - shouldNotEnforce", k8sv1.ResourceCPU, dontSetRatio, dontSetLimit, shouldNotEnforce), - ) - }) - - Context("invalid requests", func() { - const resourceAnnotationKey = "fake-key" // this is not important for this test - - DescribeTable("invalid ratio", func(ratio string, resourceName k8sv1.ResourceName) { - launcherPod := getFakeLauncherPod() - mutator := getVirtLauncherMutator() - - Expect(mutator.setResourceRatio(launcherPod, ratio, resourceAnnotationKey, resourceName)).ToNot(Succeed()) - }, - Entry("zero ratio", "0", k8sv1.ResourceCPU), - Entry("negative ratio", "-1.2", k8sv1.ResourceMemory), - ) - - Context("objects do not exist", func() { - newRequest := func(operation admissionv1.Operation, object runtime.Object, encoder runtime.Encoder) admissionv1.AdmissionRequest { - return admissionv1.AdmissionRequest{ - Operation: operation, - Object: runtime.RawExtension{ - Raw: []byte(runtime.EncodeOrDie(encoder, object)), - Object: object, - }, - } - } - - It("HCO CR object does not exist", func() { - codecFactory := serializer.NewCodecFactory(scheme.Scheme) - corev1Codec := codecFactory.LegacyCodec(k8sv1.SchemeGroupVersion) - - launcherPod := getFakeLauncherPod() - mutator := getVirtLauncherMutatorWithoutHco() - req := admission.Request{AdmissionRequest: newRequest(admissionv1.Create, launcherPod, corev1Codec)} - - res := mutator.Handle(context.TODO(), req) - Expect(res.Allowed).To(BeFalse()) - Expect(res.Result.Message).To(ContainSubstring("not found")) - }) - }) - - It("should not apply if only limit is set", func() { - launcherPod := getFakeLauncherPod() - mutator := getVirtLauncherMutator() - - launcherPod.Spec.Containers[0].Resources = k8sv1.ResourceRequirements{ - Limits: map[k8sv1.ResourceName]resource.Quantity{ - k8sv1.ResourceCPU: resource.MustParse("1"), - k8sv1.ResourceMemory: resource.MustParse("1"), - }, - } - - const ratio = "1.23" - Expect(mutator.setResourceRatio(launcherPod, ratio, resourceAnnotationKey, k8sv1.ResourceCPU)).To(Succeed()) - - Expect(mutator.setResourceRatio(launcherPod, ratio, resourceAnnotationKey, k8sv1.ResourceMemory)).To(Succeed()) - - Expect(launcherPod.Spec.Containers[0].Resources.Requests).To(BeEmpty()) - }) - }) - - DescribeTable("any operation other than CREATE should be allowed", func(operation admissionv1.Operation) { - mutator := getVirtLauncherMutator() - codecFactory := serializer.NewCodecFactory(scheme.Scheme) - corev1Codec := codecFactory.LegacyCodec(k8sv1.SchemeGroupVersion) - launcherPod := getFakeLauncherPod() - - req := admission.Request{AdmissionRequest: newRequest(admissionv1.Create, launcherPod, corev1Codec)} - - res := mutator.Handle(context.TODO(), req) - Expect(res.Allowed).To(BeFalse()) - }, - Entry("update", admissionv1.Update), - Entry("update", admissionv1.Delete), - Entry("update", admissionv1.Connect), - ) - -}) - -func getVirtLauncherMutator() *VirtLauncherMutator { - return getVirtLauncherMutatorHelper(true, true, true) -} - -func getVirtLauncherMutatorWithoutHco() *VirtLauncherMutator { - return getVirtLauncherMutatorHelper(false, true, true) -} - -func getVirtLauncherMutatorWithoutResourceQuotas(avoidCpuLimit, avoidMemoryLimit bool) *VirtLauncherMutator { - return getVirtLauncherMutatorHelper(true, !avoidCpuLimit, !avoidMemoryLimit) -} - -func getVirtLauncherMutatorHelper(hcoExists, resourceQuotaCpuExists, resourceQuotaMemoryExists bool) *VirtLauncherMutator { - var cli *commonTestUtils.HcoTestClient - var clusterObjects []runtime.Object - - if hcoExists { - clusterObjects = append(clusterObjects, getHco()) - } - if resourceQuotaCpuExists { - clusterObjects = append(clusterObjects, getResourceQuota(true, false)) - } - if resourceQuotaMemoryExists { - clusterObjects = append(clusterObjects, getResourceQuota(false, true)) - } - - cli = commonTestUtils.InitClient(clusterObjects) - mutator := NewVirtLauncherMutator(cli, hcoNamespace) - - decoder, err := admission.NewDecoder(scheme.Scheme) - ExpectWithOffset(1, err).ShouldNot(HaveOccurred()) - ExpectWithOffset(1, mutator.InjectDecoder(decoder)).Should(Succeed()) - - return mutator -} - -func getHco() *v1beta1.HyperConverged { - return &v1beta1.HyperConverged{ - ObjectMeta: metav1.ObjectMeta{ - Name: util.HyperConvergedName, - Namespace: HcoValidNamespace, - Annotations: map[string]string{}, - }, - Spec: v1beta1.HyperConvergedSpec{}, - } -} - -func getFakeLauncherPod() *k8sv1.Pod { - return &k8sv1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "virt-launcher-vmi-" + rand.String(5), - Namespace: podNamespace, - }, - Spec: k8sv1.PodSpec{ - Containers: []k8sv1.Container{k8sv1.Container{}}, - }, - } -} - -func getResourceQuota(toLimitCPU, toLimitMemory bool) *k8sv1.ResourceQuota { - rq := &k8sv1.ResourceQuota{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-resource-quota" + rand.String(5), - Namespace: podNamespace, - }, - Spec: k8sv1.ResourceQuotaSpec{ - Hard: map[k8sv1.ResourceName]resource.Quantity{}, - }, - } - - if toLimitCPU { - rq.Spec.Hard["limits.cpu"] = resource.MustParse("3000") - } - if toLimitMemory { - rq.Spec.Hard["limits.memory"] = resource.MustParse("3000G") - } - - return rq -} - -type resourceOption func(*k8sv1.ResourceRequirements) - -func getResources(options ...resourceOption) k8sv1.ResourceRequirements { - r := k8sv1.ResourceRequirements{ - Limits: map[k8sv1.ResourceName]resource.Quantity{}, - Requests: map[k8sv1.ResourceName]resource.Quantity{}, - } - - for _, option := range options { - option(&r) - } - - return r -} - -func withCpuRequest(quantityStr string) resourceOption { - return func(r *k8sv1.ResourceRequirements) { - r.Requests[k8sv1.ResourceCPU] = resource.MustParse(quantityStr) - } -} - -func withCpuLimit(quantityStr string) resourceOption { - return func(r *k8sv1.ResourceRequirements) { - r.Limits[k8sv1.ResourceCPU] = resource.MustParse(quantityStr) - } -} - -func withMemRequest(quantityStr string) resourceOption { - return func(r *k8sv1.ResourceRequirements) { - r.Requests[k8sv1.ResourceMemory] = resource.MustParse(quantityStr) - } -} - -func withMemLimit(quantityStr string) resourceOption { - return func(r *k8sv1.ResourceRequirements) { - r.Limits[k8sv1.ResourceMemory] = resource.MustParse(quantityStr) - } -} diff --git a/pkg/webhooks/setup.go b/pkg/webhooks/setup.go index b4b2d1a2d4..85b545acb8 100644 --- a/pkg/webhooks/setup.go +++ b/pkg/webhooks/setup.go @@ -47,7 +47,6 @@ func SetupWebhookWithManager(ctx context.Context, mgr ctrl.Manager, isOpenshift whHandler := validator.NewWebhookHandler(logger, mgr.GetClient(), operatorNsEnv, isOpenshift, hcoTlsSecurityProfile) nsMutator := mutator.NewNsMutator(mgr.GetClient(), operatorNsEnv) - virtLauncherMutator := mutator.NewVirtLauncherMutator(mgr.GetClient(), operatorNsEnv) hyperConvergedMutator := mutator.NewHyperConvergedMutator(mgr.GetClient()) // Make sure the certificates are mounted, this should be handled by the OLM @@ -73,7 +72,6 @@ func SetupWebhookWithManager(ctx context.Context, mgr ctrl.Manager, isOpenshift srv.TLSOpts = []func(*tls.Config){whHandler.MutateTLSConfig} srv.Register(hcoutil.HCONSWebhookPath, &webhook.Admission{Handler: nsMutator}) - srv.Register(hcoutil.HCOVirtLauncherWebhookPath, &webhook.Admission{Handler: virtLauncherMutator}) srv.Register(hcoutil.HCOMutatingWebhookPath, &webhook.Admission{Handler: hyperConvergedMutator}) srv.Register(hcoutil.HCOWebhookPath, &webhook.Admission{Handler: whHandler})