From 5a7dee2f63e02363ca86fbcc6c32cfdbb332c28b Mon Sep 17 00:00:00 2001 From: Blaine Gardner Date: Thu, 4 Nov 2021 10:49:11 -0600 Subject: [PATCH] nfs: restart nfs servers when configmap is updated When the configuration configmap is updated for a CephNFS server, the NFS application should restart to ensure it is running with the latest config. Fixes #9028 Signed-off-by: Blaine Gardner --- pkg/operator/ceph/nfs/nfs.go | 32 +++-- pkg/operator/ceph/nfs/nfs_test.go | 209 +++++++++++++++++++++++++++++ pkg/operator/ceph/nfs/spec.go | 6 + pkg/operator/ceph/nfs/spec_test.go | 7 +- 4 files changed, 241 insertions(+), 13 deletions(-) create mode 100644 pkg/operator/ceph/nfs/nfs_test.go diff --git a/pkg/operator/ceph/nfs/nfs.go b/pkg/operator/ceph/nfs/nfs.go index 937895bd1f25..95994d21dde1 100644 --- a/pkg/operator/ceph/nfs/nfs.go +++ b/pkg/operator/ceph/nfs/nfs.go @@ -18,6 +18,7 @@ limitations under the License. package nfs import ( + "crypto/sha256" "fmt" "strings" @@ -46,9 +47,10 @@ const ( var updateDeploymentAndWait = opmon.UpdateCephDeploymentAndWait type daemonConfig struct { - ID string // letter ID of daemon (e.g., a, b, c, ...) - ConfigConfigMap string // name of configmap holding config - DataPathMap *config.DataPathMap // location to store data in container + ID string // letter ID of daemon (e.g., a, b, c, ...) + ConfigConfigMap string // name of configmap holding config + ConfigConfigMapHash string // hash of configmap holding config + DataPathMap *config.DataPathMap // location to store data in container } // Create the ganesha server @@ -56,7 +58,7 @@ func (r *ReconcileCephNFS) upCephNFS(n *cephv1.CephNFS) error { for i := 0; i < n.Spec.Server.Active; i++ { id := k8sutil.IndexToName(i) - configName, err := r.createConfigMap(n, id) + configName, configHash, err := r.createConfigMap(n, id) if err != nil { return errors.Wrap(err, "failed to create config") } @@ -67,8 +69,9 @@ func (r *ReconcileCephNFS) upCephNFS(n *cephv1.CephNFS) error { } cfg := daemonConfig{ - ID: id, - ConfigConfigMap: configName, + ID: id, + ConfigConfigMap: configName, + ConfigConfigMapHash: configHash, DataPathMap: &config.DataPathMap{ HostDataDir: "", // nfs daemon does not store data on host, ... ContainerDataDir: cephclient.DefaultConfigDir, // does share data in containers using emptyDir, ... @@ -191,28 +194,35 @@ func (r *ReconcileCephNFS) generateConfigMap(n *cephv1.CephNFS, name string) *v1 return configMap } -func (r *ReconcileCephNFS) createConfigMap(n *cephv1.CephNFS, name string) (string, error) { +// return the name of the configmap, plus a hash of the data +func (r *ReconcileCephNFS) createConfigMap(n *cephv1.CephNFS, name string) (string, string, error) { // Generate configMap configMap := r.generateConfigMap(n, name) // Set owner reference err := controllerutil.SetControllerReference(n, configMap, r.scheme) if err != nil { - return "", errors.Wrapf(err, "failed to set owner reference for ceph ganesha configmap %q", configMap.Name) + return "", "", errors.Wrapf(err, "failed to set owner reference for ceph ganesha configmap %q", configMap.Name) } if _, err := r.context.Clientset.CoreV1().ConfigMaps(n.Namespace).Create(r.opManagerContext, configMap, metav1.CreateOptions{}); err != nil { if !kerrors.IsAlreadyExists(err) { - return "", errors.Wrap(err, "failed to create ganesha config map") + return "", "", errors.Wrap(err, "failed to create ganesha config map") } logger.Debugf("updating config map %q that already exists", configMap.Name) if _, err = r.context.Clientset.CoreV1().ConfigMaps(n.Namespace).Update(r.opManagerContext, configMap, metav1.UpdateOptions{}); err != nil { - return "", errors.Wrap(err, "failed to update ganesha config map") + return "", "", errors.Wrap(err, "failed to update ganesha config map") } } - return configMap.Name, nil + h := sha256.New() + if _, err := h.Write([]byte(fmt.Sprintf("%v", configMap.Data))); err != nil { + return "", "", errors.Wrapf(err, "failed to get hash of ganesha config map") + } + configHash := fmt.Sprintf("%x", h.Sum(nil)) + + return configMap.Name, configHash, nil } // Down scale the ganesha server diff --git a/pkg/operator/ceph/nfs/nfs_test.go b/pkg/operator/ceph/nfs/nfs_test.go new file mode 100644 index 000000000000..7c70b92efb42 --- /dev/null +++ b/pkg/operator/ceph/nfs/nfs_test.go @@ -0,0 +1,209 @@ +/* +Copyright 2021 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 nfs manages NFS ganesha servers for Ceph +package nfs + +import ( + "context" + "testing" + + "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + "github.com/rook/rook/pkg/client/clientset/versioned/scheme" + "github.com/rook/rook/pkg/clusterd" + cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + cephver "github.com/rook/rook/pkg/operator/ceph/version" + exectest "github.com/rook/rook/pkg/util/exec/test" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sfake "k8s.io/client-go/kubernetes/fake" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestReconcileCephNFS_createConfigMap(t *testing.T) { + s := scheme.Scheme + + clientset := k8sfake.NewSimpleClientset() + + c := &clusterd.Context{ + Executor: &exectest.MockExecutor{}, + Clientset: clientset, + } + + r := &ReconcileCephNFS{ + scheme: s, + context: c, + clusterInfo: &cephclient.ClusterInfo{ + FSID: "myfsid", + CephVersion: cephver.Octopus, + }, + cephClusterSpec: &cephv1.ClusterSpec{ + CephVersion: cephv1.CephVersionSpec{ + Image: "quay.io/ceph/ceph:v15", + }, + }, + } + + nfs := &cephv1.CephNFS{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nfs", + Namespace: "rook-ceph-test-ns", + }, + Spec: cephv1.NFSGaneshaSpec{ + RADOS: cephv1.GaneshaRADOSSpec{ + Pool: "myfs-data0", + Namespace: "nfs-test-ns", + }, + Server: cephv1.GaneshaServerSpec{ + Active: 3, + }, + }, + } + + t.Run("running multiple times should give the same hash", func(t *testing.T) { + cmName, hash1, err := r.createConfigMap(nfs, "a") + assert.NoError(t, err) + assert.Equal(t, "rook-ceph-nfs-my-nfs-a", cmName) + _, err = r.context.Clientset.CoreV1().ConfigMaps("rook-ceph-test-ns").Get(context.TODO(), cmName, metav1.GetOptions{}) + assert.NoError(t, err) + + _, hash2, err := r.createConfigMap(nfs, "a") + assert.NoError(t, err) + _, err = r.context.Clientset.CoreV1().ConfigMaps("rook-ceph-test-ns").Get(context.TODO(), cmName, metav1.GetOptions{}) + assert.NoError(t, err) + + assert.Equal(t, hash1, hash2) + }) + + t.Run("running with different IDs should give different hashes", func(t *testing.T) { + cmName, hash1, err := r.createConfigMap(nfs, "a") + assert.NoError(t, err) + assert.Equal(t, "rook-ceph-nfs-my-nfs-a", cmName) + _, err = r.context.Clientset.CoreV1().ConfigMaps("rook-ceph-test-ns").Get(context.TODO(), cmName, metav1.GetOptions{}) + assert.NoError(t, err) + + _, hash2, err := r.createConfigMap(nfs, "b") + assert.NoError(t, err) + _, err = r.context.Clientset.CoreV1().ConfigMaps("rook-ceph-test-ns").Get(context.TODO(), cmName, metav1.GetOptions{}) + assert.NoError(t, err) + + assert.NotEqual(t, hash1, hash2) + }) + + t.Run("running with different configs should give different hashes", func(t *testing.T) { + cmName, hash1, err := r.createConfigMap(nfs, "a") + assert.NoError(t, err) + assert.Equal(t, "rook-ceph-nfs-my-nfs-a", cmName) + _, err = r.context.Clientset.CoreV1().ConfigMaps("rook-ceph-test-ns").Get(context.TODO(), cmName, metav1.GetOptions{}) + assert.NoError(t, err) + + nfs2 := nfs.DeepCopy() + nfs2.Name = "nfs-two" + _, hash2, err := r.createConfigMap(nfs2, "a") + assert.NoError(t, err) + _, err = r.context.Clientset.CoreV1().ConfigMaps("rook-ceph-test-ns").Get(context.TODO(), cmName, metav1.GetOptions{}) + assert.NoError(t, err) + + assert.NotEqual(t, hash1, hash2) + }) +} + +func TestReconcileCephNFS_upCephNFS(t *testing.T) { + ns := "up-ceph-ns-namespace" + + s := scheme.Scheme + + clientset := k8sfake.NewSimpleClientset() + + executor := &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { + logger.Infof("executing command: %s %+v", command, args) + if args[0] == "auth" { + if args[1] == "get-or-create-key" { + return "{\"key\":\"mysecurekey\"}", nil + } + } + panic(errors.Errorf("unhandled command %s %v", command, args)) + }, + } + + client := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects().Build() + + c := &clusterd.Context{ + Executor: executor, + Client: client, + Clientset: clientset, + } + + r := &ReconcileCephNFS{ + scheme: s, + context: c, + clusterInfo: &cephclient.ClusterInfo{ + FSID: "myfsid", + CephVersion: cephver.Octopus, + Context: context.TODO(), + Namespace: ns, + }, + cephClusterSpec: &cephv1.ClusterSpec{ + CephVersion: cephv1.CephVersionSpec{ + Image: "quay.io/ceph/ceph:v15", + }, + }, + } + + nfs := &cephv1.CephNFS{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nfs", + Namespace: ns, + }, + Spec: cephv1.NFSGaneshaSpec{ + RADOS: cephv1.GaneshaRADOSSpec{ + Pool: "myfs-data0", + Namespace: "nfs-test-ns", + }, + Server: cephv1.GaneshaServerSpec{ + Active: 2, + }, + }, + } + + err := r.upCephNFS(nfs) + assert.NoError(t, err) + + deps, err := r.context.Clientset.AppsV1().Deployments(ns).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, deps.Items, 2) + names := []string{} + hashes := []string{} + for _, dep := range deps.Items { + names = append(names, dep.Name) + assert.Contains(t, dep.Spec.Template.Annotations, "config-hash") + hashes = append(hashes, dep.Spec.Template.Annotations["config-hash"]) + } + assert.ElementsMatch(t, []string{"rook-ceph-nfs-my-nfs-a", "rook-ceph-nfs-my-nfs-b"}, names) + assert.NotEqual(t, hashes[0], hashes[1]) + + svcs, err := r.context.Clientset.CoreV1().Services(ns).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + // Each NFS server gets a service. + assert.Len(t, svcs.Items, 2) + names = []string{} + for _, svc := range svcs.Items { + names = append(names, svc.Name) + } + assert.ElementsMatch(t, []string{"rook-ceph-nfs-my-nfs-a", "rook-ceph-nfs-my-nfs-b"}, names) +} diff --git a/pkg/operator/ceph/nfs/spec.go b/pkg/operator/ceph/nfs/spec.go index a93cab3cc639..e265e6968322 100644 --- a/pkg/operator/ceph/nfs/spec.go +++ b/pkg/operator/ceph/nfs/spec.go @@ -141,6 +141,12 @@ func (r *ReconcileCephNFS) makeDeployment(nfs *cephv1.CephNFS, cfg daemonConfig) ObjectMeta: metav1.ObjectMeta{ Name: resourceName, Labels: getLabels(nfs, cfg.ID, true), + Annotations: map[string]string{ + // set an annotation with the hash of the configmap data so that the pod will be + // re-deployed if the config in the configmap changes. otherwise, the pod won't + // restart when the config is updated. + "config-hash": cfg.ConfigConfigMapHash, + }, }, Spec: podSpec, } diff --git a/pkg/operator/ceph/nfs/spec_test.go b/pkg/operator/ceph/nfs/spec_test.go index 0c7ce77eb282..3902ee081e87 100644 --- a/pkg/operator/ceph/nfs/spec_test.go +++ b/pkg/operator/ceph/nfs/spec_test.go @@ -109,8 +109,9 @@ func TestDeploymentSpec(t *testing.T) { id := "i" configName := "rook-ceph-nfs-my-nfs-i" cfg := daemonConfig{ - ID: id, - ConfigConfigMap: configName, + ID: id, + ConfigConfigMap: configName, + ConfigConfigMapHash: "dcb0d2f5f5e86ec4929d8243cd640b8154165f8ff9b89809964fc7993e9b0101", DataPathMap: &config.DataPathMap{ HostDataDir: "", // nfs daemon does not store data on host, ... ContainerDataDir: cephclient.DefaultConfigDir, // does share data in containers using emptyDir, ... @@ -120,6 +121,8 @@ func TestDeploymentSpec(t *testing.T) { d, err := r.makeDeployment(nfs, cfg) assert.NoError(t, err) + assert.NotEmpty(t, d.Spec.Template.Annotations) + assert.Equal(t, "dcb0d2f5f5e86ec4929d8243cd640b8154165f8ff9b89809964fc7993e9b0101", d.Spec.Template.Annotations["config-hash"]) // Deployment should have Ceph labels optest.AssertLabelsContainRookRequirements(t, d.ObjectMeta.Labels, AppName)