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

[mosquitto] Remove storage fields from configinc value #1710

Merged
merged 2 commits into from
Aug 2, 2022

Conversation

terop
Copy link
Contributor

@terop terop commented Jul 26, 2022

Description of the change

Persistent storage settings for configinc have been removed because they should not have been there in the first place as they are to be specified by the user.

Benefits

The chart is aligned to the best practices used in this repository.

Possible drawbacks

The user must now specify the storage manually.

Checklist

  • [ X] Title of the PR starts with chart name (e.g. [home-assistant])
  • [ X] Chart version bumped in Chart.yaml according to semver.
  • [ X] Chart artifacthub.io/changes changelog annotation has been updated in Chart.yaml. See Artifact Hub documentation for more info.
  • [ X] Variables have been documented in the values.yaml file.

@ghost ghost added the size/XS Categorises a PR that changes 0-9 lines, ignoring generated files. label Jul 26, 2022
@terop terop force-pushed the mosquitto_add_storage_class_option branch from f4ad942 to 5991ab5 Compare July 26, 2022 11:35
@ghost ghost added precommit:ok CI status: pre-commit validation successful changelog:ok CI status: changelog validation successful lint:ok CI status: linting successful install:ok CI status: install successful labels Jul 26, 2022
@bjw-s
Copy link
Contributor

bjw-s commented Jul 28, 2022

Hi! Thanks for taking the time to raise this PR! Unfortunately it has exposed something in the chart that should not have been there in the first place. We try to keep the persistence fields as minimal as possible and let the user determine their desired values based on their needs (guided by http://docs.k8s-at-home.com/our-helm-charts/common-library-storage/).

That means we will / should be removing the following fields:

persistence:
  configinc:
    accessMode: ReadWriteOnce
    # The default storage class is used when the null value is used. Set this
    # to non-null to use a different storage class.
    storageClass: null
    size: 100Mi

@terop
Copy link
Contributor Author

terop commented Jul 29, 2022

Okay, thanks for your input @bjw-s. Should I close this PR or update it to remove the part you indicated?

@bjw-s
Copy link
Contributor

bjw-s commented Aug 1, 2022

Okay, thanks for your input @bjw-s. Should I close this PR or update it to remove the part you indicated?

It would be great if you could update the PR to remove it. Thanks so much!!

Persistence configuration in values.yaml should be kept to a minimum
so that users can implement it based on their liking.

Signed-off-by: Tero Paloheimo <[email protected]>
@terop terop force-pushed the mosquitto_add_storage_class_option branch from 5991ab5 to 03ab183 Compare August 1, 2022 15:25
@terop terop changed the title [mosquitto] Add option to specify persistence storageClass [mosquitto] Remove storage fields from configinc value Aug 1, 2022
@terop
Copy link
Contributor Author

terop commented Aug 1, 2022

PR updated to remove the fields.

@bjw-s bjw-s enabled auto-merge (squash) August 2, 2022 12:40
@bjw-s bjw-s merged commit 8d8dba0 into k8s-at-home:master Aug 2, 2022
@terop terop deleted the mosquitto_add_storage_class_option branch August 2, 2022 13:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
changelog:ok CI status: changelog validation successful install:ok CI status: install successful lint:ok CI status: linting successful precommit:ok CI status: pre-commit validation successful size/XS Categorises a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants