Fix error handling for starter controller

This commit handles the error returned by syncing operations
on the starter controller and only marks the operator as disabled
if no manila share types is available.

Author: MaysaMacedo

pkg/driver/openstack-manila/openstack_manila.go

@@ -172,12 +172,17 @@ func GetOpenStackManilaOperatorControllerConfig(ctx context.Context, flavour gen
 			return false, errors.New("manila does not provide any share types")
 		}
 
-		syncCSIDriver(ctx, c.KubeClient, c.EventRecorder)
+		err = syncCSIDriver(ctx, c.KubeClient, c.EventRecorder)
+		if err != nil {
+			return true, err
+		}
 
-		syncStorageClasses(ctx, shareTypes, c.KubeClient, c.EventRecorder, c.GuestNamespace)
+		err = syncStorageClasses(ctx, shareTypes, c.KubeClient, c.EventRecorder, c.GuestNamespace)
+		if err != nil {
+			return true, err
+		}
 
 		return true, nil
-
 	}
 
 	cfg.PreconditionInformers = []factory.Informer{c.GetCSIDriverInformer().Informer(), c.GetStorageClassInformer().Informer()}

pkg/operator/starter_controller.go

@@ -67,10 +67,11 @@ func (c *Controller) sync(ctx context.Context, syncCtx factory.SyncContext) erro
 		return nil
 	}
 
-	_, err = c.operatorControllerConfig.Precondition()
-	if err != nil {
+	valid, err := c.operatorControllerConfig.Precondition()
+	if !valid {
 		return c.setDisabledCondition(ctx, fmt.Sprintf("Pre-condition not satisfied: %v", err))
 	}
+
 	if !c.controllersRunning {
 		// Start controllers
 		for _, controller := range c.factoryControllers {
@@ -88,6 +89,11 @@ func (c *Controller) sync(ctx context.Context, syncCtx factory.SyncContext) erro
 		c.controllersRunning = true
 	}
 
+	// If the precondition was valid, let the controllers start and return any error after.
+	if err != nil {
+		return err
+	}
+
 	return c.setEnabledCondition(ctx)
 }
 

pkg/operator/starter_controller_test.go

@@ -0,0 +1,77 @@
+package operator
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	opv1 "github.com/openshift/api/operator/v1"
+	"github.com/openshift/csi-operator/pkg/operator/config"
+	"github.com/openshift/library-go/pkg/operator/v1helpers"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestValidatePreCondition(t *testing.T) {
+	testProvider := "test-driver.csi.k8s.io"
+
+	cases := []struct {
+		name                     string
+		cfg                      *config.OperatorControllerConfig
+		expectSyncError          bool
+		expectControllersRunning bool
+	}{
+		{
+			name: "Precondition valid without error",
+			cfg: &config.OperatorControllerConfig{
+				Precondition: func() (bool, error) { return true, nil },
+			},
+			expectSyncError:          false,
+			expectControllersRunning: true,
+		},
+		{
+			name: "Precondition valid with error from internal operation",
+			cfg: &config.OperatorControllerConfig{
+				Precondition: func() (bool, error) { return true, errors.New("") },
+			},
+			expectSyncError:          true,
+			expectControllersRunning: true,
+		},
+		{
+			name: "Precondition not valid without error",
+			cfg: &config.OperatorControllerConfig{
+				Precondition: func() (bool, error) { return false, nil },
+			},
+			expectSyncError:          false,
+			expectControllersRunning: false,
+		},
+		{
+			name: "Precondition not valid with error from internal operation",
+			cfg: &config.OperatorControllerConfig{
+				Precondition: func() (bool, error) { return false, errors.New("") },
+			},
+			expectSyncError:          false,
+			expectControllersRunning: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			operator := &FakeOperator{
+				ObjectMeta: metav1.ObjectMeta{Name: testProvider},
+				Spec:       opv1.OperatorSpec{ManagementState: opv1.Managed},
+			}
+			c := &Controller{
+				operatorClient:           v1helpers.NewFakeOperatorClientWithObjectMeta(&operator.ObjectMeta, &operator.Spec, &operator.Status, nil),
+				operatorControllerConfig: tc.cfg,
+			}
+			ctx := context.Background()
+			err := c.sync(ctx, nil)
+			if c.controllersRunning != tc.expectControllersRunning {
+				t.Fatalf("Unexpected CSI controller running state: %v", c.controllersRunning)
+			}
+			if err != nil && !tc.expectSyncError {
+				t.Fatalf("Did not expect an error when syncing")
+			}
+		})
+	}
+}