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.

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

Made image as deliberately optional in v1 Container struct. #48406

Merged
merged 3 commits into from
Sep 21, 2017

Conversation

gyliu513
Copy link
Contributor

@gyliu513 gyliu513 commented Jul 3, 2017

What this PR does / why we need it:
Revert #47246
Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

Mark image as deliberately optional in v1 Container struct.  Many objects in the Kubernetes API inherit the container struct and only Pods require the field to be set.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 3, 2017
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 3, 2017
@gyliu513
Copy link
Contributor Author

gyliu513 commented Jul 3, 2017

/cc @bgrant0607 @smarterclayton

@gyliu513
Copy link
Contributor Author

gyliu513 commented Jul 3, 2017

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jul 3, 2017
@smarterclayton
Copy link
Contributor

Hold on this, conversation in the other thread.

@lavalamp lavalamp added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. and removed sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Jul 13, 2017
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2017
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 23, 2017
@smarterclayton smarterclayton added this to the v1.8 milestone Sep 1, 2017
@smarterclayton
Copy link
Contributor

Marking this a candidate, still needs discussion. Not required for feature freeze but don't want to forget it.

@bgrant0607
Copy link
Member

@smarterclayton Which other thread? #47246? Indeed, a user there reported that this broke them, since validation runs in kubectl. I think we need to revert the change and backport it to 1.7. I'd even like to remove the validation from the controllers in v1beta2 in 1.9 and just have them not create pods if the field is empty.

@bgrant0607 bgrant0607 assigned kow3ns and unassigned mikedanese Sep 1, 2017
@smarterclayton
Copy link
Contributor

Agreed, revert and backport. Just qualifies as bug.

@calebamiles calebamiles modified the milestone: v1.8 Sep 2, 2017
@smarterclayton
Copy link
Contributor

Can you rebase so we can get this merged and back ported?

@smarterclayton
Copy link
Contributor

/test pull-kubernetes-bazel-build

@dims
Copy link
Member

dims commented Sep 18, 2017

/retest

@dims
Copy link
Member

dims commented Sep 19, 2017

/test all

@dims
Copy link
Member

dims commented Sep 19, 2017

@gyliu513 - Can you please see the failures and help get the tests go green? i am afraid this will miss the 1.8 otherwise.

@gyliu513
Copy link
Contributor Author

@dims I filed an issue here #52727 reporting some errors when hack/update-all.sh, can you help check? I suspect the error may cause some files can not be generated.

@gyliu513 gyliu513 force-pushed the image-optional branch 2 times, most recently from c88012d to 2a60da0 Compare September 20, 2017 03:16
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 20, 2017
@gyliu513
Copy link
Contributor Author

/retest

@gyliu513
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce-bazel

@gyliu513
Copy link
Contributor Author

@smarterclayton @dims

@gyliu513
Copy link
Contributor Author

/release-note

@k8s-ci-robot 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 Sep 20, 2017
@smarterclayton
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 20, 2017
@spiffxp
Copy link
Member

spiffxp commented Sep 20, 2017

@smarterclayton this needs an /approve no-issue or @gyliu513 needs to refer to an issue (not PR) in the "fixes #" part of the PR description before the bot will add an approved label

@smarterclayton
Copy link
Contributor

smarterclayton commented Sep 21, 2017 via email

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gyliu513, smarterclayton

Associated issue requirement bypassed by: smarterclayton

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

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

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 k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 21, 2017
@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 48406, 52819). If you want to cherry-pick this change to another branch, please follow the instructions here..

@k8s-github-robot k8s-github-robot merged commit 1a8bf2d into kubernetes:master Sep 21, 2017
@gyliu513 gyliu513 deleted the image-optional branch September 22, 2017 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.