Merge pull request #61 from kubevirt-bot/cherry-pick-59-to-release-v1.2

[release-v1.2] Revert .spec.nodeName handling and add a default namespace selector

pkg/aaq-operator/resources/cluster/aaqserver.go

@@ -17,6 +17,7 @@ const (
 	MutatingWebhookConfigurationName   = "gating-mutator"
 	validatingWebhookConfigurationName = "aaq-validator"
 	AaqServerServiceName               = "aaq-server"
+	DefaultNamespaceSelectorLabel      = "application-aware-quota/enable"
 )
 
 func createStaticAAQLockResources(args *FactoryArgs) []client.Object {
@@ -93,14 +94,23 @@ func createGatingMutatingWebhook(namespace string, c client.Client, l logr.Logge
 
 	hooks := []admissionregistrationv1.MutatingWebhook{}
 	if includeHooks {
+		namespaceSelector := cr.Spec.NamespaceSelector
+		if namespaceSelector == nil {
+			namespaceSelector = &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{Key: DefaultNamespaceSelectorLabel, Operator: metav1.LabelSelectorOpExists},
+				},
+			}
+		}
+
 		hooks = []admissionregistrationv1.MutatingWebhook{
 			{
 				Name:                    "gater.cqo.kubevirt.io",
 				AdmissionReviewVersions: []string{"v1", "v1beta1"},
 				FailurePolicy:           &failurePolicy,
 				SideEffects:             &sideEffect,
 				MatchPolicy:             &exactPolicy,
-				NamespaceSelector:       cr.Spec.NamespaceSelector,
+				NamespaceSelector:       namespaceSelector,
 				Rules: []admissionregistrationv1.RuleWithOperations{{
 					Operations: []admissionregistrationv1.OperationType{
 						admissionregistrationv1.Create,

pkg/aaq-operator/resources/crds_generated.go

@@ -2367,7 +2367,8 @@ spec:
                 type: object
               namespaceSelector:
                 description: namespaces where pods should be gated before scheduling
-                  Default to the empty LabelSelector, which matches everything.
+                  Defaults to targeting namespaces with an "application-aware-quota/enable"
+                  label key.
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.

pkg/aaq-server/handler/handler.go

@@ -11,7 +11,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"kubevirt.io/application-aware-quota/pkg/client"
 	"kubevirt.io/application-aware-quota/pkg/util"
-	"kubevirt.io/application-aware-quota/pkg/util/patch"
 	"kubevirt.io/application-aware-quota/staging/src/kubevirt.io/application-aware-quota-api/pkg/apis/core/v1alpha1"
 	"net/http"
 )
@@ -70,29 +69,19 @@ func (v Handler) mutatePod() (*admissionv1.AdmissionReview, error) {
 	if err := json.Unmarshal(v.request.Object.Raw, &pod); err != nil {
 		return nil, err
 	}
-
 	schedulingGates := pod.Spec.SchedulingGates
 	if schedulingGates == nil {
 		schedulingGates = []v1.PodSchedulingGate{}
 	}
 	schedulingGates = append(schedulingGates, v1.PodSchedulingGate{Name: util.AAQGate})
 
-	patches := []patch.PatchOperation{
-		{
-			Op:    patch.PatchAddOp,
-			Path:  "/spec/schedulingGates",
-			Value: schedulingGates,
-		},
-	}
-
-	patches = append(patches, generatePatchesForPodWithNodeName(pod)...)
-
-	patchBytes, err := patch.GeneratePatchPayload(patches...)
+	schedulingGatesBytes, err := json.Marshal(schedulingGates)
 	if err != nil {
 		return nil, err
 	}
 
-	return reviewResponseWithPatch(v.request.UID, true, http.StatusAccepted, allowPodRequest, patchBytes), nil
+	patch := fmt.Sprintf(`[{"op": "add", "path": "/spec/schedulingGates", "value": %s}]`, string(schedulingGatesBytes))
+	return reviewResponseWithPatch(v.request.UID, true, http.StatusAccepted, allowPodRequest, []byte(patch)), nil
 }
 
 func reviewResponseWithPatch(uid types.UID, allowed bool, httpCode int32,
@@ -220,61 +209,3 @@ func getResourcesNames(resourceList v1.ResourceList) []v1.ResourceName {
 	}
 	return keys
 }
-
-func generatePatchesForPodWithNodeName(pod v1.Pod) []patch.PatchOperation {
-	if pod.Spec.NodeName == "" {
-		return nil
-	}
-
-	affinity := pod.Spec.Affinity
-
-	if affinity == nil {
-		affinity = &v1.Affinity{}
-	}
-	if affinity.NodeAffinity == nil {
-		affinity.NodeAffinity = &v1.NodeAffinity{}
-	}
-	if affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil {
-		affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution = &v1.NodeSelector{}
-	}
-	affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms = append(
-		affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms,
-		v1.NodeSelectorTerm{
-			MatchFields: []v1.NodeSelectorRequirement{
-				{
-					Key:      "metadata.name",
-					Operator: v1.NodeSelectorOpIn,
-					Values:   []string{pod.Spec.NodeName},
-				},
-			},
-		},
-	)
-
-	tolerations := pod.Spec.Tolerations
-	if tolerations == nil {
-		tolerations = []v1.Toleration{}
-	}
-	tolerations = append(tolerations, v1.Toleration{
-		Key:      "", // i.e. any key
-		Operator: v1.TolerationOpExists,
-		Effect:   v1.TaintEffectNoSchedule,
-	})
-
-	return []patch.PatchOperation{
-		{
-			Op:    patch.PatchAddOp,
-			Path:  "/spec/affinity",
-			Value: affinity,
-		},
-		{
-			Op:    patch.PatchReplaceOp,
-			Path:  "/spec/nodeName",
-			Value: "",
-		},
-		{
-			Op:    patch.PatchAddOp,
-			Path:  "/spec/tolerations",
-			Value: tolerations,
-		},
-	}
-}

pkg/aaq-server/handler/handler_test.go

@@ -15,8 +15,6 @@ import (
 	"k8s.io/client-go/kubernetes/fake"
 	"kubevirt.io/application-aware-quota/pkg/client"
 	"kubevirt.io/application-aware-quota/pkg/util"
-	"kubevirt.io/application-aware-quota/pkg/util/patch"
-	aaqv1 "kubevirt.io/application-aware-quota/staging/src/kubevirt.io/application-aware-quota-api/pkg/apis/core/v1alpha1"
 	"kubevirt.io/application-aware-quota/tests/builders"
 	"net/http"
 )
@@ -194,128 +192,4 @@ var _ = Describe("Test handler of aaq server", func() {
 		Entry(" valid Update should be allowed", admissionv1.Update),
 		Entry(" valid Creation should be allowed", admissionv1.Create),
 	)
-
-	It("pod with a non-empty spec.nodeName set", func() {
-		const nodeName = "test-node"
-
-		pod := &v1.Pod{
-			Spec: v1.PodSpec{
-				NodeName: nodeName,
-			},
-		}
-
-		podBytes, err := json.Marshal(pod)
-		Expect(err).ToNot(HaveOccurred())
-
-		v := Handler{
-			request: &admissionv1.AdmissionRequest{
-				Kind: metav1.GroupVersionKind{
-					Kind: "Pod",
-				},
-				Object: runtime.RawExtension{
-					Raw:    podBytes,
-					Object: pod,
-				},
-				Operation: admissionv1.Create,
-			},
-		}
-		admissionReview, err := v.Handle()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(admissionReview.Response.Allowed).To(BeTrue())
-
-		var patches []patch.PatchOperation
-		err = json.Unmarshal(admissionReview.Response.Patch, &patches)
-		Expect(err).ToNot(HaveOccurred())
-
-		removeNodeNamePatchFound := false
-		nodeAffinityPatchFound := false
-		wildcardTolerationPatchFound := false
-
-		for _, p := range patches {
-			switch p.Path {
-
-			case "/spec/nodeName":
-				removeNodeNamePatchFound = true
-				Expect(p.Op).To(Equal(patch.PatchReplaceOp), "Patch is expected to replace nodeName with an empty string")
-				Expect(p.Value).To(BeEmpty())
-
-			case "/spec/affinity":
-				nodeAffinityPatchFound = true
-
-				var affinity *v1.Affinity
-				valueBytes, err := json.Marshal(p.Value)
-				Expect(err).ShouldNot(HaveOccurred())
-				err = json.Unmarshal(valueBytes, &affinity)
-				Expect(err).ShouldNot(HaveOccurred())
-
-				Expect(affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms).To(ContainElement(
-					v1.NodeSelectorTerm{
-						MatchFields: []v1.NodeSelectorRequirement{
-							{
-								Key:      "metadata.name",
-								Operator: v1.NodeSelectorOpIn,
-								Values:   []string{pod.Spec.NodeName},
-							},
-						},
-					},
-				),
-				)
-
-			case "/spec/tolerations":
-				wildcardTolerationPatchFound = true
-
-				var tolerations []v1.Toleration
-				valueBytes, err := json.Marshal(p.Value)
-				Expect(err).ShouldNot(HaveOccurred())
-				err = json.Unmarshal(valueBytes, &tolerations)
-				Expect(err).ShouldNot(HaveOccurred())
-
-				Expect(tolerations).To(ContainElement(
-					v1.Toleration{
-						Key:      "", // i.e. any key
-						Operator: v1.TolerationOpExists,
-						Effect:   v1.TaintEffectNoSchedule,
-					},
-				))
-			}
-		}
-
-		Expect(removeNodeNamePatchFound).To(BeTrue(), "Patch to remove nodeName is not found")
-		Expect(nodeAffinityPatchFound).To(BeTrue(), "Patch to add node affinity is not found")
-		Expect(wildcardTolerationPatchFound).To(BeTrue(), "Patch to a wildcard NoSchedule toleration is not found")
-	})
-
-	It("Operations on AAQ", func() {
-		aaq := &aaqv1.AAQ{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "AAQ",
-				APIVersion: "aaqs.aaq.kubevirt.io",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "test-aaq",
-			},
-		}
-		aaqBytes, err := json.Marshal(aaq)
-		Expect(err).ToNot(HaveOccurred())
-
-		v := Handler{
-			request: &admissionv1.AdmissionRequest{
-				Kind: metav1.GroupVersionKind{
-					Kind: aaq.Kind,
-				},
-				Object: runtime.RawExtension{
-					Raw:    aaqBytes,
-					Object: aaq,
-				},
-				Operation: admissionv1.Create,
-			},
-		}
-
-		admissionReview, err := v.Handle()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(admissionReview.Response.Allowed).To(BeFalse())
-		Expect(admissionReview.Response.Result.Code).To(Equal(int32(http.StatusForbidden)))
-		Expect(admissionReview.Response.Result.Message).To(Equal(onlySingleAAQInstaceIsAllowed))
-
-	})
 })

staging/src/kubevirt.io/application-aware-quota-api/pkg/apis/core/v1alpha1/types.go

@@ -145,7 +145,7 @@ type AAQSpec struct {
 	// PriorityClass of the AAQ control plane
 	PriorityClass *AAQPriorityClass `json:"priorityClass,omitempty"`
 	// namespaces where pods should be gated before scheduling
-	// Default to the empty LabelSelector, which matches everything.
+	// Defaults to targeting namespaces with an "application-aware-quota/enable" label key.
 	NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
 	// holds aaq configurations.
 	Configuration AAQConfiguration `json:"configuration,omitempty"`