From 656dd0f3341a3e587bc47fd4f2ea605d995d93a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Thu, 5 Aug 2021 14:09:17 +0200 Subject: [PATCH] ceph: move the admission webhook to the operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our admission webhooks will now run as part of the Operator container and not an additional deployment. This has the advantage of consuming fewer resources in the cluster and not having to manage affinities and tolerations. This only drawback is that the Secret containing the certificates is not mounted anymore and the content needs to be written inside the Operator. This is not practical since we also need to watch for the Secret content to change. Meaning that the certificates have been renewed and the webhook server needs to use them. A new approach is on its way to hopefully simplify this last issue and implement a watcher for the Secret. In the meantime, users need to use the cert-manager or renew certificates manually. Additionally, they must update the ValidatingWebhookConfiguration object with the new CA bundle. Signed-off-by: Sébastien Han --- .../rook-ceph/templates/clusterrole.yaml | 9 - .../templates/clusterrolebinding.yaml | 14 -- .../rook-ceph/templates/deployment.yaml | 14 -- .../rook-ceph/templates/serviceaccount.yaml | 8 +- cluster/examples/kubernetes/ceph/common.yaml | 29 --- .../kubernetes/ceph/operator-openshift.yaml | 20 +- .../examples/kubernetes/ceph/operator.yaml | 20 +- cmd/rook/ceph/admission.go | 43 ---- cmd/rook/ceph/ceph.go | 1 - cmd/rook/ceph/operator.go | 2 +- .../ceph/cluster/operator_watchers.go | 2 + pkg/operator/ceph/cr_manager.go | 52 +++- pkg/operator/ceph/operator.go | 59 ++--- pkg/operator/ceph/server.go | 68 ------ pkg/operator/ceph/webhook.go | 225 ++++-------------- pkg/operator/ceph/webhook_test.go | 134 ----------- tests/framework/installer/ceph_installer.go | 8 - 17 files changed, 133 insertions(+), 575 deletions(-) delete mode 100644 cmd/rook/ceph/admission.go delete mode 100644 pkg/operator/ceph/server.go delete mode 100644 pkg/operator/ceph/webhook_test.go diff --git a/cluster/charts/rook-ceph/templates/clusterrole.yaml b/cluster/charts/rook-ceph/templates/clusterrole.yaml index 2218ec33d..d9565da51 100644 --- a/cluster/charts/rook-ceph/templates/clusterrole.yaml +++ b/cluster/charts/rook-ceph/templates/clusterrole.yaml @@ -471,12 +471,3 @@ rules: verbs: - use {{- end }} ---- -kind: ClusterRole -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: rook-ceph-admission-controller-role -rules: - - apiGroups: ["ceph.rook.io"] - resources: ["*"] - verbs: ["get", "watch", "list"] diff --git a/cluster/charts/rook-ceph/templates/clusterrolebinding.yaml b/cluster/charts/rook-ceph/templates/clusterrolebinding.yaml index 36d539a25..d3c22ca74 100644 --- a/cluster/charts/rook-ceph/templates/clusterrolebinding.yaml +++ b/cluster/charts/rook-ceph/templates/clusterrolebinding.yaml @@ -260,20 +260,6 @@ subjects: name: rook-ceph-mgr namespace: {{ .Release.Namespace }} --- -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: rook-ceph-admission-controller-rolebinding -subjects: - - kind: ServiceAccount - name: rook-ceph-admission-controller - apiGroup: "" - namespace: {{ .Release.Namespace }} -roleRef: - kind: ClusterRole - name: rook-ceph-admission-controller-role - apiGroup: rbac.authorization.k8s.io ---- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: diff --git a/cluster/charts/rook-ceph/templates/deployment.yaml b/cluster/charts/rook-ceph/templates/deployment.yaml index c6402c129..7efb599eb 100644 --- a/cluster/charts/rook-ceph/templates/deployment.yaml +++ b/cluster/charts/rook-ceph/templates/deployment.yaml @@ -313,20 +313,6 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace -{{- if .Values.admissionController }} -{{- if .Values.admissionController.tolerations }} - - name: ADMISSION_CONTROLLER_TOLERATIONS - value: {{ toYaml .Values.admissionController.tolerations | quote }} -{{- end }} -{{- if .Values.admissionController.nodeAffinity }} - - name: ADMISSION_CONTROLLER_NODE_AFFINITY - value: {{ .Values.admissionController.nodeAffinity }} -{{- end }} -{{- end }} -{{- if .Values.unreachableNodeTolerationSeconds }} - - name: ROOK_UNREACHABLE_NODE_TOLERATION_SECONDS - value: {{ .Values.unreachableNodeTolerationSeconds | quote }} -{{- end }} {{- if .Values.resources }} resources: {{ toYaml .Values.resources | indent 10 }} diff --git a/cluster/charts/rook-ceph/templates/serviceaccount.yaml b/cluster/charts/rook-ceph/templates/serviceaccount.yaml index 5d3687f67..f9317cb0a 100644 --- a/cluster/charts/rook-ceph/templates/serviceaccount.yaml +++ b/cluster/charts/rook-ceph/templates/serviceaccount.yaml @@ -77,12 +77,6 @@ metadata: namespace: {{ .Release.Namespace }} {{ template "imagePullSecrets" . }} --- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: rook-ceph-admission-controller - namespace: {{ .Release.Namespace }} ---- # Service account for the purge osd job apiVersion: v1 kind: ServiceAccount @@ -90,4 +84,4 @@ metadata: name: rook-ceph-purge-osd namespace: {{ .Release.Namespace }} {{ template "imagePullSecrets" . }} ---- \ No newline at end of file +--- diff --git a/cluster/examples/kubernetes/ceph/common.yaml b/cluster/examples/kubernetes/ceph/common.yaml index aed387d5e..15ee9a612 100644 --- a/cluster/examples/kubernetes/ceph/common.yaml +++ b/cluster/examples/kubernetes/ceph/common.yaml @@ -32,35 +32,6 @@ subjects: # OLM: END OBJECTBUCKET ROLEBINDING # OLM: BEGIN OPERATOR ROLE --- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: rook-ceph-admission-controller - namespace: rook-ceph # namespace:operator ---- -kind: ClusterRole -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: rook-ceph-admission-controller-role -rules: - - apiGroups: ["ceph.rook.io"] - resources: ["*"] - verbs: ["get", "watch", "list"] ---- -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: rook-ceph-admission-controller-rolebinding -subjects: - - kind: ServiceAccount - name: rook-ceph-admission-controller - apiGroup: "" - namespace: rook-ceph # namespace:operator -roleRef: - kind: ClusterRole - name: rook-ceph-admission-controller-role - apiGroup: rbac.authorization.k8s.io ---- # The cluster role for managing all the cluster-specific resources in a namespace apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/cluster/examples/kubernetes/ceph/operator-openshift.yaml b/cluster/examples/kubernetes/ceph/operator-openshift.yaml index 5e7ce8d30..930554d0f 100644 --- a/cluster/examples/kubernetes/ceph/operator-openshift.yaml +++ b/cluster/examples/kubernetes/ceph/operator-openshift.yaml @@ -405,18 +405,6 @@ data: # The timeout value (in seconds) of Ceph commands. It should be >= 1. If this variable is not set or is an invalid value, it's default to 15. ROOK_CEPH_COMMANDS_TIMEOUT_SECONDS: "15" # CSI_VOLUME_REPLICATION_IMAGE: "quay.io/csiaddons/volumereplication-operator:v0.1.0" - - # (Optional) Admission controller NodeAffinity. - # ADMISSION_CONTROLLER_NODE_AFFINITY: "role=storage-node; storage=rook, ceph" - # (Optional) Admission controller tolerations list. Put here list of taints you want to tolerate in YAML format. - # Admission controller would be best to start on the same nodes as other ceph daemons. - # ADMISSION_CONTROLLER_TOLERATIONS: | - # - effect: NoSchedule - # key: node-role.kubernetes.io/controlplane - # operator: Exists - # - effect: NoExecute - # key: node-role.kubernetes.io/etcd - # operator: Exists --- # The deployment for the rook operator # OLM: BEGIN OPERATOR DEPLOYMENT @@ -448,6 +436,12 @@ spec: name: rook-config - mountPath: /etc/ceph name: default-config-dir + - mountPath: /etc/webhook + name: webhook-cert + ports: + - containerPort: 9443 + name: https-webhook + protocol: TCP env: - name: ROOK_CURRENT_NAMESPACE_ONLY value: "false" @@ -577,4 +571,6 @@ spec: emptyDir: {} - name: default-config-dir emptyDir: {} + - name: webhook-cert + emptyDir: {} # OLM: END OPERATOR DEPLOYMENT diff --git a/cluster/examples/kubernetes/ceph/operator.yaml b/cluster/examples/kubernetes/ceph/operator.yaml index 3393c2237..0a3d70594 100644 --- a/cluster/examples/kubernetes/ceph/operator.yaml +++ b/cluster/examples/kubernetes/ceph/operator.yaml @@ -329,18 +329,6 @@ data: # Enable volume replication controller CSI_ENABLE_VOLUME_REPLICATION: "false" # CSI_VOLUME_REPLICATION_IMAGE: "quay.io/csiaddons/volumereplication-operator:v0.1.0" - - # (Optional) Admission controller NodeAffinity. - # ADMISSION_CONTROLLER_NODE_AFFINITY: "role=storage-node; storage=rook, ceph" - # (Optional) Admission controller tolerations list. Put here list of taints you want to tolerate in YAML format. - # Admission controller would be best to start on the same nodes as other ceph daemons. - # ADMISSION_CONTROLLER_TOLERATIONS: | - # - effect: NoSchedule - # key: node-role.kubernetes.io/controlplane - # operator: Exists - # - effect: NoExecute - # key: node-role.kubernetes.io/etcd - # operator: Exists --- # OLM: BEGIN OPERATOR DEPLOYMENT apiVersion: apps/v1 @@ -371,6 +359,12 @@ spec: name: rook-config - mountPath: /etc/ceph name: default-config-dir + - mountPath: /etc/webhook + name: webhook-cert + ports: + - containerPort: 9443 + name: https-webhook + protocol: TCP env: # If the operator should only watch for cluster CRDs in the same namespace, set this to "true". # If this is not set to true, the operator will watch for cluster CRDs in all namespaces. @@ -522,4 +516,6 @@ spec: emptyDir: {} - name: default-config-dir emptyDir: {} + - name: webhook-cert + emptyDir: {} # OLM: END OPERATOR DEPLOYMENT diff --git a/cmd/rook/ceph/admission.go b/cmd/rook/ceph/admission.go deleted file mode 100644 index 8f7945e4e..000000000 --- a/cmd/rook/ceph/admission.go +++ /dev/null @@ -1,43 +0,0 @@ -/* -Copyright 2020 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 ceph - -import ( - "github.com/rook/rook/cmd/rook/rook" - operator "github.com/rook/rook/pkg/operator/ceph" - "github.com/spf13/cobra" -) - -var ( - admissionCmd = &cobra.Command{ - Use: "admission-controller", - Short: "Starts admission controller", - } -) - -func init() { - admissionCmd.Run = startAdmissionController -} - -func startAdmissionController(cmd *cobra.Command, args []string) { - rook.SetLogLevel() - rook.LogStartupInfo(admissionCmd.Flags()) - err := operator.StartAdmissionController() - if err != nil { - rook.TerminateFatal(err) - } -} diff --git a/cmd/rook/ceph/ceph.go b/cmd/rook/ceph/ceph.go index 56a18828f..705ac0b1b 100644 --- a/cmd/rook/ceph/ceph.go +++ b/cmd/rook/ceph/ceph.go @@ -58,7 +58,6 @@ func init() { Cmd.AddCommand(cleanUpCmd, operatorCmd, agentCmd, - admissionCmd, osdCmd, mgrCmd, configCmd) diff --git a/cmd/rook/ceph/operator.go b/cmd/rook/ceph/operator.go index e17db78f4..e21140469 100644 --- a/cmd/rook/ceph/operator.go +++ b/cmd/rook/ceph/operator.go @@ -84,7 +84,7 @@ func startOperator(cmd *cobra.Command, args []string) error { op := operator.New(context, volumeAttachment, rookImage, serviceAccountName) err = op.Run() if err != nil { - rook.TerminateFatal(errors.Wrap(err, "failed to run operator\n")) + rook.TerminateFatal(errors.Wrap(err, "failed to run operator")) } return nil diff --git a/pkg/operator/ceph/cluster/operator_watchers.go b/pkg/operator/ceph/cluster/operator_watchers.go index ec3ca60c7..f76765d45 100644 --- a/pkg/operator/ceph/cluster/operator_watchers.go +++ b/pkg/operator/ceph/cluster/operator_watchers.go @@ -27,6 +27,8 @@ import ( ) // StartOperatorSettingsWatch starts the operator settings watcher +// TODO: use controller runtime, so we can use a Context to cancel the watch instead of using another channel +// The cache package mentions that it'd be nice to use a Context for cancellation too func (c *ClusterController) StartOperatorSettingsWatch(stopCh chan struct{}) { operatorNamespace := os.Getenv(k8sutil.PodNamespaceEnvVar) // watch for "rook-ceph-operator-config" ConfigMap diff --git a/pkg/operator/ceph/cr_manager.go b/pkg/operator/ceph/cr_manager.go index 0c51311e2..1055de8ed 100644 --- a/pkg/operator/ceph/cr_manager.go +++ b/pkg/operator/ceph/cr_manager.go @@ -20,7 +20,6 @@ import ( "context" "github.com/pkg/errors" - cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/apis/rook.io/v1alpha2" "github.com/rook/rook/pkg/operator/ceph/cluster" "github.com/rook/rook/pkg/operator/ceph/disruption/controllerconfig" @@ -28,9 +27,15 @@ import ( mapiv1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1" healthchecking "github.com/openshift/machine-api-operator/pkg/apis/healthchecking/v1alpha1" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" clientgoscheme "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client/config" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +const ( + certDir = "/etc/webhook" ) var ( @@ -43,7 +48,11 @@ var ( } ) -func (o *Operator) startManager(namespaceToWatch string, context context.Context, mgrErrorCh chan error) { +var ( + webhookResources = []webhook.Validator{&cephv1.CephCluster{}, &cephv1.CephBlockPool{}, &cephv1.CephObjectStore{}} +) + +func (o *Operator) startManager(context context.Context, namespaceToWatch string, mgrErrorCh chan error) { logger.Info("setting up schemes") // Setup Scheme for all resources scheme := runtime.NewScheme() @@ -60,16 +69,11 @@ func (o *Operator) startManager(namespaceToWatch string, context context.Context LeaderElection: false, Namespace: namespaceToWatch, Scheme: scheme, + CertDir: certDir, } logger.Info("setting up the controller-runtime manager") - kubeConfig, err := config.GetConfig() - if err != nil { - mgrErrorCh <- errors.Wrap(err, "failed to get client config for controller-runtime manager") - return - } - - mgr, err := manager.New(kubeConfig, mgrOpts) + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOpts) if err != nil { mgrErrorCh <- errors.Wrap(err, "failed to set up overall controller-runtime manager") return @@ -77,11 +81,12 @@ func (o *Operator) startManager(namespaceToWatch string, context context.Context // options to pass to the controllers controllerOpts := &controllerconfig.Context{ - RookImage: o.rookImage, + RookImage: o.config.Image, ClusterdContext: o.context, - OperatorNamespace: o.operatorNamespace, + OperatorNamespace: o.config.OperatorNamespace, ReconcileCanaries: &controllerconfig.LockingBool{}, } + // Add the registered controllers to the manager (entrypoint for controllers) err = cluster.AddToManager(mgr, controllerOpts, o.clusterController) if err != nil { @@ -89,6 +94,29 @@ func (o *Operator) startManager(namespaceToWatch string, context context.Context return } + // Add Webhooks + // TODO: this needs a callback with a watcher for this secret so we can quickly enable the webhook if it changes + isPresent, err := isSecretPresent(context, o.context) + if err != nil { + mgrErrorCh <- errors.Wrap(err, "failed to retrieve admission webhook secret") + return + } + if isPresent { + err := createWebhookService(o.context) + if err != nil { + mgrErrorCh <- errors.Wrap(err, "failed to create admission webhook service") + return + } + logger.Info("setting up admission webhooks") + for _, resource := range webhookResources { + err = ctrl.NewWebhookManagedBy(mgr).For(resource).Complete() + if err != nil { + mgrErrorCh <- errors.Wrapf(err, "failed to register webhook for %q", resource.GetObjectKind().GroupVersionKind().Kind) + return + } + } + } + logger.Info("starting the controller-runtime manager") if err := mgr.Start(context); err != nil { mgrErrorCh <- errors.Wrap(err, "failed to run the controller-runtime manager") diff --git a/pkg/operator/ceph/operator.go b/pkg/operator/ceph/operator.go index 334afcdf2..06603fe6b 100644 --- a/pkg/operator/ceph/operator.go +++ b/pkg/operator/ceph/operator.go @@ -68,28 +68,33 @@ var ( // Operator type for managing storage type Operator struct { - context *clusterd.Context - resources []k8sutil.CustomResource - operatorNamespace string - rookImage string - securityAccount string + context *clusterd.Context + resources []k8sutil.CustomResource + config *Config // The custom resource that is global to the kubernetes cluster. // The cluster is global because you create multiple clusters in k8s clusterController *cluster.ClusterController delayedDaemonsStarted bool } +type Config struct { + OperatorNamespace string + Image string + ServiceAccount string +} + // New creates an operator instance -func New(context *clusterd.Context, volumeAttachmentWrapper attachment.Attachment, rookImage, securityAccount string) *Operator { +func New(context *clusterd.Context, volumeAttachmentWrapper attachment.Attachment, rookImage, serviceAccount string) *Operator { schemes := []k8sutil.CustomResource{opcontroller.ClusterResource, attachment.VolumeResource} - operatorNamespace := os.Getenv(k8sutil.PodNamespaceEnvVar) o := &Operator{ - context: context, - resources: schemes, - operatorNamespace: operatorNamespace, - rookImage: rookImage, - securityAccount: securityAccount, + context: context, + resources: schemes, + config: &Config{ + OperatorNamespace: os.Getenv(k8sutil.PodNamespaceEnvVar), + Image: rookImage, + ServiceAccount: serviceAccount, + }, } operatorConfigCallbacks := []func() error{ o.updateDrivers, @@ -126,10 +131,11 @@ func (o *Operator) updateOperatorLogLevel() error { // Run the operator instance func (o *Operator) Run() error { - if o.operatorNamespace == "" { + if o.config.OperatorNamespace == "" { return errors.Errorf("rook operator namespace is not provided. expose it via downward API in the rook operator manifest file using environment variable %q", k8sutil.PodNamespaceEnvVar) } + // TODO: needs a callback? opcontroller.SetCephCommandsTimeout(o.context) // Initialize signal handler and context @@ -138,20 +144,15 @@ func (o *Operator) Run() error { rookDiscover := discover.New(o.context.Clientset) if opcontroller.DiscoveryDaemonEnabled(o.context) { - if err := rookDiscover.Start(o.operatorNamespace, o.rookImage, o.securityAccount, true); err != nil { + if err := rookDiscover.Start(o.config.OperatorNamespace, o.config.Image, o.config.ServiceAccount, true); err != nil { return errors.Wrap(err, "failed to start device discovery daemonset") } } else { - if err := rookDiscover.Stop(stopContext, o.operatorNamespace); err != nil { + if err := rookDiscover.Stop(stopContext, o.config.OperatorNamespace); err != nil { return errors.Wrap(err, "failed to stop device discovery daemonset") } } - logger.Debug("checking for admission controller secrets") - err := StartControllerIfSecretPresent(stopContext, o.context, o.rookImage) - if err != nil { - return errors.Wrap(err, "failed to start webhook") - } serverVersion, err := o.context.Clientset.Discovery().ServerVersion() if err != nil { return errors.Wrap(err, "failed to get server version") @@ -178,7 +179,7 @@ func (o *Operator) Run() error { var namespaceToWatch string if os.Getenv("ROOK_CURRENT_NAMESPACE_ONLY") == "true" { logger.Infof("watching the current namespace for a ceph cluster CR") - namespaceToWatch = o.operatorNamespace + namespaceToWatch = o.config.OperatorNamespace } else { logger.Infof("watching all namespaces for ceph cluster CRs") namespaceToWatch = v1.NamespaceAll @@ -186,7 +187,7 @@ func (o *Operator) Run() error { // Start the controller-runtime Manager. mgrErrorChan := make(chan error) - go o.startManager(namespaceToWatch, stopContext, mgrErrorChan) + go o.startManager(stopContext, namespaceToWatch, mgrErrorChan) // Start the operator setting watcher go o.clusterController.StartOperatorSettingsWatch(stopChan) @@ -198,8 +199,8 @@ func (o *Operator) Run() error { logger.Infof("shutdown signal received, exiting... %v", stopContext.Err()) o.cleanup(stopChan) return nil + case err := <-mgrErrorChan: - logger.Errorf("gave up to run the operator. %v", err) o.cleanup(stopChan) return err } @@ -228,13 +229,13 @@ func (o *Operator) updateDrivers() error { return nil } - if o.operatorNamespace == "" { + if o.config.OperatorNamespace == "" { return errors.Errorf("rook operator namespace is not provided. expose it via downward API in the rook operator manifest file using environment variable %s", k8sutil.PodNamespaceEnvVar) } if opcontroller.FlexDriverEnabled(o.context) { rookAgent := agent.New(o.context.Clientset) - if err := rookAgent.Start(o.operatorNamespace, o.rookImage, o.securityAccount); err != nil { + if err := rookAgent.Start(o.config.OperatorNamespace, o.config.Image, o.config.ServiceAccount); err != nil { return errors.Wrap(err, "error starting agent daemonset") } } @@ -252,7 +253,7 @@ func (o *Operator) updateDrivers() error { return nil } - ownerRef, err := getDeploymentOwnerReference(o.context.Clientset, o.operatorNamespace) + ownerRef, err := getDeploymentOwnerReference(o.context.Clientset, o.config.OperatorNamespace) if err != nil { logger.Warningf("could not find deployment owner reference to assign to csi drivers. %v", err) } @@ -261,15 +262,15 @@ func (o *Operator) updateDrivers() error { ownerRef.BlockOwnerDeletion = &blockOwnerDeletion } - ownerInfo := k8sutil.NewOwnerInfoWithOwnerRef(ownerRef, o.operatorNamespace) + ownerInfo := k8sutil.NewOwnerInfoWithOwnerRef(ownerRef, o.config.OperatorNamespace) // create an empty config map. config map will be filled with data // later when clusters have mons - err = csi.CreateCsiConfigMap(o.operatorNamespace, o.context.Clientset, ownerInfo) + err = csi.CreateCsiConfigMap(o.config.OperatorNamespace, o.context.Clientset, ownerInfo) if err != nil { return errors.Wrap(err, "failed creating csi config map") } - go csi.ValidateAndConfigureDrivers(o.context, o.operatorNamespace, o.rookImage, o.securityAccount, serverVersion, ownerInfo) + go csi.ValidateAndConfigureDrivers(o.context, o.config.OperatorNamespace, o.config.Image, o.config.ServiceAccount, serverVersion, ownerInfo) return nil } diff --git a/pkg/operator/ceph/server.go b/pkg/operator/ceph/server.go deleted file mode 100644 index 60ed2df63..000000000 --- a/pkg/operator/ceph/server.go +++ /dev/null @@ -1,68 +0,0 @@ -/* -Copyright 2020 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 operator - -import ( - "github.com/pkg/errors" - cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" - "k8s.io/apimachinery/pkg/runtime" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" -) - -var ( - scheme = runtime.NewScheme() - resources = []webhook.Validator{&cephv1.CephCluster{}, &cephv1.CephBlockPool{}, &cephv1.CephObjectStore{}} -) - -const ( - // Default directory where TLS certs are stored - certDir = "/etc/webhook" - // Default port for server - port = 8079 -) - -// StartAdmissionController will start the server -func StartAdmissionController() error { - logger.Infof("starting the webhook for backend ceph") - err := cephv1.AddToScheme(scheme) - if err != nil { - return errors.Wrap(err, "failed to add to scheme") - } - opts := ctrl.Options{ - Scheme: scheme, - Port: port, - CertDir: certDir, - } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), opts) - if err != nil { - return errors.Wrap(err, "failed to create manager") - } - for _, resource := range resources { - err = ctrl.NewWebhookManagedBy(mgr).For(resource).Complete() - if err != nil { - return errors.Wrap(err, "failed to register webhooks") - } - } - logger.Info("starting webhook server") - err = mgr.Start(ctrl.SetupSignalHandler()) - if err != nil { - return errors.Wrap(err, "failed to start server") - } - - return nil -} diff --git a/pkg/operator/ceph/webhook.go b/pkg/operator/ceph/webhook.go index 89f1b54fc..d67577a77 100644 --- a/pkg/operator/ceph/webhook.go +++ b/pkg/operator/ceph/webhook.go @@ -18,31 +18,23 @@ package operator import ( "context" + "fmt" + "io/ioutil" "os" "github.com/pkg/errors" "github.com/rook/rook/pkg/clusterd" - "github.com/rook/rook/pkg/operator/ceph/controller" - "github.com/rook/rook/pkg/operator/ceph/csi" "github.com/rook/rook/pkg/operator/k8sutil" - v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/kubernetes" + "sigs.k8s.io/controller-runtime/pkg/webhook" ) const ( - appName = "rook-ceph-admission-controller" - secretVolumeName = "webhook-certificates" // #nosec G101 This is just a var name, not a real secret - serviceAccountName = "rook-ceph-admission-controller" - portName = "webhook-api" - servicePort int32 = 443 - serverPort int32 = 8079 - tlsDir = "/etc/webhook" - admissionControllerTolerationsEnv = "ADMISSION_CONTROLLER_TOLERATIONS" - admissionControllerNodeAffinityEnv = "ADMISSION_CONTROLLER_NODE_AFFINITY" + appName = "rook-ceph-admission-controller" + tlsPort int32 = 443 ) var ( @@ -50,16 +42,30 @@ var ( ) func isSecretPresent(ctx context.Context, context *clusterd.Context) (bool, error) { - logger.Infof("looking for secret %q", appName) - _, err := context.Clientset.CoreV1().Secrets(namespace).Get(ctx, appName, metav1.GetOptions{}) + logger.Infof("looking for admission webhook secret %q", appName) + s, err := context.Clientset.CoreV1().Secrets(namespace).Get(ctx, appName, metav1.GetOptions{}) if err != nil { // If secret is not found. All good ! Proceed with rook without admission controllers if apierrors.IsNotFound(err) { - logger.Infof("secret %q not found. proceeding without the admission controller", appName) + logger.Infof("admission webhook secret %q not found. proceeding without the admission controller", appName) return false, nil } return false, err } + + // Search for any previous admission controller deployment and if so removing it + logger.Debug("searching for old admission controller deployment") + removeOldAdmissionControllerDeployment(ctx, context) + + logger.Infof("admission webhook secret %q found", appName) + for k, data := range s.Data { + path := fmt.Sprintf("%s/%s", certDir, k) + err := ioutil.WriteFile(path, data, 0400) + if err != nil { + return false, errors.Wrapf(err, "failed to write secret content to file %q", path) + } + } + return true, nil } @@ -72,189 +78,44 @@ func createWebhookService(context *clusterd.Context) error { Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{ { - Port: servicePort, + Port: tlsPort, TargetPort: intstr.IntOrString{ - IntVal: serverPort, + IntVal: int32(webhook.DefaultPort), }, }, }, - Selector: map[string]string{ - k8sutil.AppAttr: appName, + k8sutil.AppAttr: "rook-ceph-operator", }, }, } _, err := k8sutil.CreateOrUpdateService(context.Clientset, namespace, &webhookService) - if err != nil && !apierrors.IsAlreadyExists(err) { + if err != nil { return err } + return nil } -// StartControllerIfSecretPresent will initialize the webhook if secret is detected -func StartControllerIfSecretPresent(ctx context.Context, context *clusterd.Context, admissionImage string) error { - isPresent, err := isSecretPresent(ctx, context) +func removeOldAdmissionControllerDeployment(ctx context.Context, context *clusterd.Context) { + opts := metav1.ListOptions{LabelSelector: fmt.Sprintf("app=%s", appName)} + d, err := context.Clientset.AppsV1().Deployments(namespace).List(ctx, opts) if err != nil { - return errors.Wrap(err, "failed to retrieve secret") + logger.Warningf("failed to get old admission controller deployment. %v", err) + return } - if isPresent { - err = initWebhook(ctx, context, admissionImage) - if err != nil { - return errors.Wrap(err, "failed to initialize webhook") + + if len(d.Items) > 0 { + for _, deploy := range d.Items { + var gracePeriod int64 + propagation := metav1.DeletePropagationForeground + deleteOpts := metav1.DeleteOptions{GracePeriodSeconds: &gracePeriod, PropagationPolicy: &propagation} + if err = context.Clientset.AppsV1().Deployments(namespace).Delete(ctx, deploy.Name, deleteOpts); err != nil { + logger.Warningf("failed to delete admission controller deployment %q. %v", deploy.Name, err) + return + } } + logger.Info("successfully removed old admission controller deployment. please remove old service account, cluster role and bindings manually") } - return nil -} - -func initWebhook(ctx context.Context, context *clusterd.Context, admissionImage string) error { - // At this point volume should be mounted, so proceed with creating the service and validatingwebhookconfig - err := createWebhookService(context) - if err != nil { - return errors.Wrap(err, "failed to create service") - } - err = createWebhookDeployment(ctx, context, admissionImage) - if err != nil { - return errors.Wrap(err, "failed to create deployment") - } - return nil -} - -func createWebhookDeployment(ctx context.Context, context *clusterd.Context, admissionImage string) error { - logger.Info("creating admission controller pods") - admission_parameters := []string{"ceph", - "admission-controller"} - secretVolume := getSecretVolume() - secretVolumeMount := getSecretVolumeMount() - - antiAffinity := csi.GetPodAntiAffinity(k8sutil.AppAttr, appName) - admissionControllerDeployment := getDeployment(ctx, context, secretVolume, antiAffinity, admissionImage, admission_parameters, secretVolumeMount) - - _, err := k8sutil.CreateOrUpdateDeployment(context.Clientset, &admissionControllerDeployment) - if err != nil { - return errors.Wrap(err, "failed to create admission-controller deployment") - } - - return nil -} - -func getDeployment(ctx context.Context, context *clusterd.Context, secretVolume corev1.Volume, antiAffinity corev1.PodAntiAffinity, - admissionImage string, admission_parameters []string, secretVolumeMount corev1.VolumeMount) v1.Deployment { - var replicas int32 = 2 - nodes, err := context.Clientset.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) - if err == nil { - if len(nodes.Items) == 1 { - replicas = 1 - } - } else { - logger.Errorf("failed to get nodes. Defaulting the number of replicas of admission controller pods to 2. %v", err) - } - - admissionControllerDeployment := v1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: appName, - Namespace: namespace, - }, - Spec: v1.DeploymentSpec{ - Replicas: &replicas, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - k8sutil.AppAttr: appName, - }, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Name: appName, - Namespace: namespace, - Labels: map[string]string{ - k8sutil.AppAttr: appName, - }, - }, - Spec: corev1.PodSpec{ - Volumes: []corev1.Volume{ - secretVolume, - }, - Containers: []corev1.Container{ - { - Name: appName, - Image: admissionImage, - Args: admission_parameters, - Ports: []corev1.ContainerPort{ - { - Name: portName, - ContainerPort: serverPort, - }, - }, - VolumeMounts: []corev1.VolumeMount{ - secretVolumeMount, - }, - }, - }, - ServiceAccountName: serviceAccountName, - Affinity: &corev1.Affinity{ - PodAntiAffinity: &antiAffinity, - NodeAffinity: getNodeAffinity(context.Clientset), - }, - Tolerations: getTolerations(context.Clientset), - }, - }, - }, - } - return admissionControllerDeployment -} - -func getSecretVolumeMount() corev1.VolumeMount { - secretVolumeMount := corev1.VolumeMount{ - Name: secretVolumeName, - ReadOnly: true, - MountPath: tlsDir, - } - return secretVolumeMount -} - -func getSecretVolume() corev1.Volume { - secretVolume := corev1.Volume{ - Name: secretVolumeName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: appName, - }, - }, - } - return secretVolume -} - -func getTolerations(clientset kubernetes.Interface) []corev1.Toleration { - // Add toleration if any - tolerations := []corev1.Toleration{} - tolerationsRaw, err := k8sutil.GetOperatorSetting(clientset, controller.OperatorSettingConfigMapName, admissionControllerTolerationsEnv, "") - if err != nil { - logger.Warningf("toleration will be empty because failed to read the setting. %v", err) - return tolerations - } - tolerations, err = k8sutil.YamlToTolerations(tolerationsRaw) - if err != nil { - logger.Warningf("toleration will be empty because failed to parse the setting %q. %v", tolerationsRaw, err) - return tolerations - } - return tolerations -} - -func getNodeAffinity(clientset kubernetes.Interface) *corev1.NodeAffinity { - // Add NodeAffinity if any - v1NodeAffinity := &corev1.NodeAffinity{} - nodeAffinity, err := k8sutil.GetOperatorSetting(clientset, controller.OperatorSettingConfigMapName, admissionControllerNodeAffinityEnv, "") - if err != nil { - // nodeAffinity will be empty by default in case of error - logger.Warningf("node affinity will be empty because failed to read the setting. %v", err) - return v1NodeAffinity - } - if nodeAffinity != "" { - v1NodeAffinity, err = k8sutil.GenerateNodeAffinity(nodeAffinity) - if err != nil { - logger.Warningf("node affinity will be empty because failed to parse the setting %q. %v", nodeAffinity, err) - return v1NodeAffinity - } - } - return v1NodeAffinity } diff --git a/pkg/operator/ceph/webhook_test.go b/pkg/operator/ceph/webhook_test.go deleted file mode 100644 index 0b186ebf4..000000000 --- a/pkg/operator/ceph/webhook_test.go +++ /dev/null @@ -1,134 +0,0 @@ -/* -Copyright 2020 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 operator - -import ( - "context" - "os" - "testing" - - "github.com/rook/rook/pkg/operator/ceph/controller" - "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/fake" -) - -const ( - testNamespace = "test-namespace" -) - -func TestGetTolerations(t *testing.T) { - ctx := context.TODO() - clientset := fake.NewSimpleClientset() - os.Setenv("POD_NAMESPACE", testNamespace) - - // No setting results in the default value. - cm := &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: controller.OperatorSettingConfigMapName, - Namespace: testNamespace, - }, - Data: map[string]string{}, - } - _, err := clientset.CoreV1().ConfigMaps(testNamespace).Create(ctx, cm, metav1.CreateOptions{}) - assert.NoError(t, err) - tolerations := getTolerations(clientset) - expected := []v1.Toleration{} - assert.Equal(t, expected, tolerations) - - // The invalid setting results in the default value. - cm.Data = map[string]string{ - admissionControllerTolerationsEnv: "", - } - _, err = clientset.CoreV1().ConfigMaps(testNamespace).Update(ctx, cm, metav1.UpdateOptions{}) - assert.NoError(t, err) - tolerations = getTolerations(clientset) - assert.Equal(t, expected, tolerations) - - // Correct setting result in the desired value. - cm.Data = map[string]string{ - admissionControllerTolerationsEnv: ` -- effect: NoSchedule - key: node-role.kubernetes.io/controlplane - operator: Exists`, - } - _, err = clientset.CoreV1().ConfigMaps(testNamespace).Update(ctx, cm, metav1.UpdateOptions{}) - assert.NoError(t, err) - tolerations = getTolerations(clientset) - expected = []v1.Toleration{ - { - Effect: "NoSchedule", - Key: "node-role.kubernetes.io/controlplane", - Operator: v1.TolerationOpExists, - }, - } - assert.Equal(t, expected, tolerations) -} - -func TestGetNodeAffinity(t *testing.T) { - ctx := context.TODO() - clientset := fake.NewSimpleClientset() - os.Setenv("POD_NAMESPACE", testNamespace) - - // No setting results in the default value. - cm := &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: controller.OperatorSettingConfigMapName, - Namespace: testNamespace, - }, - Data: map[string]string{}, - } - _, err := clientset.CoreV1().ConfigMaps(testNamespace).Create(ctx, cm, metav1.CreateOptions{}) - assert.NoError(t, err) - nodeAffinity := getNodeAffinity(clientset) - expected := &v1.NodeAffinity{} - assert.Equal(t, expected, nodeAffinity) - - // The invalid setting results in the default value. - cm.Data = map[string]string{ - admissionControllerNodeAffinityEnv: "", - } - _, err = clientset.CoreV1().ConfigMaps(testNamespace).Update(ctx, cm, metav1.UpdateOptions{}) - assert.NoError(t, err) - nodeAffinity = getNodeAffinity(clientset) - assert.Equal(t, expected, nodeAffinity) - - // Correct setting result in the desired value. - cm.Data = map[string]string{ - admissionControllerNodeAffinityEnv: "role=storage-node", - } - _, err = clientset.CoreV1().ConfigMaps(testNamespace).Update(ctx, cm, metav1.UpdateOptions{}) - assert.NoError(t, err) - nodeAffinity = getNodeAffinity(clientset) - expected = &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: "role", - Operator: v1.NodeSelectorOpIn, - Values: []string{"storage-node"}, - }, - }, - }, - }, - }, - } - assert.Equal(t, expected, nodeAffinity) -} diff --git a/tests/framework/installer/ceph_installer.go b/tests/framework/installer/ceph_installer.go index 9346c3712..ec6d60a51 100644 --- a/tests/framework/installer/ceph_installer.go +++ b/tests/framework/installer/ceph_installer.go @@ -466,14 +466,6 @@ func (h *CephInstaller) installRookOperator() (bool, error) { return false, err } - // disable admission controller test for Kubernetes version older than v1.16.0 - if h.settings.EnableAdmissionController && !utils.IsPlatformOpenShift() && h.k8shelper.VersionAtLeast("v1.16.0") { - if !h.k8shelper.IsPodInExpectedState("rook-ceph-admission-controller", h.settings.OperatorNamespace, "Running") { - assert.Fail(h.T(), "admission controller is not running") - return false, errors.Errorf("admission controller is not running") - } - } - discovery, err := h.k8shelper.Clientset.AppsV1().DaemonSets(h.settings.OperatorNamespace).Get(ctx, "rook-discover", metav1.GetOptions{}) if startDiscovery { assert.NoError(h.T(), err)