From 4db31f931d269f1efb3837a852e1fdf742cbb159 Mon Sep 17 00:00:00 2001 From: Simone Tiraboschi Date: Mon, 4 Mar 2024 06:57:08 +0100 Subject: [PATCH] [release-1.11] Avoid removing user labels (#2743) (#2812) * [release-1.11] Avoid removing user labels (#2743) Reconcile managed labels on operands without removing user added ones. Add unit and func tests. This is a manual cherry-pick of #2743 Signed-off-by: Simone Tiraboschi * Fix additional cases in labels handling (#2820) Fix additional cases in labels handling on operands, add more unit tests. Signed-off-by: Simone Tiraboschi --------- Signed-off-by: Simone Tiraboschi --- controllers/alerts/metrics_test.go | 12 +- controllers/alerts/reconciler.go | 4 +- controllers/commontestutils/testUtils.go | 9 + controllers/operands/aaq.go | 4 +- controllers/operands/aaq_test.go | 72 +++++ controllers/operands/cdi.go | 4 +- controllers/operands/cdi_test.go | 68 +++++ controllers/operands/cliDownload.go | 6 +- controllers/operands/cliDownload_test.go | 70 ++++- controllers/operands/cmHandler.go | 9 +- controllers/operands/dashboard_test.go | 131 +++++++++ controllers/operands/deploymentHandler.go | 4 +- .../operands/deploymentHandler_test.go | 78 ++++- controllers/operands/imageStream.go | 4 +- controllers/operands/imageStream_test.go | 20 +- controllers/operands/kubevirt.go | 6 +- controllers/operands/kubevirtConsolePlugin.go | 6 +- .../operands/kubevirtConsolePlugin_test.go | 75 ++++- controllers/operands/kubevirt_test.go | 69 +++++ controllers/operands/mtq.go | 6 +- controllers/operands/mtq_test.go | 70 +++++ controllers/operands/networkAddons.go | 4 +- controllers/operands/networkAddons_test.go | 68 +++++ controllers/operands/quickStart.go | 6 +- controllers/operands/quickStart_test.go | 134 +++++++++ controllers/operands/rbac.go | 14 +- controllers/operands/serviceHandler.go | 8 +- controllers/operands/ssp.go | 6 +- controllers/operands/ssp_test.go | 68 +++++ .../operands/virtio_win_ConfigMap_test.go | 66 +++++ pkg/util/labels.go | 20 +- pkg/util/labels_test.go | 268 +++++++++++++++++- tests/func-tests/golden_image_test.go | 18 +- tests/func-tests/labels_test.go | 24 ++ .../func-tests/update_priority_class_test.go | 4 +- .../pkg/util/labels.go | 20 +- .../onsi/gomega/gstruct/elements.go | 231 +++++++++++++++ .../gomega/gstruct/errors/nested_types.go | 72 +++++ .../github.com/onsi/gomega/gstruct/fields.go | 165 +++++++++++ .../github.com/onsi/gomega/gstruct/ignore.go | 39 +++ vendor/github.com/onsi/gomega/gstruct/keys.go | 126 ++++++++ .../github.com/onsi/gomega/gstruct/pointer.go | 58 ++++ .../github.com/onsi/gomega/gstruct/types.go | 15 + vendor/modules.txt | 2 + 44 files changed, 2095 insertions(+), 68 deletions(-) create mode 100644 vendor/github.com/onsi/gomega/gstruct/elements.go create mode 100644 vendor/github.com/onsi/gomega/gstruct/errors/nested_types.go create mode 100644 vendor/github.com/onsi/gomega/gstruct/fields.go create mode 100644 vendor/github.com/onsi/gomega/gstruct/ignore.go create mode 100644 vendor/github.com/onsi/gomega/gstruct/keys.go create mode 100644 vendor/github.com/onsi/gomega/gstruct/pointer.go create mode 100644 vendor/github.com/onsi/gomega/gstruct/types.go diff --git a/controllers/alerts/metrics_test.go b/controllers/alerts/metrics_test.go index b9b8bb61d0..003edcf5ee 100644 --- a/controllers/alerts/metrics_test.go +++ b/controllers/alerts/metrics_test.go @@ -11,6 +11,8 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/gstruct" + monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -155,7 +157,7 @@ var _ = Describe("alert tests", func() { pr := &monitoringv1.PrometheusRule{} Expect(cl.Get(context.Background(), client.ObjectKey{Namespace: r.namespace, Name: ruleName}, pr)).Should(Succeed()) - Expect(pr.Labels).Should(Equal(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring))) + Expect(pr.Labels).To(gstruct.MatchKeys(gstruct.IgnoreExtras, commontestutils.KeysFromSSMap(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring)))) Expect(ee.CheckEvents(expectedEvents)).To(BeTrue()) Expect(metrics.GetOverwrittenModificationsCount(monitoringv1.PrometheusRuleKind, ruleName)).Should(BeEquivalentTo(currentMetric)) }) @@ -406,7 +408,7 @@ var _ = Describe("alert tests", func() { role := &rbacv1.Role{} Expect(cl.Get(context.Background(), client.ObjectKey{Namespace: r.namespace, Name: roleName}, role)).Should(Succeed()) - Expect(role.Labels).Should(Equal(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring))) + Expect(role.Labels).To(gstruct.MatchKeys(gstruct.IgnoreExtras, commontestutils.KeysFromSSMap(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring)))) Expect(ee.CheckEvents(expectedEvents)).To(BeTrue()) Expect(metrics.GetOverwrittenModificationsCount("Role", roleName)).Should(BeEquivalentTo(currentMetric)) }) @@ -601,7 +603,7 @@ var _ = Describe("alert tests", func() { rb := &rbacv1.RoleBinding{} Expect(cl.Get(context.Background(), client.ObjectKey{Namespace: r.namespace, Name: roleName}, rb)).Should(Succeed()) - Expect(rb.Labels).Should(Equal(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring))) + Expect(rb.Labels).To(gstruct.MatchKeys(gstruct.IgnoreExtras, commontestutils.KeysFromSSMap(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring)))) Expect(ee.CheckEvents(expectedEvents)).To(BeTrue()) Expect(metrics.GetOverwrittenModificationsCount("RoleBinding", roleName)).Should(BeEquivalentTo(currentMetric)) }) @@ -842,7 +844,7 @@ var _ = Describe("alert tests", func() { svc := &corev1.Service{} Expect(cl.Get(context.Background(), client.ObjectKey{Namespace: r.namespace, Name: serviceName}, svc)).Should(Succeed()) - Expect(svc.Labels).Should(Equal(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring))) + Expect(svc.Labels).To(gstruct.MatchKeys(gstruct.IgnoreExtras, commontestutils.KeysFromSSMap(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring)))) Expect(ee.CheckEvents(expectedEvents)).To(BeTrue()) Expect(metrics.GetOverwrittenModificationsCount("Service", serviceName)).Should(BeEquivalentTo(currentMetric)) }) @@ -1045,7 +1047,7 @@ var _ = Describe("alert tests", func() { sm := &monitoringv1.ServiceMonitor{} Expect(cl.Get(context.Background(), client.ObjectKey{Namespace: r.namespace, Name: serviceName}, sm)).Should(Succeed()) - Expect(sm.Labels).Should(Equal(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring))) + Expect(sm.Labels).To(gstruct.MatchKeys(gstruct.IgnoreExtras, commontestutils.KeysFromSSMap(hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring)))) Expect(ee.CheckEvents(expectedEvents)).To(BeTrue()) Expect(metrics.GetOverwrittenModificationsCount("ServiceMonitor", serviceName)).Should(BeEquivalentTo(currentMetric)) }) diff --git a/controllers/alerts/reconciler.go b/controllers/alerts/reconciler.go index 5c9ded9097..a0da65ef13 100644 --- a/controllers/alerts/reconciler.go +++ b/controllers/alerts/reconciler.go @@ -173,11 +173,11 @@ func getDeploymentReference(deployment *appsv1.Deployment) metav1.OwnerReference // return true if something was changed func updateCommonDetails(required, existing *metav1.ObjectMeta) bool { if reflect.DeepEqual(required.OwnerReferences, existing.OwnerReferences) && - reflect.DeepEqual(required.Labels, existing.Labels) { + hcoutil.CompareLabels(required, existing) { return false } - hcoutil.DeepCopyLabels(required, existing) + hcoutil.MergeLabels(required, existing) if reqLen := len(required.OwnerReferences); reqLen > 0 { refs := make([]metav1.OwnerReference, reqLen) for i, ref := range required.OwnerReferences { diff --git a/controllers/commontestutils/testUtils.go b/controllers/commontestutils/testUtils.go index b8c1db440a..4b554f292e 100644 --- a/controllers/commontestutils/testUtils.go +++ b/controllers/commontestutils/testUtils.go @@ -7,6 +7,7 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" //nolint dot-imports . "github.com/onsi/gomega" //nolint dot-imports + "github.com/onsi/gomega/gstruct" gomegatypes "github.com/onsi/gomega/types" openshiftconfigv1 "github.com/openshift/api/config/v1" consolev1 "github.com/openshift/api/console/v1" @@ -445,3 +446,11 @@ func (ClusterInfoSRCPHAIMock) GetTLSSecurityProfile(_ *openshiftconfigv1.TLSSecu func (ClusterInfoSRCPHAIMock) RefreshAPIServerCR(_ context.Context, _ client.Client) error { return nil } + +func KeysFromSSMap(ssmap map[string]string) gstruct.Keys { + keys := gstruct.Keys{} + for k, v := range ssmap { + keys[k] = Equal(v) + } + return keys +} diff --git a/controllers/operands/aaq.go b/controllers/operands/aaq.go index 3fd7f6ae2c..4ef3c92f76 100644 --- a/controllers/operands/aaq.go +++ b/controllers/operands/aaq.go @@ -68,7 +68,7 @@ func (*aaqHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r } if !reflect.DeepEqual(found.Spec, aaq.Spec) || - !reflect.DeepEqual(found.Labels, aaq.Labels) { + !hcoutil.CompareLabels(aaq, found) { overwritten := false if req.HCOTriggered { req.Logger.Info("Updating existing AAQ's Spec to new opinionated values") @@ -76,7 +76,7 @@ func (*aaqHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r req.Logger.Info("Reconciling an externally updated AAQ's Spec to its opinionated values") overwritten = true } - hcoutil.DeepCopyLabels(&aaq.ObjectMeta, &found.ObjectMeta) + hcoutil.MergeLabels(&aaq.ObjectMeta, &found.ObjectMeta) aaq.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/aaq_test.go b/controllers/operands/aaq_test.go index 44d48020f2..b47199b495 100644 --- a/controllers/operands/aaq_test.go +++ b/controllers/operands/aaq_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -278,6 +279,77 @@ var _ = Describe("AAQ tests", func() { Expect(foundAAQ.Spec.CertConfig.Server.Duration.Duration.String()).Should(Equal("24h0m0s")) Expect(foundAAQ.Spec.CertConfig.Server.RenewBefore.Duration.String()).Should(Equal("12h0m0s")) }) + + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + hco.Spec.ApplicationAwareConfig = &v1beta1.ApplicationAwareConfigurations{} + hco.Spec.FeatureGates.EnableApplicationAwareQuota = ptr.To(true) + outdatedResource := NewAAQWithNameOnly(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := newAAQHandler(cl, commontestutils.GetScheme()) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &aaqv1alpha1.AAQ{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + hco.Spec.ApplicationAwareConfig = &v1beta1.ApplicationAwareConfigurations{} + hco.Spec.FeatureGates.EnableApplicationAwareQuota = ptr.To(true) + outdatedResource := NewAAQWithNameOnly(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := newAAQHandler(cl, commontestutils.GetScheme()) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &aaqv1alpha1.AAQ{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + }) Context("check cache", func() { diff --git a/controllers/operands/cdi.go b/controllers/operands/cdi.go index 8f1c05aade..aabd2dda68 100644 --- a/controllers/operands/cdi.go +++ b/controllers/operands/cdi.go @@ -67,7 +67,7 @@ func (*cdiHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r } if !reflect.DeepEqual(found.Spec, cdi.Spec) || - !reflect.DeepEqual(found.Labels, cdi.Labels) { + !util.CompareLabels(cdi, found) { overwritten := false if req.HCOTriggered { req.Logger.Info("Updating existing CDI's Spec to new opinionated values") @@ -75,7 +75,7 @@ func (*cdiHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r req.Logger.Info("Reconciling an externally updated CDI's Spec to its opinionated values") overwritten = true } - util.DeepCopyLabels(&cdi.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&cdi.ObjectMeta, &found.ObjectMeta) cdi.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/cdi_test.go b/controllers/operands/cdi_test.go index ba7833b291..70a013d414 100644 --- a/controllers/operands/cdi_test.go +++ b/controllers/operands/cdi_test.go @@ -96,6 +96,74 @@ var _ = Describe("CDI Operand", func() { })) }) + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewCDI(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newCdiHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &cdiv1beta1.CDI{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewCDI(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newCdiHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &cdiv1beta1.CDI{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + It("should set default UninstallStrategy if missing", func() { expectedResource, err := NewCDI(hco) Expect(err).ToNot(HaveOccurred()) diff --git a/controllers/operands/cliDownload.go b/controllers/operands/cliDownload.go index 47ef6206f1..df26b0196e 100644 --- a/controllers/operands/cliDownload.go +++ b/controllers/operands/cliDownload.go @@ -57,13 +57,13 @@ func (*cliDownloadHooks) updateCr(req *common.HcoRequest, Client client.Client, return false, false, errors.New("can't convert to ConsoleCLIDownload") } if !reflect.DeepEqual(found.Spec, ccd.Spec) || - !reflect.DeepEqual(found.Labels, ccd.Labels) { + !util.CompareLabels(ccd, found) { if req.HCOTriggered { req.Logger.Info("Updating existing ConsoleCLIDownload's Spec to new opinionated values") } else { req.Logger.Info("Reconciling an externally updated ConsoleCLIDownload's Spec to its opinionated values") } - util.DeepCopyLabels(&ccd.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&ccd.ObjectMeta, &found.ObjectMeta) ccd.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { @@ -180,7 +180,7 @@ func (*cliDownloadsRouteHooks) updateCr(req *common.HcoRequest, Client client.Cl } else { req.Logger.Info("Reconciling an externally updated Route Spec to its opinionated values") } - util.DeepCopyLabels(&route.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&route.ObjectMeta, &found.ObjectMeta) route.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/cliDownload_test.go b/controllers/operands/cliDownload_test.go index b3cf85b979..4f34e4afdc 100644 --- a/controllers/operands/cliDownload_test.go +++ b/controllers/operands/cliDownload_test.go @@ -9,6 +9,7 @@ import ( consolev1 "github.com/openshift/api/console/v1" routev1 "github.com/openshift/api/route/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/tools/reference" "sigs.k8s.io/controller-runtime/pkg/client" @@ -100,6 +101,72 @@ var _ = Describe("CLI Download", func() { }), ) + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewConsoleCLIDownload(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newCliDownloadHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &consolev1.ConsoleCLIDownload{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewConsoleCLIDownload(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newCliDownloadHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &consolev1.ConsoleCLIDownload{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + }) }) @@ -251,8 +318,9 @@ var _ = Describe("Cli Downloads Route", func() { Expect(hco.Status.RelatedObjects).To(ContainElement(*objectRefFound)) }, Entry("with modified labels", func(resource *routev1.Route) { - resource.Labels = map[string]string{"key": "value"} + resource.Labels = map[string]string{"app.kubernetes.io/managed-by": "value"} }), + // TODO: add another test for additional labels Entry("with modified port", func(resource *routev1.Route) { resource.Spec.Port = &routev1.RoutePort{ TargetPort: intstr.IntOrString{IntVal: 1111}, diff --git a/controllers/operands/cmHandler.go b/controllers/operands/cmHandler.go index 5f3259cfb1..3e30abfcaf 100644 --- a/controllers/operands/cmHandler.go +++ b/controllers/operands/cmHandler.go @@ -49,14 +49,17 @@ func (h cmHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r } if !reflect.DeepEqual(found.Data, h.required.Data) || - !reflect.DeepEqual(found.Labels, h.required.Labels) { + !util.CompareLabels(h.required, found) { if req.HCOTriggered { req.Logger.Info("Updating existing Configmap to new opinionated values", "name", h.required.Name) } else { req.Logger.Info("Reconciling an externally updated Configmap to its opinionated values", "name", h.required.Name) } - util.DeepCopyLabels(&h.required.ObjectMeta, &found.ObjectMeta) - h.required.DeepCopyInto(found) + util.MergeLabels(&h.required.ObjectMeta, &found.ObjectMeta) + found.Data = make(map[string]string, len(h.required.Data)) + for k, v := range h.required.Data { + found.Data[k] = v + } err := Client.Update(req.Ctx, found) if err != nil { return false, false, err diff --git a/controllers/operands/dashboard_test.go b/controllers/operands/dashboard_test.go index 3999f14222..d96d035681 100644 --- a/controllers/operands/dashboard_test.go +++ b/controllers/operands/dashboard_test.go @@ -16,6 +16,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/kubevirt/hyperconverged-cluster-operator/controllers/commontestutils" + hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" ) var _ = Describe("Dashboard tests", func() { @@ -165,5 +166,135 @@ var _ = Describe("Dashboard tests", func() { Expect(ok).Should(BeTrue()) }) }) + + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + + _ = os.Setenv(dashboardManifestLocationVarName, testFilesLocation) + cli := commontestutils.InitClient([]client.Object{}) + handlers, err := getDashboardHandlers(logger, cli, schemeForTest, hco) + Expect(err).ToNot(HaveOccurred()) + Expect(handlers).To(HaveLen(1)) + + cmList := &corev1.ConfigMapList{} + + req := commontestutils.NewReq(hco) + + By("apply the CMs", func() { + res := handlers[0].ensure(req) + Expect(res.Err).ToNot(HaveOccurred()) + Expect(res.Created).To(BeTrue()) + + Expect(cli.List(context.TODO(), cmList)).To(Succeed()) + Expect(cmList.Items).To(HaveLen(1)) + Expect(cmList.Items[0].Name).Should(Equal("grafana-dashboard-kubevirt-top-consumers")) + }) + + expectedLabels := make(map[string]map[string]string) + + By("getting opinionated labels", func() { + for _, cm := range cmList.Items { + expectedLabels[cm.Name] = make(map[string]string) + for k, v := range cm.Labels { + expectedLabels[cm.Name][k] = v + } + } + }) + + By("altering the cm objects", func() { + for _, foundResource := range cmList.Items { + for k, v := range expectedLabels[foundResource.Name] { + foundResource.Labels[k] = "wrong_" + v + } + foundResource.Labels[userLabelKey] = userLabelValue + err = cli.Update(context.TODO(), &foundResource) + Expect(err).ToNot(HaveOccurred()) + } + }) + + By("reconciling cm objects", func() { + for _, handler := range handlers { + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + } + }) + + foundResourcesList := &corev1.ConfigMapList{} + Expect(cli.List(context.TODO(), foundResourcesList)).To(Succeed()) + + for _, foundResource := range foundResourcesList.Items { + for k, v := range expectedLabels[foundResource.Name] { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + } + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + + _ = os.Setenv(dashboardManifestLocationVarName, testFilesLocation) + cli := commontestutils.InitClient([]client.Object{}) + handlers, err := getDashboardHandlers(logger, cli, schemeForTest, hco) + Expect(err).ToNot(HaveOccurred()) + Expect(handlers).To(HaveLen(1)) + + cmList := &corev1.ConfigMapList{} + + req := commontestutils.NewReq(hco) + + By("apply the CMs", func() { + res := handlers[0].ensure(req) + Expect(res.Err).ToNot(HaveOccurred()) + Expect(res.Created).To(BeTrue()) + + Expect(cli.List(context.TODO(), cmList)).To(Succeed()) + Expect(cmList.Items).To(HaveLen(1)) + Expect(cmList.Items[0].Name).Should(Equal("grafana-dashboard-kubevirt-top-consumers")) + }) + + expectedLabels := make(map[string]map[string]string) + + By("getting opinionated labels", func() { + for _, cm := range cmList.Items { + expectedLabels[cm.Name] = make(map[string]string) + for k, v := range cm.Labels { + expectedLabels[cm.Name][k] = v + } + } + }) + + By("altering the cm objects", func() { + for _, foundResource := range cmList.Items { + foundResource.Labels[userLabelKey] = userLabelValue + delete(foundResource.Labels, hcoutil.AppLabelVersion) + err = cli.Update(context.TODO(), &foundResource) + Expect(err).ToNot(HaveOccurred()) + } + }) + + By("reconciling cm objects", func() { + for _, handler := range handlers { + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + } + }) + + foundResourcesList := &corev1.ConfigMapList{} + Expect(cli.List(context.TODO(), foundResourcesList)).To(Succeed()) + + for _, foundResource := range foundResourcesList.Items { + for k, v := range expectedLabels[foundResource.Name] { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + } + }) }) }) diff --git a/controllers/operands/deploymentHandler.go b/controllers/operands/deploymentHandler.go index 6fceb24cc0..a044b14559 100644 --- a/controllers/operands/deploymentHandler.go +++ b/controllers/operands/deploymentHandler.go @@ -84,7 +84,7 @@ func (h *deploymentHooks) updateCr(req *common.HcoRequest, Client client.Client, } return true, !req.HCOTriggered, nil } - util.DeepCopyLabels(&deployment.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&deployment.ObjectMeta, &found.ObjectMeta) deployment.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { @@ -99,7 +99,7 @@ func (h *deploymentHooks) updateCr(req *common.HcoRequest, Client client.Client, // We need to check only certain fields in the deployment resource, since some of the fields // are being set by k8s. func hasCorrectDeploymentFields(found *appsv1.Deployment, required *appsv1.Deployment) bool { - return reflect.DeepEqual(found.Labels, required.Labels) && + return util.CompareLabels(required, found) && reflect.DeepEqual(found.Spec.Selector, required.Spec.Selector) && reflect.DeepEqual(found.Spec.Replicas, required.Spec.Replicas) && reflect.DeepEqual(found.Spec.Template.Spec.Containers, required.Spec.Template.Spec.Containers) && diff --git a/controllers/operands/deploymentHandler_test.go b/controllers/operands/deploymentHandler_test.go index d44db891dd..b707174645 100644 --- a/controllers/operands/deploymentHandler_test.go +++ b/controllers/operands/deploymentHandler_test.go @@ -71,7 +71,8 @@ var _ = Describe("Deployment Handler", func() { expectedDeployment.DeepCopyInto(modifiedDeployment) // modify only the labels gotLabels := modifiedDeployment.GetLabels() - gotLabels["key2"] = "value2" + gotLabels["key1"] = "wrongValue1" + gotLabels["key2"] = "newValue2" modifiedDeployment.SetLabels(gotLabels) modifiedDeployment.ObjectMeta.UID = "oldObjectUID" @@ -102,6 +103,81 @@ var _ = Describe("Deployment Handler", func() { // let's check the object UID to ensure that the object get updated and not deleted and recreated Expect(foundResource.GetUID()).To(Equal(types.UID("oldObjectUID"))) }) + + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewExpectedDeployment(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := newDeploymentHandler(cl, commontestutils.GetScheme(), NewExpectedDeployment, hco) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &appsv1.Deployment{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewExpectedDeployment(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + + removed := false + for k := range outdatedResource.Labels { + if !removed { + delete(outdatedResource.Labels, k) + removed = true + } + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := newDeploymentHandler(cl, commontestutils.GetScheme(), NewExpectedDeployment, hco) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &appsv1.Deployment{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + }) }) diff --git a/controllers/operands/imageStream.go b/controllers/operands/imageStream.go index 0b1eab74a1..3eed692538 100644 --- a/controllers/operands/imageStream.go +++ b/controllers/operands/imageStream.go @@ -176,8 +176,8 @@ func (isHooks) justBeforeComplete(_ *common.HcoRequest) { /* no implementation * func (h isHooks) compareAndUpgradeImageStream(found *imagev1.ImageStream) bool { modified := false - if !reflect.DeepEqual(h.required.Labels, found.Labels) { - util.DeepCopyLabels(&h.required.ObjectMeta, &found.ObjectMeta) + if !util.CompareLabels(h.required, found) { + util.MergeLabels(&h.required.ObjectMeta, &found.ObjectMeta) modified = true } diff --git a/controllers/operands/imageStream_test.go b/controllers/operands/imageStream_test.go index 6df200944b..53600da1a8 100644 --- a/controllers/operands/imageStream_test.go +++ b/controllers/operands/imageStream_test.go @@ -656,6 +656,9 @@ var _ = Describe("imageStream tests", func() { return testFilesLocation } + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + exists := &imagev1.ImageStream{ ObjectMeta: metav1.ObjectMeta{ Name: "test-image-stream", @@ -675,7 +678,15 @@ var _ = Describe("imageStream tests", func() { }, } exists.Labels = getLabels(hco, util.AppComponentCompute) - exists.ObjectMeta.Labels["to-be-removed"] = "test" + expectedLabels := make(map[string]string, len(exists.Labels)) + for k, v := range exists.Labels { + expectedLabels[k] = v + } + exists.Labels[userLabelKey] = userLabelValue + for k, v := range expectedLabels { + exists.Labels[k] = "wrong_" + v + } + exists.Labels[util.AppLabelManagedBy] = expectedLabels[util.AppLabelManagedBy] cli := commontestutils.InitClient([]client.Object{exists}) handlers, err := getImageStreamHandlers(logger, cli, schemeForTest, hco) @@ -690,7 +701,9 @@ var _ = Describe("imageStream tests", func() { req := commontestutils.NewReq(hco) res := handlers[0].ensure(req) Expect(res.Err).ToNot(HaveOccurred()) + Expect(res.UpgradeDone).To(BeFalse()) Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) imageStreamObjects := &imagev1.ImageStreamList{} Expect(cli.List(context.TODO(), imageStreamObjects)).To(Succeed()) @@ -714,7 +727,10 @@ var _ = Describe("imageStream tests", func() { Expect(hco.Status.RelatedObjects).To(Not(ContainElement(*objectRefOutdated))) Expect(hco.Status.RelatedObjects).To(ContainElement(*objectRefFound)) - Expect(exists.ObjectMeta.Labels).ToNot(ContainElement("to-be-removed")) + for k, v := range expectedLabels { + Expect(is.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(is.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) }) }) diff --git a/controllers/operands/kubevirt.go b/controllers/operands/kubevirt.go index 7b8131c2b4..654244794c 100644 --- a/controllers/operands/kubevirt.go +++ b/controllers/operands/kubevirt.go @@ -249,14 +249,14 @@ func (*kubevirtHooks) updateCr(req *common.HcoRequest, Client client.Client, exi } if !reflect.DeepEqual(found.Spec, virt.Spec) || - !reflect.DeepEqual(found.Labels, virt.Labels) || + !hcoutil.CompareLabels(virt, found) || !isAnnotationStateMeetingRequirements(virt.Annotations, found.Annotations) { if req.HCOTriggered { req.Logger.Info("Updating existing KubeVirt's Spec to new opinionated values") } else { req.Logger.Info("Reconciling an externally updated KubeVirt's Spec to its opinionated values") } - hcoutil.DeepCopyLabels(&virt.ObjectMeta, &found.ObjectMeta) + hcoutil.MergeLabels(&virt.ObjectMeta, &found.ObjectMeta) setAnnotationsToReqState(req.Instance, found) virt.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) @@ -804,7 +804,7 @@ func (kvPriorityClassHooks) updateCr(req *common.HcoRequest, Client client.Clien // at this point we found the object in the cache and we check if something was changed if (pc.Name == found.Name) && (pc.Value == found.Value) && - (pc.Description == found.Description) && reflect.DeepEqual(pc.Labels, found.Labels) { + (pc.Description == found.Description) && hcoutil.CompareLabels(&pc.ObjectMeta, &found.ObjectMeta) { return false, false, nil } diff --git a/controllers/operands/kubevirtConsolePlugin.go b/controllers/operands/kubevirtConsolePlugin.go index a5b0cfb746..1c1c414826 100644 --- a/controllers/operands/kubevirtConsolePlugin.go +++ b/controllers/operands/kubevirtConsolePlugin.go @@ -354,14 +354,14 @@ func (h consolePluginHooks) updateCr(req *common.HcoRequest, Client client.Clien return false, false, errors.New("can't convert to ConsolePlugin") } - if !reflect.DeepEqual(found.Spec, h.required.Spec) || - !reflect.DeepEqual(found.Labels, h.required.Labels) { + if !reflect.DeepEqual(h.required.Spec, found.Spec) || + !hcoutil.CompareLabels(h.required, found) { if req.HCOTriggered { req.Logger.Info("Updating existing ConsolePlugin to new opinionated values", "name", h.required.Name) } else { req.Logger.Info("Reconciling an externally updated ConsolePlugin to its opinionated values", "name", h.required.Name) } - hcoutil.DeepCopyLabels(&h.required.ObjectMeta, &found.ObjectMeta) + hcoutil.MergeLabels(&h.required.ObjectMeta, &found.ObjectMeta) h.required.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/kubevirtConsolePlugin_test.go b/controllers/operands/kubevirtConsolePlugin_test.go index 845d90e62e..39edc1b512 100644 --- a/controllers/operands/kubevirtConsolePlugin_test.go +++ b/controllers/operands/kubevirtConsolePlugin_test.go @@ -144,7 +144,8 @@ var _ = Describe("Kubevirt Console Plugin", func() { expectedResource := NewKVConsolePlugin(hco) outdatedResource := NewKVConsolePlugin(hco) - outdatedResource.Labels["fake_label"] = "something" + outdatedResource.Labels["app.kubernetes.io/managed-by"] = "something" + // TODO: add another test for extra labels! cl := commontestutils.InitClient([]client.Object{hco, outdatedResource, expectedConsoleConfig}) handler, _ := newKvUIPluginCRHandler(logger, cl, commontestutils.GetScheme(), hco) @@ -187,6 +188,78 @@ var _ = Describe("Kubevirt Console Plugin", func() { Expect(reflect.DeepEqual(foundResource.Labels, expectedResource.Labels)).To(BeTrue()) }) + + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewKVConsolePlugin(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler, _ := newKvUIPluginCRHandler(logger, cl, commontestutils.GetScheme(), hco) + + res := handler[0].ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &consolev1.ConsolePlugin{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewExpectedDeployment(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + removed := false + for k := range outdatedResource.Labels { + if !removed { + delete(outdatedResource.Labels, k) + removed = true + } + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := newDeploymentHandler(cl, commontestutils.GetScheme(), NewExpectedDeployment, hco) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &appsv1.Deployment{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) }) Context("Kubevirt Console Plugin and UI Proxy Deployments", func() { diff --git a/controllers/operands/kubevirt_test.go b/controllers/operands/kubevirt_test.go index 57d9c5b5d9..a4514c1bbb 100644 --- a/controllers/operands/kubevirt_test.go +++ b/controllers/operands/kubevirt_test.go @@ -293,6 +293,75 @@ Version: 1.2.3`) })) }) + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewKubeVirt(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, "app.kubernetes.io/version") + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newKubevirtHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &kubevirtcorev1.KubeVirt{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewKubeVirt(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newKubevirtHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &kubevirtcorev1.KubeVirt{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + It("should force mandatory configurations", func() { mandatoryKvFeatureGates = getMandatoryKvFeatureGates(false) hco.Spec.FeatureGates = hcov1beta1.HyperConvergedFeatureGates{ diff --git a/controllers/operands/mtq.go b/controllers/operands/mtq.go index 059656ea10..e41bbc699b 100644 --- a/controllers/operands/mtq.go +++ b/controllers/operands/mtq.go @@ -68,8 +68,8 @@ func (*mtqHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r return false, false, errors.New("can't convert to MTQ") } - if !reflect.DeepEqual(found.Spec, mtq.Spec) || - !reflect.DeepEqual(found.Labels, mtq.Labels) { + if !reflect.DeepEqual(mtq.Spec, found.Spec) || + !hcoutil.CompareLabels(mtq, found) { overwritten := false if req.HCOTriggered { req.Logger.Info("Updating existing MTQ's Spec to new opinionated values") @@ -77,7 +77,7 @@ func (*mtqHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r req.Logger.Info("Reconciling an externally updated MTQ's Spec to its opinionated values") overwritten = true } - hcoutil.DeepCopyLabels(&mtq.ObjectMeta, &found.ObjectMeta) + hcoutil.MergeLabels(&mtq.ObjectMeta, &found.ObjectMeta) mtq.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/mtq_test.go b/controllers/operands/mtq_test.go index b647142a19..84f37fbabd 100644 --- a/controllers/operands/mtq_test.go +++ b/controllers/operands/mtq_test.go @@ -9,6 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -257,6 +258,75 @@ var _ = Describe("MTQ tests", func() { Expect(foundMTQ.Spec.CertConfig.Server.Duration.Duration.String()).Should(Equal("24h0m0s")) Expect(foundMTQ.Spec.CertConfig.Server.RenewBefore.Duration.String()).Should(Equal("12h0m0s")) }) + + It("should reconcile managed labels to default without touching user added ones", func() { + hco.Spec.FeatureGates.EnableManagedTenantQuota = ptr.To(true) + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewMTQ(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + outdatedResource.Labels[hcoutil.AppLabel] = expectedLabels[hcoutil.AppLabel] + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := newMtqHandler(cl, commontestutils.GetScheme()) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &mtqv1alpha1.MTQ{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + hco.Spec.FeatureGates.EnableManagedTenantQuota = ptr.To(true) + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource := NewMTQ(hco) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := newMtqHandler(cl, commontestutils.GetScheme()) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &mtqv1alpha1.MTQ{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) }) Context("check cache", func() { diff --git a/controllers/operands/networkAddons.go b/controllers/operands/networkAddons.go index ff9ff61349..c18e7a457c 100644 --- a/controllers/operands/networkAddons.go +++ b/controllers/operands/networkAddons.go @@ -94,8 +94,8 @@ func (*cnaHooks) updateCnaCr(req *common.HcoRequest, Client client.Client, found } func (*cnaHooks) updateLabels(found *networkaddonsv1.NetworkAddonsConfig, networkAddons *networkaddonsv1.NetworkAddonsConfig) bool { - if !reflect.DeepEqual(found.Labels, networkAddons.Labels) { - util.DeepCopyLabels(&networkAddons.ObjectMeta, &found.ObjectMeta) + if !util.CompareLabels(networkAddons, found) { + util.MergeLabels(&networkAddons.ObjectMeta, &found.ObjectMeta) return true } return false diff --git a/controllers/operands/networkAddons_test.go b/controllers/operands/networkAddons_test.go index 913a08e752..99c1fab92c 100644 --- a/controllers/operands/networkAddons_test.go +++ b/controllers/operands/networkAddons_test.go @@ -128,6 +128,74 @@ var _ = Describe("CNA Operand", func() { }) + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewNetworkAddons(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newCnaHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &networkaddonsv1.NetworkAddonsConfig{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewNetworkAddons(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newCnaHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &networkaddonsv1.NetworkAddonsConfig{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + It("should add node placement if missing in CNAO", func() { existingResource, err := NewNetworkAddons(hco) Expect(err).ToNot(HaveOccurred()) diff --git a/controllers/operands/quickStart.go b/controllers/operands/quickStart.go index 386c7a4157..d063dec2c4 100644 --- a/controllers/operands/quickStart.go +++ b/controllers/operands/quickStart.go @@ -65,14 +65,14 @@ func (h qsHooks) updateCr(req *common.HcoRequest, Client client.Client, exists r return false, false, errors.New("can't convert to ConsoleQuickStart") } - if !reflect.DeepEqual(found.Spec, h.required.Spec) || - !reflect.DeepEqual(found.Labels, h.required.Labels) { + if !reflect.DeepEqual(h.required.Spec, found.Spec) || + !util.CompareLabels(h.required, found) { if req.HCOTriggered { req.Logger.Info("Updating existing ConsoleQuickStart's Spec to new opinionated values", "name", h.required.Name) } else { req.Logger.Info("Reconciling an externally updated ConsoleQuickStart's Spec to its opinionated values", "name", h.required.Name) } - util.DeepCopyLabels(&h.required.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&h.required.ObjectMeta, &found.ObjectMeta) h.required.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/quickStart_test.go b/controllers/operands/quickStart_test.go index 97a06f770f..eb571a4020 100644 --- a/controllers/operands/quickStart_test.go +++ b/controllers/operands/quickStart_test.go @@ -9,6 +9,8 @@ import ( "strings" "time" + hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" + "k8s.io/client-go/tools/reference" . "github.com/onsi/ginkgo/v2" @@ -199,5 +201,137 @@ var _ = Describe("QuickStart tests", func() { Expect(hco.Status.RelatedObjects).To(ContainElement(*objectRefFound)) }) }) + + It("should reconcile managed labels to default without touching user added ones", func() { + _ = os.Setenv(quickStartManifestLocationVarName, testFilesLocation) + + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + + cli := commontestutils.InitClient([]client.Object{qsCrd}) + handlers, err := getQuickStartHandlers(logger, cli, schemeForTest, hco) + Expect(err).ToNot(HaveOccurred()) + Expect(handlers).To(HaveLen(1)) + + quickstartObjects := &consolev1.ConsoleQuickStartList{} + + req := commontestutils.NewReq(hco) + + By("apply the quickstart CRs", func() { + res := handlers[0].ensure(req) + Expect(res.Err).ToNot(HaveOccurred()) + Expect(res.Created).To(BeTrue()) + + Expect(cli.List(context.TODO(), quickstartObjects)).To(Succeed()) + Expect(quickstartObjects.Items).To(HaveLen(1)) + Expect(quickstartObjects.Items[0].Name).Should(Equal("test-quick-start")) + }) + + expectedLabels := make(map[string]map[string]string) + + By("getting opinionated labels", func() { + for _, quickstart := range quickstartObjects.Items { + expectedLabels[quickstart.Name] = make(map[string]string) + for k, v := range quickstart.Labels { + expectedLabels[quickstart.Name][k] = v + } + } + }) + + By("altering the quickstart objects", func() { + for _, foundResource := range quickstartObjects.Items { + for k, v := range expectedLabels[foundResource.Name] { + foundResource.Labels[k] = "wrong_" + v + } + foundResource.Labels[userLabelKey] = userLabelValue + err = cli.Update(context.TODO(), &foundResource) + Expect(err).ToNot(HaveOccurred()) + } + }) + + By("reconciling quickstart objects", func() { + for _, handler := range handlers { + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + } + }) + + foundResourcesList := &consolev1.ConsoleQuickStartList{} + Expect(cli.List(context.TODO(), foundResourcesList)).To(Succeed()) + + for _, foundResource := range foundResourcesList.Items { + for k, v := range expectedLabels[foundResource.Name] { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + } + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + _ = os.Setenv(quickStartManifestLocationVarName, testFilesLocation) + + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + + cli := commontestutils.InitClient([]client.Object{qsCrd}) + handlers, err := getQuickStartHandlers(logger, cli, schemeForTest, hco) + Expect(err).ToNot(HaveOccurred()) + Expect(handlers).To(HaveLen(1)) + + quickstartObjects := &consolev1.ConsoleQuickStartList{} + + req := commontestutils.NewReq(hco) + + By("apply the quickstart CRs", func() { + res := handlers[0].ensure(req) + Expect(res.Err).ToNot(HaveOccurred()) + Expect(res.Created).To(BeTrue()) + + Expect(cli.List(context.TODO(), quickstartObjects)).To(Succeed()) + Expect(quickstartObjects.Items).To(HaveLen(1)) + Expect(quickstartObjects.Items[0].Name).Should(Equal("test-quick-start")) + }) + + expectedLabels := make(map[string]map[string]string) + + By("getting opinionated labels", func() { + for _, quickstart := range quickstartObjects.Items { + expectedLabels[quickstart.Name] = make(map[string]string) + for k, v := range quickstart.Labels { + expectedLabels[quickstart.Name][k] = v + } + } + }) + + By("altering the quickstart objects", func() { + for _, foundResource := range quickstartObjects.Items { + delete(foundResource.Labels, hcoutil.AppLabelVersion) + foundResource.Labels[userLabelKey] = userLabelValue + err = cli.Update(context.TODO(), &foundResource) + Expect(err).ToNot(HaveOccurred()) + } + }) + + By("reconciling quickstart objects", func() { + for _, handler := range handlers { + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + } + }) + + foundResourcesList := &consolev1.ConsoleQuickStartList{} + Expect(cli.List(context.TODO(), foundResourcesList)).To(Succeed()) + + for _, foundResource := range foundResourcesList.Items { + for k, v := range expectedLabels[foundResource.Name] { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + } + }) }) }) diff --git a/controllers/operands/rbac.go b/controllers/operands/rbac.go index 850f78f8b3..08f0d21f9f 100644 --- a/controllers/operands/rbac.go +++ b/controllers/operands/rbac.go @@ -47,8 +47,8 @@ func (h *roleHooks) updateCr(req *common.HcoRequest, Client client.Client, exist return false, false, errors.New("can't convert to a Role") } - if !reflect.DeepEqual(found.Labels, role.Labels) || - !reflect.DeepEqual(found.Rules, role.Rules) { + if !util.CompareLabels(role, found) || + !reflect.DeepEqual(role.Rules, found.Rules) { req.Logger.Info("Updating existing Role to its default values", "name", found.Name) @@ -56,7 +56,7 @@ func (h *roleHooks) updateCr(req *common.HcoRequest, Client client.Client, exist for i := range role.Rules { role.Rules[i].DeepCopyInto(&found.Rules[i]) } - util.DeepCopyLabels(&role.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&role.ObjectMeta, &found.ObjectMeta) err := Client.Update(req.Ctx, found) if err != nil { @@ -103,15 +103,15 @@ func (h roleBindingHooks) updateCr(req *common.HcoRequest, Client client.Client, return false, false, errors.New("can't convert to a RoleBinding") } - if !reflect.DeepEqual(found.Labels, configReaderRoleBinding.Labels) || - !reflect.DeepEqual(found.Subjects, configReaderRoleBinding.Subjects) || - !reflect.DeepEqual(found.RoleRef, configReaderRoleBinding.RoleRef) { + if !util.CompareLabels(configReaderRoleBinding, found) || + !reflect.DeepEqual(configReaderRoleBinding.Subjects, found.Subjects) || + !reflect.DeepEqual(configReaderRoleBinding.RoleRef, found.RoleRef) { req.Logger.Info("Updating existing RoleBinding to its default values", "name", found.Name) found.Subjects = make([]rbacv1.Subject, len(configReaderRoleBinding.Subjects)) copy(found.Subjects, configReaderRoleBinding.Subjects) found.RoleRef = configReaderRoleBinding.RoleRef - util.DeepCopyLabels(&configReaderRoleBinding.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&configReaderRoleBinding.ObjectMeta, &found.ObjectMeta) err := Client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/serviceHandler.go b/controllers/operands/serviceHandler.go index c1d82b30d4..158ce1d088 100644 --- a/controllers/operands/serviceHandler.go +++ b/controllers/operands/serviceHandler.go @@ -58,7 +58,7 @@ func updateService(req *common.HcoRequest, Client client.Client, exists runtime. } else { req.Logger.Info("Reconciling an externally updated Service's Spec to its opinionated values") } - util.DeepCopyLabels(&service.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&service.ObjectMeta, &found.ObjectMeta) service.Spec.ClusterIP = found.Spec.ClusterIP service.Spec.DeepCopyInto(&found.Spec) err := Client.Update(req.Ctx, found) @@ -75,7 +75,7 @@ func updateService(req *common.HcoRequest, Client client.Client, exists runtime. // When we compare current spec with expected spec by using reflect.DeepEqual, it // never returns true. func hasServiceRightFields(found *corev1.Service, required *corev1.Service) bool { - return reflect.DeepEqual(found.Labels, required.Labels) && - reflect.DeepEqual(found.Spec.Selector, required.Spec.Selector) && - reflect.DeepEqual(found.Spec.Ports, required.Spec.Ports) + return util.CompareLabels(required, found) && + reflect.DeepEqual(required.Spec.Selector, found.Spec.Selector) && + reflect.DeepEqual(required.Spec.Ports, found.Spec.Ports) } diff --git a/controllers/operands/ssp.go b/controllers/operands/ssp.go index a2803a671a..ce33774b40 100644 --- a/controllers/operands/ssp.go +++ b/controllers/operands/ssp.go @@ -98,14 +98,14 @@ func (*sspHooks) updateCr(req *common.HcoRequest, client client.Client, exists r if !ok1 || !ok2 { return false, false, errors.New("can't convert to SSP") } - if !reflect.DeepEqual(found.Spec, ssp.Spec) || - !reflect.DeepEqual(found.Labels, ssp.Labels) { + if !reflect.DeepEqual(ssp.Spec, found.Spec) || + !util.CompareLabels(ssp, found) { if req.HCOTriggered { req.Logger.Info("Updating existing SSP's Spec to new opinionated values") } else { req.Logger.Info("Reconciling an externally updated SSP's Spec to its opinionated values") } - util.DeepCopyLabels(&ssp.ObjectMeta, &found.ObjectMeta) + util.MergeLabels(&ssp.ObjectMeta, &found.ObjectMeta) ssp.Spec.DeepCopyInto(&found.Spec) err := client.Update(req.Ctx, found) if err != nil { diff --git a/controllers/operands/ssp_test.go b/controllers/operands/ssp_test.go index 585d22d839..b456556241 100644 --- a/controllers/operands/ssp_test.go +++ b/controllers/operands/ssp_test.go @@ -126,6 +126,74 @@ var _ = Describe("SSP Operands", func() { Expect(hco.Status.RelatedObjects).To(ContainElement(*objectRefFound)) }) + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, _, err := NewSSP(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newSspHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &sspv1beta2.SSP{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default on label deletion without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, _, err := NewSSP(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler := (*genericOperand)(newSspHandler(cl, commontestutils.GetScheme())) + + res := handler.ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &sspv1beta2.SSP{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + It("should create ssp with deployVmConsoleProxy feature gate enabled", func() { hco := commontestutils.NewHco() hco.Spec.FeatureGates.DeployVMConsoleProxy = ptr.To(true) diff --git a/controllers/operands/virtio_win_ConfigMap_test.go b/controllers/operands/virtio_win_ConfigMap_test.go index e531303a05..805c07159c 100644 --- a/controllers/operands/virtio_win_ConfigMap_test.go +++ b/controllers/operands/virtio_win_ConfigMap_test.go @@ -130,6 +130,72 @@ var _ = Describe("VirtioWin", func() { Expect(hco.Status.RelatedObjects).To(ContainElement(*objectRefFound)) }) + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewVirtioWinCm(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + for k, v := range expectedLabels { + outdatedResource.Labels[k] = "wrong_" + v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler, _ := newVirtioWinCmHandler(logger, cl, commontestutils.GetScheme(), hco) + res := handler[0].ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &corev1.ConfigMap{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + + It("should reconcile managed labels to default without touching user added ones", func() { + const userLabelKey = "userLabelKey" + const userLabelValue = "userLabelValue" + outdatedResource, err := NewVirtioWinCm(hco) + Expect(err).ToNot(HaveOccurred()) + expectedLabels := make(map[string]string, len(outdatedResource.Labels)) + for k, v := range outdatedResource.Labels { + expectedLabels[k] = v + } + outdatedResource.Labels[userLabelKey] = userLabelValue + delete(outdatedResource.Labels, hcoutil.AppLabelVersion) + + cl := commontestutils.InitClient([]client.Object{hco, outdatedResource}) + handler, _ := newVirtioWinCmHandler(logger, cl, commontestutils.GetScheme(), hco) + res := handler[0].ensure(req) + Expect(res.UpgradeDone).To(BeFalse()) + Expect(res.Updated).To(BeTrue()) + Expect(res.Err).ToNot(HaveOccurred()) + + foundResource := &corev1.ConfigMap{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: outdatedResource.Name, Namespace: outdatedResource.Namespace}, + foundResource), + ).ToNot(HaveOccurred()) + + for k, v := range expectedLabels { + Expect(foundResource.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(foundResource.Labels).To(HaveKeyWithValue(userLabelKey, userLabelValue)) + }) + }) Context("ConfigMap Reader Role", func() { diff --git a/pkg/util/labels.go b/pkg/util/labels.go index bff8d207c9..2889f514fd 100644 --- a/pkg/util/labels.go +++ b/pkg/util/labels.go @@ -4,13 +4,29 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func DeepCopyLabels(src, tgt *metav1.ObjectMeta) { +// MergeLabels merges src labels into tgt ones. +func MergeLabels(src, tgt *metav1.ObjectMeta) { if src.Labels == nil { return } - tgt.Labels = make(map[string]string, len(src.Labels)) + if tgt.Labels == nil { + tgt.Labels = make(map[string]string, len(src.Labels)) + } for key, val := range src.Labels { tgt.Labels[key] = val } } + +// CompareLabels reports whether src labels are contained into tgt ones; extra labels on tgt are ignored. +// It returns true if the src labels map is a subset of the tgt one. +func CompareLabels(src, tgt metav1.Object) bool { + targetLabels := tgt.GetLabels() + for key, val := range src.GetLabels() { + tgtv, ok := targetLabels[key] + if !ok || tgtv != val { + return false + } + } + return true +} diff --git a/pkg/util/labels_test.go b/pkg/util/labels_test.go index 59e38b45f4..95886074c1 100644 --- a/pkg/util/labels_test.go +++ b/pkg/util/labels_test.go @@ -3,11 +3,12 @@ package util import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var _ = Describe("Test Labels", func() { - Context("test DeepCopyLabels", func() { + Context("test MergeLabels", func() { It("should not copy the labels if the map is nil", func() { src := &metav1.ObjectMeta{ @@ -15,7 +16,7 @@ var _ = Describe("Test Labels", func() { } dest := &metav1.ObjectMeta{} - DeepCopyLabels(src, dest) + MergeLabels(src, dest) Expect(dest.Labels).To(BeNil()) }) @@ -26,7 +27,7 @@ var _ = Describe("Test Labels", func() { } dest := &metav1.ObjectMeta{} - DeepCopyLabels(src, dest) + MergeLabels(src, dest) Expect(dest.Labels).ToNot(BeNil()) Expect(dest.Labels).To(BeEmpty()) @@ -41,12 +42,271 @@ var _ = Describe("Test Labels", func() { } dest := &metav1.ObjectMeta{} - DeepCopyLabels(src, dest) + MergeLabels(src, dest) Expect(dest.Labels).ToNot(BeNil()) Expect(dest.Labels).To(HaveLen(2)) Expect(dest.Labels).To(HaveKeyWithValue("aaa", "111")) Expect(dest.Labels).To(HaveKeyWithValue("bbb", "222")) }) + + It("should merge the label map without removing dest items", func() { + src := &metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + } + dest := &metav1.ObjectMeta{ + Labels: map[string]string{ + "bbb": "444", + "ccc": "444", + "ddd": "444", + }, + } + + MergeLabels(src, dest) + + Expect(dest.Labels).ToNot(BeNil()) + Expect(dest.Labels).To(HaveLen(4)) + Expect(dest.Labels).To(HaveKeyWithValue("aaa", "111")) + Expect(dest.Labels).To(HaveKeyWithValue("bbb", "222")) + Expect(dest.Labels).To(HaveKeyWithValue("ccc", "333")) + Expect(dest.Labels).To(HaveKeyWithValue("ddd", "444")) + }) + }) + + Context("test CompareLabels", func() { + + DescribeTable("should compare source and target Labels ignoring additional labels on target", func(src, tgt metav1.ObjectMeta, expected bool) { + scm := &corev1.ConfigMap{ + ObjectMeta: src, + } + tcm := &corev1.ConfigMap{ + ObjectMeta: tgt, + } + c := CompareLabels(scm, tcm) + if expected { + Expect(c).To(BeTrue()) + } else { + Expect(c).To(BeFalse()) + } + + }, + Entry("nil labels on source and target", + metav1.ObjectMeta{ + Labels: nil, + }, + metav1.ObjectMeta{ + Labels: nil, + }, + true, + ), + Entry("nil labels on source, empty on target", + metav1.ObjectMeta{ + Labels: nil, + }, + metav1.ObjectMeta{ + Labels: make(map[string]string), + }, + true, + ), + Entry("empty on source, nil on target", + metav1.ObjectMeta{ + Labels: make(map[string]string), + }, + metav1.ObjectMeta{ + Labels: nil, + }, + true, + ), + Entry("equal on source and target", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + true, + ), + Entry("source is a subset of target", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + true, + ), + Entry("one label with a different value", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "444", + }, + }, + false, + ), + Entry("one label with empty value on source", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "444", + }, + }, + false, + ), + Entry("one label with empty value on target", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "", + }, + }, + false, + ), + Entry("one label with empty value on source, missing on target", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + }, + }, + false, + ), + Entry("one label with empty value on target, missing on source", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "", + }, + }, + true, + ), + Entry("one extra label on source", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + }, + }, + false, + ), + Entry("labels on source, nil on target", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + metav1.ObjectMeta{ + Labels: nil, + }, + false, + ), + Entry("labels on source, empty on target", + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + metav1.ObjectMeta{ + Labels: make(map[string]string), + }, + false, + ), + Entry("nil on source, labels on target", + metav1.ObjectMeta{ + Labels: nil, + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + true, + ), + Entry("empty on source, labels on target", + metav1.ObjectMeta{ + Labels: make(map[string]string), + }, + metav1.ObjectMeta{ + Labels: map[string]string{ + "aaa": "111", + "bbb": "222", + "ccc": "333", + }, + }, + true, + ), + ) + }) + }) diff --git a/tests/func-tests/golden_image_test.go b/tests/func-tests/golden_image_test.go index 5e8312b99c..cb9e775df8 100644 --- a/tests/func-tests/golden_image_test.go +++ b/tests/func-tests/golden_image_test.go @@ -120,20 +120,28 @@ var _ = Describe("golden image test", Label("data-import-cron"), Serial, Ordered ) DescribeTable("check imagestream reconciliation", func(expectedIS tests.ImageStreamConfig) { - patchOp := []byte(`[{"op": "add", "path": "/metadata/labels/test-label", "value": "test"}]`) + is := &v1.ImageStream{} + unstructured, err := cli.DynamicClient().Resource(isGVR).Namespace(imageNamespace).Get(ctx, expectedIS.Name, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured.Object, is) + Expect(err).ToNot(HaveOccurred()) + expectedValue := is.GetLabels()["app.kubernetes.io/part-of"] + Expect(expectedValue).ToNot(Equal("wrongValue")) + + patchOp := []byte(`[{"op": "replace", "path": "/metadata/labels/app.kubernetes.io~1part-of", "value": "wrong-value"}]`) Eventually(func() error { _, err := cli.DynamicClient().Resource(isGVR).Namespace(imageNamespace).Patch(ctx, expectedIS.Name, types.JSONPatchType, patchOp, metav1.PatchOptions{}) return err }).WithTimeout(time.Second * 5).WithPolling(time.Millisecond * 100).Should(Succeed()) - is := &v1.ImageStream{} - Eventually(func(g Gomega) map[string]string { + is = &v1.ImageStream{} + Eventually(func(g Gomega) string { unstructured, err := cli.DynamicClient().Resource(isGVR).Namespace(imageNamespace).Get(ctx, expectedIS.Name, metav1.GetOptions{}) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured.Object, is)).To(Succeed()) - return is.GetLabels() - }).WithTimeout(time.Second * 15).WithPolling(time.Millisecond * 100).ShouldNot(HaveKey("test-label")) + return is.GetLabels()["app.kubernetes.io/part-of"] + }).WithTimeout(time.Second * 15).WithPolling(time.Millisecond * 100).Should(Equal(expectedValue)) }, isEntries, ) diff --git a/tests/func-tests/labels_test.go b/tests/func-tests/labels_test.go index ccf3f706e1..5be4923347 100644 --- a/tests/func-tests/labels_test.go +++ b/tests/func-tests/labels_test.go @@ -4,6 +4,11 @@ import ( "context" "fmt" "strings" + "time" + + hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" + + "k8s.io/apimachinery/pkg/types" "github.com/gertd/go-pluralize" . "github.com/onsi/ginkgo/v2" @@ -34,7 +39,26 @@ var _ = Describe("Check that all the sub-resources have the required labels", La It("should have all the required labels in all the controlled resources", func() { hc := tests.GetHCO(ctx, cli) plural := pluralize.NewClient() + const kvname = "kubevirt-kubevirt-hyperconverged" + + By("removing one of the managed labels and wait for it to be added back") + kv, err := cli.KubeVirt(hc.Namespace).Get(kvname, &metav1.GetOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + expectedVersion := kv.Labels[hcoutil.AppLabelVersion] + + patch := []byte(`[{"op": "remove", "path": "/metadata/labels/app.kubernetes.io~1version"}]`) + Eventually(func() error { + _, err := cli.KubeVirt(hc.Namespace).Patch(kvname, types.JSONPatchType, patch, &metav1.PatchOptions{}) + return err + }).WithTimeout(time.Second * 5).WithPolling(time.Millisecond * 100).Should(Succeed()) + + Eventually(func(g Gomega) { + kv, err := cli.KubeVirt(hc.Namespace).Get(kvname, &metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(kv.Labels).Should(HaveKeyWithValue(hcoutil.AppLabelVersion, expectedVersion)) + }).WithTimeout(5 * time.Second).WithPolling(100 * time.Millisecond).Should(Succeed()) + By("checking all the labels") for _, resource := range hc.Status.RelatedObjects { By(fmt.Sprintf("checking labels for %s/%s", resource.Kind, resource.Name)) parts := strings.Split(resource.APIVersion, "/") diff --git a/tests/func-tests/update_priority_class_test.go b/tests/func-tests/update_priority_class_test.go index 222f9cbcbe..72af0a7779 100644 --- a/tests/func-tests/update_priority_class_test.go +++ b/tests/func-tests/update_priority_class_test.go @@ -56,7 +56,9 @@ var _ = Describe("check update priorityClass", Ordered, Serial, func() { It("should recreate the priorityClass on update", func() { GinkgoWriter.Printf("oldPriorityClassUID: %q\n", oldPriorityClassUID) - patch := []byte(`[{"op": "add", "path": "/metadata/labels/test", "value": "test"}]`) + // `~1` is the jsonpatch escapoe sequence for `\` + patch := []byte(`[{"op": "replace", "path": "/metadata/labels/app.kubernetes.io~1managed-by", "value": "test"}]`) + Eventually(func() error { _, err := cli.SchedulingV1().PriorityClasses().Patch(ctx, priorityClassName, types.JSONPatchType, patch, metav1.PatchOptions{}) return err diff --git a/tests/vendor/github.com/kubevirt/hyperconverged-cluster-operator/pkg/util/labels.go b/tests/vendor/github.com/kubevirt/hyperconverged-cluster-operator/pkg/util/labels.go index bff8d207c9..2889f514fd 100644 --- a/tests/vendor/github.com/kubevirt/hyperconverged-cluster-operator/pkg/util/labels.go +++ b/tests/vendor/github.com/kubevirt/hyperconverged-cluster-operator/pkg/util/labels.go @@ -4,13 +4,29 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func DeepCopyLabels(src, tgt *metav1.ObjectMeta) { +// MergeLabels merges src labels into tgt ones. +func MergeLabels(src, tgt *metav1.ObjectMeta) { if src.Labels == nil { return } - tgt.Labels = make(map[string]string, len(src.Labels)) + if tgt.Labels == nil { + tgt.Labels = make(map[string]string, len(src.Labels)) + } for key, val := range src.Labels { tgt.Labels[key] = val } } + +// CompareLabels reports whether src labels are contained into tgt ones; extra labels on tgt are ignored. +// It returns true if the src labels map is a subset of the tgt one. +func CompareLabels(src, tgt metav1.Object) bool { + targetLabels := tgt.GetLabels() + for key, val := range src.GetLabels() { + tgtv, ok := targetLabels[key] + if !ok || tgtv != val { + return false + } + } + return true +} diff --git a/vendor/github.com/onsi/gomega/gstruct/elements.go b/vendor/github.com/onsi/gomega/gstruct/elements.go new file mode 100644 index 0000000000..b5e5ef2e45 --- /dev/null +++ b/vendor/github.com/onsi/gomega/gstruct/elements.go @@ -0,0 +1,231 @@ +// untested sections: 6 + +package gstruct + +import ( + "errors" + "fmt" + "reflect" + "runtime/debug" + "strconv" + + "github.com/onsi/gomega/format" + errorsutil "github.com/onsi/gomega/gstruct/errors" + "github.com/onsi/gomega/types" +) + +//MatchAllElements succeeds if every element of a slice matches the element matcher it maps to +//through the id function, and every element matcher is matched. +// idFn := func(element interface{}) string { +// return fmt.Sprintf("%v", element) +// } +// +// Expect([]string{"a", "b"}).To(MatchAllElements(idFn, Elements{ +// "a": Equal("a"), +// "b": Equal("b"), +// })) +func MatchAllElements(identifier Identifier, elements Elements) types.GomegaMatcher { + return &ElementsMatcher{ + Identifier: identifier, + Elements: elements, + } +} + +//MatchAllElementsWithIndex succeeds if every element of a slice matches the element matcher it maps to +//through the id with index function, and every element matcher is matched. +// idFn := func(index int, element interface{}) string { +// return strconv.Itoa(index) +// } +// +// Expect([]string{"a", "b"}).To(MatchAllElements(idFn, Elements{ +// "0": Equal("a"), +// "1": Equal("b"), +// })) +func MatchAllElementsWithIndex(identifier IdentifierWithIndex, elements Elements) types.GomegaMatcher { + return &ElementsMatcher{ + Identifier: identifier, + Elements: elements, + } +} + +//MatchElements succeeds if each element of a slice matches the element matcher it maps to +//through the id function. It can ignore extra elements and/or missing elements. +// idFn := func(element interface{}) string { +// return fmt.Sprintf("%v", element) +// } +// +// Expect([]string{"a", "b", "c"}).To(MatchElements(idFn, IgnoreExtras, Elements{ +// "a": Equal("a"), +// "b": Equal("b"), +// })) +// Expect([]string{"a", "c"}).To(MatchElements(idFn, IgnoreMissing, Elements{ +// "a": Equal("a"), +// "b": Equal("b"), +// "c": Equal("c"), +// "d": Equal("d"), +// })) +func MatchElements(identifier Identifier, options Options, elements Elements) types.GomegaMatcher { + return &ElementsMatcher{ + Identifier: identifier, + Elements: elements, + IgnoreExtras: options&IgnoreExtras != 0, + IgnoreMissing: options&IgnoreMissing != 0, + AllowDuplicates: options&AllowDuplicates != 0, + } +} + +//MatchElementsWithIndex succeeds if each element of a slice matches the element matcher it maps to +//through the id with index function. It can ignore extra elements and/or missing elements. +// idFn := func(index int, element interface{}) string { +// return strconv.Itoa(index) +// } +// +// Expect([]string{"a", "b", "c"}).To(MatchElements(idFn, IgnoreExtras, Elements{ +// "0": Equal("a"), +// "1": Equal("b"), +// })) +// Expect([]string{"a", "c"}).To(MatchElements(idFn, IgnoreMissing, Elements{ +// "0": Equal("a"), +// "1": Equal("b"), +// "2": Equal("c"), +// "3": Equal("d"), +// })) +func MatchElementsWithIndex(identifier IdentifierWithIndex, options Options, elements Elements) types.GomegaMatcher { + return &ElementsMatcher{ + Identifier: identifier, + Elements: elements, + IgnoreExtras: options&IgnoreExtras != 0, + IgnoreMissing: options&IgnoreMissing != 0, + AllowDuplicates: options&AllowDuplicates != 0, + } +} + +// ElementsMatcher is a NestingMatcher that applies custom matchers to each element of a slice mapped +// by the Identifier function. +// TODO: Extend this to work with arrays & maps (map the key) as well. +type ElementsMatcher struct { + // Matchers for each element. + Elements Elements + // Function mapping an element to the string key identifying its matcher. + Identifier Identify + + // Whether to ignore extra elements or consider it an error. + IgnoreExtras bool + // Whether to ignore missing elements or consider it an error. + IgnoreMissing bool + // Whether to key duplicates when matching IDs. + AllowDuplicates bool + + // State. + failures []error +} + +// Element ID to matcher. +type Elements map[string]types.GomegaMatcher + +// Function for identifying (mapping) elements. +type Identifier func(element interface{}) string + +// Calls the underlying fucntion with the provided params. +// Identifier drops the index. +func (i Identifier) WithIndexAndElement(index int, element interface{}) string { + return i(element) +} + +// Uses the index and element to generate an element name +type IdentifierWithIndex func(index int, element interface{}) string + +// Calls the underlying fucntion with the provided params. +// IdentifierWithIndex uses the index. +func (i IdentifierWithIndex) WithIndexAndElement(index int, element interface{}) string { + return i(index, element) +} + +// Interface for identifing the element +type Identify interface { + WithIndexAndElement(i int, element interface{}) string +} + +// IndexIdentity is a helper function for using an index as +// the key in the element map +func IndexIdentity(index int, _ interface{}) string { + return strconv.Itoa(index) +} + +func (m *ElementsMatcher) Match(actual interface{}) (success bool, err error) { + if reflect.TypeOf(actual).Kind() != reflect.Slice { + return false, fmt.Errorf("%v is type %T, expected slice", actual, actual) + } + + m.failures = m.matchElements(actual) + if len(m.failures) > 0 { + return false, nil + } + return true, nil +} + +func (m *ElementsMatcher) matchElements(actual interface{}) (errs []error) { + // Provide more useful error messages in the case of a panic. + defer func() { + if err := recover(); err != nil { + errs = append(errs, fmt.Errorf("panic checking %+v: %v\n%s", actual, err, debug.Stack())) + } + }() + + val := reflect.ValueOf(actual) + elements := map[string]bool{} + for i := 0; i < val.Len(); i++ { + element := val.Index(i).Interface() + id := m.Identifier.WithIndexAndElement(i, element) + if elements[id] { + if !m.AllowDuplicates { + errs = append(errs, fmt.Errorf("found duplicate element ID %s", id)) + continue + } + } + elements[id] = true + + matcher, expected := m.Elements[id] + if !expected { + if !m.IgnoreExtras { + errs = append(errs, fmt.Errorf("unexpected element %s", id)) + } + continue + } + + match, err := matcher.Match(element) + if match { + continue + } + + if err == nil { + if nesting, ok := matcher.(errorsutil.NestingMatcher); ok { + err = errorsutil.AggregateError(nesting.Failures()) + } else { + err = errors.New(matcher.FailureMessage(element)) + } + } + errs = append(errs, errorsutil.Nest(fmt.Sprintf("[%s]", id), err)) + } + + for id := range m.Elements { + if !elements[id] && !m.IgnoreMissing { + errs = append(errs, fmt.Errorf("missing expected element %s", id)) + } + } + + return errs +} + +func (m *ElementsMatcher) FailureMessage(actual interface{}) (message string) { + failure := errorsutil.AggregateError(m.failures) + return format.Message(actual, fmt.Sprintf("to match elements: %v", failure)) +} + +func (m *ElementsMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return format.Message(actual, "not to match elements") +} + +func (m *ElementsMatcher) Failures() []error { + return m.failures +} diff --git a/vendor/github.com/onsi/gomega/gstruct/errors/nested_types.go b/vendor/github.com/onsi/gomega/gstruct/errors/nested_types.go new file mode 100644 index 0000000000..188492b212 --- /dev/null +++ b/vendor/github.com/onsi/gomega/gstruct/errors/nested_types.go @@ -0,0 +1,72 @@ +package errors + +import ( + "fmt" + "strings" + + "github.com/onsi/gomega/types" +) + +// A stateful matcher that nests other matchers within it and preserves the error types of the +// nested matcher failures. +type NestingMatcher interface { + types.GomegaMatcher + + // Returns the failures of nested matchers. + Failures() []error +} + +// An error type for labeling errors on deeply nested matchers. +type NestedError struct { + Path string + Err error +} + +func (e *NestedError) Error() string { + // Indent Errors. + indented := strings.Replace(e.Err.Error(), "\n", "\n\t", -1) + return fmt.Sprintf("%s:\n\t%v", e.Path, indented) +} + +// Create a NestedError with the given path. +// If err is a NestedError, prepend the path to it. +// If err is an AggregateError, recursively Nest each error. +func Nest(path string, err error) error { + if ag, ok := err.(AggregateError); ok { + var errs AggregateError + for _, e := range ag { + errs = append(errs, Nest(path, e)) + } + return errs + } + if ne, ok := err.(*NestedError); ok { + return &NestedError{ + Path: path + ne.Path, + Err: ne.Err, + } + } + return &NestedError{ + Path: path, + Err: err, + } +} + +// An error type for treating multiple errors as a single error. +type AggregateError []error + +// Error is part of the error interface. +func (err AggregateError) Error() string { + if len(err) == 0 { + // This should never happen, really. + return "" + } + if len(err) == 1 { + return err[0].Error() + } + result := fmt.Sprintf("[%s", err[0].Error()) + for i := 1; i < len(err); i++ { + result += fmt.Sprintf(", %s", err[i].Error()) + } + result += "]" + return result +} diff --git a/vendor/github.com/onsi/gomega/gstruct/fields.go b/vendor/github.com/onsi/gomega/gstruct/fields.go new file mode 100644 index 0000000000..faf07b1a25 --- /dev/null +++ b/vendor/github.com/onsi/gomega/gstruct/fields.go @@ -0,0 +1,165 @@ +// untested sections: 6 + +package gstruct + +import ( + "errors" + "fmt" + "reflect" + "runtime/debug" + "strings" + + "github.com/onsi/gomega/format" + errorsutil "github.com/onsi/gomega/gstruct/errors" + "github.com/onsi/gomega/types" +) + +//MatchAllFields succeeds if every field of a struct matches the field matcher associated with +//it, and every element matcher is matched. +// actual := struct{ +// A int +// B []bool +// C string +// }{ +// A: 5, +// B: []bool{true, false}, +// C: "foo", +// } +// +// Expect(actual).To(MatchAllFields(Fields{ +// "A": Equal(5), +// "B": ConsistOf(true, false), +// "C": Equal("foo"), +// })) +func MatchAllFields(fields Fields) types.GomegaMatcher { + return &FieldsMatcher{ + Fields: fields, + } +} + +//MatchFields succeeds if each element of a struct matches the field matcher associated with +//it. It can ignore extra fields and/or missing fields. +// actual := struct{ +// A int +// B []bool +// C string +// }{ +// A: 5, +// B: []bool{true, false}, +// C: "foo", +// } +// +// Expect(actual).To(MatchFields(IgnoreExtras, Fields{ +// "A": Equal(5), +// "B": ConsistOf(true, false), +// })) +// Expect(actual).To(MatchFields(IgnoreMissing, Fields{ +// "A": Equal(5), +// "B": ConsistOf(true, false), +// "C": Equal("foo"), +// "D": Equal("extra"), +// })) +func MatchFields(options Options, fields Fields) types.GomegaMatcher { + return &FieldsMatcher{ + Fields: fields, + IgnoreExtras: options&IgnoreExtras != 0, + IgnoreMissing: options&IgnoreMissing != 0, + } +} + +type FieldsMatcher struct { + // Matchers for each field. + Fields Fields + + // Whether to ignore extra elements or consider it an error. + IgnoreExtras bool + // Whether to ignore missing elements or consider it an error. + IgnoreMissing bool + + // State. + failures []error +} + +// Field name to matcher. +type Fields map[string]types.GomegaMatcher + +func (m *FieldsMatcher) Match(actual interface{}) (success bool, err error) { + if reflect.TypeOf(actual).Kind() != reflect.Struct { + return false, fmt.Errorf("%v is type %T, expected struct", actual, actual) + } + + m.failures = m.matchFields(actual) + if len(m.failures) > 0 { + return false, nil + } + return true, nil +} + +func (m *FieldsMatcher) matchFields(actual interface{}) (errs []error) { + val := reflect.ValueOf(actual) + typ := val.Type() + fields := map[string]bool{} + for i := 0; i < val.NumField(); i++ { + fieldName := typ.Field(i).Name + fields[fieldName] = true + + err := func() (err error) { + // This test relies heavily on reflect, which tends to panic. + // Recover here to provide more useful error messages in that case. + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("panic checking %+v: %v\n%s", actual, r, debug.Stack()) + } + }() + + matcher, expected := m.Fields[fieldName] + if !expected { + if !m.IgnoreExtras { + return fmt.Errorf("unexpected field %s: %+v", fieldName, actual) + } + return nil + } + + field := val.Field(i).Interface() + + match, err := matcher.Match(field) + if err != nil { + return err + } else if !match { + if nesting, ok := matcher.(errorsutil.NestingMatcher); ok { + return errorsutil.AggregateError(nesting.Failures()) + } + return errors.New(matcher.FailureMessage(field)) + } + return nil + }() + if err != nil { + errs = append(errs, errorsutil.Nest("."+fieldName, err)) + } + } + + for field := range m.Fields { + if !fields[field] && !m.IgnoreMissing { + errs = append(errs, fmt.Errorf("missing expected field %s", field)) + } + } + + return errs +} + +func (m *FieldsMatcher) FailureMessage(actual interface{}) (message string) { + failures := make([]string, len(m.failures)) + for i := range m.failures { + failures[i] = m.failures[i].Error() + } + return format.Message(reflect.TypeOf(actual).Name(), + fmt.Sprintf("to match fields: {\n%v\n}\n", strings.Join(failures, "\n"))) +} + +func (m *FieldsMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return format.Message(actual, "not to match fields") +} + +func (m *FieldsMatcher) Failures() []error { + return m.failures +} diff --git a/vendor/github.com/onsi/gomega/gstruct/ignore.go b/vendor/github.com/onsi/gomega/gstruct/ignore.go new file mode 100644 index 0000000000..4396573e44 --- /dev/null +++ b/vendor/github.com/onsi/gomega/gstruct/ignore.go @@ -0,0 +1,39 @@ +// untested sections: 2 + +package gstruct + +import ( + "github.com/onsi/gomega/types" +) + +//Ignore ignores the actual value and always succeeds. +// Expect(nil).To(Ignore()) +// Expect(true).To(Ignore()) +func Ignore() types.GomegaMatcher { + return &IgnoreMatcher{true} +} + +//Reject ignores the actual value and always fails. It can be used in conjunction with IgnoreMissing +//to catch problematic elements, or to verify tests are running. +// Expect(nil).NotTo(Reject()) +// Expect(true).NotTo(Reject()) +func Reject() types.GomegaMatcher { + return &IgnoreMatcher{false} +} + +// A matcher that either always succeeds or always fails. +type IgnoreMatcher struct { + Succeed bool +} + +func (m *IgnoreMatcher) Match(actual interface{}) (bool, error) { + return m.Succeed, nil +} + +func (m *IgnoreMatcher) FailureMessage(_ interface{}) (message string) { + return "Unconditional failure" +} + +func (m *IgnoreMatcher) NegatedFailureMessage(_ interface{}) (message string) { + return "Unconditional success" +} diff --git a/vendor/github.com/onsi/gomega/gstruct/keys.go b/vendor/github.com/onsi/gomega/gstruct/keys.go new file mode 100644 index 0000000000..56aed4bab7 --- /dev/null +++ b/vendor/github.com/onsi/gomega/gstruct/keys.go @@ -0,0 +1,126 @@ +// untested sections: 6 + +package gstruct + +import ( + "errors" + "fmt" + "reflect" + "runtime/debug" + "strings" + + "github.com/onsi/gomega/format" + errorsutil "github.com/onsi/gomega/gstruct/errors" + "github.com/onsi/gomega/types" +) + +func MatchAllKeys(keys Keys) types.GomegaMatcher { + return &KeysMatcher{ + Keys: keys, + } +} + +func MatchKeys(options Options, keys Keys) types.GomegaMatcher { + return &KeysMatcher{ + Keys: keys, + IgnoreExtras: options&IgnoreExtras != 0, + IgnoreMissing: options&IgnoreMissing != 0, + } +} + +type KeysMatcher struct { + // Matchers for each key. + Keys Keys + + // Whether to ignore extra keys or consider it an error. + IgnoreExtras bool + // Whether to ignore missing keys or consider it an error. + IgnoreMissing bool + + // State. + failures []error +} + +type Keys map[interface{}]types.GomegaMatcher + +func (m *KeysMatcher) Match(actual interface{}) (success bool, err error) { + if reflect.TypeOf(actual).Kind() != reflect.Map { + return false, fmt.Errorf("%v is type %T, expected map", actual, actual) + } + + m.failures = m.matchKeys(actual) + if len(m.failures) > 0 { + return false, nil + } + return true, nil +} + +func (m *KeysMatcher) matchKeys(actual interface{}) (errs []error) { + actualValue := reflect.ValueOf(actual) + keys := map[interface{}]bool{} + for _, keyValue := range actualValue.MapKeys() { + key := keyValue.Interface() + keys[key] = true + + err := func() (err error) { + // This test relies heavily on reflect, which tends to panic. + // Recover here to provide more useful error messages in that case. + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("panic checking %+v: %v\n%s", actual, r, debug.Stack()) + } + }() + + matcher, ok := m.Keys[key] + if !ok { + if !m.IgnoreExtras { + return fmt.Errorf("unexpected key %s: %+v", key, actual) + } + return nil + } + + valInterface := actualValue.MapIndex(keyValue).Interface() + + match, err := matcher.Match(valInterface) + if err != nil { + return err + } + + if !match { + if nesting, ok := matcher.(errorsutil.NestingMatcher); ok { + return errorsutil.AggregateError(nesting.Failures()) + } + return errors.New(matcher.FailureMessage(valInterface)) + } + return nil + }() + if err != nil { + errs = append(errs, errorsutil.Nest(fmt.Sprintf(".%#v", key), err)) + } + } + + for key := range m.Keys { + if !keys[key] && !m.IgnoreMissing { + errs = append(errs, fmt.Errorf("missing expected key %s", key)) + } + } + + return errs +} + +func (m *KeysMatcher) FailureMessage(actual interface{}) (message string) { + failures := make([]string, len(m.failures)) + for i := range m.failures { + failures[i] = m.failures[i].Error() + } + return format.Message(reflect.TypeOf(actual).Name(), + fmt.Sprintf("to match keys: {\n%v\n}\n", strings.Join(failures, "\n"))) +} + +func (m *KeysMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return format.Message(actual, "not to match keys") +} + +func (m *KeysMatcher) Failures() []error { + return m.failures +} diff --git a/vendor/github.com/onsi/gomega/gstruct/pointer.go b/vendor/github.com/onsi/gomega/gstruct/pointer.go new file mode 100644 index 0000000000..cc828a3251 --- /dev/null +++ b/vendor/github.com/onsi/gomega/gstruct/pointer.go @@ -0,0 +1,58 @@ +// untested sections: 3 + +package gstruct + +import ( + "fmt" + "reflect" + + "github.com/onsi/gomega/format" + "github.com/onsi/gomega/types" +) + +//PointTo applies the given matcher to the value pointed to by actual. It fails if the pointer is +//nil. +// actual := 5 +// Expect(&actual).To(PointTo(Equal(5))) +func PointTo(matcher types.GomegaMatcher) types.GomegaMatcher { + return &PointerMatcher{ + Matcher: matcher, + } +} + +type PointerMatcher struct { + Matcher types.GomegaMatcher + + // Failure message. + failure string +} + +func (m *PointerMatcher) Match(actual interface{}) (bool, error) { + val := reflect.ValueOf(actual) + + // return error if actual type is not a pointer + if val.Kind() != reflect.Ptr { + return false, fmt.Errorf("PointerMatcher expects a pointer but we have '%s'", val.Kind()) + } + + if !val.IsValid() || val.IsNil() { + m.failure = format.Message(actual, "not to be ") + return false, nil + } + + // Forward the value. + elem := val.Elem().Interface() + match, err := m.Matcher.Match(elem) + if !match { + m.failure = m.Matcher.FailureMessage(elem) + } + return match, err +} + +func (m *PointerMatcher) FailureMessage(_ interface{}) (message string) { + return m.failure +} + +func (m *PointerMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return m.Matcher.NegatedFailureMessage(actual) +} diff --git a/vendor/github.com/onsi/gomega/gstruct/types.go b/vendor/github.com/onsi/gomega/gstruct/types.go new file mode 100644 index 0000000000..48cbbe8f66 --- /dev/null +++ b/vendor/github.com/onsi/gomega/gstruct/types.go @@ -0,0 +1,15 @@ +package gstruct + +//Options is the type for options passed to some matchers. +type Options int + +const ( + //IgnoreExtras tells the matcher to ignore extra elements or fields, rather than triggering a failure. + IgnoreExtras Options = 1 << iota + //IgnoreMissing tells the matcher to ignore missing elements or fields, rather than triggering a failure. + IgnoreMissing + //AllowDuplicates tells the matcher to permit multiple members of the slice to produce the same ID when + //considered by the indentifier function. All members that map to a given key must still match successfully + //with the matcher that is provided for that key. + AllowDuplicates +) diff --git a/vendor/modules.txt b/vendor/modules.txt index 79a3a7b642..37380b1a77 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -155,6 +155,8 @@ github.com/onsi/ginkgo/v2/types ## explicit; go 1.18 github.com/onsi/gomega github.com/onsi/gomega/format +github.com/onsi/gomega/gstruct +github.com/onsi/gomega/gstruct/errors github.com/onsi/gomega/internal github.com/onsi/gomega/internal/gutil github.com/onsi/gomega/matchers