Merge pull request openshift#8693 from r4f4/capi-run-controller-fail-fix

OCPBUGS-36378: capi: start controllers after WaitGroup is created

Author: openshift-merge-bot[bot]

pkg/clusterapi/system.go

@@ -316,13 +316,6 @@ func (c *system) Run(ctx context.Context) error {
 	// We only show controller logs if the log level is DEBUG or above
 	c.logWriter = logrus.StandardLogger().WriterLevel(logrus.DebugLevel)
 
-	// Run the controllers.
-	for _, ct := range controllers {
-		if err := c.runController(ctx, ct); err != nil {
-			return fmt.Errorf("failed to run controller %q: %w", ct.Name, err)
-		}
-	}
-
 	// We create a wait group to wait for the controllers to stop,
 	// this waitgroup is a global, and is used by the Teardown function
 	// which is expected to be called when the program exits.
@@ -331,6 +324,7 @@ func (c *system) Run(ctx context.Context) error {
 		defer c.wg.Done()
 		// Stop the controllers when the context is cancelled.
 		<-ctx.Done()
+		logrus.Info("Shutting down local Cluster API controllers...")
 		for _, ct := range controllers {
 			if ct.state != nil {
 				if err := ct.state.Stop(); err != nil {
@@ -340,12 +334,14 @@ func (c *system) Run(ctx context.Context) error {
 				logrus.Infof("Stopped controller: %s", ct.Name)
 			}
 		}
+	}()
 
-		// Stop the local control plane.
-		if err := c.lcp.Stop(); err != nil {
-			logrus.Warnf("Failed to stop local Cluster API control plane: %v", err)
+	// Run the controllers.
+	for _, ct := range controllers {
+		if err := c.runController(ctx, ct); err != nil {
+			return fmt.Errorf("failed to run controller %q: %w", ct.Name, err)
 		}
-	}()
+	}
 
 	return nil
 }
@@ -377,10 +373,13 @@ func (c *system) Teardown() {
 	// Proceed to shutdown.
 	c.teardownOnce.Do(func() {
 		c.cancel()
-		logrus.Info("Shutting down local Cluster API control plane...")
 		ch := make(chan struct{})
 		go func() {
 			c.wg.Wait()
+			logrus.Info("Shutting down local Cluster API control plane...")
+			if err := c.lcp.Stop(); err != nil {
+				logrus.Warnf("Failed to stop local Cluster API control plane: %v", err)
+			}
 			close(ch)
 		}()
 		select {
@@ -463,7 +462,7 @@ func (c *system) runController(ctx context.Context, ct *controller) error {
 	// If the provider is not empty, we extract it to the binaries directory.
 	if ct.Provider != nil {
 		if err := ct.Provider.Extract(c.lcp.BinDir); err != nil {
-			logrus.Fatal(err)
+			return fmt.Errorf("failed to extract provider %q: %w", ct.Name, err)
 		}
 	}
 

pkg/infrastructure/clusterapi/clusterapi.go

@@ -128,16 +128,6 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset
 		logrus.Debugf("No pre-provisioning requirements for the %s provider", i.impl.Name())
 	}
 
-	// Run the CAPI system.
-	timer.StartTimer(infrastructureStage)
-	capiSystem := clusterapi.System()
-	if err := capiSystem.Run(ctx); err != nil {
-		return fileList, fmt.Errorf("failed to run cluster api system: %w", err)
-	}
-
-	// Grab the client.
-	cl := capiSystem.Client()
-
 	// If we're skipping bootstrap destroy, shutdown the local control plane.
 	// Otherwise, we will shut it down after bootstrap destroy.
 	// This has to execute as the last defer in the stack since previous defers might still need the local controlplane.
@@ -149,17 +139,27 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset
 	}
 
 	// Make sure to always return generated manifests, even if errors happened
-	defer func(ctx context.Context, cl client.Client) {
+	defer func(ctx context.Context) {
 		var errs []error
 		// Overriding the named return with the generated list
-		fileList, errs = i.collectManifests(ctx, cl)
+		fileList, errs = i.collectManifests(ctx, clusterapi.System().Client())
 		// If Provision returned an error, add it to the list
 		if err != nil {
-			capiSystem.CleanEtcd()
+			clusterapi.System().CleanEtcd()
 			errs = append(errs, err)
 		}
 		err = utilerrors.NewAggregate(errs)
-	}(ctx, cl)
+	}(ctx)
+
+	// Run the CAPI system.
+	timer.StartTimer(infrastructureStage)
+	capiSystem := clusterapi.System()
+	if err := capiSystem.Run(ctx); err != nil {
+		return fileList, fmt.Errorf("failed to run cluster api system: %w", err)
+	}
+
+	// Grab the client.
+	cl := capiSystem.Client()
 
 	i.appliedManifests = []client.Object{}