Merge pull request kubevirt#5333 from ashleyschuett/revert-5310-recon…

…cile-crd-prom Revert "reconcile crd to desired state"
Captured77 · Mar 29, 2021 · e7fa484 · e7fa484
2 parents 150abea + 7aedee4
commit e7fa484
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 108 deletions.
diff --git a/pkg/virt-operator/resource/apply/crds.go b/pkg/virt-operator/resource/apply/crds.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
 	extv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -48,47 +47,42 @@ func (r *Reconciler) createOrUpdateCrd(crd *extv1beta1.CustomResourceDefinition)
 		return nil
 	}
 
-	modified := resourcemerge.BoolPtr(false)
-	existing := cachedCrd.DeepCopy()
-
-	resourcemerge.EnsureCustomResourceDefinitionV1Beta1(modified, existing, *crd)
-	// there was no change to metadata and the spec fields are equal
-	if !*modified {
-		log.Log.V(4).Infof("crd %v is up-to-date", crd.GetName())
-		return nil
-	}
+	if !objectMatchesVersion(&cachedCrd.ObjectMeta, version, imageRegistry, id, r.kv.GetGeneration()) {
+		// Patch if old version
+		var ops []string
 
-	// Patch if old version
-	ops := []string{}
+		// Add Labels and Annotations Patches
+		labelAnnotationPatch, err := createLabelsAndAnnotationsPatch(&crd.ObjectMeta)
+		if err != nil {
+			return err
+		}
+		ops = append(ops, labelAnnotationPatch...)
 
-	// Add Labels and Annotations Patches
-	labelAnnotationPatch, err := createLabelsAndAnnotationsPatch(&crd.ObjectMeta)
-	if err != nil {
-		return err
-	}
-	ops = append(ops, labelAnnotationPatch...)
+		// subresource support needs to be introduced carefully after the control plane roll-over
+		// to avoid creating zombie entities which don't get processed du to ignored status updates
+		if cachedCrd.Spec.Subresources == nil || cachedCrd.Spec.Subresources.Status == nil {
+			if crd.Spec.Subresources != nil && crd.Spec.Subresources.Status != nil {
+				crd.Spec.Subresources.Status = nil
+			}
+		}
 
-	// subresource support needs to be introduced carefully after the control plane roll-over
-	// to avoid creating zombie entities which don't get processed due to ignored status updates
-	if cachedCrd.Spec.Subresources == nil || cachedCrd.Spec.Subresources.Status == nil {
-		if crd.Spec.Subresources != nil && crd.Spec.Subresources.Status != nil {
-			crd.Spec.Subresources.Status = nil
+		// Add Spec Patch
+		newSpec, err := json.Marshal(crd.Spec)
+		if err != nil {
+			return err
 		}
-	}
+		ops = append(ops, fmt.Sprintf(`{ "op": "replace", "path": "/spec", "value": %s }`, string(newSpec)))
 
-	// Add Spec Patch
-	newSpec, err := json.Marshal(crd.Spec)
-	if err != nil {
-		return err
-	}
-	ops = append(ops, fmt.Sprintf(`{ "op": "replace", "path": "/spec", "value": %s }`, string(newSpec)))
+		_, err = ext.ApiextensionsV1beta1().CustomResourceDefinitions().Patch(context.Background(), crd.Name, types.JSONPatchType, generatePatchBytes(ops), metav1.PatchOptions{})
+		if err != nil {
+			return fmt.Errorf("unable to patch crd %+v: %v", crd, err)
+		}
 
-	_, err = ext.ApiextensionsV1beta1().CustomResourceDefinitions().Patch(context.Background(), crd.Name, types.JSONPatchType, generatePatchBytes(ops), metav1.PatchOptions{})
-	if err != nil {
-		return fmt.Errorf("unable to patch crd %+v: %v", crd, err)
+		log.Log.V(2).Infof("crd %v updated", crd.GetName())
+		return nil
 	}
 
-	log.Log.V(2).Infof("crd %v updated", crd.GetName())
+	log.Log.V(4).Infof("crd %v is up-to-date", crd.GetName())
 	return nil
 }
 

diff --git a/pkg/virt-operator/resource/apply/crds_test.go b/pkg/virt-operator/resource/apply/crds_test.go
@@ -5,8 +5,6 @@ import (
 	"bytes"
 	"encoding/json"
 
-	"kubevirt.io/kubevirt/pkg/virt-operator/resource/generate/components"
-
 	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -200,76 +198,4 @@ var _ = Describe("Apply CRDs", func() {
 
 		Expect(r.rolloutNonCompatibleCRDChanges()).To(Succeed())
 	})
-
-	It("should not patch CRD on sync when they are equal", func() {
-		cachedCRD, err := components.NewKubeVirtCrd()
-		Expect(err).ToNot(HaveOccurred())
-
-		version, imageRegistry, id := getTargetVersionRegistryID(kv)
-		injectOperatorMetadata(kv, &cachedCRD.ObjectMeta, version, imageRegistry, id, true)
-
-		targetStrategy := loadTargetStrategy(cachedCRD)
-		stores.CrdCache.Add(cachedCRD)
-
-		extClient.Fake.PrependReactor("patch", "customresourcedefinitions", func(action testing.Action) (handled bool, ret runtime.Object, err error) {
-			// if patch is called we want to fail
-			Expect(true).To(BeFalse())
-
-			return true, nil, nil
-		})
-
-		r := &Reconciler{
-			kv:             kv,
-			targetStrategy: targetStrategy,
-			stores:         stores,
-			clientset:      clientset,
-			expectations:   expectations,
-		}
-
-		Expect(r.createOrUpdateCrd(cachedCRD)).To(BeNil())
-	})
-
-	It("should patch CRD on sync when it is not equal to the required CRD", func() {
-		updatedFieldName := "NewNameForCRDs"
-
-		cachedCRD, err := components.NewKubeVirtCrd()
-		Expect(err).ToNot(HaveOccurred())
-
-		version, imageRegistry, id := getTargetVersionRegistryID(kv)
-		injectOperatorMetadata(kv, &cachedCRD.ObjectMeta, version, imageRegistry, id, true)
-
-		targetStrategy := loadTargetStrategy(cachedCRD)
-		stores.CrdCache.Add(cachedCRD)
-
-		extClient.Fake.PrependReactor("patch", "customresourcedefinitions", func(action testing.Action) (handled bool, ret runtime.Object, err error) {
-			a := action.(testing.PatchActionImpl)
-			patch, err := jsonpatch.DecodePatch(a.Patch)
-			Expect(err).ToNot(HaveOccurred())
-
-			obj, err := json.Marshal(cachedCRD)
-			Expect(err).To(BeNil())
-
-			obj, err = patch.Apply(obj)
-			Expect(err).To(BeNil())
-
-			crd := &extv1beta1.CustomResourceDefinition{}
-			Expect(json.Unmarshal(obj, crd)).To(Succeed())
-			Expect(crd.Spec.Names.Plural).To(Equal(updatedFieldName))
-
-			return true, crd, nil
-		})
-
-		r := &Reconciler{
-			kv:             kv,
-			targetStrategy: targetStrategy,
-			stores:         stores,
-			clientset:      clientset,
-			expectations:   expectations,
-		}
-
-		requiredCRD := cachedCRD.DeepCopy()
-		requiredCRD.Spec.Names.Plural = updatedFieldName
-
-		Expect(r.createOrUpdateCrd(requiredCRD)).To(BeNil())
-	})
 })