diff --git a/assets/upgradePatches.json b/assets/upgradePatches.json index 038678ecc..d983c817b 100644 --- a/assets/upgradePatches.json +++ b/assets/upgradePatches.json @@ -38,5 +38,31 @@ } ] } + ], + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "v2v-vmware", + "namespace": "kubevirt-hyperconverged" + } + }, + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "vm-import-controller-config", + "namespace": "kubevirt-hyperconverged" + } + } ] } diff --git a/controllers/hyperconverged/hyperconverged_controller.go b/controllers/hyperconverged/hyperconverged_controller.go index b1c5476e3..79cf5d612 100644 --- a/controllers/hyperconverged/hyperconverged_controller.go +++ b/controllers/hyperconverged/hyperconverged_controller.go @@ -7,6 +7,8 @@ import ( "os" "reflect" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/blang/semver/v4" jsonpatch "github.com/evanphx/json-patch" "github.com/google/uuid" @@ -264,6 +266,11 @@ func (r *ReconcileHyperConverged) Reconcile(ctx context.Context, request reconci if r.firstLoop { r.firstLoopInitialization(hcoRequest) + if err := validateUpgradePatches(hcoRequest); err != nil { + logger.Error(err, "Failed validating upgrade patches file") + r.eventEmitter.EmitEvent(hcoRequest.Instance, corev1.EventTypeWarning, "Failed validating upgrade patches file", err.Error()) + os.Exit(1) + } } result, err := r.doReconcile(hcoRequest) @@ -355,7 +362,6 @@ func (r *ReconcileHyperConverged) doReconcile(req *common.HcoRequest) (reconcile // get into upgrade mode r.upgradeMode = true - r.eventEmitter.EmitEvent(req.Instance, corev1.EventTypeNormal, "UpgradeHCO", "Upgrading the HyperConverged to version "+r.ownVersion) req.Logger.Info(fmt.Sprintf("Start upgrading from version %s to version %s", knownHcoVersion, r.ownVersion)) } @@ -372,10 +378,6 @@ func (r *ReconcileHyperConverged) doReconcile(req *common.HcoRequest) (reconcile } func (r *ReconcileHyperConverged) handleUpgrade(req *common.HcoRequest, init bool) (*reconcile.Result, error) { - err := validateUpgradePatches(req) - if err != nil { - return &reconcile.Result{Requeue: true}, err - } crdStatusUpdated, err := r.updateCrdStoredVersions(req) if err != nil { @@ -1080,6 +1082,13 @@ func (r ReconcileHyperConverged) applyUpgradePatches(req *common.HcoRequest) (bo } } + for _, p := range hcoUpgradeChanges.ObjectsToBeRemoved { + removed, err := r.removeLeftover(req, knownHcoSV, p) + if err != nil { + return removed, err + } + } + tmpInstance := &hcov1beta1.HyperConverged{} err = json.Unmarshal(hcoJson, tmpInstance) if err != nil { @@ -1115,6 +1124,31 @@ func (r ReconcileHyperConverged) applyUpgradePatch(req *common.HcoRequest, hcoJs return hcoJson, nil } +func (r ReconcileHyperConverged) removeLeftover(req *common.HcoRequest, knownHcoSV semver.Version, p objectToBeRemoved) (bool, error) { + + affectedRange, err := semver.ParseRange(p.SemverRange) + if err != nil { + return false, err + } + if affectedRange(knownHcoSV) { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(p.GroupVersionKind) + gerr := r.client.Get(req.Ctx, p.ObjectKey, u) + if gerr != nil { + if apierrors.IsNotFound(gerr) { + return false, nil + } else { + req.Logger.Error(gerr, "failed looking for leftovers", "objectToBeRemoved", p) + return false, gerr + } + } + removeRelatedObject(req, p.GroupVersionKind, p.ObjectKey) + return r.deleteObj(req, u, false) + + } + return false, nil +} + var ( operatorMetrics = "hyperconverged-cluster-operator-metrics" webhookMetrics = "hyperconverged-cluster-webhook-metrics" @@ -1151,7 +1185,7 @@ func (r ReconcileHyperConverged) removeOldMetricsObjs(req *common.HcoRequest) er initOldMetricsObjects(req) for name, object := range oldMetricsObjects { - removed, err := r.deleteObj(req, object) + removed, err := r.deleteObj(req, object, true) if err != nil { return err @@ -1165,8 +1199,8 @@ func (r ReconcileHyperConverged) removeOldMetricsObjs(req *common.HcoRequest) er return nil } -func (r *ReconcileHyperConverged) deleteObj(req *common.HcoRequest, obj client.Object) (bool, error) { - removed, err := hcoutil.EnsureDeleted(req.Ctx, r.client, obj, req.Instance.Name, req.Logger, false, false) +func (r *ReconcileHyperConverged) deleteObj(req *common.HcoRequest, obj client.Object, protectNonHCOObjects bool) (bool, error) { + removed, err := hcoutil.EnsureDeleted(req.Ctx, r.client, obj, req.Instance.Name, req.Logger, false, false, protectNonHCOObjects) if err != nil { req.Logger.Error( @@ -1206,7 +1240,7 @@ func removeOldQuickStartGuides(req *common.HcoRequest, cl client.Client, require for name, existQs := range existingQSNames { if !hcoutil.ContainsString(requiredQSList, name) { req.Logger.Info("deleting ConsoleQuickStart", "name", name) - if _, err = hcoutil.EnsureDeleted(req.Ctx, cl, &existQs, req.Instance.Name, req.Logger, false, false); err != nil { + if _, err = hcoutil.EnsureDeleted(req.Ctx, cl, &existQs, req.Instance.Name, req.Logger, false, false, true); err != nil { req.Logger.Error(err, "failed to delete ConsoleQuickStart", "name", name) } } @@ -1238,6 +1272,25 @@ func removeRelatedQSObjects(req *common.HcoRequest, requiredNames []string) { } +func removeRelatedObject(req *common.HcoRequest, gvk schema.GroupVersionKind, objectKey types.NamespacedName) { + refs := make([]corev1.ObjectReference, 0, len(req.Instance.Status.RelatedObjects)) + foundRO := false + + for _, obj := range req.Instance.Status.RelatedObjects { + if obj.GroupVersionKind() == gvk && obj.Namespace == objectKey.Namespace && obj.Name == objectKey.Name { + foundRO = true + continue + } + refs = append(refs, obj) + } + + if foundRO { + req.Instance.Status.RelatedObjects = refs + req.StatusDirty = true + } + +} + // getHyperConvergedNamespacedName returns the name/namespace of the HyperConverged resource func getHyperConvergedNamespacedName() (types.NamespacedName, error) { hco := types.NamespacedName{ diff --git a/controllers/hyperconverged/hyperconverged_controller_test.go b/controllers/hyperconverged/hyperconverged_controller_test.go index e632cceb7..b3ffa32f1 100644 --- a/controllers/hyperconverged/hyperconverged_controller_test.go +++ b/controllers/hyperconverged/hyperconverged_controller_test.go @@ -1866,6 +1866,183 @@ var _ = Describe("HyperconvergedController", func() { Expect(apierrors.IsNotFound(err)).To(BeTrue()) }) }) + + Context("remove leftovers on upgrades", func() { + + It("should remove ConfigMap v2v-vmware upgrading from <= 1.6.0", func() { + + cmToBeRemoved1 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "v2v-vmware", + Namespace: namespace, + Labels: map[string]string{ + hcoutil.AppLabel: expected.hco.Name, + }, + }, + } + cmToBeRemoved2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vm-import-controller-config", + Namespace: namespace, + }, + } + cmNotToBeRemoved1 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "v2v-vmware", + Namespace: "different" + namespace, + Labels: map[string]string{ + hcoutil.AppLabel: expected.hco.Name, + }, + }, + } + cmNotToBeRemoved2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other", + Namespace: namespace, + Labels: map[string]string{ + hcoutil.AppLabel: expected.hco.Name, + }, + }, + } + + toBeRemovedRelatedObjects := []corev1.ObjectReference{ + { + APIVersion: "v1", + Kind: "ConfigMap", + Name: cmToBeRemoved1.Name, + Namespace: cmToBeRemoved1.Namespace, + ResourceVersion: "999", + }, + } + otherRelatedObjects := []corev1.ObjectReference{ + { + APIVersion: "v1", + Kind: "ConfigMap", + Name: cmNotToBeRemoved1.Name, + Namespace: cmNotToBeRemoved1.Namespace, + ResourceVersion: "999", + }, + { + APIVersion: "v1", + Kind: "ConfigMap", + Name: cmNotToBeRemoved2.Name, + Namespace: cmNotToBeRemoved2.Namespace, + ResourceVersion: "999", + }, + } + + UpdateVersion(&expected.hco.Status, hcoVersionName, "1.4.99") + + for _, objRef := range toBeRemovedRelatedObjects { + Expect(v1.SetObjectReference(&expected.hco.Status.RelatedObjects, objRef)).ToNot(HaveOccurred()) + } + for _, objRef := range otherRelatedObjects { + Expect(v1.SetObjectReference(&expected.hco.Status.RelatedObjects, objRef)).ToNot(HaveOccurred()) + } + + resources := append(expected.toArray(), cmToBeRemoved1, cmToBeRemoved2, cmNotToBeRemoved1, cmNotToBeRemoved2) + + cl := commonTestUtils.InitClient(resources) + foundResource, reconciler, requeue := doReconcile(cl, expected.hco, nil) + Expect(requeue).To(BeTrue()) + checkAvailability(foundResource, metav1.ConditionTrue) + + foundResource, _, requeue = doReconcile(cl, expected.hco, reconciler) + Expect(requeue).To(BeFalse()) + checkAvailability(foundResource, metav1.ConditionTrue) + + foundCM := &corev1.ConfigMap{} + + err := cl.Get(context.TODO(), client.ObjectKeyFromObject(cmToBeRemoved1), foundCM) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + + err = cl.Get(context.TODO(), client.ObjectKeyFromObject(cmToBeRemoved2), foundCM) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + + err = cl.Get(context.TODO(), client.ObjectKeyFromObject(cmNotToBeRemoved1), foundCM) + Expect(err).ToNot(HaveOccurred()) + + err = cl.Get(context.TODO(), client.ObjectKeyFromObject(cmNotToBeRemoved2), foundCM) + Expect(err).ToNot(HaveOccurred()) + + for _, objRef := range toBeRemovedRelatedObjects { + Expect(foundResource.Status.RelatedObjects).ToNot(ContainElement(objRef)) + } + for _, objRef := range otherRelatedObjects { + Expect(foundResource.Status.RelatedObjects).To(ContainElement(objRef)) + } + + }) + + It("should not remove ConfigMap v2v-vmware upgrading from >= 1.6.1", func() { + + cmToBeRemoved1 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "v2v-vmware", + Namespace: namespace, + Labels: map[string]string{ + hcoutil.AppLabel: expected.hco.Name, + }, + }, + } + cmToBeRemoved2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vm-import-controller-config", + Namespace: namespace, + }, + } + cmNotToBeRemoved1 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "v2v-vmware", + Namespace: "different" + namespace, + Labels: map[string]string{ + hcoutil.AppLabel: expected.hco.Name, + }, + }, + } + cmNotToBeRemoved2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other", + Namespace: namespace, + Labels: map[string]string{ + hcoutil.AppLabel: expected.hco.Name, + }, + }, + } + + UpdateVersion(&expected.hco.Status, hcoVersionName, "1.6.1") + + resources := append(expected.toArray(), cmToBeRemoved1, cmToBeRemoved2, cmNotToBeRemoved1, cmNotToBeRemoved2) + + cl := commonTestUtils.InitClient(resources) + foundResource, reconciler, requeue := doReconcile(cl, expected.hco, nil) + Expect(requeue).To(BeTrue()) + checkAvailability(foundResource, metav1.ConditionTrue) + + foundResource, _, requeue = doReconcile(cl, expected.hco, reconciler) + Expect(requeue).To(BeFalse()) + checkAvailability(foundResource, metav1.ConditionTrue) + + foundCM := &corev1.ConfigMap{} + + err := cl.Get(context.TODO(), client.ObjectKeyFromObject(cmToBeRemoved1), foundCM) + Expect(err).ToNot(HaveOccurred()) + + err = cl.Get(context.TODO(), client.ObjectKeyFromObject(cmToBeRemoved2), foundCM) + Expect(err).ToNot(HaveOccurred()) + + err = cl.Get(context.TODO(), client.ObjectKeyFromObject(cmNotToBeRemoved1), foundCM) + Expect(err).ToNot(HaveOccurred()) + + err = cl.Get(context.TODO(), client.ObjectKeyFromObject(cmNotToBeRemoved2), foundCM) + Expect(err).ToNot(HaveOccurred()) + + }) + + }) + }) Context("Aggregate Negative Conditions", func() { diff --git a/controllers/hyperconverged/test-files/upgradePatches/badObject1.json b/controllers/hyperconverged/test-files/upgradePatches/badObject1.json new file mode 100644 index 000000000..48d7b37f4 --- /dev/null +++ b/controllers/hyperconverged/test-files/upgradePatches/badObject1.json @@ -0,0 +1,16 @@ +{ + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "" + }, + "objectKey": { + "name": "v2v-vmware", + "namespace": "kubevirt-hyperconverged" + } + } + ] +} diff --git a/controllers/hyperconverged/test-files/upgradePatches/badObject1m.json b/controllers/hyperconverged/test-files/upgradePatches/badObject1m.json new file mode 100644 index 000000000..06f3c889b --- /dev/null +++ b/controllers/hyperconverged/test-files/upgradePatches/badObject1m.json @@ -0,0 +1,15 @@ +{ + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1" + }, + "objectKey": { + "name": "v2v-vmware", + "namespace": "kubevirt-hyperconverged" + } + } + ] +} diff --git a/controllers/hyperconverged/test-files/upgradePatches/badObject2.json b/controllers/hyperconverged/test-files/upgradePatches/badObject2.json new file mode 100644 index 000000000..c55aaa5ea --- /dev/null +++ b/controllers/hyperconverged/test-files/upgradePatches/badObject2.json @@ -0,0 +1,16 @@ +{ + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "v2v-vmware", + "namespace": "kubevirt-hyperconverged" + } + } + ] +} diff --git a/controllers/hyperconverged/test-files/upgradePatches/badObject2m.json b/controllers/hyperconverged/test-files/upgradePatches/badObject2m.json new file mode 100644 index 000000000..f87079a57 --- /dev/null +++ b/controllers/hyperconverged/test-files/upgradePatches/badObject2m.json @@ -0,0 +1,15 @@ +{ + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "v2v-vmware", + "namespace": "kubevirt-hyperconverged" + } + } + ] +} diff --git a/controllers/hyperconverged/test-files/upgradePatches/badObject3.json b/controllers/hyperconverged/test-files/upgradePatches/badObject3.json new file mode 100644 index 000000000..655da2c35 --- /dev/null +++ b/controllers/hyperconverged/test-files/upgradePatches/badObject3.json @@ -0,0 +1,16 @@ +{ + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "", + "namespace": "kubevirt-hyperconverged" + } + } + ] +} diff --git a/controllers/hyperconverged/test-files/upgradePatches/badObject3m.json b/controllers/hyperconverged/test-files/upgradePatches/badObject3m.json new file mode 100644 index 000000000..382c0373d --- /dev/null +++ b/controllers/hyperconverged/test-files/upgradePatches/badObject3m.json @@ -0,0 +1,15 @@ +{ + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "ConfigMap" + }, + "objectKey": { + "namespace": "kubevirt-hyperconverged" + } + } + ] +} diff --git a/controllers/hyperconverged/test-files/upgradePatches/badSemverRangeOR.json b/controllers/hyperconverged/test-files/upgradePatches/badSemverRangeOR.json new file mode 100644 index 000000000..3b064979f --- /dev/null +++ b/controllers/hyperconverged/test-files/upgradePatches/badSemverRangeOR.json @@ -0,0 +1,16 @@ +{ + "objectsToBeRemoved": [ + { + "semverRange": "= badvalue < > ", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "v2v-vmware", + "namespace": "kubevirt-hyperconverged" + } + } + ] +} diff --git a/controllers/hyperconverged/test-files/upgradePatches/upgradePatches.json b/controllers/hyperconverged/test-files/upgradePatches/upgradePatches.json index 038678ecc..d983c817b 100644 --- a/controllers/hyperconverged/test-files/upgradePatches/upgradePatches.json +++ b/controllers/hyperconverged/test-files/upgradePatches/upgradePatches.json @@ -38,5 +38,31 @@ } ] } + ], + "objectsToBeRemoved": [ + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "v2v-vmware", + "namespace": "kubevirt-hyperconverged" + } + }, + { + "semverRange": "<=1.6.0", + "groupVersionKind": { + "group": "", + "version": "v1", + "kind": "ConfigMap" + }, + "objectKey": { + "name": "vm-import-controller-config", + "namespace": "kubevirt-hyperconverged" + } + } ] } diff --git a/controllers/hyperconverged/upgradePatches.go b/controllers/hyperconverged/upgradePatches.go index f06164141..2f8e1ac5c 100644 --- a/controllers/hyperconverged/upgradePatches.go +++ b/controllers/hyperconverged/upgradePatches.go @@ -7,6 +7,9 @@ import ( "os" "strings" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "github.com/blang/semver/v4" jsonpatch "github.com/evanphx/json-patch" @@ -26,10 +29,23 @@ type hcoCRPatch struct { JSONPatch jsonpatch.Patch `json:"jsonPatch"` } +type objectToBeRemoved struct { + // SemverRange is a set of conditions which specify which versions satisfy the range + // (see https://github.com/blang/semver#ranges as a reference). + SemverRange string `json:"semverRange"` + // GroupVersionKind unambiguously identifies the kind of the object to be removed + GroupVersionKind schema.GroupVersionKind `json:"groupVersionKind"` + // objectKey contains name and namespace of the object to be removed. + ObjectKey types.NamespacedName `json:"objectKey"` +} + type UpgradePatches struct { // hcoCRPatchList is a list of upgrade patches. // Each hcoCRPatch consists in a semver range of affected source versions and a json patch to be applied during the upgrade if relevant. HCOCRPatchList []hcoCRPatch `json:"hcoCRPatchList"` + // ObjectsToBeRemoved is a list of objects to be removed on upgrades. + // Each objectToBeRemoved consists in a semver range of affected source versions and schema.GroupVersionKind and types.NamespacedName of the object to be eventually removed during the upgrade. + ObjectsToBeRemoved []objectToBeRemoved `json:"objectsToBeRemoved"` } var ( @@ -45,7 +61,7 @@ func readUpgradePatchesFromFile(req *common.HcoRequest) error { if hcoUpgradeChangesRead { return nil } - + hcoUpgradeChanges = UpgradePatches{} fileLocation := getUpgradeChangesFileLocation() file, err := os.Open(fileLocation) @@ -71,9 +87,15 @@ func validateUpgradePatches(req *common.HcoRequest) error { if err != nil { return err } - for _, p := range hcoUpgradeChanges.HCOCRPatchList { - return validateUpgradePatch(req, p) + if verr := validateUpgradePatch(req, p); verr != nil { + return verr + } + } + for _, r := range hcoUpgradeChanges.ObjectsToBeRemoved { + if verr := validateUpgradeLeftover(req, r); verr != nil { + return verr + } } return nil } @@ -98,8 +120,27 @@ func validateUpgradePatch(req *common.HcoRequest, p hcoCRPatch) error { return err } _, err = p.JSONPatch.Apply(specBytes) + // tolerate jsonpatch test failures + if err != nil && !errors.Is(err, jsonpatch.ErrTestFailed) { + return err + } + return nil +} + +func validateUpgradeLeftover(req *common.HcoRequest, r objectToBeRemoved) error { + _, err := semver.ParseRange(r.SemverRange) if err != nil { return err } + + if r.GroupVersionKind.Kind == "" { + return errors.New("missing object kind") + } + if r.GroupVersionKind.Version == "" { + return errors.New("missing object API version") + } + if r.ObjectKey.Name == "" { + return errors.New("missing object name") + } return nil } diff --git a/controllers/hyperconverged/upgradePatches_test.go b/controllers/hyperconverged/upgradePatches_test.go index a31517008..08370d51d 100644 --- a/controllers/hyperconverged/upgradePatches_test.go +++ b/controllers/hyperconverged/upgradePatches_test.go @@ -68,41 +68,100 @@ var _ = Describe("upgradePatches", func() { Expect(err.Error()).Should(HavePrefix("invalid character")) }) - It("should fail validating upgradePatches with bad semver ranges", func() { - err := copyTestFile("badSemverRange.json") - Expect(err).ToNot(HaveOccurred()) + Context("hcoCRPatchList", func() { + + It("should fail validating upgradePatches with bad semver ranges", func() { + err := copyTestFile("badSemverRange.json") + Expect(err).ToNot(HaveOccurred()) + + err = validateUpgradePatches(req) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(HavePrefix("Could not get version from string:")) + }) + + DescribeTable( + "should fail validating upgradePatches with bad patches", + func(filename, message string) { + err := copyTestFile(filename) + Expect(err).ToNot(HaveOccurred()) + + err = validateUpgradePatches(req) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(HavePrefix(message)) + }, + Entry( + "bad operation kind", + "badPatches1.json", + "Unexpected kind:", + ), + Entry( + "not on spec", + "badPatches2.json", + "can only modify spec fields", + ), + Entry( + "unexisting path", + "badPatches3.json", + "replace operation does not apply: doc is missing path:", + ), + ) - err = validateUpgradePatches(req) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).Should(HavePrefix("Could not get version from string:")) }) - DescribeTable( - "should fail validating upgradePatches with bad patches", - func(filename, message string) { - err := copyTestFile(filename) + Context("objectsToBeRemoved", func() { + + It("should fail validating upgradePatches with bad semver ranges", func() { + err := copyTestFile("badSemverRangeOR.json") Expect(err).ToNot(HaveOccurred()) err = validateUpgradePatches(req) Expect(err).To(HaveOccurred()) - Expect(err.Error()).Should(HavePrefix(message)) - }, - Entry( - "bad operation kind", - "badPatches1.json", - "Unexpected kind:", - ), - Entry( - "not on spec", - "badPatches2.json", - "can only modify spec fields", - ), - Entry( - "unexisting path", - "badPatches3.json", - "replace operation does not apply: doc is missing path:", - ), - ) + Expect(err.Error()).Should(HavePrefix("Could not get version from string:")) + }) + + DescribeTable( + "should fail validating upgradePatches with bad patches", + func(filename, message string) { + err := copyTestFile(filename) + Expect(err).ToNot(HaveOccurred()) + + err = validateUpgradePatches(req) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(HavePrefix(message)) + }, + Entry( + "empty object kind", + "badObject1.json", + "missing object kind", + ), + Entry( + "missing object kind", + "badObject1m.json", + "missing object kind", + ), + Entry( + "empty object API version", + "badObject2.json", + "missing object API version", + ), + Entry( + "missing object API version", + "badObject2m.json", + "missing object API version", + ), + Entry( + "empty object name", + "badObject3.json", + "missing object name", + ), + Entry( + "missing object name", + "badObject3m.json", + "missing object name", + ), + ) + + }) }) diff --git a/controllers/operands/imageStream.go b/controllers/operands/imageStream.go index 7bec97432..8b03d9677 100644 --- a/controllers/operands/imageStream.go +++ b/controllers/operands/imageStream.go @@ -49,7 +49,7 @@ func (iso imageStreamOperand) ensure(req *common.HcoRequest) *EnsureResult { cr := iso.operand.hooks.getEmptyCr() res := NewEnsureResult(cr) res.SetName(cr.GetName()) - deleted, err := util.EnsureDeleted(req.Ctx, iso.operand.Client, cr, req.Instance.Name, req.Logger, false, false) + deleted, err := util.EnsureDeleted(req.Ctx, iso.operand.Client, cr, req.Instance.Name, req.Logger, false, false, true) if err != nil { return res.Error(err) } diff --git a/controllers/operands/operandHandler.go b/controllers/operands/operandHandler.go index 8b0902478..d7d02ae53 100644 --- a/controllers/operands/operandHandler.go +++ b/controllers/operands/operandHandler.go @@ -200,7 +200,7 @@ func (h OperandHandler) EnsureDeleted(req *common.HcoRequest) error { for _, res := range resources { go func(o client.Object, wgr *sync.WaitGroup) { defer wgr.Done() - deleted, err := hcoutil.EnsureDeleted(tCtx, h.client, o, req.Instance.Name, req.Logger, false, true) + deleted, err := hcoutil.EnsureDeleted(tCtx, h.client, o, req.Instance.Name, req.Logger, false, true, true) if err != nil { req.Logger.Error(err, "Failed to manually delete objects") errT := ErrHCOUninstall diff --git a/pkg/util/util.go b/pkg/util/util.go index 48a569509..12b22dcdf 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -210,7 +210,7 @@ func GetRuntimeObject(ctx context.Context, c client.Client, obj client.Object, l // ComponentResourceRemoval removes the resource `obj` if it exists and belongs to the HCO // with wait=true it will wait, (util ctx timeout, please set it!) for the resource to be effectively deleted -func ComponentResourceRemoval(ctx context.Context, c client.Client, obj interface{}, hcoName string, logger logr.Logger, dryRun bool, wait bool) (bool, error) { +func ComponentResourceRemoval(ctx context.Context, c client.Client, obj interface{}, hcoName string, logger logr.Logger, dryRun bool, wait bool, protectNonHCOObjects bool) (bool, error) { resource, err := ToUnstructured(obj) if err != nil { logger.Error(err, "Failed to convert object to Unstructured") @@ -224,7 +224,7 @@ func ComponentResourceRemoval(ctx context.Context, c client.Client, obj interfac return false, err } - if !shouldDeleteResource(resource, hcoName, logger) { + if protectNonHCOObjects && !shouldDeleteResource(resource, hcoName, logger) { return false, nil } @@ -307,7 +307,7 @@ func validateDeletion(ctx context.Context, c client.Client, resource *unstructur // EnsureDeleted calls ComponentResourceRemoval if the runtime object exists // with wait=true it will wait, (util ctx timeout, please set it!) for the resource to be effectively deleted -func EnsureDeleted(ctx context.Context, c client.Client, obj client.Object, hcoName string, logger logr.Logger, dryRun bool, wait bool) (bool, error) { +func EnsureDeleted(ctx context.Context, c client.Client, obj client.Object, hcoName string, logger logr.Logger, dryRun bool, wait bool, protectNonHCOObjects bool) (bool, error) { err := GetRuntimeObject(ctx, c, obj, logger) if err != nil { @@ -320,7 +320,7 @@ func EnsureDeleted(ctx context.Context, c client.Client, obj client.Object, hcoN return false, err } - return ComponentResourceRemoval(ctx, c, obj, hcoName, logger, dryRun, wait) + return ComponentResourceRemoval(ctx, c, obj, hcoName, logger, dryRun, wait, protectNonHCOObjects) } func ContainsString(s []string, word string) bool { diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index b17f085fe..577e28b17 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -102,7 +102,7 @@ var _ = Describe("Test general utilities", func() { WithRuntimeObjects(pod). Build() - deleted, err := EnsureDeleted(ctx, cl, pod, appName, logger, false, true) + deleted, err := EnsureDeleted(ctx, cl, pod, appName, logger, false, true, true) Expect(err).ShouldNot(HaveOccurred()) Expect(deleted).To(BeTrue()) @@ -116,7 +116,7 @@ var _ = Describe("Test general utilities", func() { WithScheme(testScheme). Build() - deleted, err := EnsureDeleted(ctx, cl, pod, appName, logger, false, true) + deleted, err := EnsureDeleted(ctx, cl, pod, appName, logger, false, true, true) Expect(err).ShouldNot(HaveOccurred()) Expect(deleted).To(BeFalse()) diff --git a/pkg/webhooks/validator/validator.go b/pkg/webhooks/validator/validator.go index ed64208ac..0314a90bd 100644 --- a/pkg/webhooks/validator/validator.go +++ b/pkg/webhooks/validator/validator.go @@ -213,7 +213,7 @@ func (wh WebhookHandler) ValidateDelete(hc *v1beta1.HyperConverged) error { kv, cdi, } { - _, err := hcoutil.EnsureDeleted(ctx, wh.cli, obj, hc.Name, wh.logger, true, false) + _, err := hcoutil.EnsureDeleted(ctx, wh.cli, obj, hc.Name, wh.logger, true, false, true) if err != nil { wh.logger.Error(err, "Delete validation failed", "GVK", obj.GetObjectKind().GroupVersionKind()) return err