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

Make #19601 backward compatible. #20710

Merged
merged 1 commit into from
Feb 25, 2016
Merged

Make #19601 backward compatible. #20710

merged 1 commit into from
Feb 25, 2016
+8 −2

Conversation

fgrzadkowski
Copy link
Contributor

Move building hyperkube image to make release to make it backward compatible in release process.

Fixes #20047

@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 5, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/XS

@k8s-bot
Copy link

k8s-bot commented Feb 5, 2016

GCE e2e test build/test passed for commit 4b58f680412f8ffb1e356c0ed72b6285bc3e8ee1.

@fgrzadkowski
Copy link
Contributor Author

@david-mcmahon

david-mcmahon
david-mcmahon reviewed Feb 5, 2016
View reviewed changes
build/release.sh
# If we have these variables set then we want to build all docker images.
# Since regular build functions above don't build hyperkube we need to do
# it manually.
if [[ -n "${KUBE_DOCKER_IMAGE_TAG-}" && -n "${KUBE_DOCKER_REGISTRY-}" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies in advance for the naive questions.
Is there a better place for this? Could this go into kube::build::docker_build or another function that builds a list of passed in cluster/images directories? I don't know know all of the relationships yet, so I don't know what is appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. The build logic is well-factored, and I'd like to keep it that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this PR good to go?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe my comment wasn't clear. I agree with @david-mcmahon; this should have a different home.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@david-mcmahon kube::build::docker_build is a generic method for building docker images.

@ihmccreery Do you have some suggestions where this should go?

Copy link
Contributor

Choose a reason for hiding this comment

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

How about kube::release::package_hyberkube?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@fgrzadkowski fgrzadkowski force-pushed the backward_compatible_release branch from 4b58f68 to dcefb07 Compare February 19, 2016 16:13
@k8s-github-robot
Copy link

Labelling this PR as size/S

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 19, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 19, 2016

GCE e2e test build/test passed for commit dcefb0748f812b436720c141141981b41788413a.

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

ikehz commented Feb 19, 2016

LGTM. I'd like to see a comment about how this will skip silently if KUBE_DOCKER_IMAGE_TAG and KUBE_DOCKER_REGISTRY aren't set, but otherwise this is fine for now.

@david-mcmahon
Copy link
Contributor

Yes a 'TODO: Comment' would be best to get attention on it later.

@ikehz
Copy link
Contributor

ikehz commented Feb 19, 2016

@k8s-bot unit test this again; issue #21451.

@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 Feb 19, 2016

GCE e2e test build/test passed for commit dcefb0748f812b436720c141141981b41788413a.

@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 Feb 20, 2016

GCE e2e build/test failed for commit dcefb0748f812b436720c141141981b41788413a.

@david-mcmahon
Copy link
Contributor

ping. Can we get this in?

@fgrzadkowski
Copy link
Contributor Author

@k8s-bot test this issue: #IGNORE

@k8s-bot
Copy link

k8s-bot commented Feb 24, 2016

GCE e2e build/test failed for commit dcefb0748f812b436720c141141981b41788413a.

@fgrzadkowski
Copy link
Contributor Author

@k8s-bot test this issue: #21798

@k8s-bot
Copy link

k8s-bot commented Feb 24, 2016

GCE e2e test build/test passed for commit dcefb0748f812b436720c141141981b41788413a.

@fgrzadkowski fgrzadkowski added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Feb 24, 2016
@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 Feb 24, 2016

GCE e2e test build/test passed for commit dcefb0748f812b436720c141141981b41788413a.

@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 Feb 24, 2016

GCE e2e test build/test passed for commit dcefb0748f812b436720c141141981b41788413a.

@k8s-github-robot
Copy link

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

@david-mcmahon david-mcmahon mentioned this pull request Feb 24, 2016
Closed
@k8s-github-robot
Copy link

PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 25, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 25, 2016

GCE e2e test build/test passed for commit dcefb0748f812b436720c141141981b41788413a.

@david-mcmahon david-mcmahon mentioned this pull request Feb 25, 2016
Closed
@fgrzadkowski fgrzadkowski force-pushed the backward_compatible_release branch from dcefb07 to 6f6cb45 Compare February 25, 2016 13:25
@fgrzadkowski fgrzadkowski removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 25, 2016
@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 25, 2016
@gmarek gmarek added e2e-not-required lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Feb 25, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 25, 2016

GCE e2e test build/test passed for commit 6f6cb45.

gmarek pushed a commit that referenced this pull request Feb 25, 2016
Merge pull request #20710 from fgrzadkowski/backward_compatible_release
13d2364 
Make #19601 backward compatible.
@gmarek gmarek merged commit 13d2364 into kubernetes:master Feb 25, 2016
@colemickens colemickens mentioned this pull request Mar 2, 2016
Merged
@colemickens colemickens mentioned this pull request Sep 22, 2016
Closed
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
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@fgrzadkowski @k8s-github-robot @k8s-bot @ikehz @david-mcmahon @k8s-teamcity-mesosphere @gmarek @googlebot