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.

Sign up for GitHub

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

Jump to bottom

rbd: healer detect Kubernetes version for right StagingTargetPath #3207

Merged
merged 2 commits into from
Jun 24, 2022
Merged

rbd: healer detect Kubernetes version for right StagingTargetPath #3207

merged 2 commits into from
Jun 24, 2022

Conversation

pkalever
Copy link

@pkalever pkalever commented Jun 23, 2022 •
edited
Loading

Describe what this PR does

Kubernetes 1.24 and newer use a different path for staging the volume.
That means the CSI-driver is requested to mount the volume at an other
location, compared to previous versions of Kubernetes. CSI-drivers
implementing the volumeHealer, must receive the correct path,
otherwise the after a nodeplugin restart the NBD mounts will bailout
attempting to NodeStageVolume() call and return an error.

See-also: kubernetes/kubernetes#107065

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Fixes: #3176

Other

Related rook PR: rook/rook#10490

Credits

Thanks @nixpanic for csi-addons/kubernetes-csi-addons@3f23be1 which made the PR easy.

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

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!)
  • /retest all: run this in case the CentOS CI failed to start/report any test
    progress or results

@pkalever pkalever requested review from nixpanic, Rakshith-R, Madhu-1 and a team June 23, 2022 06:06
@mergify mergify bot added the component/rbd Issues related to RBD label Jun 23, 2022
yati1998
yati1998 reviewed Jun 23, 2022
View reviewed changes
Copy link
Contributor

@yati1998 yati1998 left a comment

Choose a reason for hiding this comment

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

LGTM

@pkalever
Copy link
Author

Related rook PR: rook/rook#10490

@Rakshith-R
Copy link
Contributor

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

@Rakshith-R
Copy link
Contributor

/retest ci/centos/mini-e2e-helm/k8s-1.24

@pkalever pkalever requested a review from yati1998 June 23, 2022 06:19
@pkalever
Copy link
Author

Diff b/w v1 and v2

Fixed a typo in the commit-msg.

@Rakshith-R
Copy link
Contributor

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

Madhu-1
Madhu-1 requested changes Jun 23, 2022
View reviewed changes
internal/rbd/rbd_healer.go Outdated Show resolved Hide resolved
internal/rbd/rbd_healer.go Show resolved Hide resolved
@pkalever pkalever requested a review from Madhu-1 June 23, 2022 07:57
Madhu-1
Madhu-1 approved these changes Jun 23, 2022
View reviewed changes
Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

squash commits into one?

@pkalever
Copy link
Author

squash commits into one?

Unless you insist, I would like to keep the deployment template changes in a separate commit, that actually helps me track them when similar changes where needed, I can always come back and check git log. Not just for this PR, if you see my commit history, I always prefer that, as I want to avoid messy commits, and keep them short as much as possible [something I learned from Michael Adam :-) ]. I also feel that makes reviewers' jobs easy.

@nixpanic nixpanic added bug Something isn't working backport-to-release-v3.6 labels Jun 23, 2022
@Rakshith-R
Copy link
Contributor

yay ! k8s 1.24 ci run is green too.
https://jenkins-ceph-csi.apps.ocp.ci.centos.org/blue/organizations/jenkins/mini-e2e_k8s-1.24/detail/mini-e2e_k8s-1.24/28/pipeline/

@pkalever pkalever added the regression This issues is a regression label Jun 23, 2022
@pkalever pkalever mentioned this pull request Jun 24, 2022
Merged
@pkalever
Copy link
Author

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2022

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

@Mergifyio rebase

@nixpanic nixpanic added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Jun 24, 2022
Prasanna Kumar Kalever added 2 commits June 24, 2022 08:21
@mergify
rbd: healer detect Kubernetes version for right StagingTargetPath
4a33607 
Kubernetes 1.24 and newer use a different path for staging the volume.
That means the CSI-driver is requested to mount the volume at an other
location, compared to previous versions of Kubernetes. CSI-drivers
implementing the volumeHealer, must receive the correct path, otherwise
the after a nodeplugin restart the NBD mounts will bailout attempting
to NodeStageVolume() call and return an error.

See-also: kubernetes/kubernetes#107065

Fixes: ceph#3176
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
@mergify
deploy: fix the staging path accordingly in the templates
be65d92 
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2022

rebase

✅ Branch has been successfully rebased

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/k8s-e2e-external-storage/1.22

@ceph-csi-bot
Copy link
Collaborator

@pkalever "ci/centos/k8s-e2e-external-storage/1.22" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2022

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/mini-e2e-helm/k8s-1.22

@ceph-csi-bot
Copy link
Collaborator

@pkalever "ci/centos/mini-e2e-helm/k8s-1.22" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2022

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@mergify mergify bot merged commit d3650ae into ceph:devel Jun 24, 2022
@mergify mergify bot mentioned this pull request Jun 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nixpanic nixpanic nixpanic approved these changes

@Madhu-1 Madhu-1 Madhu-1 approved these changes

@Rakshith-R Rakshith-R Rakshith-R approved these changes

@yati1998 yati1998 Awaiting requested review from yati1998

Assignees
No one assigned
Labels
bug Something isn't working ci/retry/e2e Label to retry e2e retesting on approved PR's component/rbd Issues related to RBD regression This issues is a regression
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rbd: rbd-nbd is not used as mounter in k8s 1.24
6 participants
@pkalever @Rakshith-R @nixpanic @ceph-csi-bot @Madhu-1 @yati1998