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

Update non-GCE Salt distros to 1.6.0, fallback to ContainerVM Docker version on GCE #7740

Merged
merged 1 commit into from
May 5, 2015
Merged

Update non-GCE Salt distros to 1.6.0, fallback to ContainerVM Docker version on GCE #7740

merged 1 commit into from
May 5, 2015

Conversation

zmerlynn
Copy link
Member

@zmerlynn zmerlynn commented May 4, 2015

This is neutral on the current version of ContainerVM, and saves us some cycles.

@zmerlynn
Copy link
Member Author

zmerlynn commented May 4, 2015

Wait, misread this Salt. Argh. We need this for GCE for the next ContainerVM, but I need special case the others.

@kubernetes kubernetes locked and limited conversation to collaborators May 4, 2015
@kubernetes kubernetes unlocked this conversation May 4, 2015
@roberthbailey roberthbailey changed the title Use built-in ContainerVM Docker version rather than installing lxc-docker WIP: Use built-in ContainerVM Docker version rather than installing lxc-docker May 4, 2015
@roberthbailey
Copy link
Contributor

I changed the PR title to "WIP". Please flip it back once it's ready for review.

@zmerlynn zmerlynn changed the title WIP: Use built-in ContainerVM Docker version rather than installing lxc-docker Use built-in ContainerVM Docker version rather than installing lxc-docker May 4, 2015
@zmerlynn
Copy link
Member Author

zmerlynn commented May 4, 2015

Ready now. Tested the override path by flipping the the cloud provider equality to verify (I have a ContainerVM image with Docker 1.6 on it, so it resulted in a downgrade).

@zmerlynn
Copy link
Member Author

zmerlynn commented May 4, 2015

cc @vmarmol @dchen1107

vmarmol
vmarmol reviewed May 4, 2015
View reviewed changes
cluster/saltbase/salt/docker/init.sls
# We are caching the Docker deb file in GCS for reliability and speed. To
# update this to a new version of docker, do the following:
# Docker is on the ContainerVM image by default. The following
# variables are provided for other cloud providers, and for testing testing and dire circumstances, to allow
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: double testing

@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

LGTM, thanks @zmerlynn! Are we planning on updating the Docker version to 1.6 sometime?

@vmarmol vmarmol added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2015
@zmerlynn
Copy link
Member Author

zmerlynn commented May 4, 2015

It's in the ContainerVM update. Do you want me to just bonk it into here since we've already e2e'd it? It would basically update Azure and AWS but not us (yet).

@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

Yeah I was thinking that for the non-GCE setups. I'm fine doing it here or in another PR.

@zmerlynn
Copy link
Member Author

zmerlynn commented May 4, 2015

I can do it here. One sec.

@zmerlynn zmerlynn changed the title Use built-in ContainerVM Docker version rather than installing lxc-docker Update non-GCE Salt distro to 1.6.0, fallback to ContainerVM Docker version on GCE May 4, 2015
@zmerlynn zmerlynn changed the title Update non-GCE Salt distro to 1.6.0, fallback to ContainerVM Docker version on GCE Update non-GCE Salt distros to 1.6.0, fallback to ContainerVM Docker version on GCE May 4, 2015
@zmerlynn
Copy link
Member Author

zmerlynn commented May 4, 2015

Upgrading non-GCE providers to 1.6.0. Tested by briefly flipped the equality again and doing an -up on a stock ContainerVM image, then verifying the versions.

@zmerlynn
Copy link
Member Author

zmerlynn commented May 4, 2015

PTAL (fixed the nit along the way)

@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

LGTM, will merge on green. Thanks again @zmerlynn!

vmarmol added a commit that referenced this pull request May 5, 2015
@vmarmol
Merge pull request #7740 from zmerlynn/docker_fallback
d159f4c 
Update non-GCE Salt distros to 1.6.0, fallback to ContainerVM Docker version on GCE
@vmarmol vmarmol merged commit d159f4c into kubernetes:master May 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vmarmol vmarmol

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zmerlynn @roberthbailey @vmarmol @googlebot