Allow to define prune by default for manual sync applications

[gaeljw]

Summary

For applications not defined with automated sync, I'd still like to define (declaratively in the YAML) that the manual sync should use pruning.

Currently pruning implies automated sync:

spec:
  syncPolicy:
    automated:
      prune: true

Motivation

I have applications on which I don't want to enable automated sync for safety but still not have to check "prune" option each time a manual sync is done.

Proposal

prune option could be moved outside of automated.

Not sure of the best way to not do a breaking change though.

[diclophis]

This would be a huge win in usability... would it work if we made a syncPolicy.manual sub-key?

[alexef]

In our use case, the issue is that we want prune to be enabled, but we lose it every time one rolls back (as automated is emptied during rollbacks and the settings are lost).

I would suggest that automated is decoupled from prune, and we have a structure like this:

spec:
  syncPolicy:
    automated:
        enabled: <boolean>
        selfHeal: <boolean>
    prune: <boolean>

so that making changes to syncPolicy.automated.enabled does not change the desired value of selfHeal, while prune becomes a policy common to automated and non-automated (manual) modes.

[leoluz]

We discussed this issue in the contributor's meeting (April 27th 23).

@alexef Your suggestion would introduce a breaking change which is something that we need to avoid as much as we can. What you described seems to me like a bug in the current rollback logic. @jannfis suggested that we could persist the current state of the spec.syncPolicy.automated in the status field and use it to set it back after the rollback with the previous values. However this seems to be a different issue than the one described by @gaeljw.

@gaeljw In your proposal you suggested to:

prune option could be moved outside of automated.

As mentioned, this would introduce a breaking change which we need to avoid. Maybe another possibility is introducing a new attribute outside of the automated section to address your use-case. Example: spec.syncPolicy.autoPrune This would be set to false by default. We would just need to define and document how precedence logic would be implemented when users provide both prune fields. Probably the spec.syncPolicy.automated.prune will have to take priority.

[gaeljw]

I'm fine with this suggestion. On one hand I find it a bit confusing but on the other hand this is definitely not something that justify a breaking change. Having both attributes with a clearly documented precedence rule sounds good.

[troligsownder]

Hello, i would also like to vote on this feature. On the UI, I configured the App to enable "Prune Last" by default, but that doesn't do nothing because "Prune" needs to be enabled as well. But that currently needs to be done manually on each single sync by ticking the "Prune" Checkbox.

[olsavmic]

A separate config option would be very much appreciated. Forgetting to call prune after deploy has caused us production issues multiple times and we can't enable autosync on all our services due to safety around data migrations.

[zbialik]

Any update on this ticket? Would love to have this as we prune using the automated sync but have recently introduced syncWindows that require manual syncs every now and again, and in one instance, we were trying to remove a workload that could only be removed when the prune option was manually checked off.

Also, we have other apps on syncWindows where I force users to manually sync except at a certain time once a week. But when they manually sync, often times the app shows up OutOfSync because of the lingering ConfigMap from previous commit.

[mmclane]

I would also like to see this. I am trying to implement Kargo which kinda assumes autosync is turned off. But when something is removed from the helm chart and then promoted with Kargo you get lingering things because pruning is disabled. Giving the option to prune with manual sync would fix this I think.

[andrii-korotkov-verkada]

I think clicking a prune checkbox during a manual sync is acceptable. Please, let me know why would that be too much hassle.

[diclophis]

@andrii-korotkov-verkada its useful to ensure that an operator does not manually sync and forget to prune. Having the "prune checkbox" default be configurable... and then configuring it to "enable prune on manual syncs by default" the manual sync button, and automatic sync sequences would be identical and less error prone.