diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index bf3564ef0..af1bb50a8 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -60,6 +60,7 @@ type cluster struct { context *clusterd.Context Namespace string Spec *cephv1.ClusterSpec + clusterMetadata metav1.ObjectMeta namespacedName types.NamespacedName mons *mon.Cluster ownerInfo *k8sutil.OwnerInfo @@ -76,6 +77,7 @@ func newCluster(ctx context.Context, c *cephv1.CephCluster, context *clusterd.Co ClusterInfo: client.AdminClusterInfo(ctx, c.Namespace, c.Name), Namespace: c.Namespace, Spec: &c.Spec, + clusterMetadata: c.ObjectMeta, context: context, namespacedName: types.NamespacedName{Namespace: c.Namespace, Name: c.Name}, monitoringRoutines: make(map[string]*controller.ClusterHealth), @@ -91,7 +93,7 @@ func newCluster(ctx context.Context, c *cephv1.CephCluster, context *clusterd.Co func (c *cluster) reconcileCephDaemons(rookImage string, cephVersion cephver.CephVersion) error { // Create a configmap for overriding ceph config settings // These settings should only be modified by a user after they are initialized - err := populateConfigOverrideConfigMap(c.context, c.Namespace, c.ownerInfo) + err := populateConfigOverrideConfigMap(c.context, c.Namespace, c.ownerInfo, c.clusterMetadata) if err != nil { return errors.Wrap(err, "failed to populate config override config map") } diff --git a/pkg/operator/ceph/cluster/cluster_external.go b/pkg/operator/ceph/cluster/cluster_external.go index d17fe1a9d..7e747138b 100644 --- a/pkg/operator/ceph/cluster/cluster_external.go +++ b/pkg/operator/ceph/cluster/cluster_external.go @@ -80,7 +80,7 @@ func (c *ClusterController) configureExternalCephCluster(cluster *cluster) error // // Only do this when doing a bit of management... logger.Infof("creating %q configmap", k8sutil.ConfigOverrideName) - err = populateConfigOverrideConfigMap(c.context, c.namespacedName.Namespace, cluster.ClusterInfo.OwnerInfo) + err = populateConfigOverrideConfigMap(c.context, c.namespacedName.Namespace, cluster.ClusterInfo.OwnerInfo, cluster.clusterMetadata) if err != nil { return errors.Wrap(err, "failed to populate config override config map") } diff --git a/pkg/operator/ceph/cluster/utils.go b/pkg/operator/ceph/cluster/utils.go index 65ba8307b..d5a728610 100644 --- a/pkg/operator/ceph/cluster/utils.go +++ b/pkg/operator/ceph/cluster/utils.go @@ -30,28 +30,83 @@ import ( // populateConfigOverrideConfigMap creates the "rook-config-override" config map // Its content allows modifying Ceph configuration flags -func populateConfigOverrideConfigMap(clusterdContext *clusterd.Context, namespace string, ownerInfo *k8sutil.OwnerInfo) error { +func populateConfigOverrideConfigMap(clusterdContext *clusterd.Context, namespace string, ownerInfo *k8sutil.OwnerInfo, clusterMetadata metav1.ObjectMeta) error { ctx := context.TODO() - placeholderConfig := map[string]string{ - k8sutil.ConfigOverrideVal: "", - } - cm := &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: k8sutil.ConfigOverrideName, - Namespace: namespace, - }, - Data: placeholderConfig, - } - - err := ownerInfo.SetControllerReference(cm) + existingCM, err := clusterdContext.Clientset.CoreV1().ConfigMaps(namespace).Get(ctx, k8sutil.ConfigOverrideName, metav1.GetOptions{}) if err != nil { - return errors.Wrapf(err, "failed to set owner reference to override configmap %q", cm.Name) - } - _, err = clusterdContext.Clientset.CoreV1().ConfigMaps(namespace).Create(ctx, cm, metav1.CreateOptions{}) - if err != nil && !kerrors.IsAlreadyExists(err) { - return errors.Wrapf(err, "failed to create override configmap %s", namespace) + if !kerrors.IsNotFound(err) { + logger.Warningf("failed to get cm %q to check labels and annotations", k8sutil.ConfigOverrideName) + return nil + } + + labels := map[string]string{} + annotations := map[string]string{} + initRequiredMetadata(clusterMetadata, labels, annotations) + + // Create the configmap since it doesn't exist yet + placeholderConfig := map[string]string{ + k8sutil.ConfigOverrideVal: "", + } + cm := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: k8sutil.ConfigOverrideName, + Namespace: namespace, + Labels: labels, + Annotations: annotations, + }, + Data: placeholderConfig, + } + + err := ownerInfo.SetControllerReference(cm) + if err != nil { + return errors.Wrapf(err, "failed to set owner reference to override configmap %q", cm.Name) + } + _, err = clusterdContext.Clientset.CoreV1().ConfigMaps(namespace).Create(ctx, cm, metav1.CreateOptions{}) + if err != nil { + return errors.Wrapf(err, "failed to create override configmap %s", namespace) + } + logger.Infof("created placeholder configmap for ceph overrides %q", cm.Name) + return nil } + // Ensure the annotations and labels are initialized + if existingCM.Annotations == nil { + existingCM.Annotations = map[string]string{} + } + if existingCM.Labels == nil { + existingCM.Labels = map[string]string{} + } + + // Add recommended labels and annotations to the existing configmap if it doesn't have any yet + updateRequired := initRequiredMetadata(clusterMetadata, existingCM.Labels, existingCM.Annotations) + if updateRequired { + _, err = clusterdContext.Clientset.CoreV1().ConfigMaps(namespace).Update(ctx, existingCM, metav1.UpdateOptions{}) + if err != nil { + logger.Warningf("failed to add recommended labels and annotations to configmap %q. %v", existingCM.Name, err) + } else { + logger.Infof("added expected labels and annotations to configmap %q", existingCM.Name) + } + } return nil } + +func initRequiredMetadata(metadata metav1.ObjectMeta, labels, annotations map[string]string) bool { + // Add the helm labels and annotations in case the user wants to install the cluster helm chart to start managing it + releaseNameAttr := "meta.helm.sh/release-name" + chartName, ok := metadata.Annotations[releaseNameAttr] + if !ok { + logger.Debug("cluster helm chart is not configured, not adding helm annotations to configmap") + return false + } + if _, ok := annotations[releaseNameAttr]; ok { + logger.Debug("cluster helm chart helm annotations already added to configmap") + return false + } + + logger.Infof("adding helm chart name %q annotation to configmap", chartName) + labels["app.kubernetes.io/managed-by"] = "Helm" + annotations[releaseNameAttr] = chartName + annotations["meta.helm.sh/release-namespace"] = metadata.Namespace + return true +} diff --git a/pkg/operator/ceph/cluster/utils_test.go b/pkg/operator/ceph/cluster/utils_test.go new file mode 100644 index 000000000..c5a80a87d --- /dev/null +++ b/pkg/operator/ceph/cluster/utils_test.go @@ -0,0 +1,105 @@ +/* +Copyright 2023 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package cluster to manage Kubernetes storage. +package cluster + +import ( + ctx "context" + "testing" + + "github.com/rook/rook/pkg/clusterd" + "github.com/rook/rook/pkg/operator/k8sutil" + "github.com/rook/rook/pkg/operator/test" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestConfigmapOverrideUpdate(t *testing.T) { + // create mocked cluster context and info + clientset := test.New(t, 3) + context := &clusterd.Context{ + Clientset: clientset, + } + ns := "test" + controllerRef := &metav1.OwnerReference{UID: "test-id"} + ownerInfo := k8sutil.NewOwnerInfoWithOwnerRef(controllerRef, "") + clusterMetadata := metav1.ObjectMeta{ + Namespace: ns, + } + + // The configmap should be created without any labels/annotations when helm not installed + err := populateConfigOverrideConfigMap(context, ns, ownerInfo, clusterMetadata) + assert.NoError(t, err) + cm, err := clientset.CoreV1().ConfigMaps(ns).Get(ctx.TODO(), k8sutil.ConfigOverrideName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, 0, len(cm.Annotations)) + assert.Equal(t, 0, len(cm.Labels)) + + // Set the labels and annotations to nil and confirm helm labels are still not added + cm.Labels = nil + cm.Annotations = nil + _, err = clientset.CoreV1().ConfigMaps(ns).Update(ctx.TODO(), cm, metav1.UpdateOptions{}) + assert.NoError(t, err) + err = populateConfigOverrideConfigMap(context, ns, ownerInfo, clusterMetadata) + assert.NoError(t, err) + cm, err = clientset.CoreV1().ConfigMaps(ns).Get(ctx.TODO(), k8sutil.ConfigOverrideName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, 0, len(cm.Annotations)) + assert.Equal(t, 0, len(cm.Labels)) + + // The configmap should be created with the helm annotations + clusterMetadata.Annotations = map[string]string{"meta.helm.sh/release-name": "my-test-cluster"} + err = populateConfigOverrideConfigMap(context, ns, ownerInfo, clusterMetadata) + assert.NoError(t, err) + cm, err = clientset.CoreV1().ConfigMaps(ns).Get(ctx.TODO(), k8sutil.ConfigOverrideName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, 2, len(cm.Annotations)) + assert.Equal(t, 1, len(cm.Labels)) + assert.Equal(t, "my-test-cluster", cm.Annotations["meta.helm.sh/release-name"]) + + // Remove the labels and annotations from the configmap and verify they are added back + // This tests the upgrade scenario where the labels and annotations will not exist + cm.Labels = nil + cm.Annotations = nil + _, err = clientset.CoreV1().ConfigMaps(ns).Update(ctx.TODO(), cm, metav1.UpdateOptions{}) + assert.NoError(t, err) + err = populateConfigOverrideConfigMap(context, ns, ownerInfo, clusterMetadata) + assert.NoError(t, err) + cm, err = clientset.CoreV1().ConfigMaps(ns).Get(ctx.TODO(), k8sutil.ConfigOverrideName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, 2, len(cm.Annotations)) + assert.Equal(t, 1, len(cm.Labels)) + assert.Equal(t, "my-test-cluster", cm.Annotations["meta.helm.sh/release-name"]) + + // The helm annotations should be added even if other non-helm properties exist + cm.Labels = map[string]string{"foo": "bar"} + cm.Annotations = map[string]string{"hello": "world"} + _, err = clientset.CoreV1().ConfigMaps(ns).Update(ctx.TODO(), cm, metav1.UpdateOptions{}) + assert.NoError(t, err) + + err = populateConfigOverrideConfigMap(context, ns, ownerInfo, clusterMetadata) + assert.NoError(t, err) + cm, err = clientset.CoreV1().ConfigMaps(ns).Get(ctx.TODO(), k8sutil.ConfigOverrideName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, 3, len(cm.Annotations)) + assert.Equal(t, 2, len(cm.Labels)) + assert.Equal(t, "world", cm.Annotations["hello"]) + assert.Equal(t, "my-test-cluster", cm.Annotations["meta.helm.sh/release-name"]) + assert.Equal(t, ns, cm.Annotations["meta.helm.sh/release-namespace"]) + assert.Equal(t, "bar", cm.Labels["foo"]) + assert.Equal(t, "Helm", cm.Labels["app.kubernetes.io/managed-by"]) +}