Skip to content

Commit 7181c84

Browse files
committed
OCPBUGS-64793: Infrastructure Topology calculation is wrong
If masters are schedulable then all nodes, not just non-controlplane nodes, should be taken into account when deciding what we tell OLM operators about the topology. Example: 1.Deploy a cluster with 3 CP nodes (schedulable) and 1 Worker Node 2.Inspect infrastructureTopology attribute of the cluster Expect: infrastructureTopology= HighlyAvailableTopologyMode
1 parent 4180662 commit 7181c84

File tree

4 files changed

+27
-12
lines changed

4 files changed

+27
-12
lines changed

pkg/asset/manifests/ingress_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,15 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) {
135135
name: "aws single node with 0 or 1 day-1 workers",
136136
installConfigBuildOptions: []icOption{icBuild.forAWS()},
137137
controlPlaneTopology: configv1.SingleReplicaTopologyMode,
138-
infrastructureTopology: configv1.SingleReplicaTopologyMode,
138+
infrastructureTopology: configv1.HighlyAvailableTopologyMode,
139139
expectedIngressAWSLBType: configv1.Classic,
140140
expectedIngressPlacement: configv1.DefaultPlacementWorkers,
141141
},
142142
{
143143
name: "aws multi-node with 1 day-1 worker",
144144
installConfigBuildOptions: []icOption{icBuild.forAWS()},
145145
controlPlaneTopology: configv1.HighlyAvailableTopologyMode,
146-
infrastructureTopology: configv1.SingleReplicaTopologyMode,
146+
infrastructureTopology: configv1.HighlyAvailableTopologyMode,
147147
expectedIngressAWSLBType: configv1.Classic,
148148
expectedIngressPlacement: configv1.DefaultPlacementWorkers,
149149
},
@@ -187,14 +187,14 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) {
187187
name: "none-platform single node with 0 or 1 day-1 workers",
188188
installConfigBuildOptions: []icOption{icBuild.forNone()},
189189
controlPlaneTopology: configv1.SingleReplicaTopologyMode,
190-
infrastructureTopology: configv1.SingleReplicaTopologyMode,
190+
infrastructureTopology: configv1.HighlyAvailableTopologyMode,
191191
expectedIngressPlacement: configv1.DefaultPlacementControlPlane,
192192
},
193193
{
194194
name: "none-platform multi-node with 1 day-1 worker",
195195
installConfigBuildOptions: []icOption{icBuild.forNone()},
196196
controlPlaneTopology: configv1.HighlyAvailableTopologyMode,
197-
infrastructureTopology: configv1.SingleReplicaTopologyMode,
197+
infrastructureTopology: configv1.HighlyAvailableTopologyMode,
198198
expectedIngressPlacement: configv1.DefaultPlacementWorkers,
199199
},
200200
{

pkg/asset/manifests/scheduler.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,9 @@ func (s *Scheduler) Generate(_ context.Context, dependencies asset.Parents) erro
6363
computeReplicas += *pool.Replicas
6464
}
6565
}
66-
if computeReplicas == 0 {
66+
if computeReplicas < 2 {
6767
// A schedulable host is required for a successful install to complete.
68-
// If the install config has 0 replicas for compute hosts, it's one of two cases:
69-
// 1. An IPI deployment with no compute hosts. The deployment can not succeed
70-
// without MastersSchedulable = true.
71-
// 2. A UPI deployment. The deployment may add compute hosts, but to ensure the
72-
// the highest probability of a successful deployment, we default to
73-
// schedulable masters.
68+
// Set the control planes to schedulable if the install config has < 2 replicas for compute hosts.
7469
logrus.Warningf("Making control-plane schedulable by setting MastersSchedulable to true for Scheduler cluster settings")
7570
config.Spec.MastersSchedulable = true
7671
}

pkg/asset/manifests/topologies.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ func determineTopologies(installConfig *types.InstallConfig) (controlPlaneTopolo
2828
for _, mp := range installConfig.Compute {
2929
numOfWorkers += ptr.Deref(mp.Replicas, 0)
3030
}
31+
if numOfWorkers < 2 {
32+
// Control planes are schedulable when there are < 2 workers.
33+
// Adjust the number of schedulable nodes here to reflect.
34+
numOfWorkers += controlPlaneReplicas
35+
}
3136

3237
switch numOfWorkers {
3338
case 0:

pkg/asset/manifests/topologies_test.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func Test_DetermineTopologies(t *testing.T) {
6666
},
6767
},
6868
expectedControlPlane: configv1.SingleReplicaTopologyMode,
69-
expectedInfra: configv1.SingleReplicaTopologyMode,
69+
expectedInfra: configv1.HighlyAvailableTopologyMode,
7070
},
7171
{
7272
desc: "should default infra to HA and controlPlane to DualReplica for 2 control replicas",
@@ -125,6 +125,21 @@ func Test_DetermineTopologies(t *testing.T) {
125125
expectedControlPlane: configv1.HighlyAvailableTopologyMode,
126126
expectedInfra: configv1.HighlyAvailableTopologyMode,
127127
},
128+
{
129+
desc: "should set infra to HA controlPlane and compute from controlPlane value",
130+
installConfig: &types.InstallConfig{
131+
ControlPlane: &types.MachinePool{
132+
Replicas: ptr.To[int64](3),
133+
},
134+
Compute: []types.MachinePool{
135+
{
136+
Replicas: ptr.To[int64](1),
137+
},
138+
},
139+
},
140+
expectedControlPlane: configv1.HighlyAvailableTopologyMode,
141+
expectedInfra: configv1.HighlyAvailableTopologyMode,
142+
},
128143
}
129144
for _, tc := range testCases {
130145
t.Run(tc.desc, func(t *testing.T) {

0 commit comments

Comments
 (0)