Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STOR-947: support disabling default StorageClass via ClusterCSIDriver #42

Merged
merged 2 commits into from
Feb 6, 2023

Conversation

dobsonj
Copy link
Member

@dobsonj dobsonj commented Dec 16, 2022

https://issues.redhat.com/browse/STOR-947
Marking this WIP until openshift/library-go#1441 merges.
/cc @openshift/storage

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dobsonj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 16, 2022
@dobsonj
Copy link
Member Author

dobsonj commented Dec 16, 2022

This failure is real (caused by bumping library-go):
level=error msg=Cluster operator storage Degraded is True with AzureFileCSIDriverOperatorCR_AzureFileDriverStaticResourcesController_SyncError: AzureFileCSIDriverOperatorCRDegraded: AzureFileDriverStaticResourcesControllerDegraded: "csidriver.yaml" (string): CSIDriver file.csi.azure.com supports Ephemeral volume lifecycle but is missing required label security.openshift.io/csi-ephemeral-volume-profile
Apparently the driver allows ephemeral (inline) volumes and we need to set the security.openshift.io/csi-ephemeral-volume-profile label.

@dobsonj
Copy link
Member Author

dobsonj commented Dec 16, 2022

Note to self: review the driver parameters and discuss with the team what would be an appropriate value for the label (i.e. restricted or privileged).

@dobsonj
Copy link
Member Author

dobsonj commented Jan 5, 2023

Note to self: review the driver parameters and discuss with the team what would be an appropriate value for the label (i.e. restricted or privileged).

We agreed that it should be privileged by default (added here) but that we should allow the admin to change the value (added here). This means we use a secure default but allow the admin to specify a more permissive policy if their use case requires it.

@dobsonj
Copy link
Member Author

dobsonj commented Jan 5, 2023

Basic functional test:

# Azure File

## patch SC, should get reverted by default
$ oc get sc
NAME                    PROVISIONER          RECLAIMPOLICY   VOLUMEBINDINGMODE      ALLOWVOLUMEEXPANSION   AGE
azurefile-csi           file.csi.azure.com   Delete          Immediate              true                   84m
managed-csi (default)   disk.csi.azure.com   Delete          WaitForFirstConsumer   true                   84m
$ oc patch sc azurefile-csi -p '{"allowVolumeExpansion":false}'
storageclass.storage.k8s.io/azurefile-csi patched
$ oc get sc azurefile-csi -o yaml | grep allowVolumeExpansion
allowVolumeExpansion: true

## Unmanaged: SC changes should persist
$ oc get clustercsidriver
NAME                 AGE
disk.csi.azure.com   85m
file.csi.azure.com   85m
$ oc edit clustercsidriver file.csi.azure.com
#  storageClassState: Unmanaged
clustercsidriver.operator.openshift.io/file.csi.azure.com edited
$ oc patch sc azurefile-csi -p '{"allowVolumeExpansion":false}'
storageclass.storage.k8s.io/azurefile-csi patched
$ oc get sc azurefile-csi -o yaml | grep allowVolumeExpansion
allowVolumeExpansion: false

## Removed: SC should be removed
$ oc edit clustercsidriver file.csi.azure.com
#  storageClassState: Removed
clustercsidriver.operator.openshift.io/file.csi.azure.com edited
$ oc get sc
NAME                    PROVISIONER          RECLAIMPOLICY   VOLUMEBINDINGMODE      ALLOWVOLUMEEXPANSION   AGE
managed-csi (default)   disk.csi.azure.com   Delete          WaitForFirstConsumer   true                   88m

## Managed: SC should be recreated. Changes should be reverted.
$ oc edit clustercsidriver file.csi.azure.com
#  storageClassState: Managed
clustercsidriver.operator.openshift.io/file.csi.azure.com edited
$ oc get sc
NAME                    PROVISIONER          RECLAIMPOLICY   VOLUMEBINDINGMODE      ALLOWVOLUMEEXPANSION   AGE
azurefile-csi           file.csi.azure.com   Delete          Immediate              true                   42s
managed-csi (default)   disk.csi.azure.com   Delete          WaitForFirstConsumer   true                   89m
$ oc patch sc azurefile-csi -p '{"allowVolumeExpansion":false}'
storageclass.storage.k8s.io/azurefile-csi patched
$ oc get sc azurefile-csi -o yaml | grep allowVolumeExpansion
allowVolumeExpansion: true

## Change csi-ephemeral-volume-profile label on CSIDriver (change should persist)
$ oc get csidriver file.csi.azure.com -o yaml
apiVersion: storage.k8s.io/v1
kind: CSIDriver
metadata:
  annotations:
    csi.openshift.io/managed: "true"
    operator.openshift.io/spec-hash: 9d1932775c60df2433bdcb362d11478167064398d9dbdade8579dbde4be91949
  creationTimestamp: "2023-01-05T20:38:36Z"
  labels:
    security.openshift.io/csi-ephemeral-volume-profile: privileged
  name: file.csi.azure.com
  resourceVersion: "5976"
  uid: 6c5e65dc-4a57-4e7f-9473-daaea3ca285b
spec:
  attachRequired: false
  fsGroupPolicy: None
  podInfoOnMount: true
  requiresRepublish: false
  storageCapacity: false
  volumeLifecycleModes:
  - Persistent
  - Ephemeral
$ oc label --overwrite csidriver file.csi.azure.com security.openshift.io/csi-ephemeral-volume-profile=restricted
csidriver.storage.k8s.io/file.csi.azure.com labeled
$ oc get csidriver file.csi.azure.com -o yaml | grep csi-ephemeral-volume-profile
    security.openshift.io/csi-ephemeral-volume-profile: restricted

## Remove csi-ephemeral-volume-profile label on CSIDriver (should be added back)
$ oc label csidriver file.csi.azure.com security.openshift.io/csi-ephemeral-volume-profile-
csidriver.storage.k8s.io/file.csi.azure.com unlabeled
$ oc get csidriver file.csi.azure.com -o yaml | grep csi-ephemeral-volume-profile
    security.openshift.io/csi-ephemeral-volume-profile: privileged

@dobsonj
Copy link
Member Author

dobsonj commented Jan 5, 2023

/retest

@dobsonj
Copy link
Member Author

dobsonj commented Jan 10, 2023

/retitle STOR-947: support disabling default StorageClass via ClusterCSIDriver

@openshift-ci openshift-ci bot changed the title WIP: STOR-947: support disabling default StorageClass via ClusterCSIDriver STOR-947: support disabling default StorageClass via ClusterCSIDriver Jan 10, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 10, 2023
@jsafrane
Copy link
Contributor

/lgtm

1 similar comment
@gnufied
Copy link
Member

gnufied commented Jan 13, 2023

/lgtm

@lpettyjo
Copy link

/docs-approved

@ropatil010
Copy link

Payload: 4.13.0-0.ci.test-2023-01-25-044031-ci-ln-8y8dp9t-latest

Pending test on csi-ephemeral-volume-profile label

Test Results:
###################test0_Default started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 13m
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 6m9s

change managed-csi volumeBindingMode
change managed-csi reclaimPolicy
change azurefile-csi volumeBindingMode
change azurefile-csi reclaimPolicy

Bydefault test change managed-csi allowVolumeExpansion to false
storageclass.storage.k8s.io/managed-csi patched
Bydefault result oc get sc managed-csi
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 6m46s

Bydefault test change azurefile-csi allowVolumeExpansion to false
storageclass.storage.k8s.io/azurefile-csi patched
Bydefault result oc get sc azurefile-csi
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 13m

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 13m
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 7m5s

oc delete sc
storageclass.storage.k8s.io "managed-csi" deleted
storageclass.storage.k8s.io "azurefile-csi" deleted

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 6s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 13s
###################test1_Removed started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Removed

Removed test change azurefile-csi allowVolumeExpansion to false
Removed test change managed-csi allowVolumeExpansion to false
storageclass.storage.k8s.io/managed-csi patched

Removed result oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 2m44s
###################test2_Managed started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Managed

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 82s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 5m13s

change managed-csi volumeBindingMode
change managed-csi reclaimPolicy
change azurefile-csi volumeBindingMode
change azurefile-csi reclaimPolicy

Managed test change managed-csi allowVolumeExpansion to false
storageclass.storage.k8s.io/managed-csi patched
Managed result oc get sc managed-csi
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 5m48s

Managed test change azurefile-csi allowVolumeExpansion to false
storageclass.storage.k8s.io/azurefile-csi patched
Managed result oc get sc azurefile-csi
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 2m10s

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 2m16s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 6m7s

oc delete sc
storageclass.storage.k8s.io "managed-csi" deleted
storageclass.storage.k8s.io "azurefile-csi" deleted

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 3s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 9s
###################test3_Unmanaged started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Unmanaged

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 2m6s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 2m12s

change managed-csi volumeBindingMode
change managed-csi reclaimPolicy
change azurefile-csi volumeBindingMode
change azurefile-csi reclaimPolicy

Unmanaged test change managed-csi allowVolumeExpansion to false
storageclass.storage.k8s.io/managed-csi patched
Unmanaged result oc get sc managed-csi
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 2m47s

Unmanaged test change azurefile-csi allowVolumeExpansion to false
storageclass.storage.k8s.io/azurefile-csi patched
Unmanaged result oc get sc azurefile-csi
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate false 2m53s

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate false 3m
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 3m6s

oc delete sc
storageclass.storage.k8s.io "managed-csi" deleted
storageclass.storage.k8s.io "azurefile-csi" deleted

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 9s
###################test4a_Multiple default sc started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Managed

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 2m2s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 5m56s
Creating sc
storageclass.storage.k8s.io/testsc created
Applying patch to sc testsc for changing to default
storageclass.storage.k8s.io/testsc patched

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 2m18s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 6m12s
testsc (default) file.csi.azure.com Delete Immediate true 13s
###################test4b_Removed started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Removed

oc get sc and check azurefile-csi should not be there, managed-csi and testsc should be there
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 7m6s
testsc (default) file.csi.azure.com Delete Immediate true 67s
###################test4c_Managed sc started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Managed

oc get sc all should be present now managed-csi, azurefile-csi, testsc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 20s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 8m47s
testsc (default) file.csi.azure.com Delete Immediate true 2m48s
Deleting all sc
storageclass.storage.k8s.io "testsc" deleted
storageclass.storage.k8s.io "managed-csi" deleted
storageclass.storage.k8s.io "azurefile-csi" deleted

oc get sc azurefile-csi and managed-csi should be back to default
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 7s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 13s
###################test5a_Managed default sc started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Managed

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 2m41s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 2m47s
###################test5b_Unmanaged default sc started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Unmanaged

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 3m55s
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 4m1s
Creating sc
storageclass.storage.k8s.io/testsc created
Applying patch to sc testsc for changing to default
storageclass.storage.k8s.io/testsc patched
Applying patch to sc managed-csi for changing to non default
storageclass.storage.k8s.io/managed-csi patched

oc get sc
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
azurefile-csi file.csi.azure.com Delete Immediate true 4m39s
managed-csi disk.csi.azure.com Delete WaitForFirstConsumer true 4m45s
testsc (default) file.csi.azure.com Delete Immediate true 37s
###################test5c_Removed sc started #################
oc get clustercsidriver file.csi.azure.com -o yaml | grep 'storageClassState'
storageClassState: Removed

oc get sc azurefile-csi should not be there, managed-csi, testsc should be there
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi disk.csi.azure.com Delete WaitForFirstConsumer true 9m23s
testsc (default) file.csi.azure.com Delete Immediate true 5m15s
Deleting sc testsc
storageclass.storage.k8s.io "testsc" deleted
Deleting sc managed-csi
storageclass.storage.k8s.io "managed-csi" deleted

managed-csi should remain
NAME PROVISIONER RECLAIMPOLICY VOLUMEBINDINGMODE ALLOWVOLUMEEXPANSION AGE
managed-csi (default) disk.csi.azure.com Delete WaitForFirstConsumer true 7s

@sferich888
Copy link

/label px-approved

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Jan 27, 2023
@dobsonj
Copy link
Member Author

dobsonj commented Jan 27, 2023

/docs-approved

/label docs-approved

@ropatil010
Copy link

Verified:
Payload: 4.13.0-0.ci.test-2023-02-06-043724-ci-ln-b8ij3kt-latest
Cluster bot: build openshift/azure-disk-csi-driver-operator#63,#42

rohitpatil@ropatil-mac Downloads % oc label --overwrite csidriver file.csi.azure.com security.openshift.io/csi-ephemeral-volume-profile=restricted
csidriver.storage.k8s.io/file.csi.azure.com labeled
rohitpatil@ropatil-mac Downloads % oc get csidriver file.csi.azure.com -o yaml | grep "security.openshift.io/csi-ephemeral-volume-profile"
security.openshift.io/csi-ephemeral-volume-profile: restricted

rohitpatil@ropatil-mac Downloads % oc label --overwrite csidriver file.csi.azure.com security.openshift.io/csi-ephemeral-volume-profile=privileged
csidriver.storage.k8s.io/file.csi.azure.com labeled
rohitpatil@ropatil-mac Downloads % oc get csidriver file.csi.azure.com -o yaml | grep "security.openshift.io/csi-ephemeral-volume-profile"
security.openshift.io/csi-ephemeral-volume-profile: privileged

May need bit additional validation codecheck/Optional.
rohitpatil@ropatil-mac Downloads % oc label --overwrite csidriver file.csi.azure.com security.openshift.io/csi-ephemeral-volume-profile=restricted123
csidriver.storage.k8s.io/file.csi.azure.com labeled
rohitpatil@ropatil-mac Downloads % oc get csidriver file.csi.azure.com -o yaml | grep "security.openshift.io/csi-ephemeral-volume-profile"
security.openshift.io/csi-ephemeral-volume-profile: restricted123

rohitpatil@ropatil-mac Downloads % oc label --overwrite csidriver file.csi.azure.com security.openshift.io/csi-ephemeral-volume-profile=abcd
csidriver.storage.k8s.io/file.csi.azure.com labeled
rohitpatil@ropatil-mac Downloads % oc get csidriver file.csi.azure.com -o yaml | grep "security.openshift.io/csi-ephemeral-volume-profile"
security.openshift.io/csi-ephemeral-volume-profile: abcd

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Feb 6, 2023
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 20ee9ab and 2 for PR HEAD 65389fe in total

@dobsonj
Copy link
Member Author

dobsonj commented Feb 6, 2023

ETRYAGAIN? The provided client secret keys for app '4f03bcab-7b63-4617-9641-0e2eeb9cc5eb' are expired.
/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 6, 2023

@dobsonj: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 4294e73 into openshift:main Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants