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

Support kubectl alpha debug default image imagePullPolicy set by the image tag #94896

Merged
merged 1 commit into from Oct 3, 2020

Conversation

wawa0210
Copy link
Contributor

@wawa0210 wawa0210 commented Sep 18, 2020

What type of PR is this?

/kind bug
/kind feature

What this PR does / why we need it:

The default imagePullPolicy of kubectl alpha debug is PullIfNotPresent rather than set according to whether the image tag is :latest.
This will cause the latest image to be executed without pull, if there is already a :latest image locally

Which issue(s) this PR fixes:

Fixes #92383

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Ephemeral containers now apply the same API defaults as initContainers and containers

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Sep 18, 2020
@wawa0210 wawa0210 changed the title Support kubectl alpha debug default image imagePullPolicy set according to the image tag Support kubectl alpha debug default image imagePullPolicy set by the image tag Sep 18, 2020
@k8s-ci-robot k8s-ci-robot added area/kubectl sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 18, 2020
@wawa0210
Copy link
Contributor Author

wawa0210 commented Sep 18, 2020

@verb
Can you help me review this pr, thank you very much

At present, there is only one method in the entire k8s to set imagePullPolicy according to the mirror tag

func SetDefaults_Container(obj *v1.Container) {
if obj.ImagePullPolicy == "" {
// Ignore error and assume it has been validated elsewhere
_, tag, _, _ := parsers.ParseImageName(obj.Image)
// Check image tag
if tag == "latest" {
obj.ImagePullPolicy = v1.PullAlways
} else {
obj.ImagePullPolicy = v1.PullIfNotPresent
}
}

But the code in kubectl is not allowed to import k8s.io/kubernetes/pkg/apis/core/v1 directly, do you have any suggestions?

@wawa0210 wawa0210 changed the title Support kubectl alpha debug default image imagePullPolicy set by the image tag [DRAFT]Support kubectl alpha debug default image imagePullPolicy set by the image tag Sep 18, 2020
@wawa0210 wawa0210 marked this pull request as draft September 18, 2020 15:59
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 18, 2020
@wawa0210 wawa0210 changed the title [DRAFT]Support kubectl alpha debug default image imagePullPolicy set by the image tag Support kubectl alpha debug default image imagePullPolicy set by the image tag Sep 18, 2020
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 19, 2020
@wawa0210 wawa0210 marked this pull request as ready for review September 19, 2020 03:50
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 19, 2020
@dims
Copy link
Member

dims commented Sep 19, 2020

/assign @verb

@wawa0210 wawa0210 force-pushed the fix-92383 branch 2 times, most recently from 62d653d to 94b532c Compare September 21, 2020 11:19
@k8s-ci-robot k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Sep 21, 2020
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 23, 2020
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 23, 2020
Copy link
Contributor

@verb verb left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

/lgtm
/retest

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 23, 2020
Copy link
Contributor

@zhouya0 zhouya0 left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 23, 2020
@wawa0210
Copy link
Contributor Author

/test pull-kubernetes-bazel-test

@zhouya0
Copy link
Contributor

zhouya0 commented Sep 24, 2020

/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 24, 2020
@wawa0210
Copy link
Contributor Author

@liggitt @dims

Can you help review this pr?

@verb
Copy link
Contributor

verb commented Sep 24, 2020

/lgtm

@dims
Copy link
Member

dims commented Sep 25, 2020

/assign @sjenning @derekwaynecarr

@liggitt
Copy link
Member

liggitt commented Oct 3, 2020

/lgtm
/approve

@liggitt liggitt added this to API review completed, 1.20 in API Reviews Oct 3, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 3, 2020
@k8s-ci-robot k8s-ci-robot merged commit 1ebf64d into kubernetes:master Oct 3, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Oct 3, 2020
@wawa0210 wawa0210 deleted the fix-92383 branch October 13, 2020 02:21
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. area/kubectl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
Status: API review completed, 1.20
Development

Successfully merging this pull request may close these issues.

imagePullPolicy does not default to Always for :latest images with ephemeral containers
8 participants