Skip to content
This repository has been archived by the owner on Aug 22, 2022. It is now read-only.

[ghost] Fix PVC creation #1360

Closed
wants to merge 1 commit into from
Closed

[ghost] Fix PVC creation #1360

wants to merge 1 commit into from

Conversation

lcotonea
Copy link
Contributor

@lcotonea lcotonea commented Jan 6, 2022

The current common file is not enough if the .Value contains persistence item is different than "content".
For exemple if you specify these following values, the persistence item shared will not be processed:

persistence:
  content:
    enabled: false
    existingClaim: "pvc-nfs-ghost"
    mountPath: "/var/lib/ghost/content"
  shared:
    enabled: true
    mountPath: /shared
    type: emptyDir

Description of the change

All persistent items are computed in the hardcoded settings.

Benefits

We can specify none, only content, or several persistent items.

Possible drawbacks

None

Applicable issues

Additional information

Checklist

  • Chart version bumped in Chart.yaml according to semver.
  • Title of the PR starts with chart name (e.g. [home-assistant])
  • Variables are documented in the README.md (this can be done with using our helm-docs wrapper ./hack/gen-helm-docs.sh stable <chart>)

The current common file is not enough if the .Value contains persistence item is different than "content".
For exemple if you specify these following values, the persistence item shared will not be processed:
```YAML
persistence:
  content:
    enabled: false
    existingClaim: "pvc-nfs-ghost"
    mountPath: "/var/lib/ghost/content"
  shared:
    enabled: true
    mountPath: /shared
    type: emptyDir
```
@ghost ghost added the precommit:failed CI status: pre-commit validation failed label Jan 6, 2022
@lcotonea lcotonea mentioned this pull request Jan 6, 2022
3 tasks
@ghost ghost added the lint:failed CI status: linting failed label Jan 6, 2022
@bjw-s
Copy link
Contributor

bjw-s commented Jan 6, 2022

As stated in the original PR, I am not a big fan of iterating over the existing values.

I tried installing the chart without enabling the content persistence item, and that seems to work fine. What would be the use case of setting this to an emptyDir if the item is not enabled?

@bjw-s
Copy link
Contributor

bjw-s commented Jan 6, 2022

Closed as discussed in #1359

@bjw-s bjw-s closed this Jan 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lint:failed CI status: linting failed precommit:failed CI status: pre-commit validation failed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants