cleanup: s^gcr.io/google_containers^gcr.io/google-containers/^g #48956
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ixdy Assign the PR to them by writing No associated issue. Update pull-request body to add a reference to an issue, or get approval with 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 |
k8s-github-robot
added
size/XXL
|
anything that goes through the registry should be fine, since GCR handles the aliasing. |
|
/test pull-kubernetes-e2e-gke |
|
/retest |
k8s-github-robot
added
the
needs-rebase
ixdy
force-pushed
the
standardize-google-containers
branch
from
83516e3 to
49c6334
Compare
k8s-github-robot
added
needs-rebase
build/lib/release.sh
Outdated
| @@ -284,7 +284,7 @@ function kube::release::create_docker_images_for_server() { | |||
| local images_dir="${RELEASE_IMAGES}/${arch}" | |||
| mkdir -p "${images_dir}" | |||
|
|
|||
| local docker_registry="${KUBE_DOCKER_REGISTRY:-gcr.io/google-containers}" | |||
There was a problem hiding this comment.
add a comment if this is special
There was a problem hiding this comment.
this line is tricky, because it matters for images which are sideloaded, so both sides need to match. I reverted this because I think the kops-aws job side-loads, using gcr.io/google_containers, but then this broke the gce job (which also side-loads) because I didn't revert its config.
additionally, anago explicitly sets this variable, which then may or may not match the side-load config.
There was a problem hiding this comment.
#49066 should have fixed the anago case. let's see if all of the jobs pass now...
|
lgtm if the comment for the special case is added |
ixdy
force-pushed
the
standardize-google-containers
branch
from
49c6334 to
ce34c2e
Compare
ixdy
force-pushed
the
standardize-google-containers
branch
from
ce34c2e to
ef420b6
Compare
ixdy
added
release-note
k8s-github-robot
removed
the
needs-rebase
ixdy
force-pushed
the
standardize-google-containers
branch
from
ef420b6 to
d59a6d7
Compare
|
kops uses the side-loaded image and hard-codes |
|
@ixdy: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
|
FYI, I talked with @zmerlynn about this a bit, and I think to solve the side-loading issue I'm going to create a separate PR which adds the Once that's done, I can come back here and try again. |
|
@ixdy PR needs rebase |
k8s-github-robot
added
the
needs-rebase
|
This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review. You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days |
What this PR does / why we need it: at some point in the distant past, docker did not support hyphens in the registry name, and so we needed to use the
google_containersalias (which GCR internally translated) to refer to thegoogle-containersproject.There's no need to use this alias anymore (since docker 1.5, I think), and it causes some developer confusion. Also, some tooling (e.g. certain
gcloudsubcommands) don't understand the underscore alias of the project.Special notes for your reviewer: this shouldn't break anything, but I'm still investigating.
I have no idea which sig this falls under.
Release note:
/release-note