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
Publish command fixes/improvements #2250
Conversation
We had been making some assumptions that the in-cluster registry was being used. We already had the code to pull images from other registries, so this wasn't actually to hard to address. Also added tests to validate.
9be5f32
to
98b77e2
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.
Had a couple of comments but otherwise looks great!
docs/guides/in-cluster-building.md
Outdated
|
||
As usual, you need to specify the `image` field on the `container` module in question. For example: | ||
You'll generally need to specify the `image` field on the `container` module in question, to indicate where the image should be published. For example: |
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 generally? Doesn't this always apply? If not, can we mention when it doesn't.
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.
You don't need that if you're publishing to your deployment registry, for example.
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.
But I'll clarify that in the docs.
if (!containerHelpers.hasDockerfile(module, module.version)) { | ||
log.setState({ msg: `Nothing to publish` }) | ||
return { published: false } | ||
} | ||
|
||
const localId = containerHelpers.getLocalImageId(module, module.version) | ||
const remoteId = containerHelpers.getPublicImageId(module) | ||
const remoteId = containerHelpers.getPublicImageId(module, tag) | ||
|
||
log.setState({ msg: `Publishing image ${remoteId}...` }) |
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.
Perhaps out of scope for this one but would be nice to stream the logs from the publish command when at verbose
log level.
docs/guides/in-cluster-building.md
Outdated
You can also set the `--tag` option on the `garden publish` command to override the tag used for images. You can both set a specific tag or you can _use template strings for the tag_. For example, you can | ||
|
||
- Set a specific tag on all published modules: `garden publish --tag "v1.2.3"` | ||
- Set a custom prefix on tags but include the Garden version hash: `garden publish --tag "v0.1-${module.hash}"` |
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 need to escape the $
for this to work, otherwise I get a zsh: bad substitution
error. We should probably mention that here.
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 updated the example to use single-quotes, which solves that matter, and documented that as well.
This should generally make the `garden publish` command a lot more usable. You can now override the default tag at publish-time, and even apply template strings on the tags. For details, please check out the updated "Publishing images" section in the in-cluster building docs, and the updated command docs.
5b15fad
to
0d65aeb
Compare
Updated @eysi09 |
It was time to give the publish command a bit of ❤️. Couple of commits:
fix(k8s): garden publish command now works with any deploymentRegistry
We had been making some assumptions that the in-cluster registry was
being used. We already had the code to pull images from other
registries, so this wasn't actually to hard to address. Also added
tests to validate.
feat(publish): add template-able --tag parameter to publish command
This should generally make the
garden publish
command a lot moreusable. You can now override the default tag at publish-time, and
even apply template strings on the tags.
From the updated in-cluster building docs:
You can also set the
--tag
option on thegarden publish
command to override the tag used for images. You can both set a specific tag or you can use template strings for the tag. For example, you cangarden publish --tag "v1.2.3"
garden publish --tag "v0.1-${module.hash}"
garden publish --tag "v0.1-${git.branch}"
Generally, you can use any template strings available for module configs for the tags, with the addition of the following:
${module.name}
— the name of module being tagged${module.version}
— the full Garden version of module being tagged, e.g.v-abcdef1234
${module.hash}
— the Garden version hash of module being tagged, e.g.abcdef1234
(i.e. without thev-
prefix)When reviewing, please give the publish command a spin, and try messing about with the
--tag
parameter.