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

rbd: cleanup some parts in the ControllerServer #5101

Open
wants to merge 7 commits into
base: devel
Choose a base branch
from

Conversation

nixpanic
Copy link
Member

Miscellaneous cleanups. This should help in restructuring the rbd package with cleaner APIs. Eventually the rbdImage, rbdVolume and rbdSnapshot types should be used as a interface/type similar to VolumeGroup provided by the internal/rbd/group package.

  1. remove unused ControllerPublishVolume and ControllerUnpublishVolume

The RBD ControllerService does not expose the PUBLISH_UNPUBLISH_VOLUME
capability, so ControllerPublishVolume and ControllerUnpublishVolume
will never get called.

  1. move cleanUpImageAndSnapReservation() into rbdSnapshot.Delete()

  2. make RequestName internal to rbdVolume

  3. use rbd.Manager within ControllerServer.DeleteVolume() and ControllerServer.DeleteSnapshot()


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!)

@mergify mergify bot added the component/rbd Issues related to RBD label Jan 22, 2025
@nixpanic nixpanic force-pushed the cleanup/rbd/controllerserver branch from 8773c14 to 24a66af Compare January 22, 2025 09:29
@nixpanic nixpanic requested a review from a team January 24, 2025 13:18
log.DebugLog(ctx, "created image %s backed for request name %s", rbdVol, rbdVol.RequestName)
requestName, err := rbdVol.GetRequestName(ctx)
if err != nil {
requestName = "<unset request name>"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nixpanic why is this special case not an error case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is only used for logging, and was not an error before this change.

@nixpanic nixpanic requested a review from Madhu-1 January 30, 2025 15:53
Madhu-1
Madhu-1 previously approved these changes Feb 3, 2025
iPraveenParihar
iPraveenParihar previously approved these changes Feb 4, 2025
@iPraveenParihar
Copy link
Contributor

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

@iPraveenParihar
Copy link
Contributor

@nixpanic, the ci test failed /DeleteVolume

  I0204 05:41:03.214520       1 utils.go:266] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 GRPC call: /csi.v1.Controller/DeleteVolume
  I0204 05:41:03.214565       1 utils.go:267] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 GRPC request: {"secrets":"***stripped***","volume_id":"0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578"}
  E0204 05:41:03.217006       1 omap.go:80] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 omap not found (pool="replicapool", namespace="", name="csi.volume.a900e70b-0bff-40c7-9503-97d5912ac578"): rados: ret=-2, No such file or directory
  W0204 05:41:03.217030       1 voljournal.go:737] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 unable to read omap keys: pool or key missing: key not found: rados: ret=-2, No such file or directory
  E0204 05:41:03.221035       1 rbd_journal.go:719] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 failed to get image id replicapool/csi-vol-a900e70b-0bff-40c7-9503-97d5912ac578: Failed as image not found (internal RBD image not found: rbd: ret=-2, No such file or directory)
  E0204 05:41:03.221103       1 utils.go:271] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 GRPC error: rpc error: code = Internal desc = BUG: failed to cast Volume to rbdVolume

The RBD ControllerService does not expose the `PUBLISH_UNPUBLISH_VOLUME`
capability, so ControllerPublishVolume and ControllerUnpublishVolume
will never get called.

In case a broken Container Orchestrator does call these operations, a
default Unimplemented error will be returned anyway.

Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
Copy link
Member Author

nixpanic commented Feb 4, 2025

@nixpanic, the ci test failed /DeleteVolume

  I0204 05:41:03.214520       1 utils.go:266] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 GRPC call: /csi.v1.Controller/DeleteVolume
  I0204 05:41:03.214565       1 utils.go:267] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 GRPC request: {"secrets":"***stripped***","volume_id":"0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578"}
  E0204 05:41:03.217006       1 omap.go:80] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 omap not found (pool="replicapool", namespace="", name="csi.volume.a900e70b-0bff-40c7-9503-97d5912ac578"): rados: ret=-2, No such file or directory
  W0204 05:41:03.217030       1 voljournal.go:737] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 unable to read omap keys: pool or key missing: key not found: rados: ret=-2, No such file or directory
  E0204 05:41:03.221035       1 rbd_journal.go:719] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 failed to get image id replicapool/csi-vol-a900e70b-0bff-40c7-9503-97d5912ac578: Failed as image not found (internal RBD image not found: rbd: ret=-2, No such file or directory)
  E0204 05:41:03.221103       1 utils.go:271] ID: 38 Req-ID: 0001-0024-5697adce-843d-491b-beba-1455aff56431-0000000000000004-a900e70b-0bff-40c7-9503-97d5912ac578 GRPC error: rpc error: code = Internal desc = BUG: failed to cast Volume to rbdVolume

Thanks! It looks like GenVolFromVolID() always returns a *rbdVolume, even if an error occurred (yuck!). Converting nil to a *rbdVolume will fail, hence the error and test failure. I'll modify it a little and check if that works better.

@nixpanic nixpanic force-pushed the cleanup/rbd/controllerserver branch from 24a66af to 6d8882f Compare February 4, 2025 17:35
@nixpanic
Copy link
Member Author

nixpanic commented Feb 4, 2025

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

@mergify mergify bot dismissed stale reviews from Madhu-1 and iPraveenParihar February 4, 2025 17:35

Pull request has been modified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/rbd Issues related to RBD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants