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

don't ship kube-registry-proxy and pause images in tars. #23605

Merged
merged 1 commit into from
Apr 15, 2016
Merged

don't ship kube-registry-proxy and pause images in tars. #23605

merged 1 commit into from
Apr 15, 2016

Conversation

mikedanese
Copy link
Member

pause is built into containervm. if it's not on the machine we should just pull
it. nobody that I'm aware of uses kube-registry-proxy and it makes build/deployment
more complicated and slower.

@mikedanese mikedanese assigned mikedanese and unassigned mikedanese Mar 29, 2016
@k8s-bot
Copy link

k8s-bot commented Mar 29, 2016

GCE e2e build/test passed for commit 133402fd80ae62244bf89bd6575effc82126474a.

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 29, 2016
@mikedanese mikedanese removed their assignment Mar 29, 2016
@ikehz
Copy link
Contributor

ikehz commented Mar 30, 2016

nobody that I'm aware of uses kube-registry-proxy and it makes build/deployment
more complicated and slower.

Does that mean we should either delete the code entirely or move it into contrib?

@ikehz
Copy link
Contributor

ikehz commented Mar 30, 2016

Looks like it was added in #12833, and would be necessary for such things.

@mikedanese
Copy link
Member Author

@ihmccreery not shipping it in our release artifact doesn't preclude us from running it. If it's not presnt on the machine it will be pulled from an external registry which seems fine. I wasn't planning on dropping support for it.

@mikedanese
Copy link
Member Author

It's rather expensive in both build time (~10 seonds) and image size to shlup this around when it's not enabled by default in any kube-up. I am also trying to reduce our dependency on docker load in the deployment process. I can split up the pause and the registry-proxy in to separate PRs if you think these should be addressed separately.

@mikedanese
Copy link
Member Author

cc @zmerlynn

@zmerlynn
Copy link
Member

One random question: Do we have a test anywhere that if we're on GCE, we never pulled :pause? It would be nice to know that if the tag were ever bumped, we'd notice.

zmerlynn
zmerlynn reviewed Mar 30, 2016
View reviewed changes
build/common.sh
) &
done

if [[ ! -z "${BUILD_PYTHON_IMAGE:-}" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

This is still needed on CoreOS for

kubernetes/cluster/saltbase/salt/kube-addons/kube-addons.sh

Line 28 in 75c1dca

function ensure_python() {
(see #18488). It's one of the reasons I've been wanting to rewrite kube-addons.sh.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'm going to push that image so we don't need this anymore.

@mikedanese mikedanese added release-note-none Denotes a PR that doesn't merit a release note. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed release-note-none Denotes a PR that doesn't merit a release note. labels Apr 1, 2016
@ikehz ikehz added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2016
@k8s-github-robot
Copy link

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@ikehz ikehz removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2016
@ikehz
Copy link
Contributor

ikehz commented Apr 4, 2016

@mikedanese Did you get the Python stuff worked out? cc @yifan-gu

@mikedanese
Copy link
Member Author

No, I'm planning on pushing a python image to gcr.io but have not yet. I'll reassign to you when ready. Thanks!

@mikedanese mikedanese assigned mikedanese and unassigned ikehz Apr 4, 2016
@k8s-bot
Copy link

k8s-bot commented Apr 4, 2016

GCE e2e build/test passed for commit 133402fd80ae62244bf89bd6575effc82126474a.

@k8s-bot
Copy link

k8s-bot commented Apr 5, 2016

GCE e2e build/test passed for commit 9c2b9dd3ad1745ac000ae679f7dde22a5e960d82.

@mikedanese
don't ship kube-registry-proxy and pause in tars.
32426d6 
pause is built into containervm. if it's not on the machine we should just pull
it. nobody that I'm aware of uses kube-registry-proxy and it makes build/deployment
more complicated and slower.
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 5, 2016
@k8s-bot
Copy link

k8s-bot commented Apr 5, 2016

GCE e2e build/test passed for commit 32426d6.

@mikedanese
Copy link
Member Author

@k8s-bot node e2e test this, issue #23878

@mikedanese mikedanese assigned ikehz and unassigned mikedanese Apr 5, 2016
@mikedanese
Copy link
Member Author

@ihmccreery ok I pushed they python image.

@mikedanese
Copy link
Member Author

@ihmccreery ping. PTAL

@mikedanese mikedanese changed the title don't ship kube-registry-proxy and pause in tars. don't ship kube-registry-proxy and pause in release tar Apr 11, 2016
@mikedanese mikedanese changed the title don't ship kube-registry-proxy and pause in release tar don't ship kube-registry-proxy and pause images in tars. Apr 11, 2016
@mikedanese mikedanese mentioned this pull request Apr 11, 2016
Merged
@ikehz ikehz added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 11, 2016
@ikehz
Copy link
Contributor

ikehz commented Apr 11, 2016

LGTM. @zmerlynn You good with this now?

@zmerlynn
Copy link
Member

LGTM

@k8s-github-robot
Copy link

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@k8s-bot
Copy link

k8s-bot commented Apr 11, 2016

GCE e2e build/test passed for commit 32426d6.

@mikedanese mikedanese mentioned this pull request Apr 12, 2016
Closed
@k8s-github-robot
Copy link

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@k8s-bot
Copy link

k8s-bot commented Apr 13, 2016

GCE e2e build/test failed for commit 32426d6.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

@mikedanese
Copy link
Member Author

@k8s-bot ok to test, issue: #19997

@k8s-bot
Copy link

k8s-bot commented Apr 14, 2016

GCE e2e build/test passed for commit 32426d6.

@luxas luxas mentioned this pull request Apr 15, 2016
Closed
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Apr 15, 2016

GCE e2e build/test passed for commit 32426d6.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 16e2e87 into kubernetes:master Apr 15, 2016
k8s-github-robot pushed a commit that referenced this pull request May 8, 2016
Merge pull request #23600 from mikedanese/addon-manager
f46f35a 
Automatic merge from submit-queue

run kube-addon-manager in a static pod

Depends on #23605 and #24108

Ref #23233
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ikehz ikehz

Labels
area/build-release lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mikedanese @k8s-bot @ikehz @zmerlynn @k8s-github-robot @k8s-teamcity-mesosphere @googlebot