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

Add support & e2e for mountOptions & efficient selinux relabelling support #3902

Merged
merged 3 commits into from
Jun 19, 2023
Merged

Add support & e2e for mountOptions & efficient selinux relabelling support #3902

merged 3 commits into from
Jun 19, 2023

Conversation

Rakshith-R
Copy link
Contributor

@Rakshith-R Rakshith-R commented Jun 13, 2023 •
edited
Loading

  • pv.Spec.MountOptions will now be added during NodePublish as well as NodeStage Request.
  • SeLinuxMount: true option is set in csidriver object.
  • e2e testcases is added to test presence of mountOptions during mounting by all three csidriver nodeplugins.

Note:

refer: https://kubernetes.io/blog/2023/04/18/kubernetes-1-27-efficient-selinux-relabeling-beta/

Resolves: #3855 #757

@mergify mergify bot added the component/testing Additional test cases or CI work label Jun 13, 2023
@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R Rakshith-R added the ci/skip/multi-arch-build skip building on multiple architectures label Jun 14, 2023
@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R Rakshith-R mentioned this pull request Jun 14, 2023
Closed
@Rakshith-R Rakshith-R added component/cephfs Issues related to CephFS and removed ci/skip/multi-arch-build skip building on multiple architectures labels Jun 14, 2023
@Rakshith-R Rakshith-R linked an issue Jun 14, 2023 that may be closed by this pull request
Honor mount options in NodeStage RPC as well #757
Closed
@Rakshith-R Rakshith-R marked this pull request as ready for review June 14, 2023 08:52
Madhu-1
Madhu-1 reviewed Jun 14, 2023
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.

Need to remove WIP from PR title

e2e/pod.go
pv.Spec.MountOptions = []string{mountOption}

// update PV
_, err = f.ClientSet.CoreV1().PersistentVolumes().Update(context.TODO(), pv, metav1.UpdateOptions{})
Copy link
Collaborator

Choose a reason for hiding this comment

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

create one ctx and reuse it.

e2e/pod.go Show resolved Hide resolved
@Rakshith-R Rakshith-R changed the title [WIP] Add support & e2e for mountOptions & efficient selinux relabelling support Add support & e2e for mountOptions & efficient selinux relabelling support Jun 14, 2023
@Rakshith-R
Copy link
Contributor Author

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

nixpanic
nixpanic reviewed Jun 14, 2023
View reviewed changes
e2e/utils.go Outdated
}
if len(mountOptions) > 0 {
// modify PV spec.MountOptions
pv, err := getBoundPV(f.ClientSet, pvc)
Copy link
Member

Choose a reason for hiding this comment

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

e2e/utils.go:786:7: shadow: declaration of "err" shadows declaration at line 774 (govet)
		pv, err := getBoundPV(f.ClientSet, pvc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e2e/utils.go:786:7: shadow: declaration of "err" shadows declaration at line 774 (govet)
		pv, err := getBoundPV(f.ClientSet, pvc)

Current version of pr does not suffer from this error.
ptal.

Madhu-1
Madhu-1 previously approved these changes Jun 14, 2023
View reviewed changes
@Rakshith-R
Copy link
Contributor Author

Raised issue for the failed ci. It is not related to this pr.
#3910

nixpanic
nixpanic previously approved these changes Jun 16, 2023
View reviewed changes
@Rakshith-R Rakshith-R mentioned this pull request Jun 16, 2023
Closed
2 tasks
@Rakshith-R
e2e: add test cases for pv.Spec.MountOptions
2e43cb9 
Signed-off-by: Rakshith R <rar@redhat.com>
@mergify mergify bot dismissed stale reviews from nixpanic and Madhu-1 June 16, 2023 12:11

Pull request has been modified.

Rakshith-R
Rakshith-R commented Jun 16, 2023
View reviewed changes
e2e/cephfs_helper.go
Comment on lines +69 to +74
// TODO: remove this once the ceph-csi driver release-v3.9 is completed
// and upgrade tests are done from v3.9 to devel.
// The mountOptions from previous are not compatible with NodeStageVolume
// request.
sc.MountOptions = []string{}

Copy link
Contributor Author

@Rakshith-R Rakshith-R Jun 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

CephFS upgrade tests pick up cephfs sc from previous release branches which still
have debug mountoption in sc yamls.
Adding this workaround untill we set upgrade test from 3.9 to devel.

I'll add a note regarding this breaking change in release notes too.
(users will need to recreate cephfs storage without debug option and
remove it from pvs too.)

added issue to track this one #3911

I'll ask re-review requests after all the ci is green.
(:sweat_smile: this pr seems to be giving more ci issues than expected.)

@Rakshith-R Rakshith-R added the ok-to-test Label to trigger E2E tests label Jun 16, 2023
@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jun 16, 2023
@Rakshith-R
Copy link
Contributor Author

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

@Rakshith-R
Copy link
Contributor Author

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

reported a new issue with logs here #3913

@Rakshith-R
Copy link
Contributor Author

Ci is finally green! 🎊
PTAL

@mergify mergify bot merged commit 54eeac2 into ceph:devel Jun 19, 2023
@iPraveenParihar iPraveenParihar mentioned this pull request Mar 28, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@nixpanic nixpanic nixpanic approved these changes

Assignees
No one assigned
Labels
component/cephfs Issues related to CephFS component/testing Additional test cases or CI work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Efficient SELinux volume relabeling Honor mount options in NodeStage RPC as well
4 participants
@Rakshith-R @nixpanic @ceph-csi-bot @Madhu-1