Skip to content

Save docker image tarfiles in _output/release-images/$arch/#47939

Merged
k8s-github-robot merged 1 commit intokubernetes:masterfrom
ixdy:docker-manifest
Jun 24, 2017
Merged

Save docker image tarfiles in _output/release-images/$arch/#47939
k8s-github-robot merged 1 commit intokubernetes:masterfrom
ixdy:docker-manifest

Conversation

@ixdy
Copy link
Copy Markdown
Contributor

@ixdy ixdy commented Jun 23, 2017

Additionally, add option KUBE_BUILD_HYPERKUBE to build hyperkube
outside of the release flow.

What this PR does / why we need it: Saves all of the docker tarfiles in a separate directory that the release scripts can use to push to a docker registry. This is easier than trying to guess which images should be pushed from the local docker engine, and supports work in kubernetes/test-infra#1400.

If we eventually use this for the official release workflow (anago) this may prevent something like #47307 from happening again.

Release note:

NONE

/release-note-none
/assign @luxas @david-mcmahon
cc @madhusudancs @roberthbailey

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 23, 2017
@ixdy ixdy changed the title Save docker image tarfiles in _output/release-images/$arch/. Save docker image tarfiles in _output/release-images/$arch/ Jun 23, 2017
@ixdy
Copy link
Copy Markdown
Contributor Author

ixdy commented Jun 23, 2017

(building hyperkube is still pretty slow, btw. I wonder if we can make progress on #42589 for 1.8?)

@k8s-github-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ixdy

Associated issue: 1400

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

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Jun 23, 2017
@@ -26,6 +26,13 @@
# This is where the final release artifacts are created locally
readonly RELEASE_STAGE="${LOCAL_OUTPUT_ROOT}/release-stage"
readonly RELEASE_DIR="${LOCAL_OUTPUT_ROOT}/release-tars"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This dir now seems named inconsistently; following the other two it should be RELEASE_TARS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

function kube::release::package_tarballs() {
# Clean out any old releases
rm -rf "${RELEASE_DIR}"
rm -rf "${RELEASE_DIR}" "${RELEASE_IMAGES}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why don't we delete $RELEASE_STAGE here? Is that cleared out elsewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the various package_*_tarballs functions do this, but a good suggestion regardless. done.

echo "${docker_tag}" > ${binary_dir}/${binary_name}.docker_tag

rm -rf ${docker_build_path}
ln "${binary_dir}/${binary_name}.tar" "${images_dir}/"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are you using ln instead of cp for efficiency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, the docker tarfiles are pretty large. for amd64 alone, ~576M for everything except hyperkube, and another 582M for hyperkube. across 5 architectures that seemed like a lot.

@roberthbailey
Copy link
Copy Markdown
Contributor

please squash, then lgtm

Additionally, add option KUBE_BUILD_HYPERKUBE to build hyperkube
outside of the release flow.
@ixdy ixdy force-pushed the docker-manifest branch from 2d518ab to a29d364 Compare June 23, 2017 20:58
@david-mcmahon david-mcmahon added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 23, 2017
@ixdy
Copy link
Copy Markdown
Contributor Author

ixdy commented Jun 23, 2017

/retest

@k8s-github-robot
Copy link
Copy Markdown

Automatic merge from submit-queue (batch tested with PRs 47650, 47936, 47939, 47986, 48006)

@k8s-github-robot k8s-github-robot merged commit 4b192f6 into kubernetes:master Jun 24, 2017
k8s-github-robot pushed a commit that referenced this pull request Jul 18, 2017
Automatic merge from submit-queue (batch tested with PRs 49043, 49001, 49057, 49066, 48102)

Always use gcr.io/google_containers for side-loaded Docker images

**What this PR does / why we need it**: #45391 changed the behavior for what registry we use in the sideloaded docker images tarfiles shipped with releases. As a result of that change, if `KUBE_DOCKER_REGISTRY` is set to anything other than `gcr.io/google_containers`, clusters will fail to start on GCE (and other places where the side-loaded images are used).

This PR reverts that change in behavior, which I believe was unintentional; we'll always use `gcr.io/google_containers` for the docker image tarfiles, but will tag the images with `$KUBE_DOCKER_REGISTRY` if different.

Also, I'm fixing a small bug in variable names that I introduced in #47939.

Note that with recent changes here and in the release repo, we don't even need to tag with `KUBE_DOCKER_REGISTRY` and `KUBE_DOCKER_IMAGE_TAG`, but that's a more extensive change, and this smaller fix is more suitable for cherry-picking to 1.7.

**Release note**:

```release-note
NONE
```
/release-note-none
/sig release
/assign @david-mcmahon
@david-mcmahon
Copy link
Copy Markdown
Contributor

david-mcmahon commented Oct 5, 2017

@ixdy I see this is only in 1.8+. How do we feel about backporting it to support all active branches?
Asking because kubernetes/release#430.

k8s-github-robot pushed a commit that referenced this pull request Nov 22, 2017
…-#47939-#49066-upstream-release-1.7

Automatic merge from submit-queue.

Automated cherry pick of #47939 #49066

Cherry pick of #47939 #49066 on release-1.7.

#47939: Save docker image tarfiles in _output/release-images/$arch/.
#49066: Always use gcr.io/google_containers for side-loaded Docker
@ixdy ixdy deleted the docker-manifest branch May 15, 2018 23:36
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. 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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants