OCPEDGE-2088, OCPEDGE-1885: Updated state transitions & tests for TNF setup job

- Setup job now sets Available condition only when ExternalEtcd isn't enabled yet (includes test)
- Fixed jobcontroller & test setup to handle variable operator status conditions
- Added test coverage for updated expectations for job controller condition transitions (Progressing, Degraded, Available)

Author: jaypoulz

pkg/tnf/operator/starter.go

@@ -4,10 +4,11 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1"
 	"os"
 	"sync"
 
+	operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1"
+
 	operatorv1 "github.com/openshift/api/operator/v1"
 	configv1informers "github.com/openshift/client-go/config/informers/externalversions"
 	v1 "github.com/openshift/client-go/config/informers/externalversions/config/v1"
@@ -69,6 +70,12 @@ func HandleDualReplicaClusters(ctx context.Context,
 
 	klog.Infof("detected DualReplica topology")
 
+	// We only set the CEO available status to false during the initial setup
+	initialSetup, err := isInitialSetup(operatorClient)
+	if err != nil {
+		return false, fmt.Errorf("could not determine if we are in the initial setup: %w", err)
+	}
+
 	runExternalEtcdSupportController(ctx, controllerContext, operatorClient, envVarGetter, kubeInformersForNamespaces,
 		configInformers, networkInformer, controlPlaneNodeInformer, etcdInformer, kubeClient)
 	runTnfResourceController(ctx, controllerContext, kubeClient, dynamicClient, operatorClient, kubeInformersForNamespaces)
@@ -78,7 +85,7 @@ func HandleDualReplicaClusters(ctx context.Context,
 	// we need node names for assigning auth and after-setup jobs to specific nodes
 	var once sync.Once
 	klog.Infof("watching for nodes...")
-	_, err := controlPlaneNodeInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
+	_, err = controlPlaneNodeInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
 			node, ok := obj.(*corev1.Node)
 			if !ok {
@@ -102,11 +109,17 @@ func HandleDualReplicaClusters(ctx context.Context,
 				klog.Infof("found 2 control plane nodes (%q, %q), creating TNF jobs", nodeList[0].GetName(), nodeList[1].GetName())
 				// the order of job creation does not matter, the jobs wait on each other as needed
 				for _, node := range nodeList {
-					runJobController(ctx, tools.JobTypeAuth, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
-					runJobController(ctx, tools.JobTypeAfterSetup, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
+					runJobController(ctx, tools.JobTypeAuth, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, jobs.DefaultConditions)
+					runJobController(ctx, tools.JobTypeAfterSetup, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, jobs.DefaultConditions)
+				}
+
+				setupConditions := jobs.DefaultConditions
+				if initialSetup {
+					setupConditions = append(setupConditions, operatorv1.OperatorStatusTypeAvailable)
 				}
-				runJobController(ctx, tools.JobTypeSetup, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
-				runJobController(ctx, tools.JobTypeFencing, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
+
+				runJobController(ctx, tools.JobTypeSetup, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, setupConditions)
+				runJobController(ctx, tools.JobTypeFencing, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, jobs.DefaultConditions)
 			})
 		},
 	})
@@ -138,6 +151,20 @@ func HandleDualReplicaClusters(ctx context.Context,
 	return true, nil
 }
 
+func isInitialSetup(operatorClient v1helpers.StaticPodOperatorClient) (bool, error) {
+	// Detect if the cluster is already running in ExternalEtcd mode
+	operatorSpec, _, _, err := operatorClient.GetStaticPodOperatorState()
+	if err != nil {
+		return false, fmt.Errorf("could not get operator spec: %w", err)
+	}
+	externalEtcdMode, err := ceohelpers.IsExternalEtcdSupport(operatorSpec)
+	if err != nil {
+		return false, fmt.Errorf("could not determine if useExternalEtcdSupport config override is set: %w", err)
+	}
+
+	return !externalEtcdMode, nil
+}
+
 func isDualReplicaTopoly(ctx context.Context, featureGateAccessor featuregates.FeatureGateAccess, configInformers configv1informers.SharedInformerFactory) (bool, error) {
 	if isDualReplicaTopology, err := ceohelpers.IsDualReplicaTopology(ctx, configInformers.Config().V1().Infrastructures().Lister()); err != nil {
 		return false, fmt.Errorf("could not determine DualReplicaTopology, aborting controller start: %w", err)
@@ -201,7 +228,7 @@ func runTnfResourceController(ctx context.Context, controllerContext *controller
 	go tnfResourceController.Run(ctx, 1)
 }
 
-func runJobController(ctx context.Context, jobType tools.JobType, nodeName *string, controllerContext *controllercmd.ControllerContext, operatorClient v1helpers.StaticPodOperatorClient, kubeClient kubernetes.Interface, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces) {
+func runJobController(ctx context.Context, jobType tools.JobType, nodeName *string, controllerContext *controllercmd.ControllerContext, operatorClient v1helpers.StaticPodOperatorClient, kubeClient kubernetes.Interface, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, conditions []string) {
 	nodeNameForLogs := "n/a"
 	if nodeName != nil {
 		nodeNameForLogs = *nodeName
@@ -214,6 +241,7 @@ func runJobController(ctx context.Context, jobType tools.JobType, nodeName *stri
 		operatorClient,
 		kubeClient,
 		kubeInformersForNamespaces.InformersFor(operatorclient.TargetNamespace).Batch().V1().Jobs(),
+		conditions,
 		[]factory.Informer{},
 		[]jobs.JobHookFunc{
 			func(_ *operatorv1.OperatorSpec, job *batchv1.Job) error {

pkg/tnf/operator/starter_test.go

@@ -2,11 +2,12 @@ package operator
 
 import (
 	"context"
-	"github.com/stretchr/testify/require"
 	"maps"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 
@@ -36,6 +37,7 @@ import (
 	"github.com/openshift/cluster-etcd-operator/pkg/operator/ceohelpers"
 	"github.com/openshift/cluster-etcd-operator/pkg/operator/operatorclient"
 	"github.com/openshift/cluster-etcd-operator/pkg/tnf/operator/dualreplicahelpers"
+	"github.com/openshift/cluster-etcd-operator/pkg/tnf/pkg/jobs"
 )
 
 type args struct {
@@ -112,6 +114,72 @@ func TestHandleDualReplicaClusters(t *testing.T) {
 	}
 }
 
+func TestSetupJobConditionsBasedOnExternalEtcd(t *testing.T) {
+	tests := []struct {
+		name                    string
+		externalEtcdEnabled     bool
+		expectedAvailableInSetup bool
+	}{
+		{
+			name:                    "Job sets the Available condition during initial setup",
+			externalEtcdEnabled:     false,
+			expectedAvailableInSetup: true,
+		},
+		{
+			name:                    "Job does not set the Available condition once ExternalEtcd is enabled",
+			externalEtcdEnabled:     true,
+			expectedAvailableInSetup: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create operator spec with or without external etcd support
+			operatorSpec := &operatorv1.StaticPodOperatorSpec{}
+			if tt.externalEtcdEnabled {
+				operatorSpec.UnsupportedConfigOverrides = runtime.RawExtension{
+					Raw: []byte(`{"useExternalEtcdSupport": true}`),
+				}
+			}
+
+			fakeOperatorClient := v1helpers.NewFakeStaticPodOperatorClient(
+				operatorSpec,
+				&operatorv1.StaticPodOperatorStatus{},
+				nil,
+				nil,
+			)
+
+			// Test isInitialSetup function
+			isInitialSetup, err := isInitialSetup(fakeOperatorClient)
+			require.NoError(t, err)
+
+			// Verify the expected behavior
+			expectedInitialSetup := !tt.externalEtcdEnabled
+			require.Equal(t, expectedInitialSetup, isInitialSetup,
+				"isInitialSetup should return %v when externalEtcdEnabled is %v",
+				expectedInitialSetup, tt.externalEtcdEnabled)
+
+			// Verify setup conditions
+			setupConditions := jobs.DefaultConditions
+			if isInitialSetup {
+				setupConditions = append(setupConditions, operatorv1.OperatorStatusTypeAvailable)
+			}
+
+			hasAvailableCondition := false
+			for _, condition := range setupConditions {
+				if condition == operatorv1.OperatorStatusTypeAvailable {
+					hasAvailableCondition = true
+					break
+				}
+			}
+
+			require.Equal(t, tt.expectedAvailableInSetup, hasAvailableCondition,
+				"Setup job should have Available condition: %v, but got: %v",
+				tt.expectedAvailableInSetup, hasAvailableCondition)
+		})
+	}
+}
+
 func getArgs(t *testing.T, dualReplicaControlPlaneEnabled, dualReplicaFeatureGateEnabled bool) args {
 
 	fakeKubeClient := fake.NewSimpleClientset()

pkg/tnf/pkg/jobs/jobcontroller.go

@@ -26,6 +26,11 @@ import (
 	"github.com/openshift/cluster-etcd-operator/pkg/tnf/pkg/tools"
 )
 
+// By default, we only set the progressing and degraded conditions for TNF setup.
+// This is because the operator is only unavailable during the transition of etcd
+// ownership between CEO and pacemaker.
+var DefaultConditions = []string{opv1.OperatorStatusTypeProgressing, opv1.OperatorStatusTypeDegraded}
+
 // TODO This based on DeploymentController in openshift/library-go
 // TODO should be moved there once it proved to be useful
 
@@ -71,6 +76,7 @@ func NewJobController(
 	operatorClient v1helpers.OperatorClient,
 	kubeClient kubernetes.Interface,
 	jobInformer batchinformersv1.JobInformer,
+	conditions []string,
 	optionalInformers []factory.Informer,
 	optionalJobHooks ...JobHookFunc,
 ) factory.Controller {
@@ -82,9 +88,7 @@ func NewJobController(
 		kubeClient,
 		jobInformer,
 	).WithConditions(
-		opv1.OperatorStatusTypeAvailable,
-		opv1.OperatorStatusTypeProgressing,
-		opv1.OperatorStatusTypeDegraded,
+		conditions...,
 	).WithExtraInformers(
 		optionalInformers...,
 	).WithJobHooks(