diff --git a/assets/webhook_deployment.yaml b/assets/webhook_deployment.yaml new file mode 100644 index 000000000..5bfdb925d --- /dev/null +++ b/assets/webhook_deployment.yaml @@ -0,0 +1,55 @@ +kind: Deployment +apiVersion: apps/v1 +metadata: + name: csi-snapshot-webhook + namespace: openshift-cluster-storage-operator +spec: + serviceName: "csi-snapshot-webhook" + replicas: 1 + selector: + matchLabels: + app: csi-snapshot-webhook + template: + metadata: + labels: + app: csi-snapshot-webhook + spec: + containers: + - name: webhook + image: ${WEBHOOK_IMAGE} + args: + - --tls-cert-file=/etc/snapshot-validation-webhook/certs/tls.crt + - --tls-private-key-file=/etc/snapshot-validation-webhook/certs/tls.key + - "--v=${LOG_LEVEL}" + - --port=8443 + ports: + - containerPort: 8443 + volumeMounts: + - name: certs + mountPath: /etc/snapshot-validation-webhook/certs + readOnly: true + imagePullPolicy: IfNotPresent + resources: + requests: + cpu: 10m + priorityClassName: "system-cluster-critical" + restartPolicy: Always + nodeSelector: + node-role.kubernetes.io/master: "" + volumes: + - name: certs + secret: + secretName: csi-snapshot-webhook-secret + tolerations: + - key: "node.kubernetes.io/unreachable" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 120 + - key: "node.kubernetes.io/not-ready" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 120 + - key: node-role.kubernetes.io/master + operator: Exists + effect: "NoSchedule" + diff --git a/manifests/07_deployment.yaml b/manifests/07_deployment.yaml index 0d2b83faf..9cb0d3dcf 100644 --- a/manifests/07_deployment.yaml +++ b/manifests/07_deployment.yaml @@ -33,6 +33,9 @@ spec: env: - name: OPERAND_IMAGE value: quay.io/openshift/origin-csi-snapshot-controller + - name: WEBHOOK_IMAGE + # TODO: replace with quay.io image + value: registry.svc.ci.openshift.org/ocp/4.7:csi-snapshot-validation-webhook - name: OPERATOR_IMAGE_VERSION value: "0.0.1-snapshot" - name: OPERAND_IMAGE_VERSION diff --git a/manifests/08_webhook_service.yaml b/manifests/08_webhook_service.yaml new file mode 100644 index 000000000..57f4a2f7d --- /dev/null +++ b/manifests/08_webhook_service.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: Service +metadata: + name: csi-snapshot-webhook + namespace: openshift-cluster-storage-operator + labels: + app: csi-snapshot-webhook + annotations: + service.beta.openshift.io/serving-cert-secret-name: csi-snapshot-webhook-secret + include.release.openshift.io/self-managed-high-availability: "true" +spec: + ports: + - name: webhook + port: 443 + targetPort: 8443 + selector: + app: csi-snapshot-webhook diff --git a/manifests/09_webhook_config.yaml b/manifests/09_webhook_config.yaml new file mode 100644 index 000000000..d1124d698 --- /dev/null +++ b/manifests/09_webhook_config.yaml @@ -0,0 +1,24 @@ +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: ValidatingWebhookConfiguration +metadata: + name: snapshot.storage.k8s.io + namespace: csi-snapshot-controller-operator + labels: + app: csi-snapshot-webhook + annotations: + service.beta.openshift.io/inject-cabundle: "true" + include.release.openshift.io/self-managed-high-availability: "true" +webhooks: + - name: volumesnapshotclasses.snapshot.storage.k8s.io + clientConfig: + service: + name: csi-snapshot-webhook + namespace: openshift-cluster-storage-operator + path: "/volumesnapshot" + rules: + - operations: [ "CREATE", "UPDATE" ] + apiGroups: ["snapshot.storage.k8s.io"] + apiVersions: ["v1", "v1beta1"] + resources: ["volumesnapshots", "volumesnapshotcontents"] + sideEffects: None + failurePolicy: Ignore diff --git a/manifests/08_clusteroperator.yaml b/manifests/10_clusteroperator.yaml similarity index 100% rename from manifests/08_clusteroperator.yaml rename to manifests/10_clusteroperator.yaml diff --git a/manifests/image-references b/manifests/image-references index 42749b32d..503b037e0 100644 --- a/manifests/image-references +++ b/manifests/image-references @@ -10,3 +10,7 @@ spec: from: kind: DockerImage name: quay.io/openshift/origin-csi-snapshot-controller + - name: csi-snapshot-validation-webhook + from: + kind: DockerImage + name: registry.svc.ci.openshift.org/ocp/4.7:csi-snapshot-validation-webhook diff --git a/pkg/generated/bindata.go b/pkg/generated/bindata.go index b083f287f..be5ee8c39 100644 --- a/pkg/generated/bindata.go +++ b/pkg/generated/bindata.go @@ -4,6 +4,7 @@ // assets/volumesnapshotclasses.yaml // assets/volumesnapshotcontents.yaml // assets/volumesnapshots.yaml +// assets/webhook_deployment.yaml package generated import ( @@ -670,6 +671,78 @@ func volumesnapshotsYaml() (*asset, error) { return a, nil } +var _webhook_deploymentYaml = []byte(`kind: Deployment +apiVersion: apps/v1 +metadata: + name: csi-snapshot-webhook + namespace: openshift-cluster-storage-operator +spec: + serviceName: "csi-snapshot-webhook" + replicas: 1 + selector: + matchLabels: + app: csi-snapshot-webhook + template: + metadata: + labels: + app: csi-snapshot-webhook + spec: + containers: + - name: webhook + image: ${WEBHOOK_IMAGE} + args: + - --tls-cert-file=/etc/snapshot-validation-webhook/certs/tls.crt + - --tls-private-key-file=/etc/snapshot-validation-webhook/certs/tls.key + - "--v=${LOG_LEVEL}" + - --port=8443 + ports: + - containerPort: 8443 + volumeMounts: + - name: certs + mountPath: /etc/snapshot-validation-webhook/certs + readOnly: true + imagePullPolicy: IfNotPresent + resources: + requests: + cpu: 10m + priorityClassName: "system-cluster-critical" + restartPolicy: Always + nodeSelector: + node-role.kubernetes.io/master: "" + volumes: + - name: certs + secret: + secretName: csi-snapshot-webhook-secret + tolerations: + - key: "node.kubernetes.io/unreachable" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 120 + - key: "node.kubernetes.io/not-ready" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 120 + - key: node-role.kubernetes.io/master + operator: Exists + effect: "NoSchedule" + +`) + +func webhook_deploymentYamlBytes() ([]byte, error) { + return _webhook_deploymentYaml, nil +} + +func webhook_deploymentYaml() (*asset, error) { + bytes, err := webhook_deploymentYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "webhook_deployment.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + // Asset loads and returns the asset for the given name. // It returns an error if the asset could not be found or // could not be loaded. @@ -726,6 +799,7 @@ var _bindata = map[string]func() (*asset, error){ "volumesnapshotclasses.yaml": volumesnapshotclassesYaml, "volumesnapshotcontents.yaml": volumesnapshotcontentsYaml, "volumesnapshots.yaml": volumesnapshotsYaml, + "webhook_deployment.yaml": webhook_deploymentYaml, } // AssetDir returns the file names below a certain @@ -773,6 +847,7 @@ var _bintree = &bintree{nil, map[string]*bintree{ "volumesnapshotclasses.yaml": {volumesnapshotclassesYaml, map[string]*bintree{}}, "volumesnapshotcontents.yaml": {volumesnapshotcontentsYaml, map[string]*bintree{}}, "volumesnapshots.yaml": {volumesnapshotsYaml, map[string]*bintree{}}, + "webhook_deployment.yaml": {webhook_deploymentYaml, map[string]*bintree{}}, }} // RestoreAsset restores an asset under the given directory diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 61551d96d..ea1da7084 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -6,6 +6,7 @@ import ( "time" operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/operatorclient" corev1 "k8s.io/api/core/v1" apiextclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" apiextinformersv1 "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensions/v1" @@ -34,11 +35,11 @@ const ( targetName = "csi-snapshot-controller" targetNamespace = "openshift-cluster-storage-operator" operatorNamespace = "openshift-cluster-storage-operator" - globalConfigName = "cluster" operatorVersionEnvName = "OPERATOR_IMAGE_VERSION" operandVersionEnvName = "OPERAND_IMAGE_VERSION" operandImageEnvName = "OPERAND_IMAGE" + webhookImageEnvName = "WEBHOOK_IMAGE" maxRetries = 15 ) @@ -49,7 +50,7 @@ var ( ) type csiSnapshotOperator struct { - client OperatorClient + client operatorclient.OperatorClient kubeClient kubernetes.Interface versionGetter status.VersionGetter eventRecorder events.Recorder @@ -70,7 +71,7 @@ type csiSnapshotOperator struct { } func NewCSISnapshotControllerOperator( - client OperatorClient, + client operatorclient.OperatorClient, crdInformer apiextinformersv1.CustomResourceDefinitionInformer, crdClient apiextclient.Interface, deployInformer appsinformersv1.DeploymentInformer, @@ -233,7 +234,7 @@ func (c *csiSnapshotOperator) enqueue(obj interface{}) { } // Sync corresponding CSISnapshotController instance. Since there is only one, sync that one. // It will check all other objects (CRDs, Deployment) and update/overwrite them as needed. - c.queue.Add(globalConfigName) + c.queue.Add(operatorclient.GlobalConfigName) } func (c *csiSnapshotOperator) eventHandler(kind string) cache.ResourceEventHandler { diff --git a/pkg/operator/operator_test.go b/pkg/operator/operator_test.go index f94a8a6b4..892d32a78 100644 --- a/pkg/operator/operator_test.go +++ b/pkg/operator/operator_test.go @@ -11,6 +11,7 @@ import ( fakeop "github.com/openshift/client-go/operator/clientset/versioned/fake" opinformers "github.com/openshift/client-go/operator/informers/externalversions" "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/generated" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/operatorclient" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourceapply" "github.com/openshift/library-go/pkg/operator/resource/resourceread" @@ -113,7 +114,7 @@ func newOperator(test operatorTest) *testContext { // Add global reactors addGenerationReactor(coreClient) - client := OperatorClient{ + client := operatorclient.OperatorClient{ Client: operatorClient.OperatorV1(), Informers: operatorInformerFactory, } @@ -733,9 +734,9 @@ func TestSync(t *testing.T) { } // Check expectedObjects.csiSnapshotController if test.expectedObjects.csiSnapshotController != nil { - actualCSISnapshotController, err := ctx.operatorClient.OperatorV1().CSISnapshotControllers().Get(context.TODO(), globalConfigName, metav1.GetOptions{}) + actualCSISnapshotController, err := ctx.operatorClient.OperatorV1().CSISnapshotControllers().Get(context.TODO(), operatorclient.GlobalConfigName, metav1.GetOptions{}) if err != nil { - t.Errorf("Failed to get CSISnapshotController %s: %v", globalConfigName, err) + t.Errorf("Failed to get CSISnapshotController %s: %v", operatorclient.GlobalConfigName, err) } sanitizeCSISnapshotController(actualCSISnapshotController) sanitizeCSISnapshotController(test.expectedObjects.csiSnapshotController) diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index f48ab2a4b..81073ef5f 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -13,6 +13,8 @@ import ( csisnapshotconfigclient "github.com/openshift/client-go/operator/clientset/versioned" informer "github.com/openshift/client-go/operator/informers/externalversions" "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/common" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/operator/webhookdeployment" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/operatorclient" "github.com/openshift/library-go/pkg/controller/controllercmd" "github.com/openshift/library-go/pkg/operator/loglevel" "github.com/openshift/library-go/pkg/operator/management" @@ -40,7 +42,7 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller } csiConfigInformers := informer.NewSharedInformerFactoryWithOptions(csiConfigClient, resync, - informer.WithTweakListOptions(singleNameListOptions(globalConfigName)), + informer.WithTweakListOptions(singleNameListOptions(operatorclient.GlobalConfigName)), ) configClient, err := configclient.NewForConfig(controllerConfig.KubeConfig) @@ -50,11 +52,17 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller configInformers := configinformer.NewSharedInformerFactoryWithOptions(configClient, resync) - operatorClient := &OperatorClient{ - csiConfigInformers, - csiConfigClient.OperatorV1(), + operatorClient := &operatorclient.OperatorClient{ + Informers: csiConfigInformers, + Client: csiConfigClient.OperatorV1(), + ExpectedConditions: []string{ + operatorv1.OperatorStatusTypeAvailable, + webhookdeployment.WebhookControllerName + operatorv1.OperatorStatusTypeAvailable, + }, } + kubeClient := ctrlctx.ClientBuilder.KubeClientOrDie(targetName) + versionGetter := status.NewVersionGetter() operator := NewCSISnapshotControllerOperator( @@ -62,7 +70,7 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller ctrlctx.APIExtInformerFactory.Apiextensions().V1().CustomResourceDefinitions(), ctrlctx.ClientBuilder.APIExtClientOrDie(targetName), ctrlctx.KubeNamespacedInformerFactory.Apps().V1().Deployments(), - ctrlctx.ClientBuilder.KubeClientOrDie(targetName), + kubeClient, versionGetter, controllerConfig.EventRecorder, os.Getenv(operatorVersionEnvName), @@ -70,12 +78,19 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller os.Getenv(operandImageEnvName), ) + webhookOperator := webhookdeployment.NewCSISnapshotWebhookController(*operatorClient, + ctrlctx.KubeNamespacedInformerFactory.Apps().V1().Deployments(), + kubeClient, + controllerConfig.EventRecorder, + os.Getenv(webhookImageEnvName), + ) + clusterOperatorStatus := status.NewClusterOperatorStatusController( targetName, []configv1.ObjectReference{ {Resource: "namespaces", Name: targetNamespace}, {Resource: "namespaces", Name: operatorNamespace}, - {Group: operatorv1.GroupName, Resource: "csisnapshotcontrollers", Name: globalConfigName}, + {Group: operatorv1.GroupName, Resource: "csisnapshotcontrollers", Name: operatorclient.GlobalConfigName}, }, configClient.ConfigV1(), configInformers.Config().V1().ClusterOperators(), @@ -108,6 +123,7 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller clusterOperatorStatus, logLevelController, managementStateController, + webhookOperator, } { go controller.Run(ctx, 1) } diff --git a/pkg/operator/webhookdeployment/webhook.go b/pkg/operator/webhookdeployment/webhook.go new file mode 100644 index 000000000..c90e656ea --- /dev/null +++ b/pkg/operator/webhookdeployment/webhook.go @@ -0,0 +1,151 @@ +package webhookdeployment + +import ( + "context" + "fmt" + "strconv" + "strings" + + operatorapi "github.com/openshift/api/operator/v1" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/generated" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/operatorclient" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/loglevel" + "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" + "github.com/openshift/library-go/pkg/operator/resource/resourceread" + "github.com/openshift/library-go/pkg/operator/v1helpers" + appsv1 "k8s.io/api/apps/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + appsinformersv1 "k8s.io/client-go/informers/apps/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/util/workqueue" +) + +type csiSnapshotWebhookController struct { + client operatorclient.OperatorClient + kubeClient kubernetes.Interface + eventRecorder events.Recorder + + queue workqueue.RateLimitingInterface + + csiSnapshotWebhookImage string +} + +const ( + WebhookControllerName = "CSISnapshotWebhookController" + webhookVersionName = "CSISnapshotWebhookDeployment" + deploymentAsset = "webhook_deployment.yaml" +) + +// NewCSISnapshotWebhookController returns a controller that creates and manages Deployment with CSI snapshot webhook. +func NewCSISnapshotWebhookController( + client operatorclient.OperatorClient, + deployInformer appsinformersv1.DeploymentInformer, + kubeClient kubernetes.Interface, + eventRecorder events.Recorder, + csiSnapshotWebhookImage string, +) factory.Controller { + c := &csiSnapshotWebhookController{ + client: client, + kubeClient: kubeClient, + eventRecorder: eventRecorder, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "csi-snapshot-controller"), + csiSnapshotWebhookImage: csiSnapshotWebhookImage, + } + + return factory.New().WithSync(c.sync).WithSyncDegradedOnError(client).WithInformers( + client.Informer(), + deployInformer.Informer(), + ).ToController(WebhookControllerName, eventRecorder.WithComponentSuffix(WebhookControllerName)) +} + +func (c *csiSnapshotWebhookController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + opSpec, opStatus, _, err := c.client.GetOperatorState() + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + if opSpec.ManagementState != operatorapi.Managed { + return nil + } + + deployment, err := c.getDeployment(opSpec) + if err != nil { + // This will set Degraded condition + return err + } + lastGeneration := resourcemerge.ExpectedDeploymentGeneration(deployment, opStatus.Generations) + deployment, _, err = resourceapply.ApplyDeployment(c.kubeClient.AppsV1(), syncCtx.Recorder(), deployment, lastGeneration) + if err != nil { + // This will set Degraded condition + return err + } + + // Compute status + // Available: at least one replica is running + deploymentAvailable := operatorapi.OperatorCondition{ + Type: WebhookControllerName + operatorapi.OperatorStatusTypeAvailable, + } + if deployment.Status.AvailableReplicas > 0 { + deploymentAvailable.Status = operatorapi.ConditionTrue + } else { + deploymentAvailable.Status = operatorapi.ConditionFalse + deploymentAvailable.Reason = "Deploying" + deploymentAvailable.Message = "Waiting for a validating webhook Deployment pod to start" + } + + // Not progressing: all replicas are at the latest version && Deployment generation matches + deploymentProgressing := operatorapi.OperatorCondition{ + Type: WebhookControllerName + operatorapi.OperatorStatusTypeProgressing, + } + if deployment.Status.ObservedGeneration != deployment.Generation { + deploymentProgressing.Status = operatorapi.ConditionTrue + deploymentProgressing.Reason = "Deploying" + msg := fmt.Sprintf("desired generation %d, current generation %d", deployment.Generation, deployment.Status.ObservedGeneration) + deploymentProgressing.Message = msg + } else { + if deployment.Spec.Replicas != nil { + if deployment.Status.UpdatedReplicas == *deployment.Spec.Replicas { + deploymentProgressing.Status = operatorapi.ConditionFalse + } else { + msg := fmt.Sprintf("%d out of %d pods running", deployment.Status.UpdatedReplicas, *deployment.Spec.Replicas) + deploymentProgressing.Status = operatorapi.ConditionTrue + deploymentProgressing.Reason = "Deploying" + deploymentProgressing.Message = msg + } + } + } + + updateGenerationFn := func(newStatus *operatorapi.OperatorStatus) error { + resourcemerge.SetDeploymentGeneration(&newStatus.Generations, deployment) + return nil + } + + _, _, err = v1helpers.UpdateStatus(c.client, + v1helpers.UpdateConditionFn(deploymentAvailable), + v1helpers.UpdateConditionFn(deploymentProgressing), + updateGenerationFn, + ) + return err +} + +func (c *csiSnapshotWebhookController) getDeployment(opSpec *operatorapi.OperatorSpec) (*appsv1.Deployment, error) { + deploymentString := string(generated.MustAsset(deploymentAsset)) + + // Replace image + deploymentString = strings.ReplaceAll(deploymentString, "${WEBHOOK_IMAGE}", c.csiSnapshotWebhookImage) + // Replace log level + if !loglevel.ValidLogLevel(opSpec.LogLevel) { + return nil, fmt.Errorf("logLevel %q is not a valid log level", opSpec.LogLevel) + } + logLevel := loglevel.LogLevelToVerbosity(opSpec.LogLevel) + deploymentString = strings.ReplaceAll(deploymentString, "${LOG_LEVEL}", strconv.Itoa(logLevel)) + + deployment := resourceread.ReadDeploymentV1OrDie([]byte(deploymentString)) + return deployment, nil + +} diff --git a/pkg/operator/webhookdeployment/webhook_test.go b/pkg/operator/webhookdeployment/webhook_test.go new file mode 100644 index 000000000..32fc8e65a --- /dev/null +++ b/pkg/operator/webhookdeployment/webhook_test.go @@ -0,0 +1,468 @@ +package webhookdeployment + +import ( + "context" + "sort" + "testing" + + "github.com/google/go-cmp/cmp" + opv1 "github.com/openshift/api/operator/v1" + fakeop "github.com/openshift/client-go/operator/clientset/versioned/fake" + opinformers "github.com/openshift/client-go/operator/informers/externalversions" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/generated" + "github.com/openshift/cluster-csi-snapshot-controller-operator/pkg/operatorclient" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + "github.com/openshift/library-go/pkg/operator/resource/resourceread" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + coreinformers "k8s.io/client-go/informers" + fakecore "k8s.io/client-go/kubernetes/fake" + core "k8s.io/client-go/testing" +) + +type operatorTest struct { + name string + image string + initialObjects testObjects + expectedObjects testObjects + expectErr bool +} + +type testContext struct { + coreClient *fakecore.Clientset + coreInformers coreinformers.SharedInformerFactory + operatorClient *fakeop.Clientset + operatorInformers opinformers.SharedInformerFactory + webhookController factory.Controller +} + +type testObjects struct { + deployment *appsv1.Deployment + csiSnapshotController *opv1.CSISnapshotController +} + +const ( + deploymentName = "csi-snapshot-webhook" + deploymentNamespace = "openshift-cluster-storage-operator" +) + +func newOperator(test operatorTest) *testContext { + // Convert to []runtime.Object + var initialDeployments []runtime.Object + if test.initialObjects.deployment != nil { + initialDeployments = []runtime.Object{test.initialObjects.deployment} + } + coreClient := fakecore.NewSimpleClientset(initialDeployments...) + coreInformerFactory := coreinformers.NewSharedInformerFactory(coreClient, 0 /*no resync */) + // Fill the informer + if test.initialObjects.deployment != nil { + coreInformerFactory.Apps().V1().Deployments().Informer().GetIndexer().Add(test.initialObjects.deployment) + } + + // Convert to []runtime.Object + var initialCSISnapshotControllers []runtime.Object + if test.initialObjects.csiSnapshotController != nil { + initialCSISnapshotControllers = []runtime.Object{test.initialObjects.csiSnapshotController} + } + operatorClient := fakeop.NewSimpleClientset(initialCSISnapshotControllers...) + operatorInformerFactory := opinformers.NewSharedInformerFactory(operatorClient, 0) + // Fill the informer + if test.initialObjects.csiSnapshotController != nil { + operatorInformerFactory.Operator().V1().CSISnapshotControllers().Informer().GetIndexer().Add(test.initialObjects.csiSnapshotController) + } + + client := operatorclient.OperatorClient{ + Client: operatorClient.OperatorV1(), + Informers: operatorInformerFactory, + } + + // Add global reactors + addGenerationReactor(coreClient) + + recorder := events.NewInMemoryRecorder("operator") + ctrl := NewCSISnapshotWebhookController( + client, + coreInformerFactory.Apps().V1().Deployments(), + coreClient, + recorder, + test.image, + ) + + return &testContext{ + webhookController: ctrl, + coreClient: coreClient, + coreInformers: coreInformerFactory, + operatorClient: operatorClient, + operatorInformers: operatorInformerFactory, + } +} + +// CSISnapshotControllers + +type csiSnapshotControllerModifier func(*opv1.CSISnapshotController) *opv1.CSISnapshotController + +func csiSnapshotController(modifiers ...csiSnapshotControllerModifier) *opv1.CSISnapshotController { + instance := &opv1.CSISnapshotController{ + TypeMeta: metav1.TypeMeta{APIVersion: opv1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{ + Name: operatorclient.GlobalConfigName, + Generation: 0, + }, + Spec: opv1.CSISnapshotControllerSpec{ + OperatorSpec: opv1.OperatorSpec{ + ManagementState: opv1.Managed, + }, + }, + Status: opv1.CSISnapshotControllerStatus{}, + } + for _, modifier := range modifiers { + instance = modifier(instance) + } + return instance +} + +func withLogLevel(logLevel opv1.LogLevel) csiSnapshotControllerModifier { + return func(i *opv1.CSISnapshotController) *opv1.CSISnapshotController { + i.Spec.LogLevel = logLevel + return i + } +} + +func withGeneration(generations ...int64) csiSnapshotControllerModifier { + return func(i *opv1.CSISnapshotController) *opv1.CSISnapshotController { + i.Generation = generations[0] + if len(generations) > 1 { + i.Status.ObservedGeneration = generations[1] + } + return i + } +} + +func withGenerations(depolymentGeneration int64) csiSnapshotControllerModifier { + + return func(i *opv1.CSISnapshotController) *opv1.CSISnapshotController { + i.Status.Generations = []opv1.GenerationStatus{ + { + Group: appsv1.GroupName, + LastGeneration: depolymentGeneration, + Name: deploymentName, + Namespace: deploymentNamespace, + Resource: "deployments", + }, + } + return i + } +} + +func withTrueConditions(conditions ...string) csiSnapshotControllerModifier { + return func(i *opv1.CSISnapshotController) *opv1.CSISnapshotController { + if i.Status.Conditions == nil { + i.Status.Conditions = []opv1.OperatorCondition{} + } + for _, c := range conditions { + i.Status.Conditions = append(i.Status.Conditions, opv1.OperatorCondition{ + Type: WebhookControllerName + c, + Status: opv1.ConditionTrue, + }) + } + return i + } +} + +func withFalseConditions(conditions ...string) csiSnapshotControllerModifier { + return func(i *opv1.CSISnapshotController) *opv1.CSISnapshotController { + if i.Status.Conditions == nil { + i.Status.Conditions = []opv1.OperatorCondition{} + } + for _, c := range conditions { + i.Status.Conditions = append(i.Status.Conditions, opv1.OperatorCondition{ + Type: WebhookControllerName + c, + Status: opv1.ConditionFalse, + }) + } + return i + } +} + +// Deployments + +type deploymentModifier func(*appsv1.Deployment) *appsv1.Deployment + +func getDeployment(args []string, image string, modifiers ...deploymentModifier) *appsv1.Deployment { + dep := resourceread.ReadDeploymentV1OrDie(generated.MustAsset(deploymentAsset)) + dep.Spec.Template.Spec.Containers[0].Args = args + dep.Spec.Template.Spec.Containers[0].Image = image + var one int32 = 1 + dep.Spec.Replicas = &one + + for _, modifier := range modifiers { + dep = modifier(dep) + } + + // Set by ApplyDeployment() + if dep.Annotations == nil { + dep.Annotations = map[string]string{} + } + resourceapply.SetSpecHashAnnotation(&dep.ObjectMeta, dep.Spec) + + return dep +} + +func withDeploymentStatus(readyReplicas, availableReplicas, updatedReplicas int32) deploymentModifier { + return func(instance *appsv1.Deployment) *appsv1.Deployment { + instance.Status.ReadyReplicas = readyReplicas + instance.Status.AvailableReplicas = availableReplicas + instance.Status.UpdatedReplicas = updatedReplicas + return instance + } +} + +func withDeploymentReplicas(replicas int32) deploymentModifier { + return func(instance *appsv1.Deployment) *appsv1.Deployment { + instance.Spec.Replicas = &replicas + return instance + } +} + +func withDeploymentGeneration(generations ...int64) deploymentModifier { + return func(instance *appsv1.Deployment) *appsv1.Deployment { + instance.Generation = generations[0] + if len(generations) > 1 { + instance.Status.ObservedGeneration = generations[1] + } + return instance + } +} + +// This reactor is always enabled and bumps Deployment generation when it gets updated. +func addGenerationReactor(client *fakecore.Clientset) { + client.PrependReactor("*", "deployments", func(action core.Action) (handled bool, ret runtime.Object, err error) { + switch a := action.(type) { + case core.CreateActionImpl: + object := a.GetObject() + deployment := object.(*appsv1.Deployment) + deployment.Generation++ + return false, deployment, nil + case core.UpdateActionImpl: + object := a.GetObject() + deployment := object.(*appsv1.Deployment) + deployment.Generation++ + return false, deployment, nil + } + return false, nil, nil + }) +} + +func TestSync(t *testing.T) { + const replica1 = 1 + const defaultImage = "csi-snahpshot-webhook-image" + var argsLevel2 = []string{"--tls-cert-file=/etc/snapshot-validation-webhook/certs/tls.crt", "--tls-private-key-file=/etc/snapshot-validation-webhook/certs/tls.key", "--v=2", "--port=8443"} + var argsLevel6 = []string{"--tls-cert-file=/etc/snapshot-validation-webhook/certs/tls.crt", "--tls-private-key-file=/etc/snapshot-validation-webhook/certs/tls.key", "--v=6", "--port=8443"} + + tests := []operatorTest{ + { + // Only CSISnapshotController exists, everything else is created + name: "initial sync", + image: defaultImage, + initialObjects: testObjects{ + csiSnapshotController: csiSnapshotController(), + }, + expectedObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, withDeploymentGeneration(1, 0)), + csiSnapshotController: csiSnapshotController( + withGenerations(1), + withTrueConditions(opv1.OperatorStatusTypeProgressing), + withFalseConditions(opv1.OperatorStatusTypeAvailable)), + }, + }, + { + // Deployment is fully deployed and its status is synced to CSISnapshotController + name: "deployment fully deployed", + image: defaultImage, + initialObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, withDeploymentGeneration(1, 1), withDeploymentStatus(replica1, replica1, replica1)), + csiSnapshotController: csiSnapshotController(withGenerations(1)), + }, + expectedObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, withDeploymentGeneration(1, 1), withDeploymentStatus(replica1, replica1, replica1)), + csiSnapshotController: csiSnapshotController( + withGenerations(1), + withTrueConditions(opv1.OperatorStatusTypeAvailable), + withFalseConditions(opv1.OperatorStatusTypeProgressing)), + }, + }, + { + // Deployment has wrong nr. of replicas, modified by user, and gets replaced by the operator. + name: "deployment modified by user", + image: defaultImage, + initialObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, + withDeploymentReplicas(2), // User changed replicas + withDeploymentGeneration(2, 1), // ... which changed Generation + withDeploymentStatus(replica1, replica1, replica1)), + csiSnapshotController: csiSnapshotController(withGenerations(1)), // the operator knows the old generation of the Deployment + }, + expectedObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, + withDeploymentReplicas(1), // The operator fixed replica count + withDeploymentGeneration(3, 1), // ... which bumps generation again + withDeploymentStatus(replica1, replica1, replica1)), + csiSnapshotController: csiSnapshotController( + withGenerations(3), // now the operator knows generation 1 + withTrueConditions(opv1.OperatorStatusTypeAvailable, opv1.OperatorStatusTypeProgressing), // Progressing due to Generation change + withFalseConditions()), + }, + }, + { + // Deployment gets degraded from some reason + name: "deployment degraded", + image: defaultImage, + initialObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, + withDeploymentGeneration(1, 1), + withDeploymentStatus(0, 0, 0)), // the Deployment has no pods + csiSnapshotController: csiSnapshotController( + withGenerations(1), + withGeneration(1, 1), + withTrueConditions(opv1.OperatorStatusTypeAvailable), + withFalseConditions(opv1.OperatorStatusTypeProgressing)), + }, + expectedObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, + withDeploymentGeneration(1, 1), + withDeploymentStatus(0, 0, 0)), // No change to the Deployment + csiSnapshotController: csiSnapshotController( + withGenerations(1), + withGeneration(1, 1), + withTrueConditions(opv1.OperatorStatusTypeProgressing), // The operator is Progressing + withFalseConditions(opv1.OperatorStatusTypeAvailable)), // The operator is not Available (no replica is running...) + }, + }, + { + // Deployment is updating pods + name: "update", + image: defaultImage, + initialObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, + withDeploymentGeneration(1, 1), + withDeploymentStatus(1 /*ready*/, 1 /*available*/, 0 /*updated*/)), // the Deployment is updating 1 pod + csiSnapshotController: csiSnapshotController( + withGenerations(1), + withGeneration(1, 1), + withTrueConditions(opv1.OperatorStatusTypeAvailable), + withFalseConditions(opv1.OperatorStatusTypeProgressing)), + }, + expectedObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, + withDeploymentGeneration(1, 1), + withDeploymentStatus(1, 1, 0)), // No change to the Deployment + csiSnapshotController: csiSnapshotController( + withGenerations(1), + withGeneration(1, 1), + withTrueConditions(opv1.OperatorStatusTypeAvailable, opv1.OperatorStatusTypeProgressing), // The operator is Progressing, but still Available + withFalseConditions()), + }, + }, + { + // User changes log level and it's projected into the Deployment + name: "log level change", + image: defaultImage, + initialObjects: testObjects{ + deployment: getDeployment(argsLevel2, defaultImage, + withDeploymentGeneration(1, 1), + withDeploymentStatus(replica1, replica1, replica1)), + csiSnapshotController: csiSnapshotController( + withGenerations(1), + withLogLevel(opv1.Trace), // User changed the log level... + withGeneration(2, 1)), //... which caused the Generation to increase + }, + expectedObjects: testObjects{ + deployment: getDeployment(argsLevel6, defaultImage, // The operator changed cmdline arguments with a new log level + withDeploymentGeneration(2, 1), // ... which caused the Generation to increase + withDeploymentStatus(replica1, replica1, replica1)), + csiSnapshotController: csiSnapshotController( + withLogLevel(opv1.Trace), + withGenerations(2), + withGeneration(2, 1), // Webhook Deployment does not update the CR, only the main controller does so. + withTrueConditions(opv1.OperatorStatusTypeAvailable, opv1.OperatorStatusTypeProgressing), // Progressing due to Generation change + withFalseConditions()), + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Initialize + ctx := newOperator(test) + + // Act + syncContext := factory.NewSyncContext("test", events.NewRecorder(ctx.coreClient.CoreV1().Events("test"), "test-operator", &corev1.ObjectReference{})) + err := ctx.webhookController.Sync(context.TODO(), syncContext) + + // Assert + // Check error + if err != nil && !test.expectErr { + t.Errorf("sync() returned unexpected error: %v", err) + } + if err == nil && test.expectErr { + t.Error("sync() unexpectedly succeeded when error was expected") + } + + // Check expectedObjects.deployment + if test.expectedObjects.deployment != nil { + actualDeployment, err := ctx.coreClient.AppsV1().Deployments(deploymentNamespace).Get(context.TODO(), deploymentName, metav1.GetOptions{}) + if err != nil { + t.Errorf("Failed to get Deployment %s: %v", deploymentName, err) + } + sanitizeDeployment(actualDeployment) + sanitizeDeployment(test.expectedObjects.deployment) + if !equality.Semantic.DeepEqual(test.expectedObjects.deployment, actualDeployment) { + t.Errorf("Unexpected Deployment %+v content:\n%s", deploymentName, cmp.Diff(test.expectedObjects.deployment, actualDeployment)) + } + } + // Check expectedObjects.csiSnapshotController + if test.expectedObjects.csiSnapshotController != nil { + actualCSISnapshotController, err := ctx.operatorClient.OperatorV1().CSISnapshotControllers().Get(context.TODO(), operatorclient.GlobalConfigName, metav1.GetOptions{}) + if err != nil { + t.Errorf("Failed to get CSISnapshotController %s: %v", operatorclient.GlobalConfigName, err) + } + sanitizeCSISnapshotController(actualCSISnapshotController) + sanitizeCSISnapshotController(test.expectedObjects.csiSnapshotController) + if !equality.Semantic.DeepEqual(test.expectedObjects.csiSnapshotController, actualCSISnapshotController) { + t.Errorf("Unexpected CSISnapshotController %+v content:\n%s", deploymentName, cmp.Diff(test.expectedObjects.csiSnapshotController, actualCSISnapshotController)) + } + } + }) + } +} + +func sanitizeDeployment(deployment *appsv1.Deployment) { + // nil and empty array are the same + if len(deployment.Labels) == 0 { + deployment.Labels = nil + } + if len(deployment.Annotations) == 0 { + deployment.Annotations = nil + } + if deployment.Annotations != nil { + deployment.Annotations["operator.openshift.io/spec-hash"] = "" + } +} + +func sanitizeCSISnapshotController(instance *opv1.CSISnapshotController) { + // Remove condition texts + for i := range instance.Status.Conditions { + instance.Status.Conditions[i].LastTransitionTime = metav1.Time{} + instance.Status.Conditions[i].Message = "" + instance.Status.Conditions[i].Reason = "" + } + // Sort the conditions by name to have consistent position in the array + sort.Slice(instance.Status.Conditions, func(i, j int) bool { + return instance.Status.Conditions[i].Type < instance.Status.Conditions[j].Type + }) +} diff --git a/pkg/operator/operatorclient.go b/pkg/operatorclient/operatorclient.go similarity index 61% rename from pkg/operator/operatorclient.go rename to pkg/operatorclient/operatorclient.go index 74a1bd089..56d57c071 100644 --- a/pkg/operator/operatorclient.go +++ b/pkg/operatorclient/operatorclient.go @@ -1,21 +1,24 @@ -package operator +package operatorclient import ( "context" - "k8s.io/client-go/tools/cache" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - operatorv1 "github.com/openshift/api/operator/v1" - operatorconfigclient "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1" operatorclientinformers "github.com/openshift/client-go/operator/informers/externalversions" + "github.com/openshift/library-go/pkg/operator/v1helpers" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" +) + +const ( + GlobalConfigName = "cluster" ) type OperatorClient struct { - Informers operatorclientinformers.SharedInformerFactory - Client operatorconfigclient.CSISnapshotControllersGetter + Informers operatorclientinformers.SharedInformerFactory + Client operatorconfigclient.CSISnapshotControllersGetter + ExpectedConditions []string } func (c OperatorClient) Informer() cache.SharedIndexInformer { @@ -23,7 +26,7 @@ func (c OperatorClient) Informer() cache.SharedIndexInformer { } func (c OperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) { - instance, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(globalConfigName) + instance, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(GlobalConfigName) if err != nil { return nil, nil, "", err } @@ -32,7 +35,7 @@ func (c OperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv } func (c OperatorClient) GetObjectMeta() (*metav1.ObjectMeta, error) { - instance, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(globalConfigName) + instance, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(GlobalConfigName) if err != nil { return nil, err } @@ -40,7 +43,7 @@ func (c OperatorClient) GetObjectMeta() (*metav1.ObjectMeta, error) { } func (c OperatorClient) UpdateOperatorSpec(resourceVersion string, spec *operatorv1.OperatorSpec) (*operatorv1.OperatorSpec, string, error) { - original, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(globalConfigName) + original, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(GlobalConfigName) if err != nil { return nil, "", err } @@ -57,10 +60,12 @@ func (c OperatorClient) UpdateOperatorSpec(resourceVersion string, spec *operato } func (c OperatorClient) UpdateOperatorStatus(resourceVersion string, status *operatorv1.OperatorStatus) (*operatorv1.OperatorStatus, error) { - original, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(globalConfigName) + original, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(GlobalConfigName) if err != nil { return nil, err } + c.addMissingConditions(status) + copy := original.DeepCopy() copy.ResourceVersion = resourceVersion copy.Status.OperatorStatus = *status @@ -73,8 +78,35 @@ func (c OperatorClient) UpdateOperatorStatus(resourceVersion string, status *ope return &ret.Status.OperatorStatus, nil } +// addMissingConditions adds all conditions that must be present to compute proper OperatorStatus CR. +// Since several controllers run in parallel, we must ensure that whichever controller runs the first sync, +// it must report conditions of the other controllers too. +func (c *OperatorClient) addMissingConditions(status *operatorv1.OperatorStatus) { + for _, cndType := range c.ExpectedConditions { + if !c.isConditionSet(status, cndType) { + cnd := operatorv1.OperatorCondition{ + Type: cndType, + Status: operatorv1.ConditionUnknown, + LastTransitionTime: metav1.Now(), + Reason: "InitalSync", + Message: "Waiting for the initial sync of the operator", + } + v1helpers.SetOperatorCondition(&status.Conditions, cnd) + } + } +} + +func (c *OperatorClient) isConditionSet(status *operatorv1.OperatorStatus, cndType string) bool { + for i := range status.Conditions { + if status.Conditions[i].Type == cndType { + return true + } + } + return false +} + func (c OperatorClient) GetOperatorInstance() (*operatorv1.CSISnapshotController, error) { - instance, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(globalConfigName) + instance, err := c.Informers.Operator().V1().CSISnapshotControllers().Lister().Get(GlobalConfigName) if err != nil { return nil, err }