ceph: move the admission webhook to the operator

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 <seb@redhat.com>

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"]

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:

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 }}

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" . }}
----
+---

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

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

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

cmd/rook/ceph/admission.go

@@ -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)
-	}
-}

cmd/rook/ceph/ceph.go

@@ -58,7 +58,6 @@ func init() {
 	Cmd.AddCommand(cleanUpCmd,
 		operatorCmd,
 		agentCmd,
-		admissionCmd,
 		osdCmd,
 		mgrCmd,
 		configCmd)

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

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

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")

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
 }
 

pkg/operator/ceph/server.go

@@ -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
-}

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
 }

pkg/operator/ceph/webhook_test.go

@@ -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)
-}

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)