add extraDeploy to add arbitrary objects to the release #441

Merged
jouve merged 2 commits from extra into main 2023-05-02 13:32:54 +00:00
jouve commented 2023-04-18 21:25:48 +00:00 (Migrated from gitea.com)

Signed-off-by: Cyril Jouve jv.cyril@gmail.com

Description of the change

add a new value extraDeploy to add arbitrary resources

inspired by bitnami charts (example

Benefits

with the change, I can deploy additional resources and keep them consistent with the chart (reuse macro, same labels, etc)., same workflow (helm upgrade), etc

Possible drawbacks

Additional information

Checklist

Signed-off-by: Cyril Jouve <jv.cyril@gmail.com> <!-- Before you open the request please review the following guidelines and tips to help it be more easily integrated: - Describe the scope of your change - i.e. what the change does. - Describe any known limitations with your change. - Please run any tests or examples that can exercise your modified code. Thank you for contributing! We will try to review, test and integrate the change as soon as we can. --> ### Description of the change <!-- Describe the scope of your change - i.e. what the change does. --> add a new value `extraDeploy` to add arbitrary resources inspired by bitnami charts ([example](https://github.com/bitnami/charts/blob/main/bitnami/postgresql/values.yaml#L58) ### Benefits <!-- What benefits will be realized by the code change? --> with the change, I can deploy additional resources and keep them consistent with the chart (reuse macro, same labels, etc)., same workflow (helm upgrade), etc ### Possible drawbacks <!-- Describe any known limitations with your change --> ### Additional information <!-- If there's anything else that's important and relevant to your pull request, mention that information here. Please remove this section if it remains empty. --> ### Checklist <!-- [Place an '[X]' (no spaces) in all applicable fields. Please remove unrelated fields.] --> - [x] Parameters are documented in the `values.yaml` and added to the `README.md` using [readme-generator-for-helm](https://github.com/bitnami-labs/readme-generator-for-helm)
pat-s (Migrated from gitea.com) approved these changes 2023-04-19 10:12:40 +00:00
pat-s (Migrated from gitea.com) left a comment

LGTM!

And thanks for the capitalization catch!

LGTM! And thanks for the capitalization catch!
luhahn (Migrated from gitea.com) reviewed 2023-04-19 11:59:15 +00:00
luhahn (Migrated from gitea.com) commented 2023-04-19 11:59:15 +00:00

I understand we might want to allow users to include and reference other values. However I'm not the biggest fan, of the tpl function, since one might be able to reference literally everything.

In Addition (no worries, I won't block this PR) do we need this? I think everything not related directly to this chart should be kept outside.

This is just my personal preference I'm open to discussions here :)

I understand we might want to allow users to include and reference other values. However I'm not the biggest fan, of the tpl function, since one might be able to reference literally everything. In Addition (no worries, I won't block this PR) do we need this? I think everything not related directly to this chart should be kept outside. This is just my personal preference I'm open to discussions here :)
luhahn (Migrated from gitea.com) commented 2023-04-19 17:35:42 +00:00

do we need this was my first reaction, too. Not blocking it either, but would be happy to discuss before a potential merge.

`do we need this` was my first reaction, too. Not blocking it either, but would be happy to discuss before a potential merge.
jouve commented 2023-04-19 19:28:14 +00:00 (Migrated from gitea.com)

I use it to add stuff like network policies

extraDeploy:
  - |
      apiVersion: networking.k8s.io/v1
      kind: NetworkPolicy
      metadata:
        name: {{ include "gitea.fullname" . }}
        labels: {{- include "gitea.labels" . | nindent 4 }}
      spec: 
        podSelector:
          matchLabels: {{- include "gitea.selectorLabels" . | nindent 10 }}
        ingress:
          - from: 
              - namespaceSelector:
                  matchLabels:
                    kubernetes.io/metadata.name: haproxy-controller      
I use it to add stuff like network policies ```yaml extraDeploy: - | apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: name: {{ include "gitea.fullname" . }} labels: {{- include "gitea.labels" . | nindent 4 }} spec: podSelector: matchLabels: {{- include "gitea.selectorLabels" . | nindent 10 }} ingress: - from: - namespaceSelector: matchLabels: kubernetes.io/metadata.name: haproxy-controller ```
justusbunsi commented 2023-04-19 19:38:36 +00:00 (Migrated from gitea.com)

Network policies. I see 😃. Do you fancy reviving #306? I see you commented there as well.

Network policies. I see 😃. Do you fancy reviving #306? I see you commented there as well.
luhahn commented 2023-04-20 08:07:15 +00:00 (Migrated from gitea.com)

This also explains the use of tpl function :)

Okay this is a neat way to include functionality, which is not yet implemented in the helm chart

This also explains the use of tpl function :) Okay this is a neat way to include functionality, which is not yet implemented in the helm chart
pat-s commented 2023-04-22 06:57:25 +00:00 (Migrated from gitea.com)

@justusbunsi @luhahn Your take - no strong opinion from my side here.

@justusbunsi @luhahn Your take - no strong opinion from my side here.
luhahn (Migrated from gitea.com) approved these changes 2023-04-22 19:05:36 +00:00
luhahn (Migrated from gitea.com) left a comment

Fine with me, adds more flexibility to the chart :)

Fine with me, adds more flexibility to the chart :)
justusbunsi (Migrated from gitea.com) approved these changes 2023-04-23 11:05:34 +00:00
justusbunsi (Migrated from gitea.com) left a comment

@jouve As soon as the failing build is resolved this can be merged. 🙂

@jouve As soon as the failing build is resolved this can be merged. 🙂
jouve commented 2023-04-24 21:35:36 +00:00 (Migrated from gitea.com)

readme-generator-for-helm copies the comment into the readme :

Password for the "gitea" user

but markdownlint -f capitalize "Gitea" between the quotes => as a workaround, I changes the quote to ` so that both agree

readme-generator-for-helm copies the comment into the readme : ``` Password for the "gitea" user ``` but markdownlint -f capitalize "Gitea" between the quotes => as a workaround, I changes the quote to ` so that both agree
pat-s commented 2023-04-25 07:15:55 +00:00 (Migrated from gitea.com)

@jouve A side remark: would it be possible not to force-push in your PRs? By doing so we cannot infer what you changed since the last conversations and only see the overall changed files of the PR.

Is there a reason you always force-push instead of doing normal commits?

A force-push can make sense in situations where you are trying out something iteratively but want to have one clear commit message. Yet it becomes problematic once there is conversation happening and people want to track the changes.

Upon merge, we would do a squash commit anyway 🙂

@jouve A side remark: would it be possible not to force-push in your PRs? By doing so we cannot infer what you changed since the last conversations and only see the overall changed files of the PR. Is there a reason you always force-push instead of doing normal commits? A force-push can make sense in situations where you are trying out something iteratively but want to have one clear commit message. Yet it becomes problematic once there is conversation happening and people want to track the changes. Upon merge, we would do a squash commit anyway 🙂
Sign in to join this conversation.
No description provided.