Skip to content

Fix docker registry used in e2e test.#67061

Merged
k8s-github-robot merged 1 commit intokubernetes:masterfrom
Random-Liu:fix-docker-registry
Aug 8, 2018
Merged

Fix docker registry used in e2e test.#67061
k8s-github-robot merged 1 commit intokubernetes:masterfrom
Random-Liu:fix-docker-registry

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

See #66055 (comment).

Fix docker registry used in e2e test, so that it works with all container runtimes.

Release note:

none

/cc @kubernetes/sig-node-pr-reviews @kubernetes/sig-testing-pr-reviews

@k8s-ci-robot k8s-ci-robot added 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. 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 Aug 7, 2018
@Random-Liu Random-Liu added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. labels Aug 7, 2018
@cblecker
Copy link
Copy Markdown
Member

cblecker commented Aug 7, 2018

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 7, 2018
Copy link
Copy Markdown
Member

@mkumatag mkumatag Aug 7, 2018

Choose a reason for hiding this comment

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

This is true only for the images which are present in docker library, so the library should come in imagename and not in dockerHubRegistry registry.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The nginx images are the only use of this const at the moment. Is it necessary to draw that distinction? The goal here is to handle docker and containerd’s normalization.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@cblecker 3 of them, not just 2.

  • community docker
  • docker in RHEL
  • containerd

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@mkumatag Please take a look at all registries below, all of them include the full "namespace":

	e2eRegistry       = "gcr.io/kubernetes-e2e-test-images"
	gcRegistry        = "k8s.gcr.io"
	PrivateRegistry   = "gcr.io/k8s-authenticated-test"
 	sampleRegistry    = "gcr.io/google-samples"

We can change dockerHubRegistry to dockerLibraryRegistry if that is preferred.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm okay with renaming to dockerLibraryRegistry

@mkumatag
Copy link
Copy Markdown
Member

mkumatag commented Aug 7, 2018

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 7, 2018
@Random-Liu
Copy link
Copy Markdown
Member Author

@mkumatag The test is failing for several days. Can we bring it back to green sooner? Are you ok with #67061 (comment)?

@mkumatag
Copy link
Copy Markdown
Member

mkumatag commented Aug 8, 2018

@mkumatag The test is failing for several days. Can we bring it back to green sooner? Are you ok with #67061 (comment)?

I'm okay with renaming to dockerLibraryRegistry

@dims
Copy link
Copy Markdown
Member

dims commented Aug 8, 2018

+1 to dockerLibraryRegistry

@cblecker
Copy link
Copy Markdown
Member

cblecker commented Aug 8, 2018

I'm good with that approach as well. Please ping when the change is complete, @Random-Liu :)

Thanks for reviewing, @mkumatag & @dims!

@Random-Liu Random-Liu force-pushed the fix-docker-registry branch from 4df2fdb to e232c4f Compare August 8, 2018 17:36
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 8, 2018
@Random-Liu
Copy link
Copy Markdown
Member Author

@cblecker @dims @mkumatag Done.

@cblecker
Copy link
Copy Markdown
Member

cblecker commented Aug 8, 2018

/lgtm
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Aug 8, 2018
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, Random-Liu

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

The pull request process is described here

Details 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

@Random-Liu
Copy link
Copy Markdown
Member Author

/test pull-kubernetes-integration

@k8s-github-robot
Copy link
Copy Markdown

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

@k8s-github-robot
Copy link
Copy Markdown

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 8a47559 into kubernetes:master Aug 8, 2018
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. kind/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. 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

Development

Successfully merging this pull request may close these issues.

6 participants