gitea/helm-gitea
Fork 169
Code Issues 54 Pull Requests 11 Actions 2 Releases 103 Activity

add global values support #322

Merged
luhahn merged 1 commits from cnfatal/helm-chart:master into master 2022-06-09 10:55:09 +00:00
Conversation 5 Commits 1 Files Changed 3 +41 -8
cnfatal commented 2022-05-23 03:11:43 +00:00
Contributor
Copy Link

add global values support for .global.storageClass .global.imageRegistry and .global.imagePullSecrets

add global values support for `.global.storageClass` `.global.imageRegistry` and `.global.imagePullSecrets`
👍 1
cnfatal force-pushed master from a87ec8a163 to f3d10f14c4 2022-05-23 03:24:27 +00:00 Compare
justusbunsi commented 2022-05-23 05:01:04 +00:00
Contributor
Copy Link

+1 for switching to readme-generator. Thanks for the effort. Would you mind splitting the two parts into different PRs? That would make a review much easier.

EDIT: Splitted into different PRs.

~~+1 for switching to readme-generator. Thanks for the effort. Would you mind splitting the two parts into different PRs? That would make a review much easier.~~ EDIT: Splitted into different PRs.
cnfatal force-pushed master from f3d10f14c4 to 3e4e94b075 2022-05-23 07:10:07 +00:00 Compare
justusbunsi added a new dependency 2022-05-27 10:32:58 +00:00
#323 generate readme Parameters from values.yaml
justusbunsi reviewed 2022-05-27 11:15:49 +00:00
justusbunsi left a comment
Contributor
Copy Link

Many thanks for your contribution. I've marked some minor things.

Many thanks for your contribution. I've marked some minor things.
templates/_helpers.tpl Outdated
@@ -35,10 +35,50 @@ Create chart name and version as used by the chart label.
Create image name and tag used by the deployment.
*/}}
{{- define "gitea.image" -}}
{{- $registry := .Values.image.registry -}}
justusbunsi commented 2022-05-27 11:07:09 +00:00
Contributor
Copy Link

.Values.image.registry does not exist yet. Do you mind adding it to the values.yaml?

In addition, we could simplify the override of $registry here.

- {{- $registry := .Values.image.registry -}} + {{- $registry := .Values.global.imageRegistry | default .Values.image.registry -}}
`.Values.image.registry` does not exist yet. Do you mind adding it to the values.yaml? In addition, we could simplify the override of `$registry` here. ```diff - {{- $registry := .Values.image.registry -}} + {{- $registry := .Values.global.imageRegistry | default .Values.image.registry -}} ```
justusbunsi commented 2022-05-27 11:28:55 +00:00
Contributor
Copy Link

.Values.image.registry does not exist yet. Do you mind adding it to the values.yaml?

Nevermind that part. Saw that it's added in the readme generator PR. ?

> `.Values.image.registry` does not exist yet. Do you mind adding it to the values.yaml? Nevermind that part. Saw that it's added in the readme generator PR. ?
cnfatal marked this conversation as resolved
templates/_helpers.tpl Outdated
@@ -42,0 +68,4 @@
Return the proper Storage Class
*/}}
{{- define "gitea.persistence.storageClass" -}}
{{- $storageClass := .Values.persistence.storageClass -}}
justusbunsi commented 2022-05-27 10:52:29 +00:00
Contributor
Copy Link

We could simplify the override of $storageClass.

- {{- $storageClass := .Values.persistence.storageClass -}} + {{- $storageClass := .Values.global.storageClass | default .Values.persistence.storageClass -}}
We could simplify the override of `$storageClass`. ```diff - {{- $storageClass := .Values.persistence.storageClass -}} + {{- $storageClass := .Values.global.storageClass | default .Values.persistence.storageClass -}} ```
cnfatal marked this conversation as resolved
templates/_helpers.tpl Outdated
@@ -42,0 +73,4 @@
{{- $storageClass = .Values.global.storageClass -}}
{{- end -}}
{{- if $storageClass -}}
{{- if (eq "-" $storageClass) -}}
justusbunsi commented 2022-05-27 10:41:14 +00:00
Contributor
Copy Link

Where does the - come from? Isn't the default value ""?

Where does the `-` come from? Isn't the default value `""`?
cnfatal commented 2022-05-31 02:46:54 +00:00
Author
Contributor
Copy Link

Ref from other charts (bitnami's) ,it says:

 ## @param persistence.storageClass PVC Storage Class for Cassandra data volume  ## If defined, storageClassName: <storageClass>  ## If set to "-", storageClassName: "", which disables dynamic provisioning  ## If undefined (the default) or set to null, no storageClassName spec is  ## set, choosing the default provisioner. (gp2 on AWS, standard on  ## GKE, AWS & OpenStack)  ##  storageClass: "" 
{{- if $storageClass -}} storageClassName: {{ $storageClass }} {{- end -}}

The empty "" and null val not pass the if check, lead to a no storageClassName spec

But the usage of "-" is really few,I'll remove it.

Ref from other charts (bitnami's) ,it says: ```yaml ## @param persistence.storageClass PVC Storage Class for Cassandra data volume ## If defined, storageClassName: <storageClass> ## If set to "-", storageClassName: "", which disables dynamic provisioning ## If undefined (the default) or set to null, no storageClassName spec is ## set, choosing the default provisioner. (gp2 on AWS, standard on ## GKE, AWS & OpenStack) ## storageClass: "" ``` ```tpl {{- if $storageClass -}} storageClassName: {{ $storageClass }} {{- end -}} ``` The empty `""` and `null` val not pass the `if` check, lead to a `no storageClassName spec` --- But the usage of "-" is really few,I'll remove it.
cnfatal marked this conversation as resolved
justusbunsi added the
kind
feature
 label 2022-05-27 11:17:43 +00:00
cnfatal force-pushed master from 3e4e94b075 to 96b27e6a8f 2022-05-31 02:57:55 +00:00 Compare
cnfatal force-pushed master from 96b27e6a8f to adcc7b974a 2022-05-31 03:07:53 +00:00 Compare
justusbunsi reviewed 2022-06-06 16:56:07 +00:00
templates/_helpers.tpl Outdated
@@ -42,0 +68,4 @@
{{- $storageClass := .Values.global.storageClass | default .Values.persistence.storageClass -}}
{{- if $storageClass -}}
storageClassName: {{ $storageClass }}
{{- end -}}
justusbunsi commented 2022-06-06 16:56:07 +00:00
Contributor
Copy Link

@cnfatal Right now generating the template fails. It seems this function uses incorrect line wrap trimming. A working function declaration would be:

{{/* Storage Class */}} {{- define "gitea.persistence.storageClass" -}} {{- $storageClass := .Values.global.storageClass | default .Values.persistence.storageClass }} {{- if $storageClass }} storageClassName: {{ $storageClass | quote }} {{- end }} {{- end -}}

And we should quote the storage class value (as done in the proposed fix). Just to be sure. :)

@cnfatal Right now generating the template fails. It seems this function uses incorrect line wrap trimming. A working function declaration would be: ``` {{/* Storage Class */}} {{- define "gitea.persistence.storageClass" -}} {{- $storageClass := .Values.global.storageClass | default .Values.persistence.storageClass }} {{- if $storageClass }} storageClassName: {{ $storageClass | quote }} {{- end }} {{- end -}} ``` And we should quote the storage class value (as done in the proposed fix). Just to be sure. :)
👍 1
cnfatal commented 2022-06-09 02:55:12 +00:00
Author
Contributor
Copy Link

you're right

you're right
cnfatal marked this conversation as resolved
cnfatal force-pushed master from adcc7b974a to 048b5c480e 2022-06-09 02:26:32 +00:00 Compare
luhahn approved these changes 2022-06-09 10:52:00 +00:00
luhahn left a comment
Member
Copy Link

LGTM

LGTM
justusbunsi approved these changes 2022-06-09 10:54:16 +00:00
justusbunsi left a comment
Contributor
Copy Link

LGTM

LGTM
luhahn merged commit 9cb822f41c into master 2022-06-09 10:55:09 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
luhahn
justusbunsi
Labels
Clear labels
has
backport
in progress
invalid
kind
breaking
kind
bug
kind
build
kind
dependency
kind
deployment
kind
docs
kind
enhancement
kind
feature
kind
lint
kind
proposal
kind
question
kind
refactor
kind
security
kind
testing
kind
translation
kind
ui
need
backport
priority
critical
priority
low
priority
maybe
priority
medium
reviewed
duplicate
reviewed
invalid
reviewed
wontfix
skip-changelog
status
blocked
status
needs-feedback
status
needs-reviews
status
wip
upstream
gitea
upstream
other
workaround
No Label
kind
feature
Milestone
No items
No Milestone
Assignees
Clear assignees
42wim 6543 a1012112796 Akkuman appleboy axifive betacat0 blumia bwko caicandong ChristopherHX DaanSelen davidsvantesson delvh denyskon fuxiaohei gary-kim garymoon giteabot guillep2k HesterG hiifong jaqra jolheiser jonasfranz jpraet kdumontnu kemzeb kerwin612 khmarbaise KN4CK3R kolaente lafriks lng2020 lotherk luhahn lunny mrsdizzie noerw pat-s puni9869 renovate-bot richmahn rossigee sapk screenshots sillyguodong silverwind smartclip_tim strk tboerger techknowlogick TheFox0x7 tobiasbp typeless volker.raschek wolfogre wxiaoguang yardenshoham yp05327 zeripath Zettat123
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Blocks
#323 generate readme Parameters from values.yaml
gitea/helm-gitea
Reference: gitea/helm-gitea#322
No description provided.
Delete Branch "cnfatal/helm-chart:master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?