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: add mkfsOptions to the StorageClass and pass them to mkfs #3692

Merged
merged 5 commits into from
Mar 8, 2023
Merged

rbd: add mkfsOptions to the StorageClass and pass them to mkfs #3692

merged 5 commits into from
Mar 8, 2023

Conversation

nixpanic
Copy link
Member

Add mkfsOptions to the StorageClass and pass them to the mkfs
command while creating the filesystem on the RBD device.

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

@mergify mergify bot added the component/rbd Issues related to RBD label Feb 23, 2023
@nixpanic
Copy link
Member Author

nixpanic commented Mar 2, 2023

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

@nixpanic nixpanic force-pushed the rbd/mkfs-options branch 2 times, most recently from f3f2ae0 to f8b52fe Compare March 2, 2023 09:52
@nixpanic
Copy link
Member Author

nixpanic commented Mar 2, 2023

/test ci/centos/upgrade-tests-rbd

@nixpanic
Copy link
Member Author

nixpanic commented Mar 2, 2023

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

@nixpanic
Copy link
Member Author

nixpanic commented Mar 2, 2023

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

@nixpanic
Copy link
Member Author

nixpanic commented Mar 2, 2023

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

@nixpanic
Copy link
Member Author

nixpanic commented Mar 2, 2023

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

@nixpanic
Copy link
Member Author

nixpanic commented Mar 3, 2023

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

1 similar comment
@nixpanic
Copy link
Member Author

nixpanic commented Mar 3, 2023

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

@nixpanic
Copy link
Member Author

nixpanic commented Mar 3, 2023

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

2 similar comments
@nixpanic
Copy link
Member Author

nixpanic commented Mar 3, 2023

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

@nixpanic
Copy link
Member Author

nixpanic commented Mar 3, 2023

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

@nixpanic nixpanic force-pushed the rbd/mkfs-options branch 2 times, most recently from 565a6c7 to d0c7849 Compare March 6, 2023 14:02
@nixpanic
Copy link
Member Author

nixpanic commented Mar 6, 2023

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

@nixpanic
Copy link
Member Author

nixpanic commented Mar 6, 2023

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

@nixpanic nixpanic marked this pull request as ready for review March 7, 2023 09:49
@nixpanic nixpanic requested a review from a team March 7, 2023 09:50
Rakshith-R
Rakshith-R reviewed Mar 7, 2023
View reviewed changes
internal/rbd/nodeserver.go
case "xfs":
args = []string{"-K", devicePath}
Copy link
Contributor

Choose a reason for hiding this comment

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

"-K" also needs to move to newly added mkfsDefultArgs ?

mkfsDefaultArgs = map[string][]string{
		"ext4": {"-m0", "-Enodiscard,lazy_itable_init=1,lazy_journal_init=1"},
		"xfs": {"-K"},
	}

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, good catch!

humblec
humblec reviewed Mar 7, 2023
View reviewed changes
internal/rbd/nodeserver.go
@@ -101,6 +101,15 @@ var (
// xfsHasReflink is set by xfsSupportsReflink(), use the function when
// checking the support for reflink.
xfsHasReflink = xfsReflinkUnset

mkfsDefaultArgs = map[string][]string{
"ext4": {"-m0", "-Enodiscard,lazy_itable_init=1,lazy_journal_init=1"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

having some references to these options and why we enable them as default options could help

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, but I don't know where they come from. This PR just moves them around...

@nixpanic nixpanic requested a review from humblec March 7, 2023 14:32
@nixpanic nixpanic requested a review from a team March 8, 2023 13:03
@nixpanic
Copy link
Member Author

nixpanic commented Mar 8, 2023

@Mergifyio rebase

nixpanic added 5 commits March 8, 2023 14:03
@nixpanic
rbd: cleanup passing mkfs arguments for NodeStageVolume
7d07b4a 
Storing the default `mkfs` arguments in a map with key per filesystem
type makes this a little more modular. It prepares th code for fetching
the `mkfs` arguments from the VolumeContext.

Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
rbd: allow setting mkfsOptions in the StorageClass
8d2f13d 
Add `mkfsOptions` to the StorageClass and pass them to the `mkfs`
command while creating the filesystem on the RBD device.

Fixes: ceph#374
Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
e2e: replace CephFS in messages for RBD tests
f30f086 
Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
e2e: add test for mkfsOptions in RBD StorageClass
6fc3dfc 
Create a 1G volume, with ext4 and only 1024 inodes. Use `df` to check if
the number of inodes is correct.

Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
rbd: do not run mkfs on a BlockMode volume
4f19b9f 
Signed-off-by: Niels de Vos <[email protected]>
@mergify
Copy link
Contributor

mergify bot commented Mar 8, 2023

rebase

✅ Branch has been successfully rebased

@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Mar 8, 2023
@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

/test ci/centos/upgrade-tests-cephfs

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

/test ci/centos/upgrade-tests-rbd

@github-actions github-actions bot removed the ok-to-test Label to trigger E2E tests label Mar 8, 2023
@mergify mergify bot merged commit 37c8f07 into ceph:devel Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@humblec humblec humblec approved these changes

@yati1998 yati1998 yati1998 approved these changes

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

Assignees
No one assigned
Labels
component/rbd Issues related to RBD
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nixpanic @humblec @yati1998 @Rakshith-R