cleanup: use rbd.Manager for fetching volumes in CSI-Addons operations
#5092
Conversation
nixpanic
force-pushed
the
cleanup/csi-addons/manager
branch
from
f000d07 to
b1569c8
Compare
|
@Mergifyio queue |
🛑 The pull request has been removed from the queue
|
nixpanic
force-pushed
the
cleanup/csi-addons/manager
branch
from
b1569c8 to
8cfe37a
Compare
|
/test ci/centos/k8s-e2e-external-storage/1.30 |
|
/test ci/centos/k8s-e2e-external-storage/1.32 |
|
/test ci/centos/mini-e2e-helm/k8s-1.30 |
|
/test ci/centos/mini-e2e-helm/k8s-1.32 |
|
/test ci/centos/mini-e2e/k8s-1.30 |
|
/test ci/centos/mini-e2e/k8s-1.32 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/k8s-e2e-external-storage/1.31 |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/mini-e2e-helm/k8s-1.31 |
|
/test ci/centos/mini-e2e/k8s-1.31 |
| @@ -32,11 +32,15 @@ import ( | |||
|
|
|||
| type EncryptionKeyRotationServer struct { | |||
| *ekr.UnimplementedEncryptionKeyRotationControllerServer | |||
| driver string | |||
There was a problem hiding this comment.
this is csiID, naming it as driver causes confusion to me. And also everywhere else we have csiID naming lets stick to that?
There was a problem hiding this comment.
csiID is a very strangely called variable. In the CSI Specification it is the name returned in the IdentityService.GetPluginInfo operation. It is not so much an ID, it is more the name of the driver (for its current deployment). Even in Kubernetes name is what is used, see deploy/rbd/kubernetes/csidriver.yaml.
csiID is also used for variables of the util.CSIIdentifier type, and that is why I prefer a different variable name for the driver-instance.
I'll send a cleanup to rename the csiID variable for the driver-instance, and will call it driverName for clarity.
There was a problem hiding this comment.
Oh, wait, in most cases it is actually the instanceID for the driver to allow multi-tenancy on the same Ceph cluster (it is used as the location of the journal where Ceph-CSI stores its metadata). I'll call it driverInstance instead.
| @@ -37,13 +37,18 @@ import ( | |||
| // of CSI-addons reclaimspace controller service spec. | |||
| type ReclaimSpaceControllerServer struct { | |||
| *rs.UnimplementedReclaimSpaceControllerServer | |||
|
|
|||
| driver string | |||
|
This pull request has been removed from the queue for the following reason: Pull request #5092 has been dequeued. The pull request rule doesn't match anymore You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. If you want to requeue this pull request, you need to post a comment with the text: |
Introduce `snapshottableVolume` and `csiAddonsVolume` types which group related functions together. Signed-off-by: Niels de Vos <[email protected]>
Signed-off-by: Niels de Vos <[email protected]>
Signed-off-by: Niels de Vos <[email protected]>
The attribute and variable `csiID` ise used for at least two different things: - name of the driver instance, used for journalling metadata - objects of the CSIIdentifier struct, composing a volume-handle By changing the name of the `csiID` attribute for driver instances to `driverInstance`, any confusion should be prevented. Signed-off-by: Niels de Vos <[email protected]>
nixpanic
force-pushed
the
cleanup/csi-addons/manager
branch
from
8cfe37a to
3f4b385
Compare
mergify
bot
dismissed stale reviews from Madhu-1 and Rakshith-R
Pull request has been modified.
ReclaimSpace and EncryptionKeyRotation now use the
Volumetype instead of the (supposed to be) internalrbdVolume.Show available bot commands
These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:
/retest ci/centos/<job-name>: retest the<job-name>after unrelatedfailure (please report the failure too!)