From b8b5bd1ed1aaf343cc70d8c350c7edf488986cfe Mon Sep 17 00:00:00 2001 From: Jeff Cantrill Date: Fri, 10 Jan 2025 16:10:43 -0500 Subject: [PATCH] LOG-6280: Only cleanup dashboard when owned by 'this' operator version --- cmd/main.go | 6 ++--- internal/metrics/dashboard/dashboards.go | 33 +++++++++++++++++------- internal/runtime/runtime.go | 21 +++++++++++++-- internal/runtime/runtime_test.go | 33 ++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 15 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index a40939b811..076dcc7abd 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -125,7 +125,7 @@ func main() { // Clean up defer func() { - if err := cleanUpResources(mgr.GetClient()); err != nil { + if err := cleanUpResources(mgr.GetClient(), mgr.GetAPIReader()); err != nil { log.V(3).Error(err, "error with resource cleanup") } }() @@ -219,9 +219,9 @@ func migrateManifestResources(k8sClient client.Client) { } } -func cleanUpResources(k8sClient client.Client) error { +func cleanUpResources(k8sClient client.Client, apiReader client.Reader) error { // Remove the dashboard config map - if err := dashboard.RemoveDashboardConfigMap(k8sClient); err != nil { + if err := dashboard.RemoveDashboardConfigMap(k8sClient, apiReader); err != nil { return err } return nil diff --git a/internal/metrics/dashboard/dashboards.go b/internal/metrics/dashboard/dashboards.go index dfbb21a1d3..77a4426957 100644 --- a/internal/metrics/dashboard/dashboards.go +++ b/internal/metrics/dashboard/dashboards.go @@ -4,18 +4,18 @@ import ( "context" _ "embed" "fmt" - staticlog "github.com/ViaQ/logerr/v2/log/static" + "github.com/openshift/cluster-logging-operator/internal/constants" "github.com/openshift/cluster-logging-operator/internal/reconcile" "github.com/openshift/cluster-logging-operator/internal/runtime" "github.com/openshift/cluster-logging-operator/internal/utils" "github.com/openshift/cluster-logging-operator/internal/utils/comparators" + "github.com/openshift/cluster-logging-operator/version" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/handler" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -60,7 +60,9 @@ func newDashboardConfigMap() *corev1.ConfigMap { ) runtime.NewConfigMapBuilder(cm). AddLabel("console.openshift.io/dashboard", "true"). - AddLabel(DashboardHashName, hash) + AddLabel(DashboardHashName, hash). + AddLabel(constants.LabelK8sVersion, version.Version). + AddLabel(constants.LabelK8sManagedBy, constants.ClusterLoggingOperator) return cm } @@ -83,14 +85,25 @@ func ReconcileForDashboards(k8sClient client.Client, reader client.Reader) error } // RemoveDashboardConfigMap removes the config map in the grafana dashboard -func RemoveDashboardConfigMap(c client.Client) (err error) { - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: DashboardName, - Namespace: DashboardNS, - }, +func RemoveDashboardConfigMap(c client.Client, r client.Reader) (err error) { + cm := newDashboardConfigMap() + + current := &corev1.ConfigMap{} + key := client.ObjectKeyFromObject(cm) + if err := r.Get(context.TODO(), key, current); err != nil { + if !errors.IsNotFound(err) { + return fmt.Errorf("failed to get %v configmap: %v", key, err) + } + return nil + } + + if runtime.Labels(current).Includes(map[string]string{ + constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, + constants.LabelK8sVersion: version.Version, + }) { + return c.Delete(context.TODO(), cm) } - return c.Delete(context.TODO(), cm) + return nil } // SetupWithManager sets up the controller with the Manager diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 2ce37f6892..97dfc26de7 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -80,8 +80,25 @@ func GroupVersionKind(o runtime.Object) schema.GroupVersionKind { return gvk } -// Labels returns the labels map for object, guaratneed to be non-nil. -func Labels(o runtime.Object) map[string]string { +type ObjectLabels map[string]string + +// Includes compares an object labels to those given and returns true if +// the object set includes the keys and also matches the values +func (objLabels ObjectLabels) Includes(other ObjectLabels) bool { + for includeKey, includeValue := range other { + if value, found := objLabels[includeKey]; found { + if value != includeValue { + return false + } + } else { + return false + } + } + return true +} + +// Labels returns the labels map for object, guaranteed to be non-nil. +func Labels(o runtime.Object) ObjectLabels { m := Meta(o) l := m.GetLabels() if l == nil { diff --git a/internal/runtime/runtime_test.go b/internal/runtime/runtime_test.go index b5505d0354..b33a7118fd 100644 --- a/internal/runtime/runtime_test.go +++ b/internal/runtime/runtime_test.go @@ -4,8 +4,10 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" + "github.com/openshift/cluster-logging-operator/internal/constants" "github.com/openshift/cluster-logging-operator/test" . "github.com/openshift/cluster-logging-operator/test/matchers" + "github.com/openshift/cluster-logging-operator/version" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,6 +37,37 @@ var _ = Describe("Object", func() { Entry("JSON string ns", test.JSONLine(nsFoo), nsFoo), ) + DescribeTable("#Labels.Includes", + func(success bool, matches map[string]string) { + obj := NewNamespace("test") + obj.Labels = map[string]string{ + "foo": "bar", + constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, + constants.LabelK8sVersion: version.Version, + } + Expect(Labels(obj).Includes(matches)).To(Equal(success)) + }, + Entry("matches a subset", true, map[string]string{ + constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, + constants.LabelK8sVersion: version.Version, + }), + Entry("matches entire set", true, map[string]string{ + "foo": "bar", + constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, + constants.LabelK8sVersion: version.Version, + }), + Entry("fails a superset", false, map[string]string{ + "foo": "bar", + constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, + constants.LabelK8sVersion: version.Version, + "xyz": "abc", + }), + Entry("fails a subset", false, map[string]string{ + constants.LabelK8sManagedBy: constants.ClusterLoggingOperator, + constants.LabelK8sVersion: "someother", + }), + ) + It("panics on bad manifest string", func() { Expect(func() { _ = Decode("bad manifest") }).To(Panic()) })