Reconcile release components by creating/deleting Argo Applications #351

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

Jump to bottom

Closed

kubasobon wants to merge 8 commits into master from argoapp-client

Contributor

kubasobon commented Sep 1, 2021 •

edited

Loading

Towards https://github.com/giantswarm/giantswarm/issues/18502

Checklist

Merge and release Remove Config CR and add ConfigReference to Release Component apiextensions#792
Merge and release Add Argo Application CR structure and constructors argoapp#42
Update go.mod versions for the above
Update changelog in CHANGELOG.md.

kubasobon self-assigned this

kubasobon force-pushed the argoapp-client branch from 87910f8 to 3aaf803 Compare

September 2, 2021 06:33

Contributor Author

kubasobon commented Sep 9, 2021

Tested with fixes:

gauss:thinkbook:~ k get application -n argocd -Lgiantswarm.io/managed-by | grep release-operator
app-operator-3.2.0                                  Synced        Healthy         release-operator
app-operator-3.2.1                                  Synced        Healthy         release-operator
aws-operator-10.2.0                                 Synced        Healthy         release-operator
aws-operator-10.3.1                                 Synced        Healthy         release-operator
aws-operator-5.7.6                                  Synced        Healthy         release-operator
aws-operator-9.3.10                                 Synced        Healthy         release-operator
capa-iam-controller-0.2.0                           Synced        Healthy         release-operator
cert-operator-0.1.0                                 Synced        Healthy         release-operator
cert-operator-1.0.1                                 Synced        Healthy         release-operator
cluster-api-bootstrap-provider-kubeadm-0.3.22-gs1   Unknown       Healthy         release-operator
cluster-api-control-plane-0.3.22-gs3                Unknown       Healthy         release-operator
cluster-api-core-0.3.22-gs4                         Synced        Healthy         release-operator
cluster-api-provider-aws-0.6.8-gs5                  Synced        Healthy         release-operator
cluster-apps-operator-0.6.0                         Synced        Healthy         release-operator
cluster-operator-0.23.18                            Synced        Healthy         release-operator
cluster-operator-3.6.0                              Synced        Healthy         release-operator
cluster-operator-3.7.1                              Synced        Healthy         release-operator
cluster-operator-3.9.0                              Synced        Healthy         release-operator
policies-aws-0.1.1                                  Synced        Healthy         release-operator
policies-common-0.1.1                               Synced        Healthy         release-operator


          ignore build artifacts

8d5edc2

kubasobon changed the title ~~Add argoapp with types + client~~ Reconcile release components by creating/deleting Argo Applications

kubasobon added 4 commits

September 9, 2021 14:42


          fixme: use WIP versions of apiextensions & argoapp

ee77883


          add AppCatalog and Argo Application RBAC rules

bab35ac


          remove old controllers and update apiextensions

ed24454

Remove old controllers for unused resources (GiantSwarm App, Config) and
common functions utilizing them. Also update apiextensions/v2 -> v3 for
Release CR to include ConfigReference field.


          add Argo Application controller

4ed13d8

kubasobon force-pushed the argoapp-client branch from 2a5628c to 4ed13d8 Compare

September 9, 2021 12:43

kubasobon requested review from a team

September 10, 2021 10:08

kubasobon marked this pull request as ready for review

September 10, 2021 10:08

MarcelMue reviewed

View reviewed changes

service/controller/release/resource/argoapps/resource.go

+              	konfigureB := extractKonfigureVariables(b)
+              	match := true
+              	match = match && konfigureA.AppName != "" && konfigureA.AppName == konfigureB.AppName

Contributor

MarcelMue Sep 10, 2021

Why can we not compare the konfigureVariables struct directly here? Or implement a method func (r *konfigureVariables) compare(input konfigureVariables) bool?

Contributor Author

kubasobon Sep 10, 2021

Good advice, let me fix that.

service/controller/release/resource/argoapps/resource.go


		}

		type konfigureVariables struct {

Contributor

MarcelMue Sep 10, 2021

Structs should always be at the top of the file.

service/controller/release/resource/argoapps/resource.go

		@@ -0,0 +1,255 @@
		package argoapps

Contributor

MarcelMue Sep 10, 2021

Can we split this up into at least two files? Maybe the state reconciliation and konfigureVariables stuff in separate files.

Contributor Author

kubasobon Sep 10, 2021

Sure, it'll make it more readable.

service/controller/release/resource/argoapps/resource.go Outdated Show resolved Hide resolved

kopiczko reviewed

View reviewed changes

Contributor

kopiczko left a comment

Reads nicely, I got a few remarks

service/controller/release/resource/apps/resource_test.go Show resolved Hide resolved

service/controller/release/resource/argoapps/resource.go Outdated Show resolved Hide resolved

service/controller/release/resource/argoapps/resource.go Outdated Show resolved Hide resolved

kubasobon added 2 commits

September 13, 2021 09:19


          (fixup) get argoapp@argoapp-client

f67b67f


          add updated unit tests

76268cd

kubasobon force-pushed the argoapp-client branch from 7ee78ad to 76268cd Compare

September 14, 2021 07:29


          go mod tidy

250e962

kopiczko approved these changes

View reviewed changes

Contributor

kopiczko left a comment

LGTM

Contributor

MarcelMue commented Sep 15, 2021

Some comments from earlier seem still valid. Should we re-review anyway?

Contributor Author

kubasobon commented Sep 16, 2021

@MarcelMue The PR is on hold until after onsite. Your remarks will be addressed.

Contributor

MarcelMue commented Sep 16, 2021

@MarcelMue The PR is on hold until after onsite. Your remarks will be addressed.

All good - just saw Pawel approve and was confused :D

Contributor

kopiczko commented Sep 16, 2021

@MarcelMue The PR is on hold until after onsite. Your remarks will be addressed.

All good - just saw Pawel approve and was confused :D

As far as I'm concerned it's fine. I should write, that I'm happy when @MarcelMue is happy.

Contributor

rossf7 commented Feb 23, 2022

@kubasobon Same here. Closing as we're migrated to flux.

rossf7 closed this

rossf7 deleted the argoapp-client branch

February 23, 2022 12:48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet