Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Add stamp option to docker_bundle rule #17

Merged
merged 2 commits into from
May 2, 2017

Conversation

ixdy
Copy link
Contributor

@ixdy ixdy commented May 1, 2017

This allows using workspace status information (such as git describe) in image tags. To differentiate from make variables (and laziness), I'm using python formatting syntax, e.g. {STABLE_BUILD_USER} or {MY_CUSTOM_VAR}.

This is basically an implementation of what I was describing in #3 (comment).

I think it'd work for what I need for kubernetes right now, though I'm not sure if this is the correct implementation, e.g. does it makes sense to do the stamping in docker_bundle? Should it also be supported in docker_build and/or docker_push? What things should be "stampable"? Do we want to be able to embed stamping info in image labels? (I might in the future, e.g. embed revision/other build data.)

(docker_push actually looks harder, since it's basically just a shell script template wrapper to the code in containerregistry.)

The proper way to do all of this would be entirely in skylark, though that depends on bazelbuild/bazel#1054 being resolved.

This allows using workspace status information (such as git describe)
in image tags.
Copy link
Contributor

@mattmoor mattmoor left a comment

Choose a reason for hiding this comment

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

Very cool, thanks!

Can you update the usage in the README.md as well?

with open(infofile) as info:
for line in info:
line = line.strip("\n")
key, value = line.split(" ")
Copy link
Contributor

Choose a reason for hiding this comment

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

you probably want line.split(" ", 1) (or maybe 2, I can never remember the sense of this)

for line in info:
line = line.strip("\n")
key, value = line.split(" ")
stamp_info[key] = value
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you print a warning when key exists in the map?

@ixdy ixdy force-pushed the docker-bundle-stamping branch 3 times, most recently from f159d08 to 8f590ca Compare May 2, 2017 01:46
@ixdy
Copy link
Contributor Author

ixdy commented May 2, 2017

Thanks for the quick review! I've addressed your comments in a follow-up commit and can squash if desired.

@mattmoor
Copy link
Contributor

mattmoor commented May 2, 2017

GitHub will let me squash and merge, but I'm happy to leave the history there. thanks for the change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants