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]: Templating in sync.paths[].mode not working since 0.13.53 #6843

Closed
andreavocado opened this issue Feb 12, 2025 · 9 comments · Fixed by #6844
Closed

[Bug]: Templating in sync.paths[].mode not working since 0.13.53 #6843

andreavocado opened this issue Feb 12, 2025 · 9 comments · Fixed by #6844
Assignees
Labels
bug feat/templating Templating in variables and configs regression A return to a previous and less advanced or worse state

Comments

@andreavocado
Copy link

Bug

Current behavior

Given following YAML:

sync:
  paths:
    - source: .
      target: /app/
      mode: ${var.syncmode || 'one-way-safe'}

produces following output when running deploy service

Error validating spec for Deploy type=container name=service (../service):

spec.sync.paths[0][mode] must be one of [one-way, one-way-safe, one-way-replica, one-way-reverse, one-way-replica-reverse, two-way, two-way-safe, two-way-resolved]

Expected behavior

It evaluates the expresion and uses the variable syncmode if given, otherwise the default 'one-way-safe'

Reproducible example

s.a.

Workaround

Use no expresions:

sync:
  paths:
    - source: .
      target: /app/
      mode: 'one-way-safe'

Suggested solution(s)

Revert or fix changes made in 0.13.53

Additional context

Works as expected untill 0.13.52.

Your environment

garden version: 0.13.53

@10ko
Copy link
Member

10ko commented Feb 12, 2025

Hi @andreavocado, thanks for reporting this.
We released some big performance improvements in 0.13.53 that included some substantial changes in the templating system.
Unfortunately it looks our tests didn't catch this specific edge case.
We'll take a look at this since we are currently working on fixing a bunch of these regressions. Ideally, we are aiming at pushing a new release the latest next week.

Apologies for the issues this might be causing.

@10ko 10ko added the feat/templating Templating in variables and configs label Feb 12, 2025
@vvagaytsev
Copy link
Collaborator

Thanks for reporting this @andreavocado!

How to you define the variables? Do you use variables or varfile?

@vvagaytsev vvagaytsev self-assigned this Feb 12, 2025
@andreavocado
Copy link
Author

andreavocado commented Feb 12, 2025

@vvagaytsev I'm using both:

In the garden.yml of the project is following defined:

varfiles:
  - garden.sync.yaml

The garden.sync.yaml contains:

sync:
  paths:
    - source: .
      target: /app/
      mode: ${var.syncmode || 'one-way-safe'}

The variable var.syncmode is defined in the garden.remote.env

syncmode=two-way-resolved

@vvagaytsev
Copy link
Collaborator

@andreavocado how to you import the garden.remote.env file into your Garden configuration?

It should be declared as:

kind: Project
varfile: garden.remote.env
...

in the Project-config or

kind: Deploy
varfiles: [garden.remote.env]
...

in the Action-config.

The config

varfiles:
  - garden.sync.yaml

looks looks like a snippet of a garden.yml config file, and not like a YAML file that declares variables. That's a bit weird.

Unfortunately, I still cannot reproduce it.

Can you please share the Project-level config and the Action-config that declares the deployment in sync-mode? Feel free to omit all sensitive data. It would be very helpful to have a minimal config to proroduce the issue.

@vvagaytsev vvagaytsev added the regression A return to a previous and less advanced or worse state label Feb 12, 2025
@andreavocado
Copy link
Author

@vvagaytsev I do my best of showing my setup ;)

The garden.remote.env should be loaded via your 4th step of Variable precedence order The environment-specific varfile (defaults to garden.< env-name >.env)

Here are the other project files:

project.garden.yml

kind: Project
apiVersion: garden.io/v1
name: marketplace
defaultEnvironment: remote
environments:
  - name: remote
providers:
  - name: kubernetes
    environments: ["remote"]
    context: gke_*
    namespace: any_namespace
    defaultHostname: any_hostname
    buildMode: cluster-buildkit
    deploymentRegistry:
      hostname: *docker.pkg.dev
      namespace: any_namespace
    imagePullSecrets:
      - name: gcr-config
        namespace: default
      - name: gar-config
        namespace: default
dotIgnoreFile: .gardenignore
sources:
  - name: service
    repositoryUrl: "git@github.com:service.git#main"

garden.yml

kind: Deploy
type: container
name: service
description: any description
dependencies:
  - build.service
  - deploy.redis
  - deploy.selenium-chrome
varfiles:
  - garden.envs.yaml
  - garden.sync.yaml
spec:
  image: some_image
  command: ["/bin/sh", "-c"]
  args:
    - "bundle exec rails server -b 0.0.0.0 -p 3000"
  ports:
    - name: http
      containerPort: 3000
      servicePort: 3000
  limits:
    cpu: 1500
    memory: 2048
  sync: ${var.sync}
  env: ${var.envVars}

garden.sync.yaml

sync:
  paths:
    - source: .
      target: /app/
      mode: ${var.syncmode || 'one-way-safe'}

@vvagaytsev
Copy link
Collaborator

vvagaytsev commented Feb 12, 2025

Thanks for sharing the config, @andreavocado!

I was able to reproduce the issue. This regression was caused by #6745 and was already fixed in #6844. I've tested it with the PR branch locally and got no error.

The PR with the fix is still in review. Once it's merged and built, you can try out the edge-bonsai release. I'll keep you posted.

Update (see the next comment)

However, I think there is one more bug related to the default environment-specific varfiles.

I think there might be one more bug in 0.13.52 (and most likely in older 0.13.x versions). Can you please check the log outputs of 0.13.52 to make sure that variable syncmode defined in the garden.remote.env actually has any effect?

In the dev console log toy should see something like this:

Syncing . to /app/ in Deployment/service (one-way-safe)

If you set syncmode to something different from "one-way-safe", then you should see it in the log message. I've just tried it locally, and it looks like Garden does not load the default env file as it's described in the docs.

I'll check whether it's an actual bug or we have outdated documentation.

The workaround is to add garden.${environment.name}.env to the list of varfiles, so it will look like:

varfiles:
  - garden.sync.yml
  - garden.${environment.name}.env

@vvagaytsev
Copy link
Collaborator

@andreavocado sorry for the confusion, I tried a wrong config. My default env varfile was in the wrong location :)

@vvagaytsev
Copy link
Collaborator

@andreavocado feel free to try out the latest edge release, it should fix the issue.

You can get it by running this command:

garden self-update edge-bonsai

@andreavocado
Copy link
Author

@vvagaytsev I tried the newest version and the sync works as expected. Thanks for your good job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feat/templating Templating in variables and configs regression A return to a previous and less advanced or worse state
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants