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.

Sign up for GitHub

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

[velero] Allow to define resource requests/limits for the containers in the upgrade/cleanup job #305

Merged
merged 2 commits into from
Sep 25, 2021
Merged

[velero] Allow to define resource requests/limits for the containers in the upgrade/cleanup job #305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3af14c4
Added the ability do define resource requests/limits for the containe…
weseven Sep 18, 2021
d63d956
Bump chart version
weseven Sep 18, 2021
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
2 changes: 1 addition & 1 deletion charts/velero/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
appVersion: 1.6.3
description: A Helm chart for velero
name: velero
version: 2.23.8
version: 2.23.9
home: https://github.com/vmware-tanzu/velero
icon: https://cdn-images-1.medium.com/max/1600/1*-9mb3AKnKdcL_QD3CMnthQ.png
sources:
Expand Down
4 changes: 4 additions & 0 deletions charts/velero/templates/cleanup-crds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ spec:
kubectl delete volumesnapshotlocation --all;
kubectl delete podvolumerestore --all;
kubectl delete crd -l app.kubernetes.io/name=velero;
{{- with .Values.kubectl.resources }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these additions be put behind a conditional so that empty "resources" fields aren't added? I'm not 100% sure that it matters, but that way there's no change in yaml for cases where the optional resources aren't specified:

    {{- if .Values.kubectl.resources }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,
thanks for the review.

I had the same concern but testing with helm template if resources is empty (either resources: {} like in values.yaml or simply resources:) nothing gets included in the final manifest. Basically, if there's something defined it gets converted in yaml and indented with 12 spaces; if there's nothing, nothing gets added as expected.

The same style of inclusion is used freely in the chart ( e.g. for the resources of the main deployment here

helm-charts/charts/velero/templates/deployment.yaml

Line 112 in 0c49944

{{- with .Values.resources }}
, the same goes for nodeSelector,affinity, tolerations, etc.).

Copy link
Collaborator

@jenting jenting Sep 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer the with syntax personally.

resources:
{{- toYaml . | nindent 12 }}
{{- end }}
restartPolicy: OnFailure
{{- with .Values.nodeSelector }}
nodeSelector:
Expand Down
8 changes: 8 additions & 0 deletions charts/velero/templates/upgrade-crds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ spec:
- /bin/sh
- -c
- /velero install --crds-only --dry-run -o yaml > /tmp/crds.yaml
{{- with .Values.kubectl.initResources }}
resources:
{{- toYaml . | nindent 12 }}
{{- end }}
volumeMounts:
- mountPath: /tmp
name: crds
Expand All @@ -62,6 +66,10 @@ spec:
- /bin/sh
- -c
- kubectl apply -f /tmp/crds.yaml
{{- with .Values.kubectl.resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- end }}
volumeMounts:
- mountPath: /tmp
name: crds
Expand Down
4 changes: 4 additions & 0 deletions charts/velero/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ kubectl:
# digest:
# kubectl image tag. If used, it will take precedence over the cluster Kubernetes version.
# tag: 1.16.15
# Resource requests/limits to specify for the upgrade/cleanup job. Optional
resources: {}
# Resource requests/limits to specify for the initContainer in the upgrade/cleanup job. Optional
initResources: {}
# Annotations to set for the upgrade/cleanup job. Optional.
annotations: {}
# Labels to set for the upgrade/cleanup job. Optional.
Expand Down