Merge pull request #1242 from fluxcd/bootstrap-health-tweaks

Always report components health in bootstrap

cmd/flux/bootstrap_git.go

@@ -134,7 +134,7 @@ func bootstrapGitCmdRun(cmd *cobra.Command, args []string) error {
 		NotificationController: rootArgs.defaults.NotificationController,
 		ManifestFile:           rootArgs.defaults.ManifestFile,
 		Timeout:                rootArgs.timeout,
-		TargetPath:             gitArgs.path.String(),
+		TargetPath:             gitArgs.path.ToSlash(),
 		ClusterDomain:          bootstrapArgs.clusterDomain,
 		TolerationKeys:         bootstrapArgs.tolerationKeys,
 	}

cmd/flux/bootstrap_github.go

@@ -168,7 +168,7 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error {
 		NotificationController: rootArgs.defaults.NotificationController,
 		ManifestFile:           rootArgs.defaults.ManifestFile,
 		Timeout:                rootArgs.timeout,
-		TargetPath:             githubArgs.path.String(),
+		TargetPath:             githubArgs.path.ToSlash(),
 		ClusterDomain:          bootstrapArgs.clusterDomain,
 		TolerationKeys:         bootstrapArgs.tolerationKeys,
 	}
@@ -180,7 +180,7 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error {
 	secretOpts := sourcesecret.Options{
 		Name:         bootstrapArgs.secretName,
 		Namespace:    rootArgs.namespace,
-		TargetPath:   githubArgs.path.String(),
+		TargetPath:   githubArgs.path.ToSlash(),
 		ManifestFile: sourcesecret.MakeDefaultOptions().ManifestFile,
 	}
 	if bootstrapArgs.tokenAuth {
@@ -208,7 +208,7 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error {
 		Namespace:         rootArgs.namespace,
 		Branch:            bootstrapArgs.branch,
 		Secret:            bootstrapArgs.secretName,
-		TargetPath:        githubArgs.path.String(),
+		TargetPath:        githubArgs.path.ToSlash(),
 		ManifestFile:      sync.MakeDefaultOptions().ManifestFile,
 		GitImplementation: sourceGitArgs.gitImplementation.String(),
 		RecurseSubmodules: bootstrapArgs.recurseSubmodules,

cmd/flux/bootstrap_gitlab.go

@@ -181,7 +181,7 @@ func bootstrapGitLabCmdRun(cmd *cobra.Command, args []string) error {
 		NotificationController: rootArgs.defaults.NotificationController,
 		ManifestFile:           rootArgs.defaults.ManifestFile,
 		Timeout:                rootArgs.timeout,
-		TargetPath:             gitlabArgs.path.String(),
+		TargetPath:             gitlabArgs.path.ToSlash(),
 		ClusterDomain:          bootstrapArgs.clusterDomain,
 		TolerationKeys:         bootstrapArgs.tolerationKeys,
 	}
@@ -224,7 +224,7 @@ func bootstrapGitLabCmdRun(cmd *cobra.Command, args []string) error {
 		Namespace:         rootArgs.namespace,
 		Branch:            bootstrapArgs.branch,
 		Secret:            bootstrapArgs.secretName,
-		TargetPath:        gitlabArgs.path.String(),
+		TargetPath:        gitlabArgs.path.ToSlash(),
 		ManifestFile:      sync.MakeDefaultOptions().ManifestFile,
 		GitImplementation: sourceGitArgs.gitImplementation.String(),
 		RecurseSubmodules: bootstrapArgs.recurseSubmodules,

cmd/flux/create_kustomization.go

@@ -19,7 +19,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"path/filepath"
 	"strings"
 	"time"
 
@@ -151,7 +150,7 @@ func createKsCmdRun(cmd *cobra.Command, args []string) error {
 			Interval: metav1.Duration{
 				Duration: createArgs.interval,
 			},
-			Path:  filepath.ToSlash(kustomizationArgs.path.String()),
+			Path:  kustomizationArgs.path.ToSlash(),
 			Prune: kustomizationArgs.prune,
 			SourceRef: kustomizev1.CrossNamespaceSourceReference{
 				Kind:      kustomizationArgs.source.Kind,

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
@@ -160,6 +175,11 @@ func kustomizationReconciled(ctx context.Context, kube client.Client, objKey cli
 			return false, err
 		}
 
+		// Detect suspended Kustomization, as this would result in an endless wait
+		if kustomization.Spec.Suspend {
+			return false, fmt.Errorf("Kustomization is suspended")
+		}
+
 		// Confirm the state we are observing is for the current generation
 		if kustomization.Generation != kustomization.Status.ObservedGeneration {
 			return false, nil

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")
-
-	// Wait till Kustomization is reconciled
-	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),
-	); err != nil {
-		return fmt.Errorf("failed waiting for Kustomization: %w", err)
-	}
-
 	b.logger.Successf("reconciled sync configuration")
+
 	return nil
 }
 
-func (b *PlainGitBootstrapper) ConfirmHealthy(ctx context.Context, install install.Options, timeout time.Duration) error {
+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
+	if err := wait.PollImmediate(pollInterval, timeout, kustomizationReconciled(
+		ctx, b.kube, objKey, &k, expectRevision),
+	); err != nil {
+		b.logger.Failuref(err.Error())
+		return err
+	}
+
+	b.logger.Successf("Kustomization reconciled successfully")
+	return nil
+}
+
+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

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

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
 }

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
 }

internal/flags/safe_relative_path.go

@@ -18,6 +18,7 @@ package flags
 
 import (
 	"fmt"
+	"path/filepath"
 	"strings"
 
 	securejoin "github.com/cyphar/filepath-securejoin"
@@ -29,6 +30,10 @@ func (p *SafeRelativePath) String() string {
 	return string(*p)
 }
 
+func (p *SafeRelativePath) ToSlash() string {
+	return filepath.ToSlash(p.String())
+}
+
 func (p *SafeRelativePath) Set(str string) error {
 	// The result of secure joining on a relative base dir is a flattened relative path.
 	cleanP, err := securejoin.SecureJoin("./", strings.TrimSpace(str))

pkg/manifestgen/sourcesecret/sourcesecret.go

@@ -18,7 +18,6 @@ package sourcesecret
 
 import (
 	"bytes"
-	"encoding/pem"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -131,12 +130,7 @@ func loadKeyPair(path string) (*ssh.KeyPair, error) {
 		return nil, fmt.Errorf("failed to open private key file: %w", err)
 	}
 
-	block, _ := pem.Decode(b)
-	if block == nil {
-		return nil, fmt.Errorf("failed to decode PEM block")
-	}
-
-	ppk, err := cryptssh.ParsePrivateKey(block.Bytes)
+	ppk, err := cryptssh.ParsePrivateKey(b)
 	if err != nil {
 		return nil, err
 	}

pkg/manifestgen/sourcesecret/sourcesecret_test.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package sourcesecret
+
+import (
+	"io/ioutil"
+	"reflect"
+	"testing"
+)
+
+func Test_loadKeyPair(t *testing.T) {
+	pk, _ := ioutil.ReadFile("testdata/rsa")
+	ppk, _ := ioutil.ReadFile("testdata/rsa.pub")
+
+	got, err := loadKeyPair("testdata/rsa")
+	if err != nil {
+		t.Errorf("loadKeyPair() error = %v", err)
+		return
+	}
+	if !reflect.DeepEqual(got.PrivateKey, pk) {
+		t.Errorf("PrivateKey %s != %s", got.PrivateKey, pk)
+	}
+	if !reflect.DeepEqual(got.PublicKey, ppk) {
+		t.Errorf("PublicKey %s != %s", got.PublicKey, ppk)
+	}
+}

pkg/manifestgen/sourcesecret/testdata/rsa

@@ -0,0 +1,38 @@
+-----BEGIN OPENSSH PRIVATE KEY-----
+b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABlwAAAAdzc2gtcn
+NhAAAAAwEAAQAAAYEA3igXbgoTAydPOiEb4VfRPM4e26S16ZXhIEt95vka8wdcru97JKFl
+3yK6i0RALylrMAyjequXTNntZm2unngMuYE3OeFoY3dE1/xcWEVu8gE6AiGv74SzQsKtQv
+lqlXhF9s2cVyfJLmt5cvQP6zRxmx8KaZL6bN3PLDMIuq4Lit3BEn9KON89+uWKjXr9jU6/
+MlaQ4jn9VYMczhMDQpTyTMFgIBLuJYa4RUQ9vpL/KvgzpXh/bSf0B7DvEt+It2Qiws+RXH
+sc/cdxEDNRVwBOeueqSFZjw83fg5p3G5mMKUG0MLiNk0gw3S7JZKRndKiyKQZ7UCpfiRsk
+YJLZGGP8BeUGC5nYLicZAYJLddHyoq8jp8AtlGcKxgAQfWTjjlI2ppJPYpM2yncgnpv8Rk
+QiDH9onIjTxx715IDiLmsjesFi8weQrPXQ/OkBY0qud4A/piHiSg2/mcQ4bD+yNXP7ka7n
+hiGI/NPPSb/Q+a9SXa72VMI3z6s4/MKXL47t1NaNAAAFiMj8bezI/G3sAAAAB3NzaC1yc2
+EAAAGBAN4oF24KEwMnTzohG+FX0TzOHtuktemV4SBLfeb5GvMHXK7veyShZd8iuotEQC8p
+azAMo3qrl0zZ7WZtrp54DLmBNznhaGN3RNf8XFhFbvIBOgIhr++Es0LCrUL5apV4RfbNnF
+cnyS5reXL0D+s0cZsfCmmS+mzdzywzCLquC4rdwRJ/SjjfPfrlio16/Y1OvzJWkOI5/VWD
+HM4TA0KU8kzBYCAS7iWGuEVEPb6S/yr4M6V4f20n9Aew7xLfiLdkIsLPkVx7HP3HcRAzUV
+cATnrnqkhWY8PN34OadxuZjClBtDC4jZNIMN0uyWSkZ3SosikGe1AqX4kbJGCS2Rhj/AXl
+BguZ2C4nGQGCS3XR8qKvI6fALZRnCsYAEH1k445SNqaST2KTNsp3IJ6b/EZEIgx/aJyI08
+ce9eSA4i5rI3rBYvMHkKz10PzpAWNKrneAP6Yh4koNv5nEOGw/sjVz+5Gu54YhiPzTz0m/
+0PmvUl2u9lTCN8+rOPzCly+O7dTWjQAAAAMBAAEAAAGBAKv82868C+YIG8UD9uKpKvrpFG
+i1BoR1HVn0N9+GAQAfNfjUvEAql4R9DXBeAVbBuRL05edFSpgbqzf+OA7FIAzJZajwwfEn
+V+vimtdXwcGng3I9BEjpMiLANoTANWzMNVYR7jRnP9ApMlf1bRGJg141VMlRGYEI46fzRp
+HHxnXWoe+hDiQjaIeCB5bqnbs1OL5O2FHb1S3LmJRNkduNFlyn5LRQE4CH4Mb3Qtn0UYnB
+p7I2LGikYr9FkoDI/74CzZkL8OK01pfgSNbVrmJ3afFra7LrMYNUqseKsWIPvwCnHjr2hL
+LRxW6DU6CYZjy02ZBpkmISCBFSaLNbh0rH47B842lqrFPrEdGKBvlHJLHqzKDCBW+PSaHD
+K30kclgO1laxx0zdTUipYPPuJLL+2iHYWMYtKdDkpS96+BjoKKen0uZUhGamk2/rCbY1Gi
+p/iWjNlDKExWjpnQd4KfyQvrds2KGj1+4loFLxT6akmi57aCj7rqKbiBfbaPuVUMp6HQAA
+AMEA8Jx79pkkCIhnA8DabHp3RBfRyaJvomka2O6XaiCdzs03U/9h8I6ROwJZ4bXXOar6n9
+KrPXC4gRIBuqxoFaBcTHNIwH5apzeptJrAXe4baKJIGjG9KWP5brbyGzOsqj0Lx1bFiiro
+eDMOQCUFjpQ5DkwQTqpInzWLvt5bixMxQutqw4iirgB065j39UzgjPbFwI1+S/vTU2S68k
+qicpRmlz/hHjZX+wEPAwaUa9nBPPWv7tOqg0CYMFDwQbuT9WkuAAAAwQD28BUPrDCD4Sl+
+EI5kE6eyk9tdpTlMtgiLbH2WJO9yr4C/GVmQytujb6YXHAtZ3IXf8Wg6sXY8JH36dGTqZP
+gJaZfzhC4SEJvCUXWim/rdxj4CQMZW7o7guMME3w5hDHitj0vyGxatp30ltwUF6/gpSiEG
+SAI1lSNkZk3Ey0OMZv5Tp0Y5HJ3SBlnystYzzDvlDq4m+cNCmH0IytZV8Udwjkd7knLQce
+gvO+vKTWf6l8nb3BlBYBV72tyKGfd9pSMAAADBAOZPMjJZtQ5yzoklmyxqUiSZl/+aB41H
+IHU89ejt7cynuzIvOi3HWJB4201Z0yaS19xX48httEyxk0MTb5oK1H6yYKAx7m9DVMCq2e
+AJRM42Hh1Eer5bh/wUqdbqrV6NWkiXP7s440ml8tAsVULCKqQPRyPo1UBkayudMBx3Ke0W
+2sKWZDMT7OzC3lR4QdyC8keLzJhfudnP5ZWstOWgkTkPoZZ6EZZBz2gMVMEVczGcMYLIub
+eulFT3H8VUH5mIjwAAAAtoaWRkZUByaWRlcgECAwQFBg==
+-----END OPENSSH PRIVATE KEY-----

pkg/manifestgen/sourcesecret/testdata/rsa.pub

@@ -0,0 +1 @@
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDeKBduChMDJ086IRvhV9E8zh7bpLXpleEgS33m+RrzB1yu73skoWXfIrqLREAvKWswDKN6q5dM2e1mba6eeAy5gTc54Whjd0TX/FxYRW7yAToCIa/vhLNCwq1C+WqVeEX2zZxXJ8kua3ly9A/rNHGbHwppkvps3c8sMwi6rguK3cESf0o43z365YqNev2NTr8yVpDiOf1VgxzOEwNClPJMwWAgEu4lhrhFRD2+kv8q+DOleH9tJ/QHsO8S34i3ZCLCz5Fcexz9x3EQM1FXAE5656pIVmPDzd+DmncbmYwpQbQwuI2TSDDdLslkpGd0qLIpBntQKl+JGyRgktkYY/wF5QYLmdguJxkBgkt10fKiryOnwC2UZwrGABB9ZOOOUjamkk9ikzbKdyCem/xGRCIMf2iciNPHHvXkgOIuayN6wWLzB5Cs9dD86QFjSq53gD+mIeJKDb+ZxDhsP7I1c/uRrueGIYj8089Jv9D5r1JdrvZUwjfPqzj8wpcvju3U1o0=