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 YAML parse: error converting YAML to JSON: when redis.enabled is set to false. #176

Closed
wants to merge 1 commit into from

Conversation

nilya
Copy link

@nilya nilya commented Jun 3, 2024

When you set redis.enabled to false Helm cannot generate valid yaml. The error is:

Error: YAML parse error on redash/templates/hook-migrations-job.yaml: error converting YAML to JSON: yaml: line 43: mapping values are not allowed in this context
helm.go:84: [debug] error converting YAML to JSON: yaml: line 43: mapping values are not allowed in this context
YAML parse error on redash/templates/hook-migrations-job.yaml
helm.sh/helm/v3/pkg/releaseutil.(*manifestFile).sort
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:146
helm.sh/helm/v3/pkg/releaseutil.SortManifests
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:106
helm.sh/helm/v3/pkg/action.(*Configuration).renderResources
        helm.sh/helm/v3/pkg/action/action.go:168
helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
        helm.sh/helm/v3/pkg/action/install.go:312
main.runInstall
        helm.sh/helm/v3/cmd/helm/install.go:314
main.newTemplateCmd.func2
        helm.sh/helm/v3/cmd/helm/template.go:95
github.com/spf13/cobra.(*Command).execute
        github.com/spf13/[email protected]/command.go:983
github.com/spf13/cobra.(*Command).ExecuteC
        github.com/spf13/[email protected]/command.go:1115
github.com/spf13/cobra.(*Command).Execute
        github.com/spf13/[email protected]/command.go:1039
main.main
        helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
        runtime/proc.go:271
runtime.goexit
        runtime/asm_amd64.s:1695

The helm template command generates output with wrong new-lines:

          env:
            - name: REDASH_DATABASE_URL
              valueFrom:
                secretKeyRef:
                  key: connectionString
                  name: redash- name: REDASH_REDIS_URL
              value: "redis://redis:6379/0"- name: PYTHONUNBUFFERED
              value: "0"
            - name: QUEUES
              value: "scheduled_queries,schemas"

This change fixes the error.
One caveat of the fix: when redis.enabled is set to true Helm adds additional new-line:

        env:
          - name: REDASH_DATABASE_URL
            valueFrom:
              secretKeyRef:
                key: connectionString
                name: redash
          - name: REDASH_REDIS_PASSWORD
            valueFrom:
              secretKeyRef:
                name: redash-redis
                key: redis-password
          - name: REDASH_REDIS_HOSTNAME
            value: redash-redis-master
          - name: REDASH_REDIS_PORT
            value: "6379"
          - name: REDASH_REDIS_NAME
            value: "0"

          ## Start primary Redash configuration

So maybe a better fix exists.

…re not allowed in this context when using redis.enabled=false.

When you set `redis.enabled` to `false` Helm cannot generate valid yaml.
The error is:

```
Error: YAML parse error on redash/templates/hook-migrations-job.yaml: error converting YAML to JSON: yaml: line 43: mapping values are not allowed in this context
helm.go:84: [debug] error converting YAML to JSON: yaml: line 43: mapping values are not allowed in this context
YAML parse error on redash/templates/hook-migrations-job.yaml
helm.sh/helm/v3/pkg/releaseutil.(*manifestFile).sort
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:146
helm.sh/helm/v3/pkg/releaseutil.SortManifests
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:106
helm.sh/helm/v3/pkg/action.(*Configuration).renderResources
        helm.sh/helm/v3/pkg/action/action.go:168
helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
        helm.sh/helm/v3/pkg/action/install.go:312
main.runInstall
        helm.sh/helm/v3/cmd/helm/install.go:314
main.newTemplateCmd.func2
        helm.sh/helm/v3/cmd/helm/template.go:95
github.com/spf13/cobra.(*Command).execute
        github.com/spf13/[email protected]/command.go:983
github.com/spf13/cobra.(*Command).ExecuteC
        github.com/spf13/[email protected]/command.go:1115
github.com/spf13/cobra.(*Command).Execute
        github.com/spf13/[email protected]/command.go:1039
main.main
        helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
        runtime/proc.go:271
runtime.goexit
        runtime/asm_amd64.s:1695
```

The `helm template` command generates output with wrong new-lines:

```yaml
          env:
            - name: REDASH_DATABASE_URL
              valueFrom:
                secretKeyRef:
                  key: connectionString
                  name: redash- name: REDASH_REDIS_URL
              value: "redis://redis:6379/0"- name: PYTHONUNBUFFERED
              value: "0"
            - name: QUEUES
              value: "scheduled_queries,schemas"
```

This change fixes the error.
One caveat of the fix: when `redis.enabled` is set to `true` Helm adds additional new-line:

```yaml
        env:
          - name: REDASH_DATABASE_URL
            valueFrom:
              secretKeyRef:
                key: connectionString
                name: redash
          - name: REDASH_REDIS_PASSWORD
            valueFrom:
              secretKeyRef:
                name: redash-redis
                key: redis-password
          - name: REDASH_REDIS_HOSTNAME
            value: redash-redis-master
          - name: REDASH_REDIS_PORT
            value: "6379"
          - name: REDASH_REDIS_NAME
            value: "0"

          ## Start primary Redash configuration
```

So maybe a better fix exists.
@frivoire
Copy link
Contributor

frivoire commented Jun 14, 2024

I've created a PR for the same issue (& other things): #180
Sorry, I did my PR before seeing yours here 😕

I would recommend to keep "my" version, because I cover both cases: postgres & redis, not just redis like here. And I don't have the "when redis.enabled is set to true, Helm adds additional new-line" issue

@lucydodo
Copy link
Member

Sorry, I missed the notification for this PR. As an aside, as @frivoire mentioned above, I'm approving #180 PR and closing this PR because that PR includes some better improvements. I apologize for the late resposne.

@lucydodo lucydodo closed this Jun 20, 2024
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 this pull request may close these issues.

3 participants