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

fix: handle differing indent levels for jobTemplate vs template #135

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions examples/app/templates/batch-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ spec:
template:
spec:
containers:
- command:
- args: {{- toYaml .Values.batchJob.pi.args | nindent 12 }}
Copy link
Owner

Choose a reason for hiding this comment

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

for CronJob nindent 12 is correct, but for Job it should be less because it does not have jobTemplate, right?

command:
- perl
- -Mbignum=bpi
- -wle
Expand All @@ -20,5 +21,6 @@ spec:
image: {{ .Values.batchJob.pi.image.repository }}:{{ .Values.batchJob.pi.image.tag
| default .Chart.AppVersion }}
name: pi
resources: {}
resources: {{- toYaml .Values.batchJob.pi.resources | nindent 14 }}
nodeSelector: {{- toYaml .Values.batchJob.nodeSelector | nindent 12 }}
Comment on lines +24 to +25
Copy link
Owner

Choose a reason for hiding this comment

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

same applies here: nindent should be less than for CronJob

restartPolicy: Never
7 changes: 4 additions & 3 deletions examples/app/templates/cron-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,18 @@ spec:
template:
spec:
containers:
- command:
- args: {{- toYaml .Values.cronJob.hello.args | nindent 12 }}
command:
- /bin/sh
- -c
- date; echo Hello from the Kubernetes cluster
env:
- name: KUBERNETES_CLUSTER_DOMAIN
value: {{ quote .Values.kubernetesClusterDomain }}
image: {{ .Values.cronJob.hello.image.repository }}:{{ .Values.cronJob.hello.image.tag
| default .Chart.AppVersion }}
imagePullPolicy: {{ .Values.cronJob.hello.imagePullPolicy }}
name: hello
resources: {}
resources: {{- toYaml .Values.cronJob.hello.resources | nindent 14 }}
nodeSelector: {{- toYaml .Values.cronJob.nodeSelector | nindent 12 }}
restartPolicy: OnFailure
schedule: {{ .Values.cronJob.schedule | quote }}
22 changes: 22 additions & 0 deletions examples/app/values.yaml
Original file line number Diff line number Diff line change
@@ -1,15 +1,37 @@
batchJob:
backoffLimit: 4
nodeSelector:
disktype: ssd
pi:
args:
- date; echo Hello from the Kubernetes cluster
image:
repository: perl
tag: 5.34.0
resources:
limits:
cpu: 200m
memory: 256Mi
requests:
cpu: 100m
memory: 128Mi
cronJob:
hello:
args:
- date; echo Hello from the Kubernetes cluster
image:
repository: busybox
tag: "1.28"
imagePullPolicy: IfNotPresent
resources:
limits:
cpu: 200m
memory: 256Mi
requests:
cpu: 100m
memory: 128Mi
nodeSelector:
disktype: ssd
schedule: '* * * * *'
fluentdElasticsearch:
fluentdElasticsearch:
Expand Down
5 changes: 3 additions & 2 deletions pkg/processor/daemonset/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ package daemonset

import (
"fmt"
"github.com/arttor/helmify/pkg/processor/pod"
"io"
"strings"
"text/template"

"github.com/arttor/helmify/pkg/processor/pod"

"github.com/arttor/helmify/pkg/helmify"
"github.com/arttor/helmify/pkg/processor"
yamlformat "github.com/arttor/helmify/pkg/yaml"
Expand Down Expand Up @@ -98,7 +99,7 @@ func (d daemonset) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstru
}

nameCamel := strcase.ToLowerCamel(name)
specMap, podValues, err := pod.ProcessSpec(nameCamel, appMeta, dae.Spec.Template.Spec)
specMap, podValues, err := pod.ProcessSpec(nameCamel, appMeta, dae.Spec.Template.Spec, dae.TypeMeta.Kind)
if err != nil {
return true, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/processor/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (d deployment) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstr
}

nameCamel := strcase.ToLowerCamel(name)
specMap, podValues, err := pod.ProcessSpec(nameCamel, appMeta, depl.Spec.Template.Spec)
specMap, podValues, err := pod.ProcessSpec(nameCamel, appMeta, depl.Spec.Template.Spec, depl.TypeMeta.Kind)
if err != nil {
return true, nil, err
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/processor/job/cron.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ package job

import (
"fmt"
"io"
"strings"
"text/template"

"github.com/arttor/helmify/pkg/helmify"
"github.com/arttor/helmify/pkg/processor"
"github.com/arttor/helmify/pkg/processor/pod"
yamlformat "github.com/arttor/helmify/pkg/yaml"
"github.com/iancoleman/strcase"
"io"
batchv1 "k8s.io/api/batch/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"strings"
"text/template"
)

var cronTempl, _ = template.New("cron").Parse(
Expand Down Expand Up @@ -105,7 +106,7 @@ func (p cron) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstructure
}

// process job pod template:
podSpecMap, podValues, err := pod.ProcessSpec(nameCamelCase, appMeta, jobObj.Spec.JobTemplate.Spec.Template.Spec)
podSpecMap, podValues, err := pod.ProcessSpec(nameCamelCase, appMeta, jobObj.Spec.JobTemplate.Spec.Template.Spec, jobObj.TypeMeta.Kind)
if err != nil {
return true, nil, err
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/processor/job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ package job

import (
"fmt"
"io"
"strings"
"text/template"

"github.com/arttor/helmify/pkg/helmify"
"github.com/arttor/helmify/pkg/processor"
"github.com/arttor/helmify/pkg/processor/pod"
yamlformat "github.com/arttor/helmify/pkg/yaml"
"github.com/iancoleman/strcase"
"io"
batchv1 "k8s.io/api/batch/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"strings"
"text/template"
)

var jobTempl, _ = template.New("job").Parse(
Expand Down Expand Up @@ -104,7 +105,7 @@ func (p job) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstructured
}
}
// process job pod template:
podSpecMap, podValues, err := pod.ProcessSpec(nameCamelCase, appMeta, jobObj.Spec.Template.Spec)
podSpecMap, podValues, err := pod.ProcessSpec(nameCamelCase, appMeta, jobObj.Spec.Template.Spec, jobObj.TypeMeta.Kind)
if err != nil {
return true, nil, err
}
Expand Down
29 changes: 20 additions & 9 deletions pkg/processor/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import (
const imagePullPolicyTemplate = "{{ .Values.%[1]s.%[2]s.imagePullPolicy }}"
const envValue = "{{ quote .Values.%[1]s.%[2]s.%[3]s.%[4]s }}"

func ProcessSpec(objName string, appMeta helmify.AppMetadata, spec corev1.PodSpec) (map[string]interface{}, helmify.Values, error) {
func ProcessSpec(objName string, appMeta helmify.AppMetadata, spec corev1.PodSpec, kind string) (map[string]interface{}, helmify.Values, error) {
baseIndent := calculateBaseIndent(kind)

values, err := processPodSpec(objName, appMeta, &spec)
if err != nil {
return nil, nil, err
Expand All @@ -39,12 +41,12 @@ func ProcessSpec(objName string, appMeta helmify.AppMetadata, spec corev1.PodSpe
return nil, nil, fmt.Errorf("%w: unable to convert podSpec to map", err)
}

specMap, values, err = processNestedContainers(specMap, objName, values, "containers")
specMap, values, err = processNestedContainers(specMap, objName, values, "containers", baseIndent)
if err != nil {
return nil, nil, err
}

specMap, values, err = processNestedContainers(specMap, objName, values, "initContainers")
specMap, values, err = processNestedContainers(specMap, objName, values, "initContainers", baseIndent)
if err != nil {
return nil, nil, err
}
Expand All @@ -63,7 +65,7 @@ func ProcessSpec(objName string, appMeta helmify.AppMetadata, spec corev1.PodSpe

// process nodeSelector if presented:
if spec.NodeSelector != nil {
err = unstructured.SetNestedField(specMap, fmt.Sprintf(`{{- toYaml .Values.%s.nodeSelector | nindent 8 }}`, objName), "nodeSelector")
err = unstructured.SetNestedField(specMap, fmt.Sprintf(`{{- toYaml .Values.%s.nodeSelector | nindent %d }}`, objName, 8+baseIndent), "nodeSelector")
if err != nil {
return nil, nil, err
}
Expand All @@ -76,14 +78,14 @@ func ProcessSpec(objName string, appMeta helmify.AppMetadata, spec corev1.PodSpe
return specMap, values, nil
}

func processNestedContainers(specMap map[string]interface{}, objName string, values map[string]interface{}, containerKey string) (map[string]interface{}, map[string]interface{}, error) {
func processNestedContainers(specMap map[string]interface{}, objName string, values map[string]interface{}, containerKey string, baseIndent int) (map[string]interface{}, map[string]interface{}, error) {
containers, _, err := unstructured.NestedSlice(specMap, containerKey)
if err != nil {
return nil, nil, err
}

if len(containers) > 0 {
containers, values, err = processContainers(objName, values, containerKey, containers)
containers, values, err = processContainers(objName, values, containerKey, containers, baseIndent)
if err != nil {
return nil, nil, err
}
Expand All @@ -97,15 +99,24 @@ func processNestedContainers(specMap map[string]interface{}, objName string, val
return specMap, values, nil
}

func processContainers(objName string, values helmify.Values, containerType string, containers []interface{}) ([]interface{}, helmify.Values, error) {
func calculateBaseIndent(resourceType string) int {
switch resourceType {
case "CronJob", "Job":
return 4 // Adjusting for the deeper nesting within a JobTemplate
default:
return 0 // Regular Template
}
}

func processContainers(objName string, values helmify.Values, containerType string, containers []interface{}, baseIndent int) ([]interface{}, helmify.Values, error) {
for i := range containers {
containerName := strcase.ToLowerCamel((containers[i].(map[string]interface{})["name"]).(string))
res, exists, err := unstructured.NestedMap(values, objName, containerName, "resources")
if err != nil {
return nil, nil, err
}
if exists && len(res) > 0 {
err = unstructured.SetNestedField(containers[i].(map[string]interface{}), fmt.Sprintf(`{{- toYaml .Values.%s.%s.resources | nindent 10 }}`, objName, containerName), "resources")
err = unstructured.SetNestedField(containers[i].(map[string]interface{}), fmt.Sprintf(`{{- toYaml .Values.%s.%s.resources | nindent %d }}`, objName, containerName, 10+baseIndent), "resources")
if err != nil {
return nil, nil, err
}
Expand All @@ -116,7 +127,7 @@ func processContainers(objName string, values helmify.Values, containerType stri
return nil, nil, err
}
if exists && len(args) > 0 {
err = unstructured.SetNestedField(containers[i].(map[string]interface{}), fmt.Sprintf(`{{- toYaml .Values.%[1]s.%[2]s.args | nindent 8 }}`, objName, containerName), "args")
err = unstructured.SetNestedField(containers[i].(map[string]interface{}), fmt.Sprintf(`{{- toYaml .Values.%[1]s.%[2]s.args | nindent %d }}`, objName, containerName, 8+baseIndent), "args")
if err != nil {
return nil, nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/processor/pod/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func Test_pod_Process(t *testing.T) {
var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeployment)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec, deploy.TypeMeta.Kind)
assert.NoError(t, err)

assert.Equal(t, map[string]interface{}{
Expand Down Expand Up @@ -162,7 +162,7 @@ func Test_pod_Process(t *testing.T) {
var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeploymentWithNoArgs)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec, deploy.TypeMeta.Kind)
assert.NoError(t, err)

assert.Equal(t, map[string]interface{}{
Expand Down Expand Up @@ -201,7 +201,7 @@ func Test_pod_Process(t *testing.T) {
var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeploymentWithTagAndDigest)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec, deploy.TypeMeta.Kind)
assert.NoError(t, err)

assert.Equal(t, map[string]interface{}{
Expand Down Expand Up @@ -240,7 +240,7 @@ func Test_pod_Process(t *testing.T) {
var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeploymentWithPort)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec, deploy.TypeMeta.Kind)
assert.NoError(t, err)

assert.Equal(t, map[string]interface{}{
Expand Down
5 changes: 3 additions & 2 deletions pkg/processor/statefulset/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ package statefulset

import (
"fmt"
"github.com/arttor/helmify/pkg/processor/pod"
"io"
"strings"
"text/template"

"github.com/arttor/helmify/pkg/processor/pod"

"github.com/arttor/helmify/pkg/helmify"
"github.com/arttor/helmify/pkg/processor"
yamlformat "github.com/arttor/helmify/pkg/yaml"
Expand Down Expand Up @@ -108,7 +109,7 @@ func (d statefulset) Process(appMeta helmify.AppMetadata, obj *unstructured.Unst
}

// process pod spec:
podSpecMap, podValues, err := pod.ProcessSpec(nameCamel, appMeta, ssSpec.Template.Spec)
podSpecMap, podValues, err := pod.ProcessSpec(nameCamel, appMeta, ssSpec.Template.Spec, ss.TypeMeta.Kind)
if err != nil {
return true, nil, err
}
Expand Down
29 changes: 24 additions & 5 deletions test_data/sample-app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ data:
name: default-oneperhost-pod-template
spec:
templates:
podTemplates:
podTemplates:
- name: default-oneperhost-pod-template
distribution: "OnePerHost"
---
Expand Down Expand Up @@ -266,7 +266,18 @@ spec:
- name: pi
image: perl:5.34.0
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
args:
- "date; echo Hello from the Kubernetes cluster"
resources:
requests:
cpu: "100m"
memory: "128Mi"
limits:
cpu: "200m"
memory: "256Mi"
restartPolicy: Never
nodeSelector:
disktype: ssd
backoffLimit: 4
---
apiVersion: batch/v1
Expand All @@ -283,11 +294,19 @@ spec:
- name: hello
image: busybox:1.28
imagePullPolicy: IfNotPresent
command:
- /bin/sh
- -c
- date; echo Hello from the Kubernetes cluster
command: ["/bin/sh", "-c"]
args:
- "date; echo Hello from the Kubernetes cluster"
resources:
requests:
cpu: "100m"
memory: "128Mi"
limits:
cpu: "200m"
memory: "256Mi"
restartPolicy: OnFailure
nodeSelector:
disktype: ssd
---
apiVersion: v1
kind: Service
Expand Down
Loading