From 233e68ecbf65cba441fb927baf524ae8a043c4b6 Mon Sep 17 00:00:00 2001 From: subhamkrai Date: Wed, 9 Aug 2023 20:10:02 +0530 Subject: [PATCH] operator: move most of discover pod setting to cm It's better to move most of discover daemon setting from env to configmap rook-ceph-operator-config. Although, we are moving to configmap, we keep reading settings from env but the priority will be configmap settings. Signed-off-by: subhamkrai (cherry picked from commit fb395800678cd9c0dae079163dc18d97ef072192) --- deploy/examples/operator-openshift.yaml | 55 +++++++++++------- deploy/examples/operator.yaml | 77 +++++++++++-------------- pkg/operator/ceph/cluster/controller.go | 6 +- pkg/operator/ceph/controller.go | 6 +- pkg/operator/discover/discover.go | 37 ++++++------ pkg/operator/discover/discover_test.go | 38 +++++++++++- 6 files changed, 128 insertions(+), 91 deletions(-) diff --git a/deploy/examples/operator-openshift.yaml b/deploy/examples/operator-openshift.yaml index b5b3ce13b..0975c1dba 100644 --- a/deploy/examples/operator-openshift.yaml +++ b/deploy/examples/operator-openshift.yaml @@ -609,6 +609,39 @@ data: CSI_CEPHFS_ATTACH_REQUIRED: "true" CSI_RBD_ATTACH_REQUIRED: "true" CSI_NFS_ATTACH_REQUIRED: "true" + # Rook Discover toleration. Will tolerate all taints with all keys. + # (Optional) Rook Discover tolerations list. Put here list of taints you want to tolerate in YAML format. + # DISCOVER_TOLERATIONS: | + # - effect: NoSchedule + # key: node-role.kubernetes.io/control-plane + # operator: Exists + # - effect: NoExecute + # key: node-role.kubernetes.io/etcd + # operator: Exists + # (Optional) Rook Discover priority class name to set on the pod(s) + # DISCOVER_PRIORITY_CLASS_NAME: "" + # (Optional) Discover Agent NodeAffinity. + # DISCOVER_AGENT_NODE_AFFINITY: | + # requiredDuringSchedulingIgnoredDuringExecution: + # nodeSelectorTerms: + # - matchExpressions: + # - key: myKey + # operator: DoesNotExist + # (Optional) Discover Agent Pod Labels. + # DISCOVER_AGENT_POD_LABELS: "key1=value1,key2=value2" + # Disable automatic orchestration when new devices are discovered + ROOK_DISABLE_DEVICE_HOTPLUG: "false" + # The duration between discovering devices in the rook-discover daemonset. + ROOK_DISCOVER_DEVICES_INTERVAL: "60m" + # DISCOVER_DAEMON_RESOURCES: | + # - name: DISCOVER_DAEMON_RESOURCES + # resources: + # limits: + # cpu: 500m + # memory: 512Mi + # requests: + # cpu: 100m + # memory: 128Mi --- # The deployment for the rook operator # OLM: BEGIN OPERATOR DEPLOYMENT @@ -659,34 +692,12 @@ spec: env: - name: ROOK_CURRENT_NAMESPACE_ONLY value: "false" - # Rook Discover toleration. Will tolerate all taints with all keys. - # Choose between NoSchedule, PreferNoSchedule and NoExecute: - # - name: DISCOVER_TOLERATION - # value: "NoSchedule" - # (Optional) Rook Discover toleration key. Set this to the key of the taint you want to tolerate - # - name: DISCOVER_TOLERATION_KEY - # value: "" - # (Optional) Rook Discover priority class name to set on the pod(s) - # - name: DISCOVER_PRIORITY_CLASS_NAME - # value: "" - # (Optional) Discover Agent NodeAffinity. - # - name: DISCOVER_AGENT_NODE_AFFINITY - # value: "role=storage-node; storage=rook, ceph" - # (Optional) Discover Agent Pod Labels. - # - name: DISCOVER_AGENT_POD_LABELS - # value: "key1=value1,key2=value2" - # The duration between discovering devices in the rook-discover daemonset. - - name: ROOK_DISCOVER_DEVICES_INTERVAL - value: "60m" # Whether to start pods as privileged that mount a host path, which includes the Ceph mon and osd pods. # Set this to true if SELinux is enabled (e.g. OpenShift) to workaround the anyuid issues. # For more details see https://github.com/rook/rook/issues/1314#issuecomment-355799641 - name: ROOK_HOSTPATH_REQUIRES_PRIVILEGED value: "true" - # Disable automatic orchestration when new devices are discovered - - name: ROOK_DISABLE_DEVICE_HOTPLUG - value: "false" # Provide customised regex as the values using comma. For eg. regex for rbd based volume, value will be like "(?i)rbd[0-9]+". # In case of more than one regex, use comma to separate between them. # Default regex will be "(?i)dm-[0-9]+,(?i)rbd[0-9]+,(?i)nbd[0-9]+" diff --git a/deploy/examples/operator.yaml b/deploy/examples/operator.yaml index 5002c3514..e3d8e3750 100644 --- a/deploy/examples/operator.yaml +++ b/deploy/examples/operator.yaml @@ -536,6 +536,39 @@ data: CSI_CEPHFS_ATTACH_REQUIRED: "true" CSI_RBD_ATTACH_REQUIRED: "true" CSI_NFS_ATTACH_REQUIRED: "true" + # Rook Discover toleration. Will tolerate all taints with all keys. + # (Optional) Rook Discover tolerations list. Put here list of taints you want to tolerate in YAML format. + # DISCOVER_TOLERATIONS: | + # - effect: NoSchedule + # key: node-role.kubernetes.io/control-plane + # operator: Exists + # - effect: NoExecute + # key: node-role.kubernetes.io/etcd + # operator: Exists + # (Optional) Rook Discover priority class name to set on the pod(s) + # DISCOVER_PRIORITY_CLASS_NAME: "" + # (Optional) Discover Agent NodeAffinity. + # DISCOVER_AGENT_NODE_AFFINITY: | + # requiredDuringSchedulingIgnoredDuringExecution: + # nodeSelectorTerms: + # - matchExpressions: + # - key: myKey + # operator: DoesNotExist + # (Optional) Discover Agent Pod Labels. + # DISCOVER_AGENT_POD_LABELS: "key1=value1,key2=value2" + # Disable automatic orchestration when new devices are discovered + ROOK_DISABLE_DEVICE_HOTPLUG: "false" + # The duration between discovering devices in the rook-discover daemonset. + ROOK_DISCOVER_DEVICES_INTERVAL: "60m" + # DISCOVER_DAEMON_RESOURCES: | + # - name: DISCOVER_DAEMON_RESOURCES + # resources: + # limits: + # cpu: 500m + # memory: 512Mi + # requests: + # cpu: 100m + # memory: 128Mi --- # OLM: BEGIN OPERATOR DEPLOYMENT apiVersion: apps/v1 @@ -589,46 +622,12 @@ spec: # If this is not set to true, the operator will watch for cluster CRDs in all namespaces. - name: ROOK_CURRENT_NAMESPACE_ONLY value: "false" - # Rook Discover toleration. Will tolerate all taints with all keys. - # Choose between NoSchedule, PreferNoSchedule and NoExecute: - # - name: DISCOVER_TOLERATION - # value: "NoSchedule" - # (Optional) Rook Discover toleration key. Set this to the key of the taint you want to tolerate - # - name: DISCOVER_TOLERATION_KEY - # value: "" - # (Optional) Rook Discover tolerations list. Put here list of taints you want to tolerate in YAML format. - # - name: DISCOVER_TOLERATIONS - # value: | - # - effect: NoSchedule - # key: node-role.kubernetes.io/control-plane - # operator: Exists - # - effect: NoExecute - # key: node-role.kubernetes.io/etcd - # operator: Exists - # (Optional) Rook Discover priority class name to set on the pod(s) - # - name: DISCOVER_PRIORITY_CLASS_NAME - # value: "" - # (Optional) Discover Agent NodeAffinity. - # - name: DISCOVER_AGENT_NODE_AFFINITY - # value: "role=storage-node; storage=rook, ceph" - # (Optional) Discover Agent Pod Labels. - # - name: DISCOVER_AGENT_POD_LABELS - # value: "key1=value1,key2=value2" - - # The duration between discovering devices in the rook-discover daemonset. - - name: ROOK_DISCOVER_DEVICES_INTERVAL - value: "60m" # Whether to start pods as privileged that mount a host path, which includes the Ceph mon and osd pods. # Set this to true if SELinux is enabled (e.g. OpenShift) to workaround the anyuid issues. # For more details see https://github.com/rook/rook/issues/1314#issuecomment-355799641 - name: ROOK_HOSTPATH_REQUIRES_PRIVILEGED value: "false" - - # Disable automatic orchestration when new devices are discovered - - name: ROOK_DISABLE_DEVICE_HOTPLUG - value: "false" - # Provide customised regex as the values using comma. For eg. regex for rbd based volume, value will be like "(?i)rbd[0-9]+". # In case of more than one regex, use comma to separate between them. # Default regex will be "(?i)dm-[0-9]+,(?i)rbd[0-9]+,(?i)nbd[0-9]+" @@ -637,16 +636,6 @@ spec: - name: DISCOVER_DAEMON_UDEV_BLACKLIST value: "(?i)dm-[0-9]+,(?i)rbd[0-9]+,(?i)nbd[0-9]+" - # - name: DISCOVER_DAEMON_RESOURCES - # value: | - # resources: - # limits: - # cpu: 500m - # memory: 512Mi - # requests: - # cpu: 100m - # memory: 128Mi - # Time to wait until the node controller will move Rook pods to other # nodes after detecting an unreachable node. # Pods affected by this setting are: diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index f24aed48d..04dc76584 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/pkg/operator/ceph/cluster/controller.go @@ -20,7 +20,6 @@ package cluster import ( "context" "fmt" - "os" "github.com/coreos/pkg/capnslog" addonsv1alpha1 "github.com/csi-addons/kubernetes-csi-addons/apis/csiaddons/v1alpha1" @@ -189,7 +188,10 @@ func add(opManagerContext context.Context, mgr manager.Manager, r reconcile.Reco // Watch for changes on the hotplug config map // TODO: to improve, can we run this against the operator namespace only? - disableVal := os.Getenv(disableHotplugEnv) + disableVal, err := k8sutil.GetOperatorSetting(opManagerContext, context.Clientset, opcontroller.OperatorSettingConfigMapName, disableHotplugEnv, "false") + if err != nil { + logger.Errorf("failed to get %s setting %s. %v", disableHotplugEnv, opcontroller.OperatorSettingConfigMapName, err) + } if disableVal != "true" { logger.Info("enabling hotplug orchestration") s := source.Kind(mgr.GetCache(), diff --git a/pkg/operator/ceph/controller.go b/pkg/operator/ceph/controller.go index b18b68ff2..a5cbed862 100644 --- a/pkg/operator/ceph/controller.go +++ b/pkg/operator/ceph/controller.go @@ -139,7 +139,7 @@ func (r *ReconcileConfig) reconcile(request reconcile.Request) (reconcile.Result reconcileOperatorLogLevel(opConfig.Data) // Reconcile discovery daemon - err = r.reconcileDiscoveryDaemon() + err = r.reconcileDiscoveryDaemon(opConfig.Data) if err != nil { return opcontroller.ImmediateRetryResult, err } @@ -158,10 +158,10 @@ func reconcileOperatorLogLevel(data map[string]string) { util.SetGlobalLogLevel(rookLogLevel, logger) } -func (r *ReconcileConfig) reconcileDiscoveryDaemon() error { +func (r *ReconcileConfig) reconcileDiscoveryDaemon(data map[string]string) error { rookDiscover := discover.New(r.context.Clientset) if opcontroller.DiscoveryDaemonEnabled(r.config.Parameters) { - if err := rookDiscover.Start(r.opManagerContext, r.config.OperatorNamespace, r.config.Image, r.config.ServiceAccount, true); err != nil { + if err := rookDiscover.Start(r.opManagerContext, r.config.OperatorNamespace, r.config.Image, r.config.ServiceAccount, data, true); err != nil { return errors.Wrap(err, "failed to start device discovery daemonset") } } else { diff --git a/pkg/operator/discover/discover.go b/pkg/operator/discover/discover.go index 26e68f06d..836240824 100644 --- a/pkg/operator/discover/discover.go +++ b/pkg/operator/discover/discover.go @@ -72,22 +72,27 @@ func New(clientset kubernetes.Interface) *Discover { } // Start the discover -func (d *Discover) Start(ctx context.Context, namespace, discoverImage, securityAccount string, useCephVolume bool) error { - err := d.createDiscoverDaemonSet(ctx, namespace, discoverImage, securityAccount, useCephVolume) +func (d *Discover) Start(ctx context.Context, namespace, discoverImage, securityAccount string, data map[string]string, useCephVolume bool) error { + err := d.createDiscoverDaemonSet(ctx, namespace, discoverImage, securityAccount, data, useCephVolume) if err != nil { return fmt.Errorf("failed to start discover daemonset. %v", err) } return nil } -func (d *Discover) createDiscoverDaemonSet(ctx context.Context, namespace, discoverImage, securityAccount string, useCephVolume bool) error { +func (d *Discover) createDiscoverDaemonSet(ctx context.Context, namespace, discoverImage, securityAccount string, data map[string]string, useCephVolume bool) error { + discoveryInterval := k8sutil.GetValue(data, discoverIntervalEnv, "") + if discoverImage == "" { + discoveryInterval = defaultDiscoverInterval + } + discoveryParameters := []string{"discover", - "--discover-interval", getEnvVar(discoverIntervalEnv, defaultDiscoverInterval)} + "--discover-interval", discoveryInterval} if useCephVolume { discoveryParameters = append(discoveryParameters, "--use-ceph-volume") } - discoverDaemonResourcesRaw := os.Getenv(discoverDaemonResourcesEnv) + discoverDaemonResourcesRaw := k8sutil.GetValue(data, discoverDaemonResourcesEnv, "") discoverDaemonResources, err := k8sutil.YamlToContainerResource(discoverDaemonResourcesRaw) if err != nil { logger.Warningf("failed to parse.%s %v", discoverDaemonResourcesRaw, err) @@ -172,7 +177,7 @@ func (d *Discover) createDiscoverDaemonSet(ctx context.Context, namespace, disco }, }, HostNetwork: false, - PriorityClassName: os.Getenv(discoverDaemonsetPriorityClassNameEnv), + PriorityClassName: k8sutil.GetValue(data, discoverDaemonsetPriorityClassNameEnv, ""), }, }, }, @@ -186,26 +191,27 @@ func (d *Discover) createDiscoverDaemonSet(ctx context.Context, namespace, disco } // Add toleration if any - tolerationValue := os.Getenv(discoverDaemonsetTolerationEnv) + tolerationValue := k8sutil.GetValue(data, discoverDaemonsetTolerationEnv, "") if tolerationValue != "" { ds.Spec.Template.Spec.Tolerations = []v1.Toleration{ { Effect: v1.TaintEffect(tolerationValue), Operator: v1.TolerationOpExists, - Key: os.Getenv(discoverDaemonsetTolerationKeyEnv), + Key: k8sutil.GetValue(data, discoverDaemonsetTolerationKeyEnv, ""), }, } } - tolerationsRaw := os.Getenv(discoverDaemonsetTolerationsEnv) + tolerationsRaw := k8sutil.GetValue(data, discoverDaemonsetTolerationsEnv, "") tolerations, err := k8sutil.YamlToTolerations(tolerationsRaw) if err != nil { logger.Warningf("failed to parse %s. %+v", tolerationsRaw, err) } + logger.Infof("tolerations: %v", tolerations) ds.Spec.Template.Spec.Tolerations = append(ds.Spec.Template.Spec.Tolerations, tolerations...) // Add NodeAffinity if any - nodeAffinity := os.Getenv(discoverDaemonSetNodeAffinityEnv) + nodeAffinity := k8sutil.GetValue(data, discoverDaemonSetNodeAffinityEnv, "") if nodeAffinity != "" { v1NodeAffinity, err := k8sutil.GenerateNodeAffinity(nodeAffinity) if err != nil { @@ -215,9 +221,10 @@ func (d *Discover) createDiscoverDaemonSet(ctx context.Context, namespace, disco NodeAffinity: v1NodeAffinity, } } + logger.Infof("nodeAffinity: %s", v1NodeAffinity) } - podLabels := os.Getenv(discoverDaemonSetPodLabelsEnv) + podLabels := k8sutil.GetValue(data, discoverDaemonSetPodLabelsEnv, "") if podLabels != "" { podLabels := k8sutil.ParseStringToLabels(podLabels) // Override / Set the app label even if set by the user as @@ -255,14 +262,6 @@ func getLabels() map[string]string { return labels } -func getEnvVar(varName string, defaultValue string) string { - envValue := os.Getenv(varName) - if envValue != "" { - return envValue - } - return defaultValue -} - // ListDevices lists all devices discovered on all nodes or specific node if node name is provided. func ListDevices(ctx context.Context, clusterdContext *clusterd.Context, namespace, nodeName string) (map[string][]sys.LocalDisk, error) { // convert the host name label to the k8s node name to look up the configmap with the devices diff --git a/pkg/operator/discover/discover_test.go b/pkg/operator/discover/discover_test.go index 0ce007a11..667e88bdc 100644 --- a/pkg/operator/discover/discover_test.go +++ b/pkg/operator/discover/discover_test.go @@ -24,6 +24,7 @@ import ( cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" discoverDaemon "github.com/rook/rook/pkg/daemon/discover" + "github.com/rook/rook/pkg/operator/ceph/controller" "github.com/rook/rook/pkg/operator/k8sutil" "github.com/rook/rook/pkg/operator/test" @@ -62,7 +63,7 @@ func TestStartDiscoveryDaemonset(t *testing.T) { _, err := clientset.CoreV1().Pods("rook-system").Create(ctx, &pod, metav1.CreateOptions{}) assert.NoError(t, err) // start a basic cluster - err = a.Start(ctx, namespace, "rook/rook:myversion", "mysa", false) + err = a.Start(ctx, namespace, "rook/rook:myversion", "mysa", map[string]string{}, false) assert.Nil(t, err) // check daemonset parameters @@ -83,6 +84,41 @@ func TestStartDiscoveryDaemonset(t *testing.T) { image := agentDS.Spec.Template.Spec.Containers[0].Image assert.Equal(t, "rook/rook:myversion", image) assert.Nil(t, agentDS.Spec.Template.Spec.Tolerations) + + // Test with rook override configmap setting + opConfigCM := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: controller.OperatorSettingConfigMapName, + Namespace: namespace, + }, + Data: map[string]string{ + "DISCOVER_TOLERATIONS": "- effect: NoSchedule\n key: node-role.kubernetes.io/control-plane\n operator: Exists\n- effect: NoExecute\n key: node-role.kubernetes.io/etcd\n operator: Exists", + }, + } + + cm, err := clientset.CoreV1().ConfigMaps(namespace).Create(ctx, opConfigCM, metav1.CreateOptions{}) + assert.NoError(t, err) + + // start a basic cluster + err = a.Start(ctx, namespace, "rook/rook:myversion", "mysa", cm.Data, false) + assert.Nil(t, err) + + agentDS, err = clientset.AppsV1().DaemonSets(namespace).Get(ctx, "rook-discover", metav1.GetOptions{}) + assert.NoError(t, err) + + want := []v1.Toleration{ + { + Key: "node-role.kubernetes.io/control-plane", + Operator: v1.TolerationOpExists, + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "node-role.kubernetes.io/etcd", + Operator: v1.TolerationOpExists, + Effect: v1.TaintEffectNoExecute, + }, + } + assert.Equal(t, want, agentDS.Spec.Template.Spec.Tolerations) } func TestGetAvailableDevices(t *testing.T) {