Backward compatibility in EmptyDirVolumeSource after v1.7.8 #56505

yue9944882 · 2017-11-28T17:41:30Z

In lower versions of k8s client sdk (golang, java..), requests can be incompatible with apiserver after v1.7.8 because of this commit. The field SizeLimit inEmptyDirVolumeSource struct is no longer a value but a pointer of resource.Quantity. And the default value of SizeLimit is {"Medium":"", "SizeLimit":"0"} but now a nil.

So this PR make apiserver able to process client request made by lower versions of k8s client sdk without simply throw out an error:

Forbidden: pod updates may not change fields other than spec.containers[ ].image, spec.initContainers[].image, spec.activeDeadlineSeconds or spec.tolerations (only additions to existing tolerations)

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #56381

Special notes for your reviewer:

Release note:

Provides compatibility of fields SizeLimit in types.EmptyDirVolumeSource since v1.7.8

k8s-ci-robot · 2017-11-28T17:41:33Z

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.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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.

yue9944882 · 2017-11-28T17:57:53Z

/assign @jbeda

dims · 2017-11-29T03:18:25Z

/ok-to-test

yue9944882 · 2017-12-01T04:13:48Z

hello? Anyone following up with this PR?
Thx

liggitt · 2017-12-01T04:26:43Z

pkg/apis/core/validation/validation.go

+			if mungedPod.Spec.Volumes[idx].EmptyDir.SizeLimit != nil {
+				// If EmptyDir.SizeLimit is set to "0", then take SizeLimit as a nil value
+				if mungedPod.Spec.Volumes[idx].EmptyDir.SizeLimit.String() == "0" && vol.EmptyDir.SizeLimit == nil {
+					mungedPod.Spec.Volumes[idx].EmptyDir.SizeLimit = nil


This field should already be dropped by this line prior to validation:

kubernetes/pkg/api/pod/util.go

Line 242 in c933067

podSpec.Volumes[i].EmptyDir.SizeLimit = nil

It seems that this DropDisabledAlphaFields function is written after 1.8.0 release.
But [v1.7.8~v1.8.0) didn't have this before validation, can I submit this PR onto release-1.7 branch?

It would be better to add the DropDisabledAlphaFields part to the 1.7.x branch than this

Ok, I will update this PR dat way.

Hi, Could you review my PR again plz? Thx

yue9944882 · 2017-12-02T05:47:00Z

/area apiserver

yue9944882 · 2017-12-04T04:59:36Z

@liggitt PTAL.Thx.

liggitt · 2017-12-04T21:43:19Z

/lgtm

k8s-github-robot · 2017-12-04T21:44:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, yue9944882

Associated issue: 56381

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

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

~~pkg/api/OWNERS~~ [liggitt]
~~pkg/registry/OWNERS~~ [liggitt]

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

yue9944882 · 2017-12-04T23:59:09Z

@liggitt @jbeda @yujuhong @jingxu97 Do you think about cherry-picking it？

liggitt · 2017-12-05T00:44:23Z

That means it's pending approval by the 1.7 release branch manager.

liggitt · 2017-12-05T01:30:39Z

^ @wojtek-t

wojtek-t · 2017-12-05T08:04:57Z

The PR obviously makes perfect sense for cherrypicking.

@yue9944882 @liggitt - can one of you add user-friendly release note to it?

yue9944882 · 2017-12-05T08:14:34Z

@wojtek-t PTAL. Release note updated.

wojtek-t · 2017-12-05T08:24:44Z

Thanks - approved.

k8s-github-robot · 2017-12-05T08:25:22Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-12-05T09:25:15Z

Automatic merge from submit-queue.

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 28, 2017

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 28, 2017

k8s-github-robot assigned jbeda and yujuhong Nov 28, 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 Nov 28, 2017

yujuhong assigned jingxu97 Nov 28, 2017

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 Nov 29, 2017

liggitt self-assigned this Dec 1, 2017

liggitt reviewed Dec 1, 2017

View reviewed changes

yue9944882 changed the base branch from master to release-1.7 December 1, 2017 04:49

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 1, 2017

k8s-github-robot added do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 1, 2017

yue9944882 changed the base branch from release-1.7 to master December 1, 2017 04:49

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 1, 2017

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2017

This comment has been minimized.

Sign in to view

k8s-ci-robot added the area/apiserver label Dec 2, 2017

Apply DropDisabledAlphaFields for pod create/update validation

7efae04

yue9944882 force-pushed the judge_sizelimit_backward_compatibility branch from a384ae0 to 7efae04 Compare December 2, 2017 15:15

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

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

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

liggitt assigned liggitt and wojtek-t and unassigned jbeda, liggitt, yujuhong and jingxu97 Dec 5, 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 Dec 5, 2017

wojtek-t added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. labels Dec 5, 2017

k8s-github-robot merged commit 7b3a07d into kubernetes:release-1.7 Dec 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backward compatibility in EmptyDirVolumeSource after v1.7.8 #56505

Backward compatibility in EmptyDirVolumeSource after v1.7.8 #56505

yue9944882 commented Nov 28, 2017 •

edited

Loading

k8s-ci-robot commented Nov 28, 2017

yue9944882 commented Nov 28, 2017

dims commented Nov 29, 2017

yue9944882 commented Dec 1, 2017

liggitt Dec 1, 2017 •

edited

Loading

yue9944882 Dec 1, 2017 •

edited

Loading

liggitt Dec 1, 2017

yue9944882 Dec 1, 2017

yue9944882 Dec 1, 2017

This comment has been minimized.

yue9944882 commented Dec 2, 2017

yue9944882 commented Dec 4, 2017

liggitt commented Dec 4, 2017

k8s-github-robot commented Dec 4, 2017

yue9944882 commented Dec 4, 2017 •

edited

Loading

liggitt commented Dec 5, 2017

liggitt commented Dec 5, 2017

wojtek-t commented Dec 5, 2017

yue9944882 commented Dec 5, 2017

wojtek-t commented Dec 5, 2017

k8s-github-robot commented Dec 5, 2017

k8s-github-robot commented Dec 5, 2017

Backward compatibility in EmptyDirVolumeSource after v1.7.8 #56505

Backward compatibility in EmptyDirVolumeSource after v1.7.8 #56505

Conversation

yue9944882 commented Nov 28, 2017 • edited Loading

k8s-ci-robot commented Nov 28, 2017

yue9944882 commented Nov 28, 2017

dims commented Nov 29, 2017

yue9944882 commented Dec 1, 2017

liggitt Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

yue9944882 Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

liggitt Dec 1, 2017

Choose a reason for hiding this comment

yue9944882 Dec 1, 2017

Choose a reason for hiding this comment

yue9944882 Dec 1, 2017

Choose a reason for hiding this comment

This comment has been minimized.

yue9944882 commented Dec 2, 2017

yue9944882 commented Dec 4, 2017

liggitt commented Dec 4, 2017

k8s-github-robot commented Dec 4, 2017

yue9944882 commented Dec 4, 2017 • edited Loading

liggitt commented Dec 5, 2017

liggitt commented Dec 5, 2017

wojtek-t commented Dec 5, 2017

yue9944882 commented Dec 5, 2017

wojtek-t commented Dec 5, 2017

k8s-github-robot commented Dec 5, 2017

k8s-github-robot commented Dec 5, 2017

yue9944882 commented Nov 28, 2017 •

edited

Loading

liggitt Dec 1, 2017 •

edited

Loading

yue9944882 Dec 1, 2017 •

edited

Loading

yue9944882 commented Dec 4, 2017 •

edited

Loading