Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open to adding args/extraArgs? #108

Closed
foot opened this issue Apr 13, 2023 · 4 comments
Closed

Open to adding args/extraArgs? #108

foot opened this issue Apr 13, 2023 · 4 comments

Comments

@foot
Copy link

foot commented Apr 13, 2023

Awesome project!

We're using helmify in a kubebuilder project as mentioned in the README.md.

Now we're looking into ways to allow the user to configure the controller a bit more when they install the helm chart. I'm not sure what the most idiomatic patterns for doing this but args is one that comes to mind. Would the helmify project be open to a change like that?

For example:

  • extracting args from the deployment into a the values.yaml for direct control during helm install
  • extraArgs as a convenience to add additional configuration without overriding the defaults?

I imagine it work similarly to the securityContext extraction already in place.

Happy to put together a PR if this makes sense?

@foot foot changed the title [discussion] Open to adding args/extraArgs? Open to adding args/extraArgs? Apr 13, 2023
@arttor
Copy link
Owner

arttor commented Apr 13, 2023

Hi, we can definitely think about adding a new flag since we already have a couple of them. Please describe which spec needs to be templated. Maybe we should template it by default without adding the flag.

On the other hand, it is not the best way to control too many specs with flags. Please check out #59 - having a config is a better idea but the problem is that this feature is quite big compared to quckfix with a flag.

@foot
Copy link
Author

foot commented Apr 14, 2023

Ah sorry! I actually meant args/extraArgs for a pod, not for the helmify cli.

But as you say you may not want this behaviour for some reason and so would want to disable it via a flag to helmify too..

Proposed changes would look like this main...bigkevmcd:helmify:args

The diff from 0.4.1 and that branch using cat test_data/sample-app.yaml | helmify looks like this

diff --git a/chart/templates/deployment.yaml b/chart/templates/deployment.yaml
index f23af3a..89a8a87 100644
--- a/chart/templates/deployment.yaml
+++ b/chart/templates/deployment.yaml
@@ -18,10 +18,7 @@ spec:
       {{- include "chart.selectorLabels" . | nindent 8 }}
     spec:
       containers:
-      - args:
-        - --health-probe-bind-address=:8081
-        - --metrics-bind-address=127.0.0.1:8080
-        - --leader-elect
+      - args: {{- toYaml .Values.myapp.app.args | nindent 10 }}
         command:
         - /manager
         env:
@@ -65,9 +62,7 @@ spec:
           name: props
         - mountPath: /usr/share/nginx/html
           name: sample-pv-storage
-      - args:
-        - --secure-listen-address=0.0.0.0:8443
-        - --v=10
+      - args: {{- toYaml .Values.myapp.proxySidecar.args | nindent 10 }}
         env:
         - name: KUBERNETES_CLUSTER_DOMAIN
           value: {{ quote .Values.kubernetesClusterDomain }}
diff --git a/chart/values.yaml b/chart/values.yaml
index c7e5a6e..b26d4a1 100644
--- a/chart/values.yaml
+++ b/chart/values.yaml
@@ -1,11 +1,13 @@
 batchJob:
   backoffLimit: 4
   pi:
+    args: []
     image:
       repository: perl
       tag: 5.34.0
 cronJob:
   hello:
+    args: []
     image:
       repository: busybox
       tag: "1.28"
@@ -13,6 +15,7 @@ cronJob:
   schedule: '* * * * *'
 fluentdElasticsearch:
   fluentdElasticsearch:
+    args: []
     image:
       repository: quay.io/fluentd_elasticsearch/fluentd
       tag: v2.5.2
@@ -53,6 +56,10 @@ mySecretVars:
   var2: ""
 myapp:
   app:
+    args:
+    - --health-probe-bind-address=:8081
+    - --metrics-bind-address=127.0.0.1:8080
+    - --leader-elect
     containerSecurityContext:
       allowPrivilegeEscalation: false
     image:
@@ -69,6 +76,9 @@ myapp:
     region: east
     type: user-node
   proxySidecar:
+    args:
+    - --secure-listen-address=0.0.0.0:8443
+    - --v=10
     image:
       repository: gcr.io/kubebuilder/kube-rbac-proxy
       tag: v0.8.0
@@ -92,6 +102,7 @@ pvc:
     storageRequest: 3Gi
 web:
   nginx:
+    args: []
     image:
       repository: registry.k8s.io/nginx-slim
       tag: "0.8"

@arttor
Copy link
Owner

arttor commented Apr 14, 2023

looks nice, i think it is a good idea to template container args, especially for Jobs and CronJobs

@arttor
Copy link
Owner

arttor commented Apr 28, 2023

@bigkevmcd @foot thank you for your contribution! The changes are available in v0.4.2 release.

@arttor arttor closed this as completed Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants