woodpecker-ci / helm

This repo contains the helm charts of the Woodpecker project.
Apache License 2.0
20 stars 16 forks source link

Bad indent setting for extraVolumes? #118

Closed dominic-p closed 7 months ago

dominic-p commented 8 months ago

This is a follow up from #97. In my testing, it looks like the extra volume mounts are added at the wrong indent level which breaks the YAML and the entire chart. If I run helm template ... I get YAML that looks like this:

# Source: woodpecker/charts/server/templates/statefulset.yaml
apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: woodpecker-server
spec:
  serviceName: woodpecker-server-headless
  template:
    spec:
      serviceAccountName: default
      securityContext:
        {}
      containers:
        - name: server
          securityContext:
            {}
          image: "docker.io/woodpeckerci/woodpecker-server:next"
          imagePullPolicy: IfNotPresent
          resources:
            {}
          volumeMounts:
          - name: data
            mountPath: /var/lib/woodpecker
            - mountPath: /etc/registry
              name: reg-cred
              readOnly: true
          env:
            - name: ...

I removed some irrelevant parts for clarity. You can see that the mountPath stanza is indented a bit too far.

pat-s commented 8 months ago

There's an unreleased fix in #98, but overall I agree, we need tests for this. I am about to add some.

pat-s commented 8 months ago

@dominic-p I've asserted the volume templating in #121. Does it work for you in v1.0.0 or is it still an issue?

dominic-p commented 8 months ago

Thanks for working on this. I tried to test 1.0.0 tonight, but I can't seem to download it from either https://woodpecker-ci.org or oci://registry-1.docker.io/woodpeckerci/helm. In both cases I get:

Error: failed to download "woodpecker/woodpecker" at version "1.0.0"

If I look at https://woodpecker-ci.org/index.yaml, I'm not seeing a reference to the 1.0.0 version. I'm guessing it will just take some time for the new release to be deployed.

pat-s commented 8 months ago

There was an error during the release, 1.0.1 is now available.

dominic-p commented 7 months ago

Thanks for working on that. I just tested 1.0.1 and extraVolumes are working as expected for me.