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

Optional template variables seem to be broken since 0.12.13 #2241

Closed
tvollstaedt opened this issue Feb 3, 2021 · 7 comments · Fixed by #2243
Closed

Optional template variables seem to be broken since 0.12.13 #2241

tvollstaedt opened this issue Feb 3, 2021 · 7 comments · Fixed by #2243

Comments

@tvollstaedt
Copy link
Contributor

tvollstaedt commented Feb 3, 2021

Bug

After upgrading garden to v0.12.13, my workflow broke. I've boiled it down to handling of optional variables, in my case it's the definition of a Dockerfile within a container module.

Current Behavior

The container module tries to pull the given image instead of building it. This is because it doesn't recognize a Dockerfile, given through the instruction dockerfile: ${var.dockerfile}.

Expected behavior

The workflow should work exactly the same as with 0.12.12.

Reproducible example

garden.yml

kind: Project
name: example
providers:
  - name: local-kubernetes
    setupIngressController: null
    namespace: example
variables:
  dockerfile: Dockerfile.example
---
kind: Module
type: container
name: example
allowPublish: false
dockerfile: ${var.dockerfile}?
image: example
services:
  - name: nginx

Dockerfile.example:

FROM nginx

garden 0.12.12:

thomas@SYSTEM ~/d/p/g/broken-optional-variables > curl -sL https://get.garden.io/install.sh | zsh -s 0.12.12
...
❊ Installing the Garden CLI ❊

→ Downloading https://github.com/garden-io/garden/releases/download/0.12.12/garden-0.12.12-linux-amd64.tar.gz...
→ Extracting to /home/thomas/.garden/bin...

🌺🌻  Garden has been successfully installed 🌷💐
...
thomas@SYSTEM ~/d/p/g/broken-optional-variables > garden deploy
Deploy

✔ providers                 → Getting status... → Done
✔ example                   → Syncing module sources (1 file)... → Done (took 0 sec)
✔ example                   → Building example:v-e59908670c... → Done (took 1.5 sec)
✔ nginx                     → Deploying version v-e59908670c... → Done (took 3.3 sec)
   ℹ nginx                     → Resources ready

Done!

garden 0.12.13 onwards:

thomas@SYSTEM ~/d/p/g/broken-optional-variables > curl -sL https://get.garden.io/install.sh | zsh -s 0.12.13
...
❊ Installing the Garden CLI ❊

→ Downloading https://github.com/garden-io/garden/releases/download/0.12.13/garden-0.12.13-linux-amd64.tar.gz...
→ Extracting to /home/thomas/.garden/bin...

🌺🌻  Garden has been successfully installed 🌷💐
...
thomas@SYSTEM ~/d/p/g/broken-optional-variables > garden deploy
Deploy

✔ providers                 → Getting status... → Done
✔ example                   → Getting build status for v-a8ee0b0019... → Done (took 0 sec)
✖ nginx                     → Deploying version v-a8ee0b0019...
   ℹ nginx                     → Waiting for resources to be ready...

Failed deploying service 'nginx' (from module 'example'). Here is the output:
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Error deploying nginx: Failed - Error: ImagePullBackOff

━━━ Events ━━━
Deployment nginx: ScalingReplicaSet - Scaled up replica set nginx-6cc88f679c to 1
Deployment nginx: ScalingReplicaSet - Scaled up replica set nginx-5b64898cc to 1
Deployment nginx: ScalingReplicaSet - Scaled down replica set nginx-6cc88f679c to 0
Deployment nginx: ScalingReplicaSet - Scaled up replica set nginx-f94bd598d to 1
Deployment nginx: ScalingReplicaSet - Scaled down replica set nginx-5b64898cc to 0
Deployment nginx: ScalingReplicaSet - Scaled up replica set nginx-54765649b8 to 1
Deployment nginx: ScalingReplicaSet - Scaled down replica set nginx-f94bd598d to 0
Deployment nginx: ScalingReplicaSet - Scaled up replica set nginx-7c685f68f to 1
Deployment nginx: ScalingReplicaSet - Scaled down replica set nginx-54765649b8 to 0
Deployment nginx: ScalingReplicaSet - (combined from similar events): Scaled down replica set nginx-9bb9bbfc4 to 0
Pod nginx-5958cd8b9d-gj574: Scheduled - Successfully assigned example/nginx-5958cd8b9d-gj574 to docker-desktop
Pod nginx-5958cd8b9d-gj574: Pulling - Pulling image "example"
Pod nginx-5958cd8b9d-gj574: Failed - Failed to pull image "example": rpc error: code = Unknown desc = Error response from
daemon: pull access denied for example, repository does not exist or may require 'docker login': denied: requested access to
the resource is denied
Pod nginx-5958cd8b9d-gj574: Failed - Error: ErrImagePull
Pod nginx-5958cd8b9d-gj574: BackOff - Back-off pulling image "example"
Pod nginx-5958cd8b9d-gj574: Failed - Error: ImagePullBackOff

━━━ Pod logs ━━━
<Showing last 30 lines per pod in this Deployment. Run the following command for complete logs>
$ kubectl -n example --context=docker-desktop logs deployment/nginx

****** nginx-5958cd8b9d-gj574 ******
------ nginx ------<no logs>
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
1 deploy task(s) failed!

See error.log for detailed error message
thomas@SYSTEM ~/d/p/g/broken-optional-variables >

Workaround

Stay on 0.12.12 or omit optional variables (not applicable in my case).

Suggested solution(s)

Fix handling of optional variables.

Additional context

I didn't test other scenarios than the one with dockerfile listed in the example above. It may a corner case of dockerfile, but I believe it's the general template variable handling that has changed with 0.12.13.

Your environment

  • OS: Ubuntu on WSL2
  • How I'm running Kubernetes: Docker Desktop for Windows

garden version

0.12.13

@edvald
Copy link
Collaborator

edvald commented Feb 3, 2021

Thanks for the detailed report @tvollstaedt. I'm gonna take a quick look. One quick thing to validate this: Does the example work if you hardcode the dockerfile value, instead of the template reference?

@tvollstaedt
Copy link
Contributor Author

Thanks for the detailed report @tvollstaedt. I'm gonna take a quick look. One quick thing to validate this: Does the example work if you hardcode the dockerfile value, instead of the template reference?

Yep, hardcoding or simply removing the ? fixes the example.

@edvald
Copy link
Collaborator

edvald commented Feb 3, 2021

Got it, thanks! I'll address this ahead of tomorrow's release.

@edvald
Copy link
Collaborator

edvald commented Feb 3, 2021

Actually I'm so far failing to reproduce this. Using the exact example you provided (adding the Dockerfile.example file with just a FROM nginx statement), it works just fine. And I added some tests to be sure the optional template strings work as expected, just to make sure.

One possibility is that it's already something fixed on the current master, or that there's something different in our environments. I'm also using Docker Desktop, albeit on Mac. Perhaps you could test the latest edge build? https://github.com/garden-io/garden/releases/tag/untagged-a1a702f18adcb4d63d5e

@edvald
Copy link
Collaborator

edvald commented Feb 3, 2021

There have also been a couple of patch releases since 0.12.13, so I'd be curious if 0.12.15 has the same problem.

@tvollstaedt
Copy link
Contributor Author

Hi Jon, thanks for looking into it! I've just tried the latest 0.12.15 with an empty kubernetes cluster and still got the same result.
It only works if your cluster / daemon did not build/tag that image before.

Anything else that could differ between our systems in garden's point of view?

thomas@SYSTEM ~/d/p/g/broken-optional-variables > lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 20.04.1 LTS
Release:        20.04
Codename:       focal
thomas@SYSTEM ~/d/p/g/broken-optional-variables > cat Dockerfile.example
FROM nginx
thomas@SYSTEM ~/d/p/g/broken-optional-variables > cat garden.yml
kind: Project
name: example
providers:
  - name: local-kubernetes
    setupIngressController: null
    namespace: example
variables:
  dockerfile: Dockerfile.example
---
kind: Module
type: container
name: example
allowPublish: false
dockerfile: ${var.dockerfile}?
image: example
services:
  - name: nginx

thomas@SYSTEM ~/d/p/g/broken-optional-variables > garden -v
0.12.15
thomas@SYSTEM ~/d/p/g/broken-optional-variables > garden deploy
Deploy

✔ providers                 → Getting status... → Cached
   ℹ Run with --force-refresh to force a refresh of provider statuses.
✔ example                   → Getting build status for v-a8ee0b0019... → Done (took 0 sec)
✖ nginx                     → Deploying version v-a8ee0b0019...
   ℹ nginx                     → Waiting for resources to be ready...

Failed deploying service 'nginx' (from module 'example'). Here is the output:
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Error deploying nginx: ImagePullBackOff - Back-off pulling image "example"

━━━ Events ━━━
Deployment nginx: ScalingReplicaSet - Scaled up replica set nginx-5958cd8b9d to 1
Pod nginx-5958cd8b9d-n2xc7: Scheduled - Successfully assigned example/nginx-5958cd8b9d-n2xc7 to docker-desktop
Pod nginx-5958cd8b9d-n2xc7: Pulling - Pulling image "example"
Pod nginx-5958cd8b9d-n2xc7: Failed - Failed to pull image "example": rpc error: code = Unknown desc = Error response from
daemon: pull access denied for example, repository does not exist or may require 'docker login': denied: requested access to
the resource is denied
Pod nginx-5958cd8b9d-n2xc7: Failed - Error: ErrImagePull
Pod nginx-5958cd8b9d-n2xc7: BackOff - Back-off pulling image "example"
Pod nginx-5958cd8b9d-n2xc7: Failed - Error: ImagePullBackOff

━━━ Pod logs ━━━
<Showing last 30 lines per pod in this Deployment. Run the following command for complete logs>
$ kubectl -n example --context=docker-desktop logs deployment/nginx

****** nginx-5958cd8b9d-n2xc7 ******
------ nginx ------<no logs>
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
1 deploy task(s) failed!

See error.log for detailed error message

@edvald
Copy link
Collaborator

edvald commented Feb 3, 2021

I think I figured it out. That was a subtle one. I'll see if I can fix this ahead of the release, want to make sure we don't accidentally break something else.

edvald added a commit that referenced this issue Feb 3, 2021
Previously, we would immediately resolve an optional template string
as undefined during partial resolution, which would cause problems when
using optional template strings for variables that are only available
later in the process. We now leave them unchanged until the final
resolution occurs.

Fixes #2241
thsig pushed a commit that referenced this issue Feb 4, 2021
Previously, we would immediately resolve an optional template string
as undefined during partial resolution, which would cause problems when
using optional template strings for variables that are only available
later in the process. We now leave them unchanged until the final
resolution occurs.

Fixes #2241
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants