-
Notifications
You must be signed in to change notification settings - Fork 134
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
Scope build meta-data keys by config file #112
Conversation
Gnarly problem! |
bd6200a
to
2b920f7
Compare
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 seems pretty reasonable to me!
Unfortunately, whilst this is compatible with the same version, if you mix and match accidentally (which is quite easy to do), it's API incompatible with previous versions.
IMO we need to make this at least work ok with the 1.8.x series of metadata keys, and then probably bump the version to v2.0.0 to release this. |
Oh, interesting! I hadn’t really considered that. Yeah, we should bump to 2.0 Also, we should change the output message as well… that meta-data key message is obtuse, and could be much friendlier. |
We haven’t cut a new version yet, I was hestitant to. Glad I didn’t! Did this crop up for someone who was accidentally using /me ponders a GitHub bot that sends PRs to lint and update your BK pipeline plugin version deps. |
Any reason we need the backward compat? Why not cut v2.0.0 and in the release notes mention it? |
Uhoh. I think v2.0.0 broke our builds :( |
More context required @jufemaiz ;) |
@lox confirmation on #112 (comment) Release notes didn't comment on breaking change and that's the exact message (within reason) I saw by bumping to v2.0.0. v1.8.4 works fine. We are having to do intermediary build injecting agent held SSH creds via build arguments in so that we can access private git repo so we don't do a docker-compose plugin based build but obviously need to tag the metadata for future uses by docker-compose plugin.
Using v2.0.0 plugin threw the following:
Which then attempted to do a standard docker-compose build (but no build arguments so no dice). Would be good to know what the key is there to attempt to further debug. |
Sorry about that @jufemaiz! We're trying to avoid having the metadata key schemes being a public API, but we should have been clearer about that. Might multi-stage docker builds be a good solution for you? https://medium.com/@kevinsimper/how-we-do-docker-multi-stage-builds-and-secure-sharing-of-private-repositories-956eda84f3b8 Alternately, we could prioritize build args to the docker-compose plugin. |
No worries @lox. Would you like me to open as a separate issue? |
That would be great if you could @jufemaiz! |
This updates the meta-data key used for storing pre-built images to include the docker-compose config file name. This allows you to pre-build and run the same named service from two completely different docker-compose.yml files, without any fear of clashing.
For example:
The above two will now set and get the following meta-data keys:
If you specify multiple configs with the array syntax they're joined with a dash. For example, the following:
Sets the following meta-data key:
I don't think this should have any backwards compatibility problems, unless people were depending on the meta-data key, but as that's not configurable I'd consider it private.
Fixes #111