-
-
Notifications
You must be signed in to change notification settings - Fork 646
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.
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
Nicer local docker iteration #7065
Nicer local docker iteration #7065
Conversation
@@ -0,0 +1,15 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this script is confusing. It reads like random words strung together. This script appears to build the two images and then run an interactive bash session in a new container started from the travis image. But the name doesn't really imply any of that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your description is correct, and I will edit the filename / commenting to indicate this!
|
||
set -euxo pipefail | ||
|
||
CENTOS_SNAPSHOT_TAG='pants_centos6_snapshot' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is likely to lead to trouble and confusion, as you're overwriting the same tag with different versions, and it may be unclear which version you're currently using. It's already bad enough that we use :latest
in production, which makes our CI runs unreproducible.
Possibly we should generate a unique tag based on the git sha or something, and have the travis image depend on that tag (passed in via ARG for desktop use, or hard-coded for CI use). This solves a problem in both development runs and "production" CI usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of the second paragraph a lot (and the first) and will be able to look into it more later today.
@@ -0,0 +1,32 @@ | |||
# This script is intended to be sourced, not executed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not bake this into the image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love to, but didn't want to enforce that people use the kinds of ls
options I personally prefer (although with that rationale, why not propagate an env var from the invoking shell? not sure). If we can use this PR to converge on a "nicer than bare-metal" bash environment for interactive usage that would be lovely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you are kinda enforcing it, no? Having the file be in the image doesn't mean that people have to use it when running a container.
Closing due to being stale + |
Problem
I was looking into #7064 and was always a little annoyed that we couldn't plug in more functionality to our docker image, or modify the centos6 image when iterating on image generation locally.
Solution
BASE_IMAGE
arg to thetravis_ci
Dockerfile to allow modifying both the centos6 image and the travis image.bootstrap-docker-snapshot-image.sh
to build thetravis_ci
image, run it, and add some nice shell environment tweaks that are useful when debugging locally.