2nd try at using a vanity GCR name #57824

thockin · 2018-01-04T05:56:32Z

The 2nd commit here is the changes relative to the reverted PR. Please focus review attention on that.

This is the 2nd attempt. The previous try (#57573) was reverted while we
figured out the regional mirrors (oops).

New plan: k8s.gcr.io is a read-only facade that auto-detects your source
region (us, eu, or asia for now) and pulls from the closest. To publish
an image, push k8s-staging.gcr.io and it will be synced to the regionals
automatically (similar to today). For now the staging is an alias to
gcr.io/google_containers (the legacy URL).

When we move off of google-owned projects (working on it), then we just
do a one-time sync, and change the google-internal config, and nobody
outside should notice.

We can, in parallel, change the auto-sync into a manual sync - send a PR
to "promote" something from staging, and a bot activates it. Nice and
visible, easy to keep track of.

xref kubernetes/release#281

TL;DR:

The new staging-k8s.gcr.io is where we push images. It is literally an alias to gcr.io/google_containers (the existing repo) and is hosted in the US.
The contents of staging-k8s.gcr.io are automatically synced to {asia,eu,us)-k8s.gcr.io.
The new k8s.gcr.io will be a read-only alias to whichever regional repo is closest to you.
In the future, images will be promoted from staging to regional "prod" more explicitly and auditably.

Use "k8s.gcr.io" for pulling container images rather than "gcr.io/google_containers".  Images are already synced, so this should not impact anyone materially.
   
Documentation and tools should all convert to the new name. Users should take note of this in case they see this new name in the system.

cblecker · 2018-01-04T07:49:46Z

CHANGELOG-1.4.md

@@ -177,15 +177,15 @@ filename | sha256 hash
 ### Other notable changes

 * kube-apiserver now drops unneeded path information if an older version of Windows kubectl sends it. ([#44586](https://github.com/kubernetes/kubernetes/pull/44586), [@mml](https://github.com/mml))
-* Bump gcr.io/google_containers/glbc from 0.8.0 to 0.9.2. Release notes: [0.9.0](https://github.com/kubernetes/ingress/releases/tag/0.9.0), [0.9.1](https://github.com/kubernetes/ingress/releases/tag/0.9.1), [0.9.2](https://github.com/kubernetes/ingress/releases/tag/0.9.2) ([#43098](https://github.com/kubernetes/kubernetes/pull/43098), [@timstclair](https://github.com/timstclair))
+* Bump k8s.gcr.io/glbc from 0.8.0 to 0.9.2. Release notes: [0.9.0](https://github.com/kubernetes/ingress/releases/tag/0.9.0), [0.9.1](https://github.com/kubernetes/ingress/releases/tag/0.9.1), [0.9.2](https://github.com/kubernetes/ingress/releases/tag/0.9.2) ([#43098](https://github.com/kubernetes/kubernetes/pull/43098), [@timstclair](https://github.com/timstclair))


Is it worth updating historial changelogs?

I waffle on this. The image may eventually cease to exist at the old URL? Or maybe we simply say it won't?

My thought is, the application configuration and manifests for, say 1.4, will still be looking for the image at gcr.io/google_containers/glbc. Modifying the changelog, doesn't go back and create a new patch release of 1.4 updating the config/manifests to look at the new place.

I'd say it would be worth doing for any version we are actually going to go back and change. If this is just for 1.10 forward, then I'd suggest leaving the historical change logs be.

cblecker · 2018-01-04T07:51:12Z

FYI cross is failing across the board: #57822

dekkagaijin · 2018-01-04T19:31:38Z

build/build-image/cross/Makefile


 push: build
-	gcloud docker -- push gcr.io/google_containers/$(IMAGE):$(TAG)
+	gcloud docker -- push k8s-staging.gcr.io/$(IMAGE):$(TAG)


Thinking about it now, I think it's a good idea to manually authenticate for k8s-staging for as long as the workflow continues to use gcloud. This makes the registry change backwards-compatible with older versions of gcloud.

s/gcloud docker -- push k8s-staging.gcr.io/gcloud docker -s k8s-staging -- push k8s-staging.gcr.io/g

dekkagaijin · 2018-01-04T20:01:58Z

build/debian-base/Makefile

@@ -14,7 +14,7 @@

 all: build

-REGISTRY ?= gcr.io/google-containers
+REGISTRY ?= k8s-staging.gcr.io


I'd modify this and other similar Makefiles to use gcloud docker -s $(REGISTRY) -- push (subbing REGISTRY for PREFIX or whatever the case may be, but I understand that this is a more involved change.
Otherwise, it'll require another gcloud docker update to be pushed to authenticate to k8s-staging.gcr.io, which wouldn't be the end of the world.

dekkagaijin · 2018-01-04T20:23:16Z

cluster/common.sh

@@ -478,7 +478,7 @@ function stage-images() {

  local docker_cmd=("docker")

-  if [[ "${KUBE_DOCKER_REGISTRY}" == "gcr.io/"* ]]; then
+  if [[ "${KUBE_DOCKER_REGISTRY}" =~ "gcr.io/" ]]; then
    local docker_push_cmd=("gcloud" "docker")


surprised this works without gcloud getting angry at the lack of --, but this might have to change as well depending on what the value of KUBE_DOCKER_REGISTRY is. How's this:

if [[ "${KUBE_DOCKER_REGISTRY}" =~ "staging-k8s\.gcr\.io/" ]]; then local docker_push_cmd=("gcloud" "docker" "-s" "staging-k8s.gcr.io" "--") elif [[ "${KUBE_DOCKER_REGISTRY}" =~ "gcr\.io/" ]]; then local docker_push_cmd=("gcloud" "docker" "--") else local docker_push_cmd=("${docker_cmd[@]}") fi

Once staging becomes a default, we only need the -- part, right?

dekkagaijin · 2018-01-16T18:32:38Z

Pushes should now be made to staging-k8s.gcr.io. k8s.gcr.io will be turned into a read-only alias for the local replicas (i.e. (asia|eu|us)-k8s.gcr.io).

thockin · 2018-01-19T04:54:14Z

/retest

thockin · 2018-01-19T04:55:51Z

Un-holding this. I think it works now. Let's see what tests say. PTAL

thockin · 2018-01-19T05:55:57Z

/retest

fejta-bot · 2018-02-07T12:40:40Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-02-07T15:28:39Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

cblecker · 2018-02-07T17:43:54Z

/lgtm cancel

dekkagaijin · 2018-02-07T18:39:45Z

test/kubemark/start-kubemark.sh

@@ -221,7 +221,7 @@ function create-and-upload-hollow-node-image {
 function create-and-upload-hollow-node-image-bazel {
  RETRIES=3
  for attempt in $(seq 1 ${RETRIES}); do
-    if ! bazel run //cluster/images/kubemark:push --define PROJECT="${PROJECT}"; then
+    if ! bazel run //cluster/images/kubemark:push --define REGISTRY="${FULL_REGISTRY}"; then


This works with the current implementation of docker_build, since it's just concatenating the strings under the covers, but it would be good to be able to be able to assign $PROJECT/ as a base repository for kubemark in the BUILD target, in case that ever changes in the future.

? PROJECT is a faux construct, AFAICT. gcr.io/google_containers is the registry. If you want something else, specify the whole thing.

In the Docker sense, 'registry' == hostname[:port] and 'repository' == path of the URLesque image name: https://docs.docker.com/registry/spec/api/#overview
I'm worried that the a future implementation of docker_push might make use of that assumption.

Suggestions added in the relevant places...

cblecker · 2018-02-07T21:44:54Z

cluster/images/kubemark/BUILD

-    registry = "gcr.io",
-    repository = "$(PROJECT)/kubemark",
+    registry = "$(REGISTRY)",
+    repository = "kubemark",


@ixdy @BenTheElder Does this look okay from the bazel side?

This looks like valid bazel, seems fine to me.
Edit: typically repository is foo/bar but it looks like this has already been discussed so.

thockin · 2018-02-07T21:45:15Z

/retest

BenTheElder · 2018-02-07T22:30:44Z

/retest
this looks like a known flake in the go tests, @mikedanese did your timeout bump go in?

cblecker · 2018-02-07T22:33:00Z

@thockin If the test failure is a flake, you should be good to squash

BenTheElder · 2018-02-07T22:54:16Z

All green ✅

dekkagaijin · 2018-02-07T23:29:25Z

cluster/images/kubemark/BUILD

-    registry = "gcr.io",
-    repository = "$(PROJECT)/kubemark",
+    registry = "$(REGISTRY)",
+    repository = "kubemark",


registry = "$(REGISTRY)", repository = "$(REPO_PATH_BASE)kubemark",

dekkagaijin · 2018-02-07T23:37:07Z

test/kubemark/start-kubemark.sh

  echo "Copying kubemark binary to ${MAKE_DIR}"
  cp "${KUBEMARK_BIN}" "${MAKE_DIR}"
  CURR_DIR=`pwd`
  cd "${MAKE_DIR}"
  RETRIES=3
  for attempt in $(seq 1 ${RETRIES}); do
-    if ! REGISTRY="${CONTAINER_REGISTRY}" PROJECT="${PROJECT}" make "${KUBEMARK_IMAGE_MAKE_TARGET}"; then
+    if ! REGISTRY="${FULL_REGISTRY}" make "${KUBEMARK_IMAGE_MAKE_TARGET}"; then


if ! REGISTRY="${REGISTRY}" REPO_PATH_BASE="${REPO_PATH_BASE}" make "${KUBEMARK_IMAGE_MAKE_TARGET}"; then

dekkagaijin · 2018-02-07T23:39:12Z

test/kubemark/start-kubemark.sh

+else
+  FULL_REGISTRY=staging-k8s.gcr.io
+fi
+


# TODO: Simplify. The caller should have to pass a full registry or nothing. if [[ -n "${CONTAINER_REGISTRY}" ]]; then REGISTRY="${CONTAINER_REGISTRY}" else REGISTRY=staging-k8s.gcr.io fi if [[ -n "${PROJECT}" ]]; then REPO_PATH_BASE="${PROJECT}/" fi

dekkagaijin · 2018-02-07T23:39:28Z

test/kubemark/start-kubemark.sh

@@ -221,7 +221,7 @@ function create-and-upload-hollow-node-image {
 function create-and-upload-hollow-node-image-bazel {
  RETRIES=3
  for attempt in $(seq 1 ${RETRIES}); do
-    if ! bazel run //cluster/images/kubemark:push --define PROJECT="${PROJECT}"; then
+    if ! bazel run //cluster/images/kubemark:push --define REGISTRY="${FULL_REGISTRY}"; then


if ! bazel run //cluster/images/kubemark:push --define REGISTRY="${REGISTRY}" --define REPO_PATH_BASE ="${REPO_PATH_BASE}"; then

I prefer it to be simpler - one variable.

I'll leave refactoring of this to the team that owns it ( @shyamjvs and @wojtek-t can overrule if needed)

This is the 2nd attempt. The previous was reverted while we figured out the regional mirrors (oops). New plan: k8s.gcr.io is a read-only facade that auto-detects your source region (us, eu, or asia for now) and pulls from the closest. To publish an image, push k8s-staging.gcr.io and it will be synced to the regionals automatically (similar to today). For now the staging is an alias to gcr.io/google_containers (the legacy URL). When we move off of google-owned projects (working on it), then we just do a one-time sync, and change the google-internal config, and nobody outside should notice. We can, in parallel, change the auto-sync into a manual sync - send a PR to "promote" something from staging, and a bot activates it. Nice and visible, easy to keep track of.

thockin · 2018-02-08T05:14:38Z

squashed.

cblecker · 2018-02-08T05:49:20Z

/lgtm

k8s-ci-robot · 2018-02-08T05:49:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, thockin

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 OWNERS Files:

~~api/OWNERS~~ [thockin]
~~build/OWNERS~~ [cblecker,thockin]
~~cluster/OWNERS~~ [thockin]
~~cmd/kubeadm/OWNERS~~ [thockin]
~~cmd/kubelet/OWNERS~~ [thockin]
~~docs/api-reference/OWNERS~~ [thockin]
~~examples/OWNERS~~ [thockin]
~~hack/OWNERS~~ [cblecker,thockin]
~~pkg/api/OWNERS~~ [thockin]
~~pkg/kubectl/OWNERS~~ [thockin]
~~pkg/kubelet/OWNERS~~ [thockin]
~~pkg/volume/OWNERS~~ [thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~test/OWNERS~~ [thockin]

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

k8s-github-robot · 2018-02-08T06:50:25Z

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

fejta-bot · 2018-02-08T09:19:18Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

k8s-github-robot · 2018-02-08T10:18:57Z

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

k8s-ci-robot · 2018-02-08T10:49:07Z

@thockin: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gke	`3586986`	link	`/test pull-kubernetes-e2e-gke`
pull-kubernetes-unit	`3586986`	link	`/test pull-kubernetes-unit`

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.

k8s-github-robot · 2018-02-08T11:29:32Z

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

shyamjvs · 2018-02-08T12:22:26Z

Oops.. seems like I failed to stop it on time. IIUC this would create problems with our kubemark setup. See point 3 on #59567.

shyamjvs · 2018-02-08T12:40:02Z

Sorry, I think I misunderstood the change. You seem to define a FULL_REGISTRY that composes REGISTRY and PROJECT, and those value are taken from here which luckily also continues to take the same previous values here.

I'll however send a PR to unify these values into one single source of truth to avoid bugs in future.

thockin · 2018-02-09T16:26:27Z

Thanks Shyam - Composing REGISTRY and PROJECT makes assumptions that no longer hold :)

dekkagaijin · 2018-02-09T20:53:27Z

@shyamjvs would be good to split on the first '/' of the new single source of truth, when passing values to bazel run //cluster/images/kubemark:push in test/kubemark/start-kubemark.sh:
https://github.com/kubernetes/kubernetes/blob/master/cluster/images/kubemark/BUILD#L12
https://github.com/bazelbuild/rules_docker#container_push

k8s-github-robot assigned crassirostris and zmerlynn Jan 4, 2018

thockin added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2018

cblecker reviewed Jan 4, 2018

View reviewed changes

thockin force-pushed the gcr-vanity branch from fdcf1ba to 5873251 Compare January 4, 2018 20:51

dekkagaijin reviewed Jan 4, 2018

View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 5, 2018

thockin force-pushed the gcr-vanity branch from 5873251 to 2a3cddc Compare January 17, 2018 18:59

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 17, 2018

thockin mentioned this pull request Jan 17, 2018

Use k8s.gcr.io vanity domain for container images #54174

Merged

thockin force-pushed the gcr-vanity branch from 2a3cddc to 31abf8d Compare January 17, 2018 19:43

This was referenced Jan 18, 2018

Pushes go to staging-k8s.gcr.io kubernetes/kube-state-metrics#349

Merged

Pushes go to staging-k8s.gcr.io kubernetes/test-infra#6319

Merged

thockin removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2018

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 7, 2018

thockin force-pushed the gcr-vanity branch from a4c78b7 to 851b256 Compare February 7, 2018 17:52

dekkagaijin reviewed Feb 7, 2018

View reviewed changes

cblecker reviewed Feb 7, 2018

View reviewed changes

dekkagaijin reviewed Feb 7, 2018

View reviewed changes

thockin force-pushed the gcr-vanity branch from 851b256 to 3586986 Compare February 8, 2018 05:14

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 8, 2018

k8s-github-robot merged commit fb340a4 into kubernetes:master Feb 8, 2018

shyamjvs mentioned this pull request Feb 8, 2018

Problems with kubemark image lifecycle #59567

Closed

jpbetz mentioned this pull request Mar 9, 2018

Bump to etcd 3.1.12 to pick up critical fix #60998

Merged

2nd try at using a vanity GCR name #57824

2nd try at using a vanity GCR name #57824

Conversation

thockin commented Jan 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cblecker commented Jan 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dekkagaijin commented Jan 16, 2018

thockin commented Jan 19, 2018

thockin commented Jan 19, 2018

thockin commented Jan 19, 2018

fejta-bot commented Feb 7, 2018

fejta-bot commented Feb 7, 2018

cblecker commented Feb 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenTheElder Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

thockin commented Feb 7, 2018

BenTheElder commented Feb 7, 2018 • edited Loading

cblecker commented Feb 7, 2018

BenTheElder commented Feb 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Feb 8, 2018

cblecker commented Feb 8, 2018

k8s-ci-robot commented Feb 8, 2018

k8s-github-robot commented Feb 8, 2018

fejta-bot commented Feb 8, 2018

k8s-github-robot commented Feb 8, 2018

k8s-ci-robot commented Feb 8, 2018 • edited Loading

k8s-github-robot commented Feb 8, 2018

shyamjvs commented Feb 8, 2018

shyamjvs commented Feb 8, 2018

thockin commented Feb 9, 2018

dekkagaijin commented Feb 9, 2018

thockin commented Jan 4, 2018 •

edited

Loading

BenTheElder Feb 7, 2018 •

edited

Loading

BenTheElder commented Feb 7, 2018 •

edited

Loading

k8s-ci-robot commented Feb 8, 2018 •

edited

Loading