clusterctl init: add flag for retrying cert-manager readiness check

This introduces a new clusterctl init flag
"--retry-cert-manager-readiness-check" that allows to retry
the check for an already installed cert-manager, which
by default is only attempted once before a new cert-manager
installation is started.

When enabled, cert-manager readiness check will be retried
for the duration specified in clusterctl config file's
cert-manager.timeout entry or for a default timeout.

See: kubernetes-sigs#11960

Author: azych

cmd/clusterctl/client/client_test.go

@@ -251,7 +251,7 @@ type fakeCertManagerClient struct {
 
 var _ cluster.CertManagerClient = &fakeCertManagerClient{}
 
-func (p *fakeCertManagerClient) EnsureInstalled(_ context.Context) error {
+func (p *fakeCertManagerClient) EnsureInstalled(_ context.Context, _ bool) error {
 	return nil
 }
 

cmd/clusterctl/client/cluster/cert_manager.go

@@ -72,7 +72,7 @@ type CertManagerUpgradePlan struct {
 type CertManagerClient interface {
 	// EnsureInstalled makes sure cert-manager is running and its API is available.
 	// This is required to install a new provider.
-	EnsureInstalled(ctx context.Context) error
+	EnsureInstalled(ctx context.Context, retryReadinessCheck bool) error
 
 	// EnsureLatestVersion checks the cert-manager version currently installed, and if it is
 	// older than the version currently suggested by clusterctl, upgrades it.
@@ -155,11 +155,11 @@ func (cm *certManagerClient) certManagerNamespaceExists(ctx context.Context) (bo
 
 // EnsureInstalled makes sure cert-manager is running and its API is available.
 // This is required to install a new provider.
-func (cm *certManagerClient) EnsureInstalled(ctx context.Context) error {
+func (cm *certManagerClient) EnsureInstalled(ctx context.Context, retryReadinessCheck bool) error {
 	log := logf.Log
 
 	// Checking if a version of cert manager supporting cert-manager-test-resources.yaml is already installed and properly working.
-	if err := cm.waitForAPIReady(ctx, false); err == nil {
+	if err := cm.waitForAPIReady(ctx, retryReadinessCheck); err == nil {
 		log.Info("Skipping installing cert-manager as it is already installed")
 		return nil
 	}
@@ -555,13 +555,14 @@ func (cm *certManagerClient) waitForAPIReady(ctx context.Context, retry bool) er
 		return err
 	}
 
+	waitTimeout := cm.getWaitTimeout()
 	for i := range testObjs {
 		o := testObjs[i]
 
 		// Create the Kubernetes object.
 		// This is wrapped with a retry as the cert-manager API may not be available
-		// yet, so we need to keep retrying until it is.
-		if err := cm.pollImmediateWaiter(ctx, waitCertManagerInterval, cm.getWaitTimeout(), func(ctx context.Context) (bool, error) {
+		// yet, so we need to keep retrying until it is.createObjcreateObj
+		if err := cm.pollImmediateWaiter(ctx, waitCertManagerInterval, waitTimeout, func(ctx context.Context) (bool, error) {
 			if err := cm.createObj(ctx, o); err != nil {
 				// If retrying is disabled, return the error here.
 				if !retry {

cmd/clusterctl/client/cluster/cert_manager_test.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	. "github.com/onsi/gomega"
+	"github.com/pkg/errors"
 	admissionregistration "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -808,6 +809,67 @@ func Test_certManagerClient_EnsureLatestVersion(t *testing.T) {
 	}
 }
 
+func Test_certManagerClient_EnsureInstalled(t *testing.T) {
+	tests := []struct {
+		name                     string
+		retryReadinessCheck      bool
+		expectedError            error
+		expectedCertManagerCalls int
+	}{
+		{
+			name:                "Retries checking for existing cert-manager",
+			retryReadinessCheck: true,
+			// because of current EnsureInstalled logic, where the code to check for existing cert-manager
+			// and code to do a new installation if that check failed call the same methods/functions
+			// (or those methods/functions aren't really mockable from unit test POV), counting the number
+			// of calls to cm.configClient.CertManager() seems like a reasonable way to differntiate between paths
+			// and test the retry logic
+			expectedCertManagerCalls: 1,
+		},
+		{
+			name:                     "Checks for existing cert-manager only once",
+			retryReadinessCheck:      false,
+			expectedCertManagerCalls: 3,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			fakeConfigClient := newFakeConfig()
+			// make the proxy NewClient() calls return two errors on two initial calls, then nil's
+			proxy := test.NewFakeProxy().WithNewClientErrors(
+				errors.New("fail1"),
+				errors.New("fail2"),
+			)
+			pollImmediateWaiter := func(ctx context.Context, _ time.Duration, _ time.Duration, f wait.ConditionWithContextFunc) error {
+				// mimic non-test behavior
+				return wait.PollUntilContextTimeout(ctx, 0, 10*time.Second, true, f)
+			}
+			repo := repository.NewMemoryRepository().
+				WithPaths("root", "components.yaml").
+				WithDefaultVersion(config.CertManagerDefaultVersion).
+				WithFile(config.CertManagerDefaultVersion, "components.yaml", certManagerDeploymentYaml)
+			repositoryClientFactory := func(ctx context.Context, provider config.Provider, configClient config.Client, _ ...repository.Option) (repository.Client, error) {
+				return repository.New(ctx, provider, configClient, repository.InjectRepository(repo))
+			}
+
+			cm := newCertManagerClient(fakeConfigClient, repositoryClientFactory, proxy, pollImmediateWaiter)
+
+			err := cm.EnsureInstalled(context.TODO(), tt.retryReadinessCheck)
+
+			if tt.expectedError != nil {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err).To(MatchError(tt.expectedError))
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+			}
+			g.Expect(fakeConfigClient.certManagerCalls).To(Equal(tt.expectedCertManagerCalls))
+		})
+	}
+}
+
 func newFakeConfig() *fakeConfigClient {
 	fakeReader := test.NewFakeReader()
 
@@ -821,23 +883,26 @@ func newFakeConfig() *fakeConfigClient {
 type fakeConfigClient struct {
 	fakeReader     *test.FakeReader
 	internalclient config.Client
+
+	certManagerCalls int
 }
 
 var _ config.Client = &fakeConfigClient{}
 
-func (f fakeConfigClient) CertManager() config.CertManagerClient {
+func (f *fakeConfigClient) CertManager() config.CertManagerClient {
+	f.certManagerCalls++
 	return f.internalclient.CertManager()
 }
 
-func (f fakeConfigClient) Providers() config.ProvidersClient {
+func (f *fakeConfigClient) Providers() config.ProvidersClient {
 	return f.internalclient.Providers()
 }
 
-func (f fakeConfigClient) Variables() config.VariablesClient {
+func (f *fakeConfigClient) Variables() config.VariablesClient {
 	return f.internalclient.Variables()
 }
 
-func (f fakeConfigClient) ImageMeta() config.ImageMetaClient {
+func (f *fakeConfigClient) ImageMeta() config.ImageMetaClient {
 	return f.internalclient.ImageMeta()
 }
 

cmd/clusterctl/client/init.go

@@ -76,6 +76,10 @@ type InitOptions struct {
 	// WaitProviderTimeout sets the timeout per provider wait installation
 	WaitProviderTimeout time.Duration
 
+	// RetryCertManagerReadinessCheck instructs the init command to retry the check for cert-manager readiness
+	// before attempting to install it.
+	RetryCertManagerReadinessCheck bool
+
 	// SkipTemplateProcess allows for skipping the call to the template processor, including also variable replacement in the component YAML.
 	// NOTE this works only if the rawYaml is a valid yaml by itself, like e.g when using envsubst/the simple processor.
 	skipTemplateProcess bool
@@ -142,7 +146,7 @@ func (c *clusterctlClient) Init(ctx context.Context, options InitOptions) ([]Com
 
 	// Before installing the providers, ensure the cert-manager Webhook is in place.
 	certManager := clusterClient.CertManager()
-	if err := certManager.EnsureInstalled(ctx); err != nil {
+	if err := certManager.EnsureInstalled(ctx, options.RetryCertManagerReadinessCheck); err != nil {
 		return nil, err
 	}
 

cmd/clusterctl/cmd/init.go

@@ -40,6 +40,7 @@ type initOptions struct {
 	validate                  bool
 	waitProviders             bool
 	waitProviderTimeout       int
+	retryCertManagerCheck     bool
 }
 
 var initOpts = &initOptions{}
@@ -117,6 +118,8 @@ func init() {
 		"Wait timeout per provider installation in seconds. This value is ignored if --wait-providers is false")
 	initCmd.Flags().BoolVar(&initOpts.validate, "validate", true,
 		"If true, clusterctl will validate that the deployments will succeed on the management cluster.")
+	initCmd.Flags().BoolVar(&initOpts.retryCertManagerCheck, "retry-cert-manager-readiness-check", false,
+		"If true, clusterctl will retry checking for an existing cert-manager readiness using cert-manager.timeout entry value from the config file.")
 
 	initCmd.AddCommand(initListImagesCmd)
 	RootCmd.AddCommand(initCmd)
@@ -131,19 +134,20 @@ func runInit() error {
 	}
 
 	options := client.InitOptions{
-		Kubeconfig:                client.Kubeconfig{Path: initOpts.kubeconfig, Context: initOpts.kubeconfigContext},
-		CoreProvider:              initOpts.coreProvider,
-		BootstrapProviders:        initOpts.bootstrapProviders,
-		ControlPlaneProviders:     initOpts.controlPlaneProviders,
-		InfrastructureProviders:   initOpts.infrastructureProviders,
-		IPAMProviders:             initOpts.ipamProviders,
-		RuntimeExtensionProviders: initOpts.runtimeExtensionProviders,
-		AddonProviders:            initOpts.addonProviders,
-		TargetNamespace:           initOpts.targetNamespace,
-		LogUsageInstructions:      true,
-		WaitProviders:             initOpts.waitProviders,
-		WaitProviderTimeout:       time.Duration(initOpts.waitProviderTimeout) * time.Second,
-		IgnoreValidationErrors:    !initOpts.validate,
+		Kubeconfig:                     client.Kubeconfig{Path: initOpts.kubeconfig, Context: initOpts.kubeconfigContext},
+		CoreProvider:                   initOpts.coreProvider,
+		BootstrapProviders:             initOpts.bootstrapProviders,
+		ControlPlaneProviders:          initOpts.controlPlaneProviders,
+		InfrastructureProviders:        initOpts.infrastructureProviders,
+		IPAMProviders:                  initOpts.ipamProviders,
+		RuntimeExtensionProviders:      initOpts.runtimeExtensionProviders,
+		AddonProviders:                 initOpts.addonProviders,
+		TargetNamespace:                initOpts.targetNamespace,
+		LogUsageInstructions:           true,
+		WaitProviders:                  initOpts.waitProviders,
+		WaitProviderTimeout:            time.Duration(initOpts.waitProviderTimeout) * time.Second,
+		IgnoreValidationErrors:         !initOpts.validate,
+		RetryCertManagerReadinessCheck: initOpts.retryCertManagerCheck,
 	}
 
 	if _, err := c.Init(ctx, options); err != nil {

cmd/clusterctl/internal/test/fake_proxy.go

@@ -17,6 +17,7 @@ limitations under the License.
 package test
 
 import (
+	"container/list"
 	"context"
 	"errors"
 
@@ -46,6 +47,8 @@ type FakeProxy struct {
 	namespace string
 	objs      []client.Object
 	available *bool
+
+	newClientErrors *list.List
 }
 
 var (
@@ -80,6 +83,12 @@ func (f *FakeProxy) GetConfig() (*rest.Config, error) {
 }
 
 func (f *FakeProxy) NewClient(_ context.Context) (client.Client, error) {
+	firstInserted := f.newClientErrors.Back()
+	if firstInserted != nil {
+		f.newClientErrors.Remove(firstInserted)
+		return nil, firstInserted.Value.(error)
+	}
+
 	if f.cs != nil {
 		return f.cs, nil
 	}
@@ -149,7 +158,8 @@ func (f *FakeProxy) GetResourceNames(_ context.Context, _, _ string, _ []client.
 
 func NewFakeProxy() *FakeProxy {
 	return &FakeProxy{
-		namespace: "default",
+		namespace:       "default",
+		newClientErrors: list.New(),
 	}
 }
 
@@ -163,6 +173,13 @@ func (f *FakeProxy) WithNamespace(n string) *FakeProxy {
 	return f
 }
 
+func (f *FakeProxy) WithNewClientErrors(errs ...error) *FakeProxy {
+	for _, err := range errs {
+		f.newClientErrors.PushFront(err)
+	}
+	return f
+}
+
 // WithProviderInventory can be used as a fast track for setting up test scenarios requiring an already initialized management cluster.
 // NB. this method adds an items to the Provider inventory, but it doesn't install the corresponding provider; if the
 // test case requires the actual provider to be installed, use the fake client to install both the provider