Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Refine ApplicationSet SyncPolicy API to be less like ArgoCD. #55

Merged
merged 1 commit into from Nov 11, 2020

Conversation

dgoodwin
Copy link
Contributor

The ArgoCD core Application.Spec.SyncPolicy is available as part of the
embedded ApplicationSet.Spec.Template.

However we inherited some of this API within the ApplicationSet.Spec
where it does not make sense.

This PR drops the "automated" sub-struct, as automated is really the
only sync policy for an ApplicationSet. We do not support the concept of
a manual Sync from a UI or CLI tool as we would in ArgoCD itself, and as
far as I know this is not desirable.

This PR also drops the "initialSync" field, as I do not believe this
concept makes sense for an ApplicationSet. Applications are always
synced for their ApplicationSet.

ApplicationSet.Spec.SyncPolicy.Automated.Prune then moves up to
ApplicationSet.Spec.SyncPolicy.Prune.

The ArgoCD core Application.Spec.SyncPolicy is available as part of the
embedded ApplicationSet.Spec.Template.

However we inherited some of this API within the ApplicationSet.Spec
where it does not make sense.

This PR drops the "automated" sub-struct, as automated is really the
only sync policy for an ApplicationSet. We do not support the concept of
a manual Sync from a UI or CLI tool as we would in ArgoCD itself, and as
far as I know this is not desirable.

This PR also drops the "initialSync" field, as I do not believe this
concept makes sense for an ApplicationSet. Applications are always
synced for their ApplicationSet.

ApplicationSet.Spec.SyncPolicy.Automated.Prune then moves up to
ApplicationSet.Spec.SyncPolicy.Prune.
@dgoodwin
Copy link
Contributor Author

This PR is for consideration and may be controversial, or I may be missing vision on how this is intended to be used. Core assumption I'm making here is that there will be no UI around ApplicationSets where you click a button to reconcile to your Applications. It just happens via the standard watch/reconcile controller loop.

If I am mistaken about this then this PR should be closed.

However if not I think the API should be adjusted to better reflect what is possible and prevent Application SyncPolicy concepts from slipping in.

Additionally I changed Prune to SkipPrune, as I believe pruning should be the default behavior, and thus the go default of false would better align with SkipPrune.

@@ -1,13 +1,11 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed as this is where "make generate" is dropping the file, not sure why the mismatch.

@OmerKahani
Copy link
Contributor

Related to #32

---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.3.0
creationTimestamp: null
labels:
Copy link
Member

Choose a reason for hiding this comment

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

why delete this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the autogenerated output from make generate, typically projects would let these be generated rather than maintained manually.

However was this file being maintained manually? It might explain the filename that confused me.

Or were these labels injected and I'm using an incorrect version of something.

@xianlubird xianlubird merged commit b72b26e into argoproj:master Nov 11, 2020
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.

None yet

3 participants