storageclass ceph add imageformat parameter #45805

weiwei04 · 2017-05-15T04:26:20Z

What this PR does / why we need it:
Add a imageformat parameter for StorageClass(ceph rbd)

k8s hard coded ceph imageformat 1, according to ceph manual, imageformat 1 was deprecated, we should add an extra ceph parameter to set ceph rbd imageformat. Ceph rbd imageformat can only be 1 or 2, set the default value to 1.

Release note:

Allow StorageClass Ceph RBD to specify image format and image features.

k8s-ci-robot · 2017-05-15T04:26:27Z

Hi @weiwei04. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

weiwei04 · 2017-05-17T02:23:13Z

/assign @saad-ali please take a look, thanks.

weiwei04 · 2017-05-19T01:47:52Z

@saad-ali any comments on this pr?

weiwei04 · 2017-06-02T06:40:01Z

/assign @rootfs would you please take a look, thanks.

rootfs · 2017-06-02T12:46:55Z

pkg/volume/rbd/rbd.go

 		default:
 			return nil, fmt.Errorf("invalid option %q for volume plugin %s", k, r.plugin.GetPluginName())
 		}
 	}
 	// sanity check
+	if imageFormat != rbdImageFormat1 && imageFormat != rbdImageFormat2 {
+		return nil, fmt.Errorf("invalid ceph imageformat %s, neither 1 or 2", imageFormat)


fmt.Error("invalid ..., expecting %s or %s", imageFormat1, imageFormat2)

rootfs · 2017-06-02T12:47:55Z

@k8s-bot ok to test

rootfs · 2017-06-02T12:52:29Z

Though image format 1 is deprecated, it is still used by krbd (which rbd volume is based on). Newer kernel starts to support format 2 but many linux distros don't have them yet. The plan is to wait till they pick up the newer kernel and we add image format options.

weiwei04 · 2017-06-04T03:31:30Z

@k8s-bot pull-kubernetes-federation-e2e-gce test this

weiwei04 · 2017-06-04T04:40:40Z

@rootfs modified the error log, please take a look, thanks.

rootfs · 2017-06-04T13:28:47Z

@leseb

rootfs · 2017-06-04T13:33:04Z

@weiwei04 please update readme and more diagnostic messages when older krbd failes to map v2 image

k8s-ci-robot · 2017-06-16T12:15:04Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

cofyc · 2017-06-16T12:50:42Z

Bump to verify CLA.

rootfs · 2017-06-16T13:21:47Z

examples/persistent-volume-provisioning/README.md

@@ -192,6 +193,10 @@ parameters:
 * `pool`: Ceph RBD pool. Default is "rbd".
 * `userId`: Ceph client ID that is used to map the RBD image. Default is the same as `adminId`.
 * `userSecretName`: The name of Ceph Secret for `userId` to map RBD image. It must exist in the same namespace as PVCs. It is required.
+* `imageFormat`: Ceph RBD image format, "1" or "2". Default is "1".
+* `imageFeatures`: Ceph RBD image format 2 features, comma delimited. This is optional, and only be used if you set `imageFormat` to "2". For a complete list of available image features, please refer to [RBD docs](http://docs.ceph.com/docs/master/man/8/rbd/). By default, all features (except for striping) will be enabled.


Not all features are supported and unfortunately RBD docs are not well updated.

rootfs · 2017-06-16T13:24:47Z

@weiwei04 @cofyc The readme and feature validation need some work, see my closed PR here
#43816

We cannot turn on exclusive lock feature for now, exclusive lock and advisory lock cannot work together. For now, just turn on layering feature and we'll revisit exclusive lock later.

weiwei04 · 2017-06-19T06:39:12Z

/release-note

leseb

@rootfs lgtm

cofyc · 2017-06-22T16:41:36Z

@rootfs
Is there anything else I need to fix? Are you ok with RBD new parameters, and theirs default behaviors?

rootfs · 2017-06-22T16:54:05Z

/lgtm

thanks for contributing!

rootfs · 2017-06-22T16:54:14Z

/approve

eparis · 2017-06-22T16:59:15Z

/approve no-issue

k8s-github-robot · 2017-06-22T16:59:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: eparis, rootfs, weiwei04

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~examples/OWNERS~~ [eparis]
~~pkg/volume/rbd/OWNERS~~ [rootfs]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-06-23T00:49:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eparis, rootfs, weiwei04

Associated issue requirement bypassed by: eparis

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~examples/OWNERS~~ [eparis]
~~pkg/volume/rbd/OWNERS~~ [rootfs]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-06-23T18:38:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eparis, rootfs, weiwei04

Associated issue requirement bypassed by: eparis

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~examples/OWNERS~~ [eparis]
~~pkg/volume/rbd/OWNERS~~ [rootfs]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-06-23T22:36:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eparis, rootfs, weiwei04

Associated issue requirement bypassed by: eparis

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~examples/OWNERS~~ [eparis]
~~pkg/volume/rbd/OWNERS~~ [rootfs]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-06-23T23:00:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eparis, rootfs, weiwei04

Associated issue requirement bypassed by: eparis

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~examples/OWNERS~~ [eparis]
~~pkg/volume/rbd/OWNERS~~ [rootfs]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

ianchakeres · 2017-06-23T23:41:12Z

/sig storage

k8s-github-robot · 2017-06-23T23:43:37Z

Automatic merge from submit-queue

ianchakeres · 2017-06-23T23:53:45Z

examples/persistent-volume-provisioning/README.md

@@ -209,6 +209,7 @@ parameters:
    pool: kube
    userId: kube
    userSecretName: ceph-secret-user
+    imageFormat: "1"


Does "1" need to be in double quotes?

Yes, it need to be in double quotes, because parameters is a map of string to string.

ianchakeres · 2017-06-23T23:56:51Z

Will there ever be a case when imageFormat will be nil or ""? I'm not sure exactly where in the lifecycle of a PV Provision() is called.

For example, if a PV were created before this code was merged.

cofyc · 2017-06-26T08:42:36Z

@ianchakeres

hi,

imageFormat type is string, so it will never to nil. And it will never be empty string (""), because imageFormat is invalid option before this code is merged.

After this code is merged, imageFormat can be missing (not configured) or one of possible values ("1" or "2"). If imageFormat is configured, Provisioner will use configured value to create PVs. If imageFormat is missing, Provisioner will use default value ("1") to create PVs, so old RBD StorageClasses which do not (and can not) specify imageFormat parameter will continue work as before.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 15, 2017

k8s-github-robot assigned saad-ali and smarterclayton May 15, 2017

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels May 15, 2017

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 15, 2017

rootfs reviewed Jun 2, 2017

View reviewed changes

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 2, 2017

weiwei04 force-pushed the ceph-imageformat-2 branch from d9ed3db to 36eada0 Compare June 4, 2017 01:59

k8s-ci-robot removed the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 16, 2017

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jun 16, 2017

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 16, 2017

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 16, 2017

cofyc force-pushed the ceph-imageformat-2 branch from 28adc3a to 60f8ae5 Compare June 16, 2017 12:54

rootfs reviewed Jun 16, 2017

View reviewed changes

k8s-ci-robot removed the release-note-label-needed label Jun 19, 2017

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jun 19, 2017

leseb approved these changes Jun 19, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 22, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2017

k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Jun 23, 2017

k8s-github-robot merged commit e554dd6 into kubernetes:master Jun 23, 2017

ianchakeres reviewed Jun 23, 2017

View reviewed changes

PellePedro mentioned this pull request Jul 25, 2017

rbd: image is locked by other nodes ceph/ceph-container#642

Closed

This was referenced Aug 13, 2017

Added imageFormat and imageFeatures for rbd kubernetes/examples#63

Merged

Added rbd imageFormat and imageFeatures kubernetes/website#4805

Merged

weiwei04 deleted the ceph-imageformat-2 branch September 18, 2017 03:50

aemneina mentioned this pull request Oct 8, 2017

Kubernetes 1.7.4 controller-manager behaves like 1.6 for Ceph volume controller rancher/rancher#9944

Closed

cofyc mentioned this pull request Nov 28, 2017

Add an option to specify supported image features to allow kubernetes-retired/external-storage#481

Closed

wenjun93 mentioned this pull request Jan 8, 2018

rbd volume plugin should support image feature adjustment according to kernel version #57960

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

storageclass ceph add imageformat parameter #45805

storageclass ceph add imageformat parameter #45805

weiwei04 commented May 15, 2017 •

edited

Loading

k8s-ci-robot commented May 15, 2017

weiwei04 commented May 17, 2017 •

edited

Loading

weiwei04 commented May 19, 2017

weiwei04 commented Jun 2, 2017

rootfs Jun 2, 2017

rootfs commented Jun 2, 2017

rootfs commented Jun 2, 2017

weiwei04 commented Jun 4, 2017

weiwei04 commented Jun 4, 2017

rootfs commented Jun 4, 2017

rootfs commented Jun 4, 2017

k8s-ci-robot commented Jun 16, 2017

cofyc commented Jun 16, 2017

rootfs Jun 16, 2017

rootfs commented Jun 16, 2017

weiwei04 commented Jun 19, 2017

leseb left a comment

cofyc commented Jun 22, 2017

rootfs commented Jun 22, 2017

rootfs commented Jun 22, 2017

eparis commented Jun 22, 2017 •

edited

Loading

k8s-github-robot commented Jun 22, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

ianchakeres commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

ianchakeres Jun 23, 2017

cofyc Jun 26, 2017

ianchakeres commented Jun 23, 2017

cofyc commented Jun 26, 2017 •

edited

Loading

storageclass ceph add imageformat parameter #45805

storageclass ceph add imageformat parameter #45805

Conversation

weiwei04 commented May 15, 2017 • edited Loading

k8s-ci-robot commented May 15, 2017

weiwei04 commented May 17, 2017 • edited Loading

weiwei04 commented May 19, 2017

weiwei04 commented Jun 2, 2017

rootfs Jun 2, 2017

Choose a reason for hiding this comment

rootfs commented Jun 2, 2017

rootfs commented Jun 2, 2017

weiwei04 commented Jun 4, 2017

weiwei04 commented Jun 4, 2017

rootfs commented Jun 4, 2017

rootfs commented Jun 4, 2017

k8s-ci-robot commented Jun 16, 2017

cofyc commented Jun 16, 2017

rootfs Jun 16, 2017

Choose a reason for hiding this comment

rootfs commented Jun 16, 2017

weiwei04 commented Jun 19, 2017

leseb left a comment

Choose a reason for hiding this comment

cofyc commented Jun 22, 2017

rootfs commented Jun 22, 2017

rootfs commented Jun 22, 2017

eparis commented Jun 22, 2017 • edited Loading

k8s-github-robot commented Jun 22, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

ianchakeres commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

ianchakeres Jun 23, 2017

Choose a reason for hiding this comment

cofyc Jun 26, 2017

Choose a reason for hiding this comment

ianchakeres commented Jun 23, 2017

cofyc commented Jun 26, 2017 • edited Loading

weiwei04 commented May 15, 2017 •

edited

Loading

weiwei04 commented May 17, 2017 •

edited

Loading

eparis commented Jun 22, 2017 •

edited

Loading

cofyc commented Jun 26, 2017 •

edited

Loading