Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Garbage collection dry run #2063

Merged

Conversation

jan-schumacher
Copy link

@jan-schumacher jan-schumacher commented May 18, 2019

I added a new syncGarbageCollection.dry option for helm chart and deployment. With this option activated it is irrelevant how syncGarbageCollection.enabled is set, the garbage collection will run in a trace mode. Only logs appear in the console but no resources are touched.
I created a Test for this behaviour in the style of the existing tests.

Fixes #1990

Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Great idea, thanks for putting your time into this ✋ (high five!)

I made a few suggestions, see what you think. If you don't think you'll have capacity to spend more time on this PR, no worries, just let us know :-)

cmd/fluxd/main.go Outdated Show resolved Hide resolved
cluster/kubernetes/sync.go Outdated Show resolved Hide resolved
@hiddeco hiddeco added the helm label May 21, 2019
@hiddeco hiddeco changed the title Garbage collection dry run #1990 Garbage collection dry run May 21, 2019
@hiddeco hiddeco removed the helm label May 21, 2019
@jan-schumacher
Copy link
Author

ci/circleci: e2e-testing — Your tests failed on CircleCI

Unfortunately I can’t run the e2e tests because I miss the app kind. Is that an alias?

@hiddeco
Copy link
Member

hiddeco commented May 22, 2019

@jan-schumacher kind is Kubernetes in Docker, it should also work with minikube (after rebasing on master) by running make e2e.

@2opremio
Copy link
Contributor

Unfortunately I can’t run the e2e tests because I miss the app kind. Is that an alias?

@jan-schumacher You will be able to run them locally if you rebase on master (thanks to #2063 ) by running make e2e

Regardless, the current error is:

Error: render error in "flux/templates/deployment.yaml": template: flux/templates/deployment.yaml:194:24: executing "flux/templates/deployment.yaml" at <.Values.syncGarbageCollection.enabled>: enabled is not a method but has arguments
Exited with code 1

@hiddeco hiddeco added this to the v1.13.0 milestone May 22, 2019
@jan-schumacher
Copy link
Author

Unfortunately I can’t run the e2e tests because I miss the app kind. Is that an alias?

@jan-schumacher You will be able to run them locally if you rebase on master (thanks to #2063 ) by running make e2e

Regardless, the current error is:

Error: render error in "flux/templates/deployment.yaml": template: flux/templates/deployment.yaml:194:24: executing "flux/templates/deployment.yaml" at <.Values.syncGarbageCollection.enabled>: enabled is not a method but has arguments
Exited with code 1

@hiddeco
Does that come from the change of the equation?

{{- if .Values.syncGarbageCollection.enabled and not .Values.syncGarbageCollection.dry}}
- --sync-garbage-collection={{ .Values.syncGarbageCollection.enabled }}
{{- else if .Values.syncGarbageCollection.dry }}
- --sync-garbage-collection-dry={{ .Values.syncGarbageCollection.dry }}
{{- end }}

Should I rather do it like this?

{{- if .Values.syncGarbageCollection.enabled }}
{{- if not .Values.syncGarbageCollection.dry }}
- --sync-garbage-collection={{ .Values.syncGarbageCollection.enabled }}
{{- end }}
{{- else if .Values.syncGarbageCollection.dry }}
- --sync-garbage-collection-dry={{ .Values.syncGarbageCollection.dry }}
{{- end }}

@hiddeco
Copy link
Member

hiddeco commented May 22, 2019

@jan-schumacher please take a look at the Helm documentation about operators https://helm.sh/docs/chart_template_guide/#operators-are-functions

@jan-schumacher
Copy link
Author

jan-schumacher commented May 23, 2019

@jan-schumacher please take a look at the Helm documentation about operators https://helm.sh/docs/chart_template_guide/#operators-are-functions

I see, thanks mate ;) not this is what i call syntactical candy.

@hiddeco
Copy link
Member

hiddeco commented May 24, 2019

@jan-schumacher this is shaping up well, can you smash your commits together into one?

@jan-schumacher jan-schumacher force-pushed the Garbage-collection-dry-run-#1990 branch 2 times, most recently from 9f53771 to e3a28e4 Compare May 24, 2019 10:00
cluster/kubernetes/sync.go Outdated Show resolved Hide resolved
Before this change there was no ability to run the garbage collection
process in a dry run, without touching any resources.

After this change you can enable the option sync-garbage-collection-dry
to only log what would be garbage collected, rather than deleting.
@jan-schumacher jan-schumacher force-pushed the Garbage-collection-dry-run-#1990 branch from e3a28e4 to cbbe90c Compare May 24, 2019 12:26
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Looking good, thanks @jan-schumacher 🥇

@hiddeco hiddeco merged commit 4062cb6 into fluxcd:master May 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Garbage collection dry-run
4 participants