From 70ec15171dab2ccd9a9f916ae2ff2b4b7190f09a Mon Sep 17 00:00:00 2001 From: Patryk Matuszak Date: Mon, 20 Jan 2025 11:54:07 +0100 Subject: [PATCH 1/2] Remove CSI webhook due to deprecation --- .../webhook_clusterrole.yaml | 11 --- .../webhook_clusterrolebinding.yaml | 12 --- .../webhook_config.yaml | 42 ---------- .../webhook_deployment.yaml | 83 ------------------- .../webhook_service.yaml | 18 ---- .../webhook_serviceaccount.yaml | 5 -- assets/release/release-aarch64.json | 3 +- assets/release/release-x86_64.json | 3 +- .../config/config-openapi-spec.json | 6 +- docs/user/howto_config.md | 4 +- .../microshift/pkg/config/storage.go | 10 +-- okd/src/README.md | 1 - .../greenboot/microshift-running-check.sh | 2 +- packaging/microshift/config.yaml | 3 +- pkg/assets/scheme.go | 4 + pkg/components/csi-snapshot-controller.go | 57 +++++++------ pkg/config/config_test.go | 3 +- pkg/config/storage.go | 10 +-- pkg/config/storage_test.go | 11 ++- scripts/auto-rebase/assets.yaml | 8 -- scripts/auto-rebase/rebase.sh | 12 --- test/suites/standard1/configuration.robot | 11 ++- 22 files changed, 59 insertions(+), 260 deletions(-) delete mode 100644 assets/components/csi-snapshot-controller/webhook_clusterrole.yaml delete mode 100644 assets/components/csi-snapshot-controller/webhook_clusterrolebinding.yaml delete mode 100644 assets/components/csi-snapshot-controller/webhook_config.yaml delete mode 100644 assets/components/csi-snapshot-controller/webhook_deployment.yaml delete mode 100644 assets/components/csi-snapshot-controller/webhook_service.yaml delete mode 100644 assets/components/csi-snapshot-controller/webhook_serviceaccount.yaml diff --git a/assets/components/csi-snapshot-controller/webhook_clusterrole.yaml b/assets/components/csi-snapshot-controller/webhook_clusterrole.yaml deleted file mode 100644 index 322dd21fa6c..00000000000 --- a/assets/components/csi-snapshot-controller/webhook_clusterrole.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: csi-snapshot-webhook-clusterrole -rules: -- apiGroups: ["snapshot.storage.k8s.io"] - resources: ["volumesnapshotclasses"] - verbs: ["get", "list", "watch"] -- apiGroups: ["groupsnapshot.storage.k8s.io"] - resources: ["volumegroupsnapshotclasses"] - verbs: ["get", "list", "watch"] diff --git a/assets/components/csi-snapshot-controller/webhook_clusterrolebinding.yaml b/assets/components/csi-snapshot-controller/webhook_clusterrolebinding.yaml deleted file mode 100644 index 007207e86b4..00000000000 --- a/assets/components/csi-snapshot-controller/webhook_clusterrolebinding.yaml +++ /dev/null @@ -1,12 +0,0 @@ -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: csi-snapshot-webhook-clusterrolebinding -subjects: - - kind: ServiceAccount - name: csi-snapshot-webhook - namespace: ${CONTROLPLANE_NAMESPACE} -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: csi-snapshot-webhook-clusterrole diff --git a/assets/components/csi-snapshot-controller/webhook_config.yaml b/assets/components/csi-snapshot-controller/webhook_config.yaml deleted file mode 100644 index 6c4f5399852..00000000000 --- a/assets/components/csi-snapshot-controller/webhook_config.yaml +++ /dev/null @@ -1,42 +0,0 @@ -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - name: snapshot.storage.k8s.io - labels: - app: csi-snapshot-webhook - annotations: - service.beta.openshift.io/inject-cabundle: "true" - include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" -webhooks: - - name: volumesnapshotclasses.snapshot.storage.k8s.io - clientConfig: - service: - name: csi-snapshot-webhook - namespace: kube-system - path: /volumesnapshot - rules: - - operations: ["CREATE", "UPDATE"] - apiGroups: ["snapshot.storage.k8s.io"] - apiVersions: ["v1beta1", "v1"] - resources: ["volumesnapshotclasses"] - admissionReviewVersions: - - v1 - - v1beta1 - sideEffects: None - failurePolicy: Ignore - - name: volumegroupsnapshotclasses.groupsnapshot.storage.k8s.io - clientConfig: - service: - name: csi-snapshot-webhook - namespace: openshift-cluster-storage-operator - path: /volumegroupsnapshot - rules: - - operations: ["CREATE", "UPDATE"] - apiGroups: ["groupsnapshot.storage.k8s.io"] - apiVersions: ["v1alpha1"] - resources: ["volumegroupsnapshotclasses"] - admissionReviewVersions: - - v1 - sideEffects: None - failurePolicy: Ignore diff --git a/assets/components/csi-snapshot-controller/webhook_deployment.yaml b/assets/components/csi-snapshot-controller/webhook_deployment.yaml deleted file mode 100644 index 1ac61c90226..00000000000 --- a/assets/components/csi-snapshot-controller/webhook_deployment.yaml +++ /dev/null @@ -1,83 +0,0 @@ -kind: Deployment -apiVersion: apps/v1 -metadata: - name: csi-snapshot-webhook - namespace: kube-system -spec: - serviceName: "csi-snapshot-webhook" - selector: - matchLabels: - app: csi-snapshot-webhook - strategy: - type: RollingUpdate - rollingUpdate: - maxUnavailable: 1 - maxSurge: 0 - template: - metadata: - annotations: - target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' - openshift.io/required-scc: restricted-v2 - labels: - app: csi-snapshot-webhook - spec: - serviceAccount: csi-snapshot-webhook - containers: - - name: webhook - image: '{{ .ReleaseImage.csi_snapshot_validation_webhook }}' - args: - - --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 - ports: - - containerPort: 8443 - volumeMounts: - - name: certs - mountPath: /etc/snapshot-validation-webhook/certs - readOnly: true - optional: true - imagePullPolicy: IfNotPresent - resources: - requests: - cpu: 10m - memory: 20Mi - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - runAsUser: 65534 - terminationMessagePolicy: FallbackToLogsOnError - affinity: - podAntiAffinity: - preferredDuringSchedulingIgnoredDuringExecution: - - weight: 100 - podAffinityTerm: - labelSelector: - matchLabels: - app: csi-snapshot-webhook - topologyKey: kubernetes.io/hostname - restartPolicy: Always - securityContext: - runAsNonRoot: true - seccompProfile: - type: RuntimeDefault - 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/assets/components/csi-snapshot-controller/webhook_service.yaml b/assets/components/csi-snapshot-controller/webhook_service.yaml deleted file mode 100644 index ddd576182d9..00000000000 --- a/assets/components/csi-snapshot-controller/webhook_service.yaml +++ /dev/null @@ -1,18 +0,0 @@ -apiVersion: v1 -kind: Service -metadata: - name: csi-snapshot-webhook - namespace: kube-system - labels: - app: csi-snapshot-webhook - hypershift.openshift.io/allow-guest-webhooks: "true" - annotations: - service.beta.openshift.io/serving-cert-secret-name: csi-snapshot-webhook-secret - capability.openshift.io/name: CSISnapshot -spec: - ports: - - name: webhook - port: 443 - targetPort: 8443 - selector: - app: csi-snapshot-webhook diff --git a/assets/components/csi-snapshot-controller/webhook_serviceaccount.yaml b/assets/components/csi-snapshot-controller/webhook_serviceaccount.yaml deleted file mode 100644 index 3d56d25c29e..00000000000 --- a/assets/components/csi-snapshot-controller/webhook_serviceaccount.yaml +++ /dev/null @@ -1,5 +0,0 @@ -apiVersion: v1 -kind: ServiceAccount -metadata: - name: csi-snapshot-webhook - namespace: kube-system diff --git a/assets/release/release-aarch64.json b/assets/release/release-aarch64.json index 8ac45b239fb..022a728758b 100644 --- a/assets/release/release-aarch64.json +++ b/assets/release/release-aarch64.json @@ -11,7 +11,6 @@ "pod": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:bcc1c450c4888d6b759636e508ee77f49592642454d4b59c908da0ae5297a31c", "service-ca-operator": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:49f294bfc07141d8d238566373651845cc5137dcf93f9de2029b448d79b26235", "lvms_operator": "registry.redhat.io/lvms4/lvms-rhel9-operator@sha256:bd6dc4d6e90fdbcdb844759e203c9c591abc5ac29a956257a90bda101a37b76e", - "csi-snapshot-controller": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:610c51e225c86c53aa1a30fd354f744e1e62a7578169f161969522fe88c27af4", - "csi-snapshot-validation-webhook": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:fac64c91ea6032efdfecf24de6ba1684549a3dcfccd20388e7bdd8a49910df15" + "csi-snapshot-controller": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:610c51e225c86c53aa1a30fd354f744e1e62a7578169f161969522fe88c27af4" } } diff --git a/assets/release/release-x86_64.json b/assets/release/release-x86_64.json index 401e0df125d..5f7b60b04cb 100644 --- a/assets/release/release-x86_64.json +++ b/assets/release/release-x86_64.json @@ -11,7 +11,6 @@ "pod": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:937e6ee08ea4bd899126c8daf2a9fa8c27ea3850257baeb278ecd8cad207fc47", "service-ca-operator": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:a870f9818c582a6dda22d591b0121bc1331c26eafc97bf007bcaaacb9c43e743", "lvms_operator": "registry.redhat.io/lvms4/lvms-rhel9-operator@sha256:bd6dc4d6e90fdbcdb844759e203c9c591abc5ac29a956257a90bda101a37b76e", - "csi-snapshot-controller": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:31d9234ca87bcb644468c00195368de69383310dad22fdb88b4ec0a55eda627a", - "csi-snapshot-validation-webhook": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:45de22ed8e8644046c024ca117aba239b9fa585c1f847a4b2be83a272cba7c3d" + "csi-snapshot-controller": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:31d9234ca87bcb644468c00195368de69383310dad22fdb88b4ec0a55eda627a" } } diff --git a/cmd/generate-config/config/config-openapi-spec.json b/cmd/generate-config/config/config-openapi-spec.json index f158b127498..4bf916cd035 100755 --- a/cmd/generate-config/config/config-openapi-spec.json +++ b/cmd/generate-config/config/config-openapi-spec.json @@ -408,7 +408,7 @@ ] }, "optionalCsiComponents": { - "description": "OptionalCSIComponents is a user defined slice of CSIComponent values. These value tell MicroShift which\nadditional, non-driver, CSI controllers to deploy on start. MicroShift will deploy snapshot controller\nand webhook when no components are specified. This preserves the current deployment behavior of existing\nclusters. Users must set `.storage.optionalCsiComponents: []` to explicitly tell MicroShift not to deploy any CSI\ncomponents. The CSI Driver is excluded as it is typically deployed via the same manifest as the accompanying\nstorage driver. Like CSIStorageDriver, uninstallation is not supported as this can lead to orphaned storage\nobjects.\nAllowed values are: unset, [], or one or more of [\"snapshot-controller\", \"snapshot-webhook\"]", + "description": "OptionalCSIComponents is a user defined slice of CSIComponent values. These value tell MicroShift which\nadditional, non-driver, CSI controllers to deploy on start. MicroShift will deploy snapshot controller\nand webhook when no components are specified. This preserves the current deployment behavior of existing\nclusters. Users must set `.storage.optionalCsiComponents: []` to explicitly tell MicroShift not to deploy any CSI\ncomponents. The CSI Driver is excluded as it is typically deployed via the same manifest as the accompanying\nstorage driver. Like CSIStorageDriver, uninstallation is not supported as this can lead to orphaned storage\nobjects.\nAllowed values are: unset, [], or one or more of [\"snapshot-controller\"]", "type": "array", "items": { "description": "OptionalCsiComponent values determine which CSI components MicroShift should deploy. Currently only csi snapshot components\nare supported.", @@ -416,13 +416,11 @@ "enum": [ "none", "snapshot-controller", - "snapshot-webhook", "" ] }, "example": [ - "snapshot-controller", - "snapshot-webhook" + "snapshot-controller" ] } } diff --git a/docs/user/howto_config.md b/docs/user/howto_config.md index dfb612097e6..f8ba35f0097 100644 --- a/docs/user/howto_config.md +++ b/docs/user/howto_config.md @@ -359,9 +359,9 @@ specifying supported values under `.storage` node of the MicroShift config in th storage optionalCsiComponents: **ARRAY**. ``` - - Expected values are: `['csi-snapshot-controller', 'csi-snapshot-webhook', 'none']`. `'none'` is mutually exclusive + - Expected values are: `['csi-snapshot-controller', 'none']`. `'none'` is mutually exclusive with all other values. - - Empty array defaults to deploying `snapshot-controller` and `snapshot-webhook`. + - Empty array defaults to deploying `snapshot-controller`. ### Automated Uninstallation is Not Supported diff --git a/etcd/vendor/github.com/openshift/microshift/pkg/config/storage.go b/etcd/vendor/github.com/openshift/microshift/pkg/config/storage.go index b53c93024a1..3408ebd7977 100644 --- a/etcd/vendor/github.com/openshift/microshift/pkg/config/storage.go +++ b/etcd/vendor/github.com/openshift/microshift/pkg/config/storage.go @@ -26,7 +26,7 @@ const ( // OptionalCsiComponent values determine which CSI components MicroShift should deploy. Currently only csi snapshot components // are supported. -// +kubebuilder:validation:Enum:=none;snapshot-controller;snapshot-webhook;"" +// +kubebuilder:validation:Enum:=none;snapshot-controller;"" type OptionalCsiComponent string const ( @@ -39,8 +39,6 @@ const ( CsiComponentNone OptionalCsiComponent = "none" // CsiComponentSnapshot causes MicroShift to deploy the CSI Snapshot controller. CsiComponentSnapshot OptionalCsiComponent = "snapshot-controller" - // CsiComponentSnapshotWebhook causes MicroShift to deploy the CSI Snapshot Validation Webhook. - CsiComponentSnapshotWebhook OptionalCsiComponent = "snapshot-webhook" // CsiComponentNullAlias is equivalent to not specifying a value. It exists because controller-gen generates // default empty-array values as [""], instead of []. Failing to include this odd value would mean the generated // /etc/microshift/config.default.yaml would break if passed to MicroShift. @@ -64,9 +62,9 @@ type Storage struct { // components. The CSI Driver is excluded as it is typically deployed via the same manifest as the accompanying // storage driver. Like CSIStorageDriver, uninstallation is not supported as this can lead to orphaned storage // objects. - // Allowed values are: unset, [], or one or more of ["snapshot-controller", "snapshot-webhook"] + // Allowed values are: unset, [], or one or more of ["snapshot-controller"] // +kubebuilder:validation:Optional - // +kubebuilder:example={"snapshot-controller", "snapshot-webhook"} + // +kubebuilder:example={"snapshot-controller"} OptionalCSIComponents []OptionalCsiComponent `json:"optionalCsiComponents,omitempty"` } @@ -75,7 +73,7 @@ func (s Storage) driverIsValid() (isSupported bool) { } func (s Storage) csiComponentsAreValid() []string { - supported := sets.New[OptionalCsiComponent](CsiComponentSnapshot, CsiComponentSnapshotWebhook, CsiComponentNone, + supported := sets.New[OptionalCsiComponent](CsiComponentSnapshot, CsiComponentNone, CsiComponentNullAlias) unsupported := sets.New[string]() diff --git a/okd/src/README.md b/okd/src/README.md index 9e7a9dc102f..732cb6466c9 100644 --- a/okd/src/README.md +++ b/okd/src/README.md @@ -38,7 +38,6 @@ > oc get pods NAMESPACE NAME READY STATUS RESTARTS AGE kube-system csi-snapshot-controller-7d6c78bc58-5p7tb 1/1 Running 0 8m52s - kube-system csi-snapshot-webhook-5598db6db4-rmrpx 1/1 Running 0 8m54s openshift-dns dns-default-2q89q 2/2 Running 0 7m34s openshift-dns node-resolver-k2c5h 1/1 Running 0 8m54s openshift-ingress router-default-db4b598b9-x8lvb 1/1 Running 0 8m52s diff --git a/packaging/greenboot/microshift-running-check.sh b/packaging/greenboot/microshift-running-check.sh index 1de5d6041ee..3981f27e224 100755 --- a/packaging/greenboot/microshift-running-check.sh +++ b/packaging/greenboot/microshift-running-check.sh @@ -145,7 +145,7 @@ if lvmsShouldBeDeployed; then PODS_NS_LIST+=(openshift-storage) PODS_CT_LIST+=(2) fi -declare -a csi_components=('csi-snapshot-controller' 'csi-snapshot-webhook') +declare -a csi_components=('csi-snapshot-controller') csi_pods_ct=0 for csi_c in "${csi_components[@]}"; do if csiComponentShouldBeDeployed "${csi_c}"; then diff --git a/packaging/microshift/config.yaml b/packaging/microshift/config.yaml index 55c82d529e6..e0f3b368038 100644 --- a/packaging/microshift/config.yaml +++ b/packaging/microshift/config.yaml @@ -369,10 +369,9 @@ storage: # components. The CSI Driver is excluded as it is typically deployed via the same manifest as the accompanying # storage driver. Like CSIStorageDriver, uninstallation is not supported as this can lead to orphaned storage # objects. - # Allowed values are: unset, [], or one or more of ["snapshot-controller", "snapshot-webhook"] + # Allowed values are: unset, [], or one or more of ["snapshot-controller"] # example: # - snapshot-controller - # - snapshot-webhook optionalCsiComponents: - "" diff --git a/pkg/assets/scheme.go b/pkg/assets/scheme.go index 3d5a2a607ee..c3e49aee625 100644 --- a/pkg/assets/scheme.go +++ b/pkg/assets/scheme.go @@ -2,6 +2,7 @@ package assets import ( sccv1 "github.com/openshift/api/security/v1" + arv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -27,4 +28,7 @@ func init() { if err := sccv1.AddToScheme(scheme); err != nil { panic(err) } + if err := arv1.AddToScheme(scheme); err != nil { + panic(err) + } } diff --git a/pkg/components/csi-snapshot-controller.go b/pkg/components/csi-snapshot-controller.go index 810e1c18318..57a9dc898bd 100644 --- a/pkg/components/csi-snapshot-controller.go +++ b/pkg/components/csi-snapshot-controller.go @@ -4,6 +4,12 @@ import ( "context" "fmt" + arv1 "k8s.io/api/admissionregistration/v1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" @@ -16,7 +22,7 @@ func startCSISnapshotController(ctx context.Context, cfg *config.Config, kubecon if len(cfg.Storage.OptionalCSIComponents) == 0 { // Upgraded clusters will not have set .storage.*, so we need to support the prior default behavior and deploy // CSI snapshots when .storage.optionalCsiComponents is nil. - csiComps.Insert(config.CsiComponentSnapshot, config.CsiComponentSnapshotWebhook) + csiComps.Insert(config.CsiComponentSnapshot) } else if csiComps.Has(config.CsiComponentNone) { // User set a zero-len slice, indicating that the cluster supports optional CSI components, and that they should // be disabled. @@ -25,34 +31,15 @@ func startCSISnapshotController(ctx context.Context, cfg *config.Config, kubecon } var ( - whSA = []string{"components/csi-snapshot-controller/webhook_serviceaccount.yaml"} - whCfg = []string{"components/csi-snapshot-controller/webhook_config.yaml"} - whDeploy = []string{"components/csi-snapshot-controller/webhook_deployment.yaml"} - whSvc = []string{"components/csi-snapshot-controller/webhook_service.yaml"} - cr = []string{"components/csi-snapshot-controller/clusterrole.yaml"} - crb = []string{"components/csi-snapshot-controller/clusterrolebinding.yaml"} - sa = []string{"components/csi-snapshot-controller/serviceaccount.yaml"} - deploy = []string{"components/csi-snapshot-controller/csi_controller_deployment.yaml"} + cr = []string{"components/csi-snapshot-controller/clusterrole.yaml"} + crb = []string{"components/csi-snapshot-controller/clusterrolebinding.yaml"} + sa = []string{"components/csi-snapshot-controller/serviceaccount.yaml"} + deploy = []string{"components/csi-snapshot-controller/csi_controller_deployment.yaml"} ) - // Deploy Webhook - //nolint:nestif - if csiComps.Has(config.CsiComponentSnapshotWebhook) { - klog.Infof("deploying CSI snapshot webhook") - if err := assets.ApplyServiceAccounts(ctx, whSA, kubeconfigPath); err != nil { - return fmt.Errorf("apply service account: %w", err) - } - if err := assets.ApplyDeployments(ctx, whDeploy, renderTemplate, renderParamsFromConfig(cfg, nil), kubeconfigPath); err != nil { - return fmt.Errorf("apply deployment: %w", err) - } - if err := assets.ApplyValidatingWebhookConfiguration(ctx, whCfg, kubeconfigPath); err != nil { - return fmt.Errorf("apply validationWebhookConfiguration: %w", err) - } - if err := assets.ApplyDeployments(ctx, deploy, renderTemplate, renderParamsFromConfig(cfg, nil), kubeconfigPath); err != nil { - return fmt.Errorf("apply deployments: %w", err) - } - } else { - klog.Warningf("CSI snapshot webhook is disabled") + if err := deleteCSIWebhook(ctx, kubeconfigPath); err != nil { + klog.Warningf("Failed to delete resources of CSI Webhook: %v", err) + return err } // Deploy CSI Controller Deployment @@ -68,11 +55,23 @@ func startCSISnapshotController(ctx context.Context, cfg *config.Config, kubecon if err := assets.ApplyServiceAccounts(ctx, sa, kubeconfigPath); err != nil { return fmt.Errorf("apply service account: %w", err) } - if err := assets.ApplyServices(ctx, whSvc, nil, map[string]interface{}{}, kubeconfigPath); err != nil { - return fmt.Errorf("apply service: %w", err) + if err := assets.ApplyDeployments(ctx, deploy, renderTemplate, renderParamsFromConfig(cfg, nil), kubeconfigPath); err != nil { + return fmt.Errorf("apply deployments: %w", err) } } else { klog.Warningf("CSI snapshot controller is disabled") } return nil } + +func deleteCSIWebhook(ctx context.Context, kubeconfigPath string) error { + klog.Infof("Deleting resources of CSI Webhook") + return assets.DeleteGeneric(ctx, []runtime.Object{ + &arv1.ValidatingWebhookConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "snapshot.storage.k8s.io"}}, + &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "csi-snapshot-webhook", Namespace: "kube-system"}}, + &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "csi-snapshot-webhook", Namespace: "kube-system"}}, + &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: "csi-snapshot-webhook-clusterrole"}}, + &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: "csi-snapshot-webhook-clusterrolebinding"}}, + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "csi-snapshot-webhook", Namespace: "kube-system"}}, + }, kubeconfigPath) +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4d36dbbed2f..e484a045762 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -377,13 +377,12 @@ func TestGetActiveConfigFromYAML(t *testing.T) { driver: "none" optionalCsiComponents: - "snapshot-controller" - - "snapshot-webhook" `), expected: func() *Config { c := mkDefaultConfig() c.Storage = Storage{ Driver: CsiDriverNone, - OptionalCSIComponents: []OptionalCsiComponent{CsiComponentSnapshot, CsiComponentSnapshotWebhook}, + OptionalCSIComponents: []OptionalCsiComponent{CsiComponentSnapshot}, } return c }(), diff --git a/pkg/config/storage.go b/pkg/config/storage.go index b53c93024a1..3408ebd7977 100644 --- a/pkg/config/storage.go +++ b/pkg/config/storage.go @@ -26,7 +26,7 @@ const ( // OptionalCsiComponent values determine which CSI components MicroShift should deploy. Currently only csi snapshot components // are supported. -// +kubebuilder:validation:Enum:=none;snapshot-controller;snapshot-webhook;"" +// +kubebuilder:validation:Enum:=none;snapshot-controller;"" type OptionalCsiComponent string const ( @@ -39,8 +39,6 @@ const ( CsiComponentNone OptionalCsiComponent = "none" // CsiComponentSnapshot causes MicroShift to deploy the CSI Snapshot controller. CsiComponentSnapshot OptionalCsiComponent = "snapshot-controller" - // CsiComponentSnapshotWebhook causes MicroShift to deploy the CSI Snapshot Validation Webhook. - CsiComponentSnapshotWebhook OptionalCsiComponent = "snapshot-webhook" // CsiComponentNullAlias is equivalent to not specifying a value. It exists because controller-gen generates // default empty-array values as [""], instead of []. Failing to include this odd value would mean the generated // /etc/microshift/config.default.yaml would break if passed to MicroShift. @@ -64,9 +62,9 @@ type Storage struct { // components. The CSI Driver is excluded as it is typically deployed via the same manifest as the accompanying // storage driver. Like CSIStorageDriver, uninstallation is not supported as this can lead to orphaned storage // objects. - // Allowed values are: unset, [], or one or more of ["snapshot-controller", "snapshot-webhook"] + // Allowed values are: unset, [], or one or more of ["snapshot-controller"] // +kubebuilder:validation:Optional - // +kubebuilder:example={"snapshot-controller", "snapshot-webhook"} + // +kubebuilder:example={"snapshot-controller"} OptionalCSIComponents []OptionalCsiComponent `json:"optionalCsiComponents,omitempty"` } @@ -75,7 +73,7 @@ func (s Storage) driverIsValid() (isSupported bool) { } func (s Storage) csiComponentsAreValid() []string { - supported := sets.New[OptionalCsiComponent](CsiComponentSnapshot, CsiComponentSnapshotWebhook, CsiComponentNone, + supported := sets.New[OptionalCsiComponent](CsiComponentSnapshot, CsiComponentNone, CsiComponentNullAlias) unsupported := sets.New[string]() diff --git a/pkg/config/storage_test.go b/pkg/config/storage_test.go index 4a4434ba3f3..40bf80b6894 100644 --- a/pkg/config/storage_test.go +++ b/pkg/config/storage_test.go @@ -57,7 +57,7 @@ func TestStorage_IsValid(t *testing.T) { name: "is valid when a driver is set and csi-components are valid", fields: fields{ Driver: CsiDriverLVMS, - CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot, CsiComponentSnapshotWebhook}, + CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot}, }, want: []error{}, }, @@ -73,7 +73,7 @@ func TestStorage_IsValid(t *testing.T) { name: "is valid when the driver is unset and csi-components are valid", fields: fields{ Driver: CsiDriverUnset, - CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot, CsiComponentSnapshotWebhook}, + CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot}, }, want: []error{}, }, @@ -81,7 +81,7 @@ func TestStorage_IsValid(t *testing.T) { name: "is invalid when driver is valid, but csi-components are invalid", fields: fields{ Driver: CsiDriverLVMS, - CSIComponents: []OptionalCsiComponent{"foobar", CsiComponentSnapshot, CsiComponentSnapshotWebhook}, + CSIComponents: []OptionalCsiComponent{"foobar", CsiComponentSnapshot}, }, want: []error{ fmt.Errorf("invalid CSI components: [foobar]"), @@ -91,7 +91,7 @@ func TestStorage_IsValid(t *testing.T) { name: "is invalid when driver is invalid, but CSI components are valid", fields: fields{ Driver: "foobar", - CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot, CsiComponentSnapshotWebhook}, + CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot}, }, want: []error{ fmt.Errorf("invalid driver \"foobar\""), @@ -125,7 +125,6 @@ func TestStorage_csiComponentIsValid(t *testing.T) { fields: fields{ CSIComponents: []OptionalCsiComponent{ CsiComponentSnapshot, - CsiComponentSnapshotWebhook, }, }, want: []string{}, @@ -230,7 +229,7 @@ func TestStorage_noneIsMutuallyExclusive(t *testing.T) { { name: "passes when none is not in a list of values", fields: fields{ - CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot, CsiComponentSnapshotWebhook}, + CSIComponents: []OptionalCsiComponent{CsiComponentSnapshot}, }, want: nil, want1: true, diff --git a/scripts/auto-rebase/assets.yaml b/scripts/auto-rebase/assets.yaml index 2cbef5ec165..00c2c7a3cf9 100644 --- a/scripts/auto-rebase/assets.yaml +++ b/scripts/auto-rebase/assets.yaml @@ -89,14 +89,6 @@ assets: files: - file: csi_controller_deployment.yaml - file: serviceaccount.yaml - - file: webhook_config.yaml - - file: webhook_deployment.yaml - - file: webhook_service.yaml - - file: webhook_serviceaccount.yaml - - file: webhook_clusterrole.yaml - src: ./rbac/ - - file: webhook_clusterrolebinding.yaml - src: ./rbac/ - file: volumesnapshotclasses.yaml - file: volumesnapshotcontents.yaml - file: volumesnapshots.yaml diff --git a/scripts/auto-rebase/rebase.sh b/scripts/auto-rebase/rebase.sh index c9c76d0564c..fb863f8825d 100755 --- a/scripts/auto-rebase/rebase.sh +++ b/scripts/auto-rebase/rebase.sh @@ -789,18 +789,6 @@ EOF yq -i 'del(.spec.template.spec.priorityClassName) | del(.spec.template.spec.containers[0].securityContext.seccompProfile)' $target yq -i 'with(.spec.template.spec.containers[0].securityContext; .runAsUser = 65534)' $target - local target="${REPOROOT}/assets/components/csi-snapshot-controller/webhook_deployment.yaml" - yq -i '.metadata.namespace = "kube-system"' $target - yq -i '.spec.template.spec.containers[0].image = "{{ .ReleaseImage.csi_snapshot_validation_webhook }}"' $target - yq -i 'with(.spec.template.spec.containers[0].args; .[] |= sub("\${LOG_LEVEL}", "2") )' $target - yq -i 'del(.spec.template.spec.priorityClassName)' $target - yq -i 'with(.spec.template.spec.containers[0].securityContext; .runAsUser = 65534)' $target - - yq -i '.metadata.namespace = "kube-system"' "${REPOROOT}/assets/components/csi-snapshot-controller/webhook_service.yaml" - yq -i '.metadata.namespace = "kube-system"' "${REPOROOT}/assets/components/csi-snapshot-controller/webhook_serviceaccount.yaml" - - yq -i '.webhooks[0].clientConfig.service.namespace="kube-system"' "${REPOROOT}/assets/components/csi-snapshot-controller/webhook_config.yaml" - yq -i '.metadata.namespace = "kube-system"' "${REPOROOT}/assets/components/csi-snapshot-controller/serviceaccount.yaml" local target="${REPOROOT}/assets/components/csi-snapshot-controller/05_operand_rbac.yaml" diff --git a/test/suites/standard1/configuration.robot b/test/suites/standard1/configuration.robot index 7fd76635700..6fb6f1e0035 100644 --- a/test/suites/standard1/configuration.robot +++ b/test/suites/standard1/configuration.robot @@ -79,7 +79,7 @@ Deploy MicroShift With LVMS By Default [Documentation] Verify that LVMS and CSI snapshotting are deployed when config fields are null. [Setup] Deploy Storage Config ${LVMS_DEFAULT} LVMS Is Deployed - CSI Snapshot Controller And Webhook Are Deployed + CSI Snapshot Controller Is Deployed [Teardown] Run Keywords ... Remove Storage Drop In Config ... Restart MicroShift @@ -89,7 +89,7 @@ Deploy MicroShift Without LVMS ... components are still deployed. [Setup] Deploy Storage Config ${LVMS_DISABLED} - CSI Snapshot Controller And Webhook Are Deployed + CSI Snapshot Controller Is Deployed Run Keyword And Expect Error 1 != 0 ... LVMS Is Deployed [Teardown] Run Keywords @@ -102,7 +102,7 @@ Deploy MicroShift Without CSI Snapshotter LVMS Is Deployed Run Keyword And Expect Error 1 != 0 - ... CSI Snapshot Controller And Webhook Are Deployed + ... CSI Snapshot Controller Is Deployed [Teardown] Run Keywords ... Remove Storage Drop In Config @@ -172,7 +172,6 @@ LVMS Is Deployed Wait Until Resource Exists daemonset vg-manager openshift-storage 120s Named Daemonset Should Be Available vg-manager openshift-storage 120s -CSI Snapshot Controller And Webhook Are Deployed - [Documentation] Wait for CSI snapshot controller and webhook to be deployed +CSI Snapshot Controller Is Deployed + [Documentation] Wait for CSI snapshot controller to be deployed Named Deployment Should Be Available csi-snapshot-controller kube-system 120s - Named Deployment Should Be Available csi-snapshot-webhook kube-system 120s From b4c1a5d2d3521371e848a14159775cc9b38cf34f Mon Sep 17 00:00:00 2001 From: Patryk Matuszak Date: Tue, 21 Jan 2025 09:58:19 +0100 Subject: [PATCH 2/2] Test the removal --- test/assets/csi-webhook-deletion-diff.yaml | 33 ++++++++++++++++++++ test/suites/upgrade/upgrade-successful.robot | 9 ++++++ 2 files changed, 42 insertions(+) create mode 100644 test/assets/csi-webhook-deletion-diff.yaml diff --git a/test/assets/csi-webhook-deletion-diff.yaml b/test/assets/csi-webhook-deletion-diff.yaml new file mode 100644 index 00000000000..aa8325f01b3 --- /dev/null +++ b/test/assets/csi-webhook-deletion-diff.yaml @@ -0,0 +1,33 @@ +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: csi-snapshot-webhook-clusterrole +--- +kind: ClusterRoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: csi-snapshot-webhook-clusterrolebinding +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: snapshot.storage.k8s.io +--- +kind: Deployment +apiVersion: apps/v1 +metadata: + name: csi-snapshot-webhook + namespace: kube-system +--- +apiVersion: v1 +kind: Service +metadata: + name: csi-snapshot-webhook + namespace: kube-system +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: csi-snapshot-webhook + namespace: kube-system diff --git a/test/suites/upgrade/upgrade-successful.robot b/test/suites/upgrade/upgrade-successful.robot index 29ed75f1d5d..a30f5c52fe8 100644 --- a/test/suites/upgrade/upgrade-successful.robot +++ b/test/suites/upgrade/upgrade-successful.robot @@ -18,6 +18,8 @@ Test Tags ostree ${TARGET_REF} ${EMPTY} ${BOOTC_REGISTRY} ${EMPTY} +${CSI_WEBHOOK_DIFF} ./assets/csi-webhook-deletion-diff.yaml + *** Test Cases *** Upgrade @@ -40,6 +42,13 @@ Upgrade Validate SELinux With Backup ${future_backup} END + # Skip CSI webhook deletion check if upgrade is to "crel" because (at the time of writing) + # it doesn't yet contain the deletion. + IF 'microshift-crel' not in '${TARGET_REF}' + # Verifies that CSI Webhook resources are cleaned up after migration + Oc Wait -f ${CSI_WEBHOOK_DIFF} --for=Delete --timeout=${DEFAULT_WAIT_TIMEOUT} + END + *** Keywords *** Setup