From 8dc46bfdcd225fc63198dc340462d933f101035c Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Sun, 20 Aug 2023 14:28:23 +0900 Subject: [PATCH 1/5] Support multiple sources and revisions (ApplicationPhaseCommentReconciler) --- internal/argocd/application.go | 26 ++++++++++ .../applicationphasecomment_controller.go | 18 +++---- internal/notification/comment.go | 48 ++++++++++--------- 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/internal/argocd/application.go b/internal/argocd/application.go index d6965932..8b3c336e 100644 --- a/internal/argocd/application.go +++ b/internal/argocd/application.go @@ -17,6 +17,32 @@ func GetDeployedRevision(a argocdv1alpha1.Application) string { return a.Status.OperationState.Operation.Sync.Revision } +type SourceRevision struct { + Source argocdv1alpha1.ApplicationSource + Revision string +} + +func GetSourceRevisions(app argocdv1alpha1.Application) []SourceRevision { + if app.Status.OperationState == nil { + return nil + } + sources := app.Spec.GetSources() + revisions := app.Status.OperationState.Operation.Sync.Revisions + if revisions == nil { + revisions = []string{app.Status.OperationState.Operation.Sync.Revision} + } + size := min(len(sources), len(revisions)) + + sourceRevisions := make([]SourceRevision, size) + for i := 0; i < size; i++ { + sourceRevisions[i] = SourceRevision{ + Source: sources[i], + Revision: revisions[i], + } + } + return sourceRevisions +} + // GetDeploymentURL returns the deployment URL in annotations func GetDeploymentURL(a argocdv1alpha1.Application) string { if a.Annotations == nil { diff --git a/internal/controller/applicationphasecomment_controller.go b/internal/controller/applicationphasecomment_controller.go index 0a628ec0..b64a2f69 100644 --- a/internal/controller/applicationphasecomment_controller.go +++ b/internal/controller/applicationphasecomment_controller.go @@ -60,17 +60,19 @@ func (r *ApplicationPhaseCommentReconciler) Reconcile(ctx context.Context, req c if err != nil { logger.Info("unable to determine Argo CD URL", "error", err) } - comment := notification.NewCommentOnOnPhaseChanged(app, argocdURL) - if comment == nil { + comments := notification.NewCommentsOnOnPhaseChanged(app, argocdURL) + if len(comments) == 0 { logger.Info("no comment on this phase event", "phase", phase) return ctrl.Result{}, nil } - if err := r.Notification.CreateComment(ctx, *comment, app); err != nil { - logger.Error(err, "unable to create a comment") - r.Recorder.Eventf(&app, corev1.EventTypeWarning, "CreateCommentError", - "unable to create a comment by %s: %s", phase, err) - } else { - r.Recorder.Eventf(&app, corev1.EventTypeNormal, "CreatedComment", "created a comment by %s", phase) + for _, comment := range comments { + if err := r.Notification.CreateComment(ctx, comment, app); err != nil { + logger.Error(err, "unable to create a comment") + r.Recorder.Eventf(&app, corev1.EventTypeWarning, "CreateCommentError", + "unable to create a comment by %s: %s", phase, err) + } else { + r.Recorder.Eventf(&app, corev1.EventTypeNormal, "CreatedComment", "created a comment by %s", phase) + } } return ctrl.Result{}, nil } diff --git a/internal/notification/comment.go b/internal/notification/comment.go index cabbfa18..ec0b844c 100644 --- a/internal/notification/comment.go +++ b/internal/notification/comment.go @@ -20,60 +20,64 @@ type Comment struct { Body string } -func NewCommentOnOnPhaseChanged(app argocdv1alpha1.Application, argocdURL string) *Comment { - if app.Spec.Source == nil { - return nil +func NewCommentsOnOnPhaseChanged(app argocdv1alpha1.Application, argocdURL string) []Comment { + sourceRevisions := argocd.GetSourceRevisions(app) + var comments []Comment + for _, sourceRevision := range sourceRevisions { + comment := generateCommentOnPhaseChanged(app, argocdURL, sourceRevision) + if comment == nil { + continue + } + comments = append(comments, *comment) } - repository := github.ParseRepositoryURL(app.Spec.Source.RepoURL) + return comments +} + +func generateCommentOnPhaseChanged(app argocdv1alpha1.Application, argocdURL string, sourceRevision argocd.SourceRevision) *Comment { + repository := github.ParseRepositoryURL(sourceRevision.Source.RepoURL) if repository == nil { return nil } - revision := argocd.GetDeployedRevision(app) - if revision == "" { - return nil - } - body := generateCommentOnPhaseChanged(app, argocdURL) + body := generateCommentBodyOnPhaseChanged(app, argocdURL, sourceRevision) if body == "" { return nil } return &Comment{ GitHubRepository: *repository, - Revision: revision, + Revision: sourceRevision.Revision, Body: body, } } -func generateCommentOnPhaseChanged(app argocdv1alpha1.Application, argocdURL string) string { - phase := argocd.GetOperationPhase(app) - if phase == "" { +func generateCommentBodyOnPhaseChanged(app argocdv1alpha1.Application, argocdURL string, sourceRevision argocd.SourceRevision) string { + if app.Status.OperationState == nil { return "" } - revision := argocd.GetDeployedRevision(app) argocdApplicationURL := fmt.Sprintf("%s/applications/%s", argocdURL, app.Name) - + phase := app.Status.OperationState.Phase switch phase { case synccommon.OperationRunning: - return fmt.Sprintf(":warning: Syncing [%s](%s) to %s", app.Name, argocdApplicationURL, revision) + return fmt.Sprintf(":warning: Syncing [%s](%s) to %s", app.Name, argocdApplicationURL, sourceRevision.Revision) case synccommon.OperationSucceeded: - return fmt.Sprintf(":white_check_mark: Synced [%s](%s) to %s", app.Name, argocdApplicationURL, revision) + return fmt.Sprintf(":white_check_mark: Synced [%s](%s) to %s", app.Name, argocdApplicationURL, sourceRevision.Revision) case synccommon.OperationFailed, synccommon.OperationError: return fmt.Sprintf("## :x: Sync %s: [%s](%s)\nError while syncing to %s:\n%s", phase, app.Name, argocdApplicationURL, - revision, - generateSyncResultComment(app), + sourceRevision.Revision, + generateSyncResultComment(app.Status.OperationState.SyncResult), ) } return "" } -func generateSyncResultComment(app argocdv1alpha1.Application) string { - if app.Status.OperationState.SyncResult == nil { +func generateSyncResultComment(syncResult *argocdv1alpha1.SyncOperationResult) string { + if syncResult == nil { return "" } var b strings.Builder - for _, r := range app.Status.OperationState.SyncResult.Resources { + for _, r := range syncResult.Resources { namespacedName := r.Namespace + "/" + r.Name switch r.Status { case synccommon.ResultCodeSyncFailed, synccommon.ResultCodePruneSkipped: From 5477ae055fc1b8703dc9b9ce68531b53b206ec45 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Sun, 20 Aug 2023 14:50:32 +0900 Subject: [PATCH 2/5] Support multiple sources and revisions (ApplicationHealthCommentReconciler) --- internal/argocd/application.go | 15 +++------ .../applicationhealthcomment_controller.go | 32 ++++++++++-------- internal/notification/comment.go | 33 +++++++++++-------- 3 files changed, 41 insertions(+), 39 deletions(-) diff --git a/internal/argocd/application.go b/internal/argocd/application.go index 8b3c336e..b2f99683 100644 --- a/internal/argocd/application.go +++ b/internal/argocd/application.go @@ -6,26 +6,19 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// GetDeployedRevision returns the last synced revision -func GetDeployedRevision(a argocdv1alpha1.Application) string { - if a.Status.OperationState == nil { - return "" - } - if a.Status.OperationState.Operation.Sync == nil { - return "" - } - return a.Status.OperationState.Operation.Sync.Revision -} - type SourceRevision struct { Source argocdv1alpha1.ApplicationSource Revision string } +// GetSourceRevisions returns the last synced revisions func GetSourceRevisions(app argocdv1alpha1.Application) []SourceRevision { if app.Status.OperationState == nil { return nil } + if app.Status.OperationState.Operation.Sync == nil { + return nil + } sources := app.Spec.GetSources() revisions := app.Status.OperationState.Operation.Sync.Revisions if revisions == nil { diff --git a/internal/controller/applicationhealthcomment_controller.go b/internal/controller/applicationhealthcomment_controller.go index 90d4be63..f52e120e 100644 --- a/internal/controller/applicationhealthcomment_controller.go +++ b/internal/controller/applicationhealthcomment_controller.go @@ -59,7 +59,6 @@ func (r *ApplicationHealthCommentReconciler) Reconcile(ctx context.Context, req if !app.DeletionTimestamp.IsZero() { return ctrl.Result{}, nil } - deployedRevision := argocd.GetDeployedRevision(app) var appHealth argocdcommenterv1.ApplicationHealth if err := r.Client.Get(ctx, req.NamespacedName, &appHealth); err != nil { @@ -81,39 +80,44 @@ func (r *ApplicationHealthCommentReconciler) Reconcile(ctx context.Context, req } logger.Info("created an ApplicationHealth") } - if deployedRevision == appHealth.Status.LastHealthyRevision { - return ctrl.Result{}, nil - } argocdURL, err := argocd.GetExternalURL(ctx, r.Client, req.Namespace) if err != nil { logger.Info("unable to determine Argo CD URL", "error", err) } - comment := notification.NewCommentOnOnHealthChanged(app, argocdURL) - if comment == nil { + comments := notification.NewCommentsOnOnHealthChanged(app, argocdURL) + if len(comments) == 0 { logger.Info("no comment on this health event") return ctrl.Result{}, nil } - if err := r.Notification.CreateComment(ctx, *comment, app); err != nil { - logger.Error(err, "unable to create a comment") - r.Recorder.Eventf(&app, corev1.EventTypeWarning, "CreateCommentError", - "unable to create a comment by %s: %s", app.Status.Health.Status, err) - } else { - r.Recorder.Eventf(&app, corev1.EventTypeNormal, "CreatedComment", "created a comment by %s", app.Status.Health.Status) + currentRevision := comments[0].Revision + if appHealth.Status.LastHealthyRevision == currentRevision { + logger.Info("current revision is already healthy", "revision", currentRevision) + return ctrl.Result{}, nil + } + + for _, comment := range comments { + if err := r.Notification.CreateComment(ctx, comment, app); err != nil { + logger.Error(err, "unable to create a comment") + r.Recorder.Eventf(&app, corev1.EventTypeWarning, "CreateCommentError", + "unable to create a comment by %s: %s", app.Status.Health.Status, err) + } else { + r.Recorder.Eventf(&app, corev1.EventTypeNormal, "CreatedComment", "created a comment by %s", app.Status.Health.Status) + } } if app.Status.Health.Status != health.HealthStatusHealthy { return ctrl.Result{}, nil } patch := client.MergeFrom(appHealth.DeepCopy()) - appHealth.Status.LastHealthyRevision = deployedRevision + appHealth.Status.LastHealthyRevision = currentRevision if err := r.Client.Status().Patch(ctx, &appHealth, patch); err != nil { logger.Error(err, "unable to patch lastHealthyRevision of ApplicationHealth") return ctrl.Result{}, client.IgnoreNotFound(err) } logger.Info("patched lastHealthyRevision of ApplicationHealth") r.Recorder.Eventf(&appHealth, corev1.EventTypeNormal, "UpdateLastHealthyRevision", - "patched lastHealthyRevision to %s", deployedRevision) + "patched lastHealthyRevision to %s", currentRevision) return ctrl.Result{}, nil } diff --git a/internal/notification/comment.go b/internal/notification/comment.go index ec0b844c..ccf26a5a 100644 --- a/internal/notification/comment.go +++ b/internal/notification/comment.go @@ -87,31 +87,36 @@ func generateSyncResultComment(syncResult *argocdv1alpha1.SyncOperationResult) s return b.String() } -func NewCommentOnOnHealthChanged(app argocdv1alpha1.Application, argocdURL string) *Comment { - if app.Spec.Source == nil { - return nil +func NewCommentsOnOnHealthChanged(app argocdv1alpha1.Application, argocdURL string) []Comment { + sourceRevisions := argocd.GetSourceRevisions(app) + var comments []Comment + for _, sourceRevision := range sourceRevisions { + comment := generateCommentOnHealthChanged(app, argocdURL, sourceRevision) + if comment == nil { + continue + } + comments = append(comments, *comment) } - repository := github.ParseRepositoryURL(app.Spec.Source.RepoURL) + return comments +} + +func generateCommentOnHealthChanged(app argocdv1alpha1.Application, argocdURL string, sourceRevision argocd.SourceRevision) *Comment { + repository := github.ParseRepositoryURL(sourceRevision.Source.RepoURL) if repository == nil { return nil } - revision := argocd.GetDeployedRevision(app) - if revision == "" { - return nil - } - body := generateCommentOnHealthChanged(app, argocdURL) + body := generateCommentBodyOnHealthChanged(app, argocdURL, sourceRevision) if body == "" { return nil } return &Comment{ GitHubRepository: *repository, - Revision: revision, + Revision: sourceRevision.Revision, Body: body, } } -func generateCommentOnHealthChanged(app argocdv1alpha1.Application, argocdURL string) string { - revision := argocd.GetDeployedRevision(app) +func generateCommentBodyOnHealthChanged(app argocdv1alpha1.Application, argocdURL string, sourceRevision argocd.SourceRevision) string { argocdApplicationURL := fmt.Sprintf("%s/applications/%s", argocdURL, app.Name) switch app.Status.Health.Status { case health.HealthStatusHealthy: @@ -120,7 +125,7 @@ func generateCommentOnHealthChanged(app argocdv1alpha1.Application, argocdURL st app.Status.Health.Status, app.Name, argocdApplicationURL, - revision, + sourceRevision.Revision, ) case health.HealthStatusDegraded: return fmt.Sprintf("## %s %s: [%s](%s)\nDeployed %s", @@ -128,7 +133,7 @@ func generateCommentOnHealthChanged(app argocdv1alpha1.Application, argocdURL st app.Status.Health.Status, app.Name, argocdApplicationURL, - revision, + sourceRevision.Revision, ) } return "" From 5a9f053cdd3b96d6c3c7af473e1907cbcc69ed54 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Fri, 1 Sep 2023 19:13:19 +0900 Subject: [PATCH 3/5] Replace with `sources` in `Application` --- e2e_test/applications/app1.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/e2e_test/applications/app1.yaml b/e2e_test/applications/app1.yaml index 0ac34170..5bf722b0 100644 --- a/e2e_test/applications/app1.yaml +++ b/e2e_test/applications/app1.yaml @@ -9,10 +9,11 @@ metadata: - resources-finalizer.argocd.argoproj.io spec: project: default - source: - repoURL: https://github.com/int128/argocd-commenter-e2e-test - targetRevision: FIXTURE_BASE_BRANCH - path: app1 + # https://argo-cd.readthedocs.io/en/stable/user-guide/multiple_sources/ + sources: + - repoURL: https://github.com/int128/argocd-commenter-e2e-test + targetRevision: FIXTURE_BASE_BRANCH + path: app1 destination: server: https://kubernetes.default.svc namespace: app1 From aeb22e5f5ad3bc315d22f85d23483a2004a58313 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Sat, 2 Sep 2023 12:33:41 +0900 Subject: [PATCH 4/5] Fix e2e-test --- e2e_test/waitforapp/main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e_test/waitforapp/main.go b/e2e_test/waitforapp/main.go index 9f11cf50..70db9186 100644 --- a/e2e_test/waitforapp/main.go +++ b/e2e_test/waitforapp/main.go @@ -114,6 +114,10 @@ func newApplicationStatus(app *argocdv1alpha1.Application) ApplicationStatus { Sync: string(app.Status.Sync.Status), Health: string(app.Status.Health.Status), } + // for multiple sources + if len(app.Status.Sync.Revisions) > 0 { + s.Revision = app.Status.Sync.Revisions[0] + } if app.Status.OperationState != nil { s.Operation = string(app.Status.OperationState.Phase) } From 8aa01632487d5a88762fb9d7b5ea9430d794aee6 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Sat, 2 Sep 2023 21:15:12 +0900 Subject: [PATCH 5/5] Change to app3 --- e2e_test/applications/app1.yaml | 9 ++++----- e2e_test/applications/app3.yaml | 9 +++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/e2e_test/applications/app1.yaml b/e2e_test/applications/app1.yaml index 5bf722b0..0ac34170 100644 --- a/e2e_test/applications/app1.yaml +++ b/e2e_test/applications/app1.yaml @@ -9,11 +9,10 @@ metadata: - resources-finalizer.argocd.argoproj.io spec: project: default - # https://argo-cd.readthedocs.io/en/stable/user-guide/multiple_sources/ - sources: - - repoURL: https://github.com/int128/argocd-commenter-e2e-test - targetRevision: FIXTURE_BASE_BRANCH - path: app1 + source: + repoURL: https://github.com/int128/argocd-commenter-e2e-test + targetRevision: FIXTURE_BASE_BRANCH + path: app1 destination: server: https://kubernetes.default.svc namespace: app1 diff --git a/e2e_test/applications/app3.yaml b/e2e_test/applications/app3.yaml index 52e938e7..d6fb9fcf 100644 --- a/e2e_test/applications/app3.yaml +++ b/e2e_test/applications/app3.yaml @@ -9,10 +9,11 @@ metadata: - resources-finalizer.argocd.argoproj.io spec: project: default - source: - repoURL: https://github.com/int128/argocd-commenter-e2e-test - targetRevision: FIXTURE_BASE_BRANCH - path: app3 + # https://argo-cd.readthedocs.io/en/stable/user-guide/multiple_sources/ + sources: + - repoURL: https://github.com/int128/argocd-commenter-e2e-test + targetRevision: FIXTURE_BASE_BRANCH + path: app3 destination: server: https://kubernetes.default.svc namespace: test3-fixture