From ea451e7e498c9c3e052c24644f93a2bbdea9b955 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hello@hidde.co>
Date: Fri, 9 Apr 2021 13:47:54 +0200
Subject: [PATCH] Always report components health in bootstrap

This is useful in case the `Kustomization` does not reconcile
successfully because for example the controller(s) are in a crash loop,
which is not visible in the resource itself.

Signed-off-by: Hidde Beydals <hello@hidde.co>
---
 internal/bootstrap/bootstrap.go           | 29 ++++++++++++++++------
 internal/bootstrap/bootstrap_plain_git.go | 30 ++++++++++++++++-------
 internal/bootstrap/bootstrap_provider.go  |  4 +--
 internal/bootstrap/git/git.go             |  1 +
 internal/bootstrap/git/gogit/gogit.go     | 11 +++++++++
 5 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/internal/bootstrap/bootstrap.go b/internal/bootstrap/bootstrap.go
index 45fc0803..34d54840 100644
--- a/internal/bootstrap/bootstrap.go
+++ b/internal/bootstrap/bootstrap.go
@@ -58,11 +58,15 @@ type Reconciler interface {
 	// ReconcileSyncConfig reconciles the sync configuration by generating
 	// the sync manifests with the provided values, committing them to Git
 	// and pushing to remote if there are any changes.
-	ReconcileSyncConfig(ctx context.Context, options sync.Options, pollInterval, timeout time.Duration) error
+	ReconcileSyncConfig(ctx context.Context, options sync.Options) error
 
-	// ConfirmHealthy confirms that the components and extra components in
-	// install.Options are healthy.
-	ConfirmHealthy(ctx context.Context, options install.Options, timeout time.Duration) error
+	// ReportKustomizationHealth reports about the health of the
+	// Kustomization synchronizing the components.
+	ReportKustomizationHealth(ctx context.Context, options sync.Options, pollInterval, timeout time.Duration) error
+
+	// ReportComponentsHealth reports about the health for the components
+	// and extra components in install.Options.
+	ReportComponentsHealth(ctx context.Context, options install.Options, timeout time.Duration) error
 }
 
 type RepositoryReconciler interface {
@@ -89,11 +93,22 @@ func Run(ctx context.Context, reconciler Reconciler, manifestsBase string,
 	if err := reconciler.ReconcileSourceSecret(ctx, secretOpts); err != nil {
 		return err
 	}
-	if err := reconciler.ReconcileSyncConfig(ctx, syncOpts, pollInterval, timeout); err != nil {
+	if err := reconciler.ReconcileSyncConfig(ctx, syncOpts); err != nil {
 		return err
 	}
-	if err := reconciler.ConfirmHealthy(ctx, installOpts, timeout); err != nil {
-		return err
+
+	var healthErrCount int
+	if err := reconciler.ReportKustomizationHealth(ctx, syncOpts, pollInterval, timeout); err != nil {
+		healthErrCount++
+	}
+	if err := reconciler.ReportComponentsHealth(ctx, installOpts, timeout); err != nil {
+		healthErrCount++
+	}
+	if healthErrCount > 0 {
+		// Composing a "smart" error message here from the returned
+		// errors does not result in any useful information for the
+		// user, as both methods log the failures they run into.
+		err = fmt.Errorf("bootstrap failed with %d health check failure(s)", healthErrCount)
 	}
 
 	return err
diff --git a/internal/bootstrap/bootstrap_plain_git.go b/internal/bootstrap/bootstrap_plain_git.go
index b3ee125e..19fbae18 100644
--- a/internal/bootstrap/bootstrap_plain_git.go
+++ b/internal/bootstrap/bootstrap_plain_git.go
@@ -34,7 +34,6 @@ import (
 	kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1beta1"
 
 	"github.com/fluxcd/flux2/internal/bootstrap/git"
-
 	"github.com/fluxcd/flux2/internal/utils"
 	"github.com/fluxcd/flux2/pkg/log"
 	"github.com/fluxcd/flux2/pkg/manifestgen/install"
@@ -207,7 +206,7 @@ func (b *PlainGitBootstrapper) ReconcileSourceSecret(ctx context.Context, option
 	return nil
 }
 
-func (b *PlainGitBootstrapper) ReconcileSyncConfig(ctx context.Context, options sync.Options, pollInterval, timeout time.Duration) error {
+func (b *PlainGitBootstrapper) ReconcileSyncConfig(ctx context.Context, options sync.Options) error {
 	// Confirm that sync configuration does not overwrite existing config
 	if curPath, err := kustomizationPathDiffers(ctx, b.kube, client.ObjectKey{Name: options.Name, Namespace: options.Namespace}, options.TargetPath); err != nil {
 		return fmt.Errorf("failed to determine if sync configuration would overwrite existing Kustomization: %w", err)
@@ -283,22 +282,35 @@ func (b *PlainGitBootstrapper) ReconcileSyncConfig(ctx context.Context, options
 	if _, err = utils.ExecKubectlCommand(ctx, utils.ModeStderrOS, b.kubeconfig, b.kubecontext, kubectlArgs...); err != nil {
 		return err
 	}
-	b.logger.Successf("applied sync manifests")
+	b.logger.Successf("reconciled sync configuration")
 
-	// Wait till Kustomization is reconciled
+	return nil
+}
+
+func (b *PlainGitBootstrapper) ReportKustomizationHealth(ctx context.Context, options sync.Options, pollInterval, timeout time.Duration) error {
+	head, err := b.git.Head()
+	if err != nil {
+		return err
+	}
+
+	objKey := client.ObjectKey{Name: options.Name, Namespace: options.Namespace}
+
+	b.logger.Waitingf("waiting for Kustomization %q to be reconciled", objKey.String())
+
+	expectRevision := fmt.Sprintf("%s/%s", options.Branch, head)
 	var k kustomizev1.Kustomization
-	expectRevision := fmt.Sprintf("%s/%s", options.Branch, commit)
 	if err := wait.PollImmediate(pollInterval, timeout, kustomizationReconciled(
-		ctx, b.kube, client.ObjectKey{Name: options.Name, Namespace: options.Namespace}, &k, expectRevision),
+		ctx, b.kube, objKey, &k, expectRevision),
 	); err != nil {
-		return fmt.Errorf("failed waiting for Kustomization: %w", err)
+		b.logger.Failuref(err.Error())
+		return err
 	}
 
-	b.logger.Successf("reconciled sync configuration")
+	b.logger.Successf("Kustomization reconciled successfully")
 	return nil
 }
 
-func (b *PlainGitBootstrapper) ConfirmHealthy(ctx context.Context, install install.Options, timeout time.Duration) error {
+func (b *PlainGitBootstrapper) ReportComponentsHealth(ctx context.Context, install install.Options, timeout time.Duration) error {
 	cfg, err := utils.KubeConfig(b.kubeconfig, b.kubecontext)
 	if err != nil {
 		return err
diff --git a/internal/bootstrap/bootstrap_provider.go b/internal/bootstrap/bootstrap_provider.go
index 5a0bdea3..367b2c72 100644
--- a/internal/bootstrap/bootstrap_provider.go
+++ b/internal/bootstrap/bootstrap_provider.go
@@ -168,7 +168,7 @@ func (o sshHostnameOption) applyGitProvider(b *GitProviderBootstrapper) {
 	b.sshHostname = string(o)
 }
 
-func (b *GitProviderBootstrapper) ReconcileSyncConfig(ctx context.Context, options sync.Options, pollInterval, timeout time.Duration) error {
+func (b *GitProviderBootstrapper) ReconcileSyncConfig(ctx context.Context, options sync.Options) error {
 	repo, err := b.getRepository(ctx)
 	if err != nil {
 		return err
@@ -187,7 +187,7 @@ func (b *GitProviderBootstrapper) ReconcileSyncConfig(ctx context.Context, optio
 		}
 		options.URL = syncURL
 	}
-	return b.PlainGitBootstrapper.ReconcileSyncConfig(ctx, options, pollInterval, timeout)
+	return b.PlainGitBootstrapper.ReconcileSyncConfig(ctx, options)
 }
 
 // ReconcileRepository reconciles an organization or user repository with the
diff --git a/internal/bootstrap/git/git.go b/internal/bootstrap/git/git.go
index cbafadf9..86466e79 100644
--- a/internal/bootstrap/git/git.go
+++ b/internal/bootstrap/git/git.go
@@ -47,5 +47,6 @@ type Git interface {
 	Commit(message Commit) (string, error)
 	Push(ctx context.Context) error
 	Status() (bool, error)
+	Head() (string, error)
 	Path() string
 }
diff --git a/internal/bootstrap/git/gogit/gogit.go b/internal/bootstrap/git/gogit/gogit.go
index 619dd552..b11acd0c 100644
--- a/internal/bootstrap/git/gogit/gogit.go
+++ b/internal/bootstrap/git/gogit/gogit.go
@@ -212,6 +212,17 @@ func (g *GoGit) Status() (bool, error) {
 	return status.IsClean(), nil
 }
 
+func (g *GoGit) Head() (string, error) {
+	if g.repository == nil {
+		return "", git.ErrNoGitRepository
+	}
+	head, err := g.repository.Head()
+	if err != nil {
+		return "", err
+	}
+	return head.Hash().String(), nil
+}
+
 func (g *GoGit) Path() string {
 	return g.path
 }
-- 
GitLab