rbd: setup encryption if rbdVol exits during CreateVol #3422
Conversation
This commit adds code to setup encryption on a rbdVol being repaired in a followup CreateVolume request. This is fixes a bug wherein encryption metadata may not have been set in previous request due to container restart. Fixes: ceph#3402 Signed-off-by: Rakshith R <[email protected]>
Rakshith-R
force-pushed
the
enc-fix-for-create-req
branch
from
e91b849
to
4bda0a5
Compare
|
This pr is ready for review. refer: |
Without the fix, Prov would get OOMKilled during set metadata step for DEK and encryption Status, and provisioning would get completed in a followup req with empty encryption status like seen in the issue linked. With fix, provision tries to setupencryption again in subsequent req leading to repeated OOMkills but raising mem limit leads to provisioning of PVC which can be successfully. |
| @@ -508,6 +508,15 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C | |||
|
|
|||
| return nil, err | |||
| } | |||
|
|
|||
| default: | |||
| // setup encryption again to make sure everything is in place. | |||
There was a problem hiding this comment.
Question:
- This is only for PVC. What about the PVC Clone and PVC Snapshot?
- What about the fileEncryption?
There was a problem hiding this comment.
Question:
- This is only for PVC. What about the PVC Clone and PVC Snapshot?
For clone
ceph-csi/internal/rbd/rbd_journal.go
Line 337 in 07e9ded
For snapshot, just few ones above to copy encryption config
- What about the fileEncryption?
This pr handles for block encryption,
We can handle the same for file encryption in another pr.
@irq0 can you please take a look at this?
There was a problem hiding this comment.
Open an issue so that we dont forget it?
| @@ -508,6 +508,15 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C | |||
|
|
|||
| return nil, err | |||
| } | |||
|
|
|||
| default: | |||
| // setup encryption again to make sure everything is in place. | |||
There was a problem hiding this comment.
Open an issue so that we dont forget it?
|
/test ci/centos/k8s-e2e-external-storage/1.23 |
|
/test ci/centos/k8s-e2e-external-storage/1.24 |
|
/test ci/centos/k8s-e2e-external-storage/1.25 |
|
/test ci/centos/mini-e2e-helm/k8s-1.23 |
|
/test ci/centos/mini-e2e-helm/k8s-1.24 |
|
/test ci/centos/mini-e2e-helm/k8s-1.25 |
|
/test ci/centos/mini-e2e/k8s-1.23 |
|
/test ci/centos/mini-e2e/k8s-1.24 |
|
/test ci/centos/mini-e2e/k8s-1.25 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/upgrade-tests-rbd |
This commit adds code to setup encryption on a rbdVol being repaired in a followup CreateVolume request. This is fixes a bug wherein encryption metadata may not have been set in previous request due to container restart.
Fixes: #3402
Signed-off-by: Rakshith R [email protected]