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

use k8s.gcr.io/pause instead of kubernetes/pause #69631

Merged
merged 1 commit into from
Oct 12, 2018
Merged

use k8s.gcr.io/pause instead of kubernetes/pause #69631

merged 1 commit into from
Oct 12, 2018

Conversation

Huang-Wei
Copy link
Member

What this PR does / why we need it:

According to testing-conventions, we should use grc.io instead of docker hub.

This PR changes occurrences of kubernetes/pause to k8s.gcr.io/pause.

Special notes for your reviewer:

Release note:

NONE

/kind cleanup
/sig testing

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/testing Categorizes an issue or PR as relevant to SIG Testing. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 10, 2018
@Huang-Wei
Copy link
Member Author

Kindly cc @fejta @ixdy @rmmh @spiffxp for approval.

@spiffxp
Copy link
Member

spiffxp commented Oct 10, 2018

While you're here, I think you missed an occurrence

pkg/kubelet/dockershim/libdocker/helpers_test.go
54:			Inspected: dockertypes.ImageInspect{RepoTags: []string{"docker.io/kubernetes/pause:latest"}},
55:			Image:     "kubernetes/pause:latest",

@Huang-Wei
Copy link
Member Author

@spiffxp thanks pointing out.

Not quite sure if RepoTags: []string{"docker.io/kubernetes/pause:latest"} has specific semantics. Let me change that also to see if it works.

@k8s-ci-robot k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. labels Oct 11, 2018
@ixdy
Copy link
Member

ixdy commented Oct 11, 2018

Wow, the kubernetes/pause image in docker hub is 3 years old. Good catch!

Can you also update these to use the newest tag, 3.1? latest on k8s.gcr.io points to the 1.0 tag, which is definitely not latest.

@Huang-Wei
Copy link
Member Author

@ixdy sure, updated to use k8s.gcr.io/pause:3.1.

ixdy
ixdy reviewed Oct 11, 2018
View reviewed changes
pkg/kubelet/dockershim/libdocker/helpers_test.go Outdated
@@ -51,8 +51,8 @@ func TestMatchImageTagOrSHA(t *testing.T) {
Output: true,
},
{
Inspected: dockertypes.ImageInspect{RepoTags: []string{"docker.io/kubernetes/pause:latest"}},
Image: "kubernetes/pause:latest",
Inspected: dockertypes.ImageInspect{RepoTags: []string{"k8s.gcr.io/pause:3.1"}},
Copy link
Member

Choose a reason for hiding this comment

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

hm, actually, maybe we should have left this one? I'm not sure what exactly this test is doing, but the docker.io bit may have been relevant here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was also a little hesitant on this.

Kindly ping @yujuhong: do you see any side effect of this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't change this. The test verifies that the two match even with the docker.io difference.

Copy link
Member Author

Choose a reason for hiding this comment

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

@yujuhong thanks for clarifying! I will keep it as is.

@ixdy
Copy link
Member

ixdy commented Oct 12, 2018

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Huang-Wei, ixdy

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 12, 2018
@k8s-ci-robot k8s-ci-robot merged commit 4d4c906 into kubernetes:master Oct 12, 2018
@Huang-Wei Huang-Wei deleted the use-gcr-io-pause branch October 13, 2018 06:24
@spiffxp spiffxp mentioned this pull request Apr 30, 2019
Closed
This was referenced Apr 30, 2019
Closed
Merged
k8s-ci-robot added a commit that referenced this pull request May 2, 2019
@k8s-ci-robot
Merge pull request #77292 from cblecker/automated-cherry-pick-of-#696…
3146b27 
…31-upstream-release-1.12

Automated cherry pick of #69631: use k8s.gcr.io/pause instead of kubernetes/pause
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ixdy ixdy ixdy left review comments

@yujuhong yujuhong yujuhong left review comments

@janetkuo janetkuo Awaiting requested review from janetkuo

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

Assignees

@ixdy ixdy

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Huang-Wei @spiffxp @ixdy @k8s-ci-robot @yujuhong