Skip to content
This repository has been archived by the owner on May 8, 2024. It is now read-only.

[stable/openebs]: update openebs charts to 2.4.0 #174

Merged
merged 3 commits into from
Dec 15, 2020

Conversation

akhilerm
Copy link
Member

@akhilerm akhilerm commented Dec 4, 2020

  • update chart version
  • update values.yaml
  • update README

Signed-off-by: Akhil Mohan akhil.mohan@mayadata.io

Special notes for your reviewer:

The PR should be squashed and merged.

Checklist

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [stable/openebs])

Comment on lines +32 to +35
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- toYaml . | nindent 8 }}
{{- end }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sonasingh46 @prateekpandey14 Should we use with or if.

  • We need to be consistent across all the repos.
  • We need to be consistent in the templates. For some fields with and for some if are used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my two cents. You should stay with Helm defaults. If you do helm create test.. you will see that in the deployment file

{{- with .Values.imagePullSecrets }}
      imagePullSecrets:
        {{- toYaml . | nindent 8 }}
      {{- end }}
...
{{- with .Values.nodeSelector }}
      nodeSelector:
        {{- toYaml . | nindent 8 }}
      {{- end }}
      {{- with .Values.affinity }}
      affinity:
        {{- toYaml . | nindent 8 }}
      {{- end }}
      {{- with .Values.tolerations }}
      tolerations:
        {{- toYaml . | nindent 8 }}
      {{- end }}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great. Then I will follow this pattern,

@survivant
Copy link
Contributor

will it include cstor-operator too ?

@akhilerm
Copy link
Member Author

will it include cstor-operator too ?

@survivant this is the PR for cstor-operator charts

@akhilerm akhilerm removed the pr/hold-merge The PR should not be merged now label Dec 14, 2020
- update chart version
- update values.yaml
- update README

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
- update chart version
- update values.yaml
- update README

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
- update chart version
- update values.yaml
- update README

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants