Skip to content
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

fix: garden publish command to respect publishId #6052

Merged
merged 22 commits into from
May 17, 2024
Merged

Conversation

vvagaytsev
Copy link
Collaborator

@vvagaytsev vvagaytsev commented May 16, 2024

What this PR does / why we need it:

This PR fixes the behavior of garden publish command. See #6050 for additional context and technical details.

Which issue(s) this PR fixes:

Fixes #6050 #5045 #4796

Reverts #4740
Reopens #4095
Patches #4808

Special notes for your reviewer:

This PR does not address possible Action Router problems mentioned in #6050. There might be no issues at all in the Action Router. That should be considered separately, and another PR can be created if any fix is required.

@vvagaytsev vvagaytsev force-pushed the fix/publish-command branch 2 times, most recently from 28a354c to d50350e Compare May 16, 2024 13:20
The parsed image comes from a helper function.
That helpers function uses `splitLast` instead of `splitFirst`
with `:`-delimiter to get the tag name. That looks correct.
The only diff between `getContainerBuildActionOutputs` and `k8sGetContainerBuildActionOutputs`
was in the deploymentImageId calculation that depended
on the presence of deployment registry config.
Align the behaviour with the `k8sPublishContainerBuild`
which does not compare local and remote images.

The repeated tagging is idemponent and should not be too costly.
It was used only in one place.
Publish ID is used only by `publish` command,
and it should not be used for internal deployment registries
and while Build-actions resolution.
@vvagaytsev vvagaytsev marked this pull request as ready for review May 17, 2024 15:19
@vvagaytsev vvagaytsev changed the title fix: garden publish command fix: garden publish command May 17, 2024
@vvagaytsev vvagaytsev changed the title fix: garden publish command fix: garden publish command to respect publishId May 17, 2024
edvald
edvald previously approved these changes May 17, 2024
Copy link
Collaborator

@edvald edvald left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@vvagaytsev vvagaytsev added this pull request to the merge queue May 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 17, 2024
@vvagaytsev vvagaytsev added this pull request to the merge queue May 17, 2024
Merged via the queue into main with commit e30ab0b May 17, 2024
40 checks passed
@vvagaytsev vvagaytsev deleted the fix/publish-command branch May 17, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: garden publish command does not respect the spec.publishId when container type is used
2 participants