Skip to content

Comments

Use sls_build for building docker images#39467

Merged
cachedout merged 1 commit intosaltstack:developfrom
gtmanfred:sls_build
Feb 24, 2017
Merged

Use sls_build for building docker images#39467
cachedout merged 1 commit intosaltstack:developfrom
gtmanfred:sls_build

Conversation

@gtmanfred
Copy link
Contributor

@gtmanfred gtmanfred commented Feb 16, 2017

What does this PR do?

Allows the use of docker.sls_build with building images for docker.image_present.

Also fix some bad logic with clearing out the created docker containers after a build was finished. Without the reorganization around the dryrun check, we would leave behind the container that was used to create the image. The logic is fixed here. 5eed60b?diff=split#diff-8f4981346bb8d093255737c1cd281ef6R5986

Tests written?

No

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was removed because there is no reason to add a name to the container when we are just going to delete it at the end.

@gtmanfred
Copy link
Contributor Author

Go Go Jenkins!

@cachedout
Copy link
Contributor

@gtmanfred
Copy link
Contributor Author

gtmanfred commented Feb 17, 2017 via email

@cachedout
Copy link
Contributor

@gtmanfred Merge conflict now I'm afraid.

@gtmanfred gtmanfred force-pushed the sls_build branch 4 times, most recently from 9adb935 to 14e166d Compare February 22, 2017 17:40
@gtmanfred
Copy link
Contributor Author

Ok, all the tests should pass on the last one, and I went through and just squashed all the changes down to one commit because i bungled the rebase.

Thanks!
Daniel

@gtmanfred
Copy link
Contributor Author

Go Go Jenkins!

@gtmanfred gtmanfred force-pushed the sls_build branch 2 times, most recently from f8e8b1b to 5eed60b Compare February 22, 2017 18:22
@gtmanfred
Copy link
Contributor Author

Go Go Jenkins!

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify the reason that this needs to be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name is only used for generating a container with a name, we never use that name, we always use the id_ when referencing it

However in the docker.image_present state, we add the tag ':latest' to any image that does not already have a tag in it's name variable, and : is not a valid character in the name of a container, though we still want the tag in the name of the image.

@gtmanfred
Copy link
Contributor Author

gtmanfred commented Feb 23, 2017

rebased again 😃 darn merge forwards!

@gtmanfred
Copy link
Contributor Author

Go Go Jenkins!

@cachedout cachedout merged commit 9eb2399 into saltstack:develop Feb 24, 2017
@gtmanfred gtmanfred deleted the sls_build branch February 24, 2017 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants