rbd: remove topologyConstrainedPools parameter #4499

iPraveenParihar · 2024-03-14T12:23:15Z

Describe what this PR does

This commit removes the topologyConstrainedPools parameter from PV volumeAttributes as it is not required.

Related issues

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

Checklist:

Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
Reviewed the developer guide on Submitting a Pull Request
Pending release notes updated with breaking and/or notable changes for the next major release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.

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 unrelated
failure (please report the failure too!)

iPraveenParihar · 2024-03-14T12:24:06Z

verified that topologyConstrainedPools has been removed.

[pm@dhcp53-176 examples]$ k get pv pvc-5aaf7aef-fda5-4a57-af7c-abbb737d77bb -o jsonpath='{.spec.csi.volumeAttributes}' | jq
{
  "clusterID": "rook-ceph",
  "encryptionKMSID": "azure-kms",
  "encryptionType": "block",
  "imageFeatures": "layering",
  "imageName": "csi-vol-1292393c-b86a-4ff7-abb7-52750879d83a",
  "journalPool": "replicapool",
  "pool": "rp",
  "storage.kubernetes.io/csiProvisionerIdentity": "1710418563048-5228-rook-ceph.rbd.csi.ceph.com"
}

internal/rbd/controllerserver.go

internal/rbd/rbd_util.go

mergify · 2024-03-27T11:20:59Z

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

internal/util/topology.go

internal/util/util.go

nixpanic · 2024-04-04T10:47:07Z

@Mergifyio rebase

mergify · 2024-04-04T10:47:21Z

rebase

✅ Branch has been successfully rebased

ceph-csi-bot · 2024-04-04T10:52:17Z

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

ceph-csi-bot · 2024-04-04T10:52:17Z

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

nixpanic · 2024-04-05T07:37:12Z

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

nixpanic · 2024-04-05T07:39:21Z

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

spurious failure, the logs show that an image was not cleaned up (yet?):

[FAILED] backend images not matching kubernetes resource count,image count 1 kubernetes resource count 0

nixpanic · 2024-04-05T07:44:04Z

@Mergifyio requeue

mergify · 2024-04-05T07:44:09Z

requeue

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

Madhu-1 · 2024-04-05T08:33:09Z

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

spurious failure, the logs show that an image was not cleaned up (yet?):
[FAILED] backend images not matching kubernetes resource count,image count 1 kubernetes resource count 0

i think we need to update the validation function to print rbd info and rbd status of each images to get more clarity what happened. it looks like the image is opened by someone but not closed? not sure who

nixpanic · 2024-04-05T10:29:20Z

@Mergifyio queue

mergify · 2024-04-05T10:29:24Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c146724

This commit removes the `topologyConstrainedPools` parameter from PV volumeAttributes as it is not required. Signed-off-by: Praveen M <[email protected]>

Signed-off-by: Praveen M <[email protected]>

This commit replaces the user implemented function `CheckSliceContains()` with `slices.Contains()` function introduced in Go 1.21. Signed-off-by: Praveen M <[email protected]>

ceph-csi-bot · 2024-04-05T10:30:58Z

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

ceph-csi-bot · 2024-04-05T10:30:59Z

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

ceph-csi-bot · 2024-04-05T10:31:00Z

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

ceph-csi-bot · 2024-04-05T10:31:01Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2024-04-05T10:31:01Z

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

ceph-csi-bot · 2024-04-05T10:31:02Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2024-04-05T10:31:02Z

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

ceph-csi-bot · 2024-04-05T10:31:02Z

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

ceph-csi-bot · 2024-04-05T10:31:03Z

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

ceph-csi-bot · 2024-04-05T10:31:03Z

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

ceph-csi-bot · 2024-04-05T10:31:04Z

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

mergify bot added the component/rbd Issues related to RBD label Mar 14, 2024

nixpanic reviewed Mar 14, 2024

View reviewed changes

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

iPraveenParihar requested a review from nixpanic March 26, 2024 06:46

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch from 6d70eb8 to f438135 Compare March 27, 2024 08:07

iPraveenParihar marked this pull request as ready for review March 27, 2024 08:07

nixpanic reviewed Mar 27, 2024

View reviewed changes

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

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch 3 times, most recently from 9aada71 to 21a71d3 Compare March 27, 2024 09:25

Madhu-1 reviewed Mar 27, 2024

View reviewed changes

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

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch from 21a71d3 to b85269a Compare March 27, 2024 11:20

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch from b85269a to 442ba67 Compare March 27, 2024 11:21

iPraveenParihar requested a review from Madhu-1 March 27, 2024 12:17

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch 2 times, most recently from edad60f to e6a17c9 Compare March 28, 2024 05:24

nixpanic reviewed Mar 28, 2024

View reviewed changes

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

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch 3 times, most recently from c8c1d71 to a4632ad Compare March 29, 2024 03:07

iPraveenParihar requested a review from nixpanic March 29, 2024 05:37

nixpanic reviewed Mar 29, 2024

View reviewed changes

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

iPraveenParihar requested a review from nixpanic April 1, 2024 03:36

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch from a7ad618 to 2b72bf8 Compare April 1, 2024 03:41

nixpanic approved these changes Apr 3, 2024

View reviewed changes

nixpanic requested a review from a team April 3, 2024 08:19

Madhu-1 approved these changes Apr 3, 2024

View reviewed changes

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

Madhu-1 mentioned this pull request Apr 5, 2024

Log RBD info and RBD status in E2E validateRBDImageCount function in case of failure #4547

Closed

iPraveenParihar added 3 commits April 5, 2024 10:30

rbd: remove topologyConstrainedPools parameter

921a302

This commit removes the `topologyConstrainedPools` parameter from PV volumeAttributes as it is not required. Signed-off-by: Praveen M <[email protected]>

cephfs: refactor code for improved reusability

8d318d2

Signed-off-by: Praveen M <[email protected]>

cleanup: use slices package

da9ef00

This commit replaces the user implemented function `CheckSliceContains()` with `slices.Contains()` function introduced in Go 1.21. Signed-off-by: Praveen M <[email protected]>

iPraveenParihar force-pushed the remove/topologyConstrainedPools-param branch from 92bd938 to da9ef00 Compare April 5, 2024 10:30

mergify bot added the ok-to-test Label to trigger E2E tests label Apr 5, 2024

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

nixpanic mentioned this pull request Apr 5, 2024

ci: run snapshot tests and expand tests #4545

Merged

mergify bot merged commit c146724 into ceph:devel Apr 5, 2024

Uh oh!

rbd: remove topologyConstrainedPools parameter #4499

rbd: remove topologyConstrainedPools parameter #4499

Uh oh!

Conversation

iPraveenParihar commented Mar 14, 2024

Describe what this PR does

Related issues

Future concerns

Uh oh!

iPraveenParihar commented Mar 14, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mergify bot commented Mar 27, 2024

Uh oh!

Uh oh!

Uh oh!

nixpanic commented Apr 4, 2024

Uh oh!

mergify bot commented Apr 4, 2024

✅ Branch has been successfully rebased

Uh oh!

ceph-csi-bot commented Apr 4, 2024

Uh oh!

ceph-csi-bot commented Apr 4, 2024

Uh oh!

nixpanic commented Apr 5, 2024

Uh oh!

nixpanic commented Apr 5, 2024

Uh oh!

nixpanic commented Apr 5, 2024

Uh oh!

mergify bot commented Apr 5, 2024

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

Uh oh!

Madhu-1 commented Apr 5, 2024

Uh oh!

nixpanic commented Apr 5, 2024

Uh oh!

mergify bot commented Apr 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ The pull request has been merged automatically

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

ceph-csi-bot commented Apr 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mergify bot commented Apr 5, 2024 •

edited

Loading