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

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

Closed
10ko opened this issue May 15, 2024 — with Linear · 0 comments · Fixed by #6052
Closed

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

10ko opened this issue May 15, 2024 — with Linear · 0 comments · Fixed by #6052

Comments

Copy link
Member

10ko commented May 15, 2024

Garden Bonsai (0.13) Bug

The 0.13 behavior of publish commands differs from 0.12. That's a regression in 0.13. See the details below.

Current behavior

When container action- or module-type is used, Garden does not respect the spec.publishId (for actions) and image (for modules) configurations. Instead, it seems to fall back to the local image id.

Expected behavior

The explicit spec.publishId and image must be respected.

Reproducible example

Consider the project below, it contains 1 build action called image-action and 1 module called image-module.

Project config

Click to expand project.garden.yml
apiVersion: garden.io/v1
kind: Project
name: repro-6050

environments:
  - name: local

providers:
  - name: local-kubernetes
    environments: [local]
Click to expand action.garden.yml
kind: Build
description: Desc
type: container
name: image-action
allowPublish: true
spec:
  publishId: "${var.publishRegistry}/image-build:${var.publishTag}"
Click to expand module.garden.yml
kind: Module
description: Desc
type: container
name: image-module
allowPublish: true
image: "${var.publishRegistry}/image-build:${var.publishTag}"

0.13 Behavior

Run the publish command for build action:

> garden publish image-action --var publishTag="v1.0.0" --var publishRegistry="publish.registry.example"

and for module:

> garden publish image-module --var publishTag="v1.0.0" --var publishRegistry="publish.registry.example"

and observe the console output.

Both commands publish the image with its local Garden version used as a tag:

Publishing image publish.registry.example/image-build:v-00dc0a96b1...

This is wrong. Instead, it should behave like in 0.12 as its shown below.

0.12 Behavior

Pre-requisites:

  1. Install the latest garden 0.12
  2. Edit project.garden.yml to have apiVersion: garden.io/v0
  3. Remove action.garden.yml or rename it to not be recognied as a Garden config file (i.e. its name should not end with .garden.yml)

Run the publish command:

> garden publish image-module --var publishTag="v1.0.0" --var publishRegistry="publish.registry.example"

and observe the console output.

It will have the correct tag in the target image:

Publishing image publish.registry.example/image-build:v1.0.0...

Workaround

None.

Suggested solution(s)

Fix the action router to make sure, that publishContainerBuild is getting called consistently for the relevant plugins and providers.

Additional context

Some findings, thoughts and ideas.

Module-to-Action Converter

The fact that both Action- and Module-based configs had behaved incorrectly indicated that's it's likely not an issue in the Module-to-Actions converted. The converter's code was inspected and no issues were found so far.

Action Router

Debugging the reproducible example shew that publishContainerBuild handler of the container-plugin was never called from 0.13 code. That's indicates a potential issue in the action router. The router dispatches the handlers dynamically, and it might get a wrong one for this scenario. This is likely a root cause, but some deeper investigation should be done to verify that.

Prior to #4740 the behaviour was correct, because handler k8sPublishContainerBuild was delegating to publishContainerBuild that, in turn, was reading the publishId value properly.

In #4740 the handlers were decoupled, It's still fine to decouple those.

So, #4740 actually uncovered another issue (this one) that should be fixed now. We need to make sure, that publishContainerBuild is not an orphan and is getting called consistently for the matching plugins and providers.

Update. #4740 introduced some unintended behavior and seems to be relevant to the root cause.

Build Action Outputs

The issue might also be relevant to #4808 and earlier changes or original implementation of getContainerBuildActionOutputs and k8sGetContainerBuildActionOutputs.

We need to realize the semantics of deploymentImageId and deploymentImageName for different contexts (combinations of plugins, providers and lack/presence of a deployment registry configs) and make it clear and test-covered.

We should also revisit various helpers in plugins/container/helpers.ts to make sure there is no unnecessary complexity and all helpers are documented properly and have clear enough behaviour.

Docs

The behaviour of the publish command is described in:

Your environment

garden version 0.13.30 (it seems to happen in all 0.13.x versions so far)

@vvagaytsev vvagaytsev self-assigned this May 15, 2024
@vvagaytsev vvagaytsev changed the title Make sure publish command is consistent between 0.12 and 0.13. Make sure publish command is consistent between 0.12 and 0.13 May 16, 2024
@vvagaytsev vvagaytsev changed the title Make sure publish command is consistent between 0.12 and 0.13 [Bug] Make sure publish command is consistent between 0.12 and 0.13 May 16, 2024
@vvagaytsev vvagaytsev changed the title [Bug] Make sure publish command is consistent between 0.12 and 0.13 [Bug]: Make sure publish command is consistent between 0.12 and 0.13 May 16, 2024
@vvagaytsev vvagaytsev changed the title [Bug]: Make sure publish command is consistent between 0.12 and 0.13 [Bug]: garden publish command does not respect the --tag flag when container type is used May 16, 2024
@vvagaytsev vvagaytsev changed the title [Bug]: garden publish command does not respect the --tag flag when container type is used [Bug]: garden publish command does not respect the spec.publishId when container type is used May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants