rbd: use ioctx locks for key rotation #4734

black-dragon74 · 2024-07-26T07:29:11Z

Describe what this PR does

This PR introduces rados ioctx locks for key rotation operations which will help with synchronizing
operations when dealing with ROX/RWX volumes.

nixpanic

This code looks very much like what is used in CephFS. Can you move it to a shared helper package under internal/util/lock/...? You can have a

package lock

interface Lock {
    LockExclusive(ctx) error
    Unlock(ctx) error
}

type lock struct {
    lockName, lockCookie, lockDesc string
    ioctx *rados.Ioctx
}

func NewLock(ioctx, volumeID, duration) Lock {
    return &lock{
        ioctx: ioctx,
        ...
    }
}

func (lck *Lock) LockExclusive(ctx context.Context, ioctx *rados.Ioctx, volumeID string) error {
    return lck.ioctx.LockExclusive(...)
}

func (lck *Lock) Unlock(ctx) error {
   ...
}

internal/rbd/encryption.go

Madhu-1

small nit

internal/util/lock/lock.go

nixpanic

Looks good! Just a small change needed for a little simpler lock API.

internal/util/lock/lock.go

nixpanic · 2024-07-30T09:59:30Z

@Mergifyio queue

mergify · 2024-07-30T09:59:35Z

queue

🛑 The pull request has been removed from the queue `default`

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

that abstracts rados Ioctx Lock/Unlocks Signed-off-by: Niraj Yadav <niryadav@redhat.com>

Signed-off-by: Niraj Yadav <niryadav@redhat.com>

ceph-csi-bot · 2024-07-30T10:21:34Z

/test ci/centos/k8s-e2e-external-storage/1.29

ceph-csi-bot · 2024-07-30T10:21:35Z

/test ci/centos/k8s-e2e-external-storage/1.27

ceph-csi-bot · 2024-07-30T10:21:35Z

/test ci/centos/mini-e2e-helm/k8s-1.29

ceph-csi-bot · 2024-07-30T10:21:35Z

/test ci/centos/mini-e2e-helm/k8s-1.27

ceph-csi-bot · 2024-07-30T10:21:38Z

/test ci/centos/mini-e2e-helm/k8s-1.28

ceph-csi-bot · 2024-07-30T10:21:39Z

/test ci/centos/mini-e2e/k8s-1.28

black-dragon74 · 2024-07-30T11:23:36Z

@Mergifyio queue

mergify · 2024-07-30T11:23:40Z

queue

🛑 The pull request has been removed from the queue `default`

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

Madhu-1 · 2024-07-30T11:25:40Z

Warning: A secret was passed to "sh" using Groovy String interpolation, which is insecure.
Affected argument(s) used the following variable(s): [CREDS_USER]
See https://jenkins.io/redirect/groovy-string-interpolation for details.

ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no root@n27-38-30.pool.ci.centos.org 'podman login --authfile=~/.podman-auth.json --username=**** --password=**** registry-.apps.ocp.cloud.ci.centos.org'
Error: authenticating creds for "registry-.apps.ocp.cloud.ci.centos.org": pinging container registry registry-****.apps.ocp.cloud.ci.centos.org: Get "https://registry-****.apps.ocp.cloud.ci.centos.org/v2/": tls: failed to verify certificate: x509: certificate signed by unknown authority
script returned exit code 125

@nixpanic CI is failing with above error, any idea about it?

nixpanic · 2024-07-30T11:50:35Z

I think there was some certificate rotation on the OpenShift service that provides the routing. This gives a similar error for the Jenkins Pod:

curl -v https://jenkins-ceph-csi.apps.ocp.cloud.ci.centos.org/

The TLS certificate was updated today:

$ curl -v -k https://registry-ceph-csi.apps.ocp.cloud.ci.centos.org
* Host registry-ceph-csi.apps.ocp.cloud.ci.centos.org:443 was resolved.
* IPv6: (none)
* IPv4: 54.174.203.175, 54.145.63.179
*   Trying 54.174.203.175:443...
* Connected to registry-ceph-csi.apps.ocp.cloud.ci.centos.org (54.174.203.175) port 443
...
* Server certificate:
*  subject: CN=api.ocp.cloud.ci.centos.org
*  start date: Jul 30 08:42:03 2024 GMT
...

nixpanic · 2024-07-30T12:28:15Z

@Mergifyio requeue

mergify · 2024-07-30T12:28:19Z

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

nixpanic · 2024-07-30T12:28:46Z

@Mergifyio queue

mergify · 2024-07-30T12:28:49Z

queue

🛑 The pull request has been removed from the queue `default`

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

ceph-csi-bot · 2024-07-30T12:29:46Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2024-07-30T12:29:47Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2024-07-30T12:29:47Z

/test ci/centos/k8s-e2e-external-storage/1.27

ceph-csi-bot · 2024-07-30T12:29:47Z

/test ci/centos/mini-e2e-helm/k8s-1.27

ceph-csi-bot · 2024-07-30T12:29:47Z

/test ci/centos/k8s-e2e-external-storage/1.28

ceph-csi-bot · 2024-07-30T12:29:48Z

/test ci/centos/mini-e2e/k8s-1.27

ceph-csi-bot · 2024-07-30T12:29:48Z

/test ci/centos/mini-e2e-helm/k8s-1.28

ceph-csi-bot · 2024-07-30T12:29:48Z

/test ci/centos/k8s-e2e-external-storage/1.29

ceph-csi-bot · 2024-07-30T12:29:48Z

/test ci/centos/k8s-e2e-external-storage/1.30

ceph-csi-bot · 2024-07-30T12:29:49Z

/test ci/centos/mini-e2e/k8s-1.28

ceph-csi-bot · 2024-07-30T12:29:49Z

/test ci/centos/mini-e2e-helm/k8s-1.29

ceph-csi-bot · 2024-07-30T12:29:49Z

/test ci/centos/mini-e2e-helm/k8s-1.30

ceph-csi-bot · 2024-07-30T12:29:50Z

/test ci/centos/mini-e2e/k8s-1.29

ceph-csi-bot · 2024-07-30T12:29:50Z

/test ci/centos/mini-e2e/k8s-1.30

nixpanic · 2024-07-30T12:30:25Z

@Mergifyio requeue

mergify · 2024-07-30T12:30:31Z

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

black-dragon74 self-assigned this Jul 26, 2024

mergify bot added the component/rbd Issues related to RBD label Jul 26, 2024

black-dragon74 force-pushed the rwx-test branch 2 times, most recently from dc19aae to 21ccbda Compare July 26, 2024 07:44

nixpanic reviewed Jul 26, 2024

View reviewed changes

black-dragon74 force-pushed the rwx-test branch 2 times, most recently from 66120a8 to 1c3de73 Compare July 26, 2024 10:28

black-dragon74 requested review from nixpanic and Madhu-1 July 29, 2024 05:05

Madhu-1 requested changes Jul 29, 2024

View reviewed changes

internal/rbd/encryption.go Outdated Show resolved Hide resolved

internal/rbd/encryption.go Outdated Show resolved Hide resolved

internal/rbd/encryption.go Outdated Show resolved Hide resolved

black-dragon74 force-pushed the rwx-test branch 2 times, most recently from 7ca683d to f1ea33a Compare July 29, 2024 07:40

Madhu-1 reviewed Jul 29, 2024

View reviewed changes

internal/util/lock/lock.go Outdated Show resolved Hide resolved

black-dragon74 force-pushed the rwx-test branch from f1ea33a to e18b42d Compare July 29, 2024 08:02

Madhu-1 approved these changes Jul 29, 2024

View reviewed changes

nixpanic requested changes Jul 30, 2024

View reviewed changes

internal/util/lock/lock.go Outdated Show resolved Hide resolved

black-dragon74 force-pushed the rwx-test branch from e18b42d to 7e415d0 Compare July 30, 2024 07:29

nixpanic approved these changes Jul 30, 2024

View reviewed changes

Madhu-1 approved these changes Jul 30, 2024

View reviewed changes

black-dragon74 added 3 commits July 30, 2024 10:21

util: Add IOCtxLock interface

7a34111

that abstracts rados Ioctx Lock/Unlocks Signed-off-by: Niraj Yadav <niryadav@redhat.com>

rbd: use ioctx locks for key rotation

3300012

Signed-off-by: Niraj Yadav <niryadav@redhat.com>

cephfs: refactor locks using IOCtxLock interface

0ec6248

Signed-off-by: Niraj Yadav <niryadav@redhat.com>

black-dragon74 force-pushed the rwx-test branch from 7e415d0 to 0ec6248 Compare July 30, 2024 10:21

mergify bot added the ok-to-test Label to trigger E2E tests label Jul 30, 2024

ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 30, 2024

nixpanic added the ok-to-test Label to trigger E2E tests label Jul 30, 2024

ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 30, 2024

mergify bot merged commit 439a03f into ceph:devel Jul 30, 2024
52 checks passed

rbd: use ioctx locks for key rotation #4734

rbd: use ioctx locks for key rotation #4734

Conversation

black-dragon74 commented Jul 26, 2024 • edited Loading

Describe what this PR does

nixpanic left a comment

Choose a reason for hiding this comment

Madhu-1 left a comment

Choose a reason for hiding this comment

nixpanic left a comment

Choose a reason for hiding this comment

nixpanic commented Jul 30, 2024

mergify bot commented Jul 30, 2024 • edited Loading

🛑 The pull request has been removed from the queue default

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

black-dragon74 commented Jul 30, 2024

mergify bot commented Jul 30, 2024

🛑 The pull request has been removed from the queue default

Madhu-1 commented Jul 30, 2024

nixpanic commented Jul 30, 2024

nixpanic commented Jul 30, 2024

mergify bot commented Jul 30, 2024

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

nixpanic commented Jul 30, 2024

mergify bot commented Jul 30, 2024 • edited Loading

🛑 The pull request has been removed from the queue default

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

ceph-csi-bot commented Jul 30, 2024

nixpanic commented Jul 30, 2024

mergify bot commented Jul 30, 2024

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

black-dragon74 commented Jul 26, 2024 •

edited

Loading

mergify bot commented Jul 30, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

🛑 The pull request has been removed from the queue `default`

mergify bot commented Jul 30, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`