Skip to content

Commit 77e2932

Browse files
committed
OCPEDGE-2088, OCPEDGE-1885: Updated state transitions & tests for TNF setup job
- Setup job now sets Available condition only when ExternalEtcd hasn't transitioned yet - Fixed jobcontroller & test setup to handle variable operator status conditions - Added test coverage for updated expectations for job controller condition transitions (Progressing, Degraded, Available) Depends on #1484 Depends on #1483
1 parent cbf0499 commit 77e2932

File tree

7 files changed

+293
-33
lines changed

7 files changed

+293
-33
lines changed

pkg/operator/ceohelpers/external_etcd_status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func GetExternalEtcdClusterStatus(ctx context.Context,
123123
// Check readiness for transition
124124
externalEtcdStatus.IsReadyForEtcdTransition = v1helpers.IsOperatorConditionTrue(opStatus.Conditions, etcd.OperatorConditionExternalEtcdReadyForTransition)
125125

126-
// Check if etcd has completed transition
126+
// Check if etcd is running externally
127127
externalEtcdStatus.HasExternalEtcdCompletedTransition = v1helpers.IsOperatorConditionTrue(opStatus.Conditions, etcd.OperatorConditionExternalEtcdHasCompletedTransition)
128128

129129
if externalEtcdStatus.IsEtcdRunningInCluster {

pkg/operator/externaletcdsupportcontroller/external_etcd_support_controller_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ func TestExternalEtcdSupportController(t *testing.T) {
5353
name: "ExternalEtcd cluster but bootstrap not completed",
5454
staticPodStatus: testutils.StaticPodOperatorStatus(
5555
testutils.WithLatestRevision(3),
56-
testutils.WithNodeStatusAtCurrentRevision(2),
57-
testutils.WithNodeStatusAtCurrentRevision(2),
58-
testutils.WithNodeStatusAtCurrentRevision(2),
56+
testutils.WithNodeStatusAtCurrentRevision(3),
57+
testutils.WithNodeStatusAtCurrentRevision(3),
58+
testutils.WithNodeStatusAtCurrentRevision(3),
5959
),
6060
expectedConfigMapExists: false,
6161
expectedErr: nil,
@@ -65,9 +65,9 @@ func TestExternalEtcdSupportController(t *testing.T) {
6565
name: "ExternalEtcd cluster and bootstrap completed",
6666
staticPodStatus: testutils.StaticPodOperatorStatus(
6767
testutils.WithLatestRevision(3),
68-
testutils.WithNodeStatusAtCurrentRevision(2),
69-
testutils.WithNodeStatusAtCurrentRevision(2),
70-
testutils.WithNodeStatusAtCurrentRevision(2),
68+
testutils.WithNodeStatusAtCurrentRevision(3),
69+
testutils.WithNodeStatusAtCurrentRevision(3),
70+
testutils.WithNodeStatusAtCurrentRevision(3),
7171
testutils.WithOperatorCondition("EtcdRunningInCluster", operatorv1.ConditionTrue),
7272
),
7373
expectedConfigMapExists: true,

pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ func TestTargetConfigController(t *testing.T) {
7575
},
7676
{
7777
name: "BackupVar Test HappyPath",
78-
7978
staticPodStatus: u.StaticPodOperatorStatus(
8079
u.WithLatestRevision(3),
8180
u.WithNodeStatusAtCurrentRevision(3),

pkg/tnf/operator/starter.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,23 @@ func handleNodes(
172172

173173
// the order of job creation does not matter, the jobs wait on each other as needed
174174
for _, node := range nodeList {
175-
runJobController(ctx, tools.JobTypeAuth, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
176-
runJobController(ctx, tools.JobTypeAfterSetup, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
175+
runJobController(ctx, tools.JobTypeAuth, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, jobs.DefaultConditions)
176+
runJobController(ctx, tools.JobTypeAfterSetup, &node.Name, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, jobs.DefaultConditions)
177177
}
178-
runJobController(ctx, tools.JobTypeSetup, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
179-
runJobController(ctx, tools.JobTypeFencing, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces)
178+
179+
hasExternalEtcdCompletedTransition, err := ceohelpers.HasExternalEtcdCompletedTransition(ctx, operatorClient)
180+
if err != nil {
181+
klog.Errorf("failed to get external etcd status: %v", err)
182+
return
183+
}
184+
185+
setupConditions := jobs.DefaultConditions
186+
if !hasExternalEtcdCompletedTransition {
187+
setupConditions = jobs.AllConditions
188+
}
189+
190+
runJobController(ctx, tools.JobTypeSetup, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, setupConditions)
191+
runJobController(ctx, tools.JobTypeFencing, nil, controllerContext, operatorClient, kubeClient, kubeInformersForNamespaces, jobs.DefaultConditions)
180192
}
181193

182194
func waitForEtcdBootstrapCompleted(ctx context.Context, operatorClient v1helpers.StaticPodOperatorClient) error {
@@ -246,7 +258,7 @@ func runTnfResourceController(ctx context.Context, controllerContext *controller
246258
go tnfResourceController.Run(ctx, 1)
247259
}
248260

249-
func runJobController(ctx context.Context, jobType tools.JobType, nodeName *string, controllerContext *controllercmd.ControllerContext, operatorClient v1helpers.StaticPodOperatorClient, kubeClient kubernetes.Interface, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces) {
261+
func runJobController(ctx context.Context, jobType tools.JobType, nodeName *string, controllerContext *controllercmd.ControllerContext, operatorClient v1helpers.StaticPodOperatorClient, kubeClient kubernetes.Interface, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, conditions []string) {
250262
nodeNameForLogs := "n/a"
251263
if nodeName != nil {
252264
nodeNameForLogs = *nodeName
@@ -259,6 +271,7 @@ func runJobController(ctx context.Context, jobType tools.JobType, nodeName *stri
259271
operatorClient,
260272
kubeClient,
261273
kubeInformersForNamespaces.InformersFor(operatorclient.TargetNamespace).Batch().V1().Jobs(),
274+
conditions,
262275
[]factory.Informer{},
263276
[]jobs.JobHookFunc{
264277
func(_ *operatorv1.OperatorSpec, job *batchv1.Job) error {

pkg/tnf/operator/starter_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ import (
3737
"github.com/openshift/cluster-etcd-operator/pkg/operator/ceohelpers"
3838
"github.com/openshift/cluster-etcd-operator/pkg/operator/operatorclient"
3939
u "github.com/openshift/cluster-etcd-operator/pkg/testutils"
40+
"github.com/openshift/cluster-etcd-operator/pkg/tnf/pkg/etcd"
41+
"github.com/openshift/cluster-etcd-operator/pkg/tnf/pkg/jobs"
4042
)
4143

4244
type args struct {
@@ -111,6 +113,70 @@ func TestHandleDualReplicaClusters(t *testing.T) {
111113
}
112114
}
113115

116+
func TestSetupJobConditionsBasedOnExternalEtcd(t *testing.T) {
117+
tests := []struct {
118+
name string
119+
isReadyForEtcdTransition bool
120+
expectedAvailableInSetup bool
121+
}{
122+
{
123+
name: "Job sets the Available condition when not ready for etcd transition",
124+
isReadyForEtcdTransition: false,
125+
expectedAvailableInSetup: true,
126+
},
127+
{
128+
name: "Job does not set the Available condition when ready for etcd transition",
129+
isReadyForEtcdTransition: true,
130+
expectedAvailableInSetup: false,
131+
},
132+
}
133+
134+
for _, tt := range tests {
135+
t.Run(tt.name, func(t *testing.T) {
136+
// Set up operator status with the appropriate condition
137+
operatorStatus := &operatorv1.StaticPodOperatorStatus{}
138+
if tt.isReadyForEtcdTransition {
139+
operatorStatus.Conditions = []operatorv1.OperatorCondition{
140+
{
141+
Type: etcd.OperatorConditionExternalEtcdHasCompletedTransition,
142+
Status: operatorv1.ConditionTrue,
143+
},
144+
}
145+
}
146+
147+
fakeOperatorClient := v1helpers.NewFakeStaticPodOperatorClient(
148+
&operatorv1.StaticPodOperatorSpec{},
149+
operatorStatus,
150+
nil,
151+
nil,
152+
)
153+
154+
hasExternalEtcdCompletedTransition, err := ceohelpers.HasExternalEtcdCompletedTransition(context.Background(), fakeOperatorClient)
155+
if err != nil {
156+
t.Errorf("failed to get external etcd status: %v", err)
157+
}
158+
159+
// Determine setup conditions based on the etcd transition status
160+
setupConditions := jobs.DefaultConditions
161+
if !hasExternalEtcdCompletedTransition {
162+
setupConditions = jobs.AllConditions
163+
}
164+
165+
hasAvailableCondition := false
166+
for _, condition := range setupConditions {
167+
if condition == operatorv1.OperatorStatusTypeAvailable {
168+
hasAvailableCondition = true
169+
break
170+
}
171+
}
172+
173+
require.Equalf(t, tt.expectedAvailableInSetup, hasAvailableCondition,
174+
"Setup job should have Available condition: %v, but got: %v",
175+
tt.expectedAvailableInSetup, hasAvailableCondition)
176+
})
177+
}
178+
}
179+
114180
func getArgs(t *testing.T, dualReplicaControlPlaneEnabled bool) args {
115181

116182
fakeKubeClient := fake.NewSimpleClientset()

pkg/tnf/pkg/jobs/jobcontroller.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ import (
2626
"github.com/openshift/cluster-etcd-operator/pkg/tnf/pkg/tools"
2727
)
2828

29+
// By default, we only set the progressing and degraded conditions for TNF setup.
30+
// This is because the operator is only unavailable during the transition of etcd
31+
// ownership between CEO and pacemaker.
32+
var DefaultConditions = []string{opv1.OperatorStatusTypeProgressing, opv1.OperatorStatusTypeDegraded}
33+
var AllConditions = []string{opv1.OperatorStatusTypeAvailable, opv1.OperatorStatusTypeProgressing, opv1.OperatorStatusTypeDegraded}
34+
2935
// TODO This based on DeploymentController in openshift/library-go
3036
// TODO should be moved there once it proved to be useful
3137

@@ -71,6 +77,7 @@ func NewJobController(
7177
operatorClient v1helpers.OperatorClient,
7278
kubeClient kubernetes.Interface,
7379
jobInformer batchinformersv1.JobInformer,
80+
conditions []string,
7481
optionalInformers []factory.Informer,
7582
optionalJobHooks ...JobHookFunc,
7683
) factory.Controller {
@@ -82,9 +89,7 @@ func NewJobController(
8289
kubeClient,
8390
jobInformer,
8491
).WithConditions(
85-
opv1.OperatorStatusTypeAvailable,
86-
opv1.OperatorStatusTypeProgressing,
87-
opv1.OperatorStatusTypeDegraded,
92+
conditions...,
8893
).WithExtraInformers(
8994
optionalInformers...,
9095
).WithJobHooks(

0 commit comments

Comments
 (0)