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

Autoxhelm create ArgoCD application #1355

Closed
wants to merge 46 commits into from

Conversation

Alan-Cha
Copy link
Member

No description provided.

@Alan-Cha Alan-Cha force-pushed the autoxhelm4 branch 4 times, most recently from 54ed544 to 5b82f23 Compare October 14, 2022 15:08
@Alan-Cha Alan-Cha marked this pull request as ready for review October 14, 2022 18:29
autox/informer.go Outdated Show resolved Hide resolved
@sriumcp sriumcp mentioned this pull request Oct 17, 2022
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

Idempotence of autox + a few minor naming changes, and visibility changes (for kubeclient).

autox/application.tpl Outdated Show resolved Hide resolved
autox/application.tpl Outdated Show resolved Hide resolved
targetRevision: {{ .chart.version }}
syncPolicy:
automated:
prune: true
Copy link
Member

Choose a reason for hiding this comment

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

Document what prune and selfHeal mean.

Copy link
Member Author

Choose a reason for hiding this comment

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

prune:

https://argo-cd.readthedocs.io/en/stable/user-guide/auto_sync/#automatic-pruning

By default (and as a safety mechanism), automated sync will not delete resources when Argo CD detects the resource is no longer defined in Git. To prune the resources, a manual sync can always be performed (with pruning checked). Pruning can also be enabled to happen automatically as part of the automated sync by... setting this option.

@sriumcp @kalantar If this is related to git, then should we get rid of it?

selfHeal:

https://argo-cd.readthedocs.io/en/stable/user-guide/auto_sync/#automatic-self-healing

By default, changes that are made to the live cluster will not trigger automated sync. To enable automatic sync when the live cluster's state deviates from the state defined in Git... set this option.

We want the changes to trigger a sync but I'm not sure what how Argo CD's Git usage means in relationship with what we are doing.

autox/config.go Outdated Show resolved Hide resolved
autox/config.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
@sriumcp sriumcp self-requested a review October 17, 2022 16:03
)

type chartAction int64

const (
releaseAction chartAction = 0
deleteAction chartAction = 1

applicationTemplateFilePath string = "application.tpl"
Copy link
Member

Choose a reason for hiding this comment

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

Decide between embed and using an arguCD typed application struct.

@sriumcp sriumcp self-requested a review October 17, 2022 16:04
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

ignore approve!

@Alan-Cha Alan-Cha force-pushed the autoxhelm4 branch 5 times, most recently from dc45464 to 0c00b4e Compare November 3, 2022 20:58
autox/application.tpl Outdated Show resolved Hide resolved
autox/config.go Show resolved Hide resolved
autox/watcher.go Outdated Show resolved Hide resolved
autox/watcher.go Outdated Show resolved Hide resolved
autox/k8sclient.go Outdated Show resolved Hide resolved
autox/watcher.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

Please see initial comments. More comments to follow.

autox/application.tpl Outdated Show resolved Hide resolved
autox/config.go Outdated Show resolved Hide resolved
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

Please see requested changes.

autox/informer.go Outdated Show resolved Hide resolved
autox/watcher.go Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

Please see comments on informer.

autox/application.tpl Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
@Alan-Cha Alan-Cha force-pushed the autoxhelm4 branch 2 times, most recently from be66969 to 29072d0 Compare November 22, 2022 05:20
sriumcp and others added 3 commits November 22, 2022 00:24
Signed-off-by: Srinivasan Parthasarathy <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Srinivasan Parthasarathy <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

Please see a small change requested for delete logic.

autox/informer.go Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

Minor comment.

autox/watcher_test.go Outdated Show resolved Hide resolved
Signed-off-by: Alan Cha <[email protected]>
@sriumcp sriumcp self-requested a review November 29, 2022 18:32
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Update workflows

Signed-off-by: Alan Cha <[email protected]>
autox/application.tpl Show resolved Hide resolved
autox/config.go Outdated Show resolved Hide resolved
autox/watcher.go Outdated Show resolved Hide resolved
base/util.go Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved
autox/informer.go Outdated Show resolved Hide resolved

// autoX handler will call on applyHelmRelease and deleteHelmRelease
applyHelmReleaseInvocations := 0
applyHelmRelease = func(releaseName string, specGroupName string, releaseSpec releaseSpec, namespace string, additionalValues map[string]string) error {
Copy link
Member

Choose a reason for hiding this comment

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

Query the cluster. Do not mock applyHelmRelease and delete. Instead query the cluster and make sure it looks right (e.g., no objects after delete; appropriate application objects after apply).

Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
Signed-off-by: Alan Cha <[email protected]>
@Alan-Cha Alan-Cha mentioned this pull request Jan 11, 2023
@Alan-Cha
Copy link
Member Author

See #1387

@Alan-Cha Alan-Cha closed this Jan 11, 2023
@Alan-Cha Alan-Cha deleted the autoxhelm4 branch May 10, 2023 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants