Skip to content

Commit bc127b2

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 2b8f0c7 commit bc127b2

File tree

3 files changed

+18
-5
lines changed

3 files changed

+18
-5
lines changed

pkg/asset/manifests/ingress_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) {
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
},
@@ -194,7 +194,7 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) {
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/topologies.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func determineTopologies(installConfig *types.InstallConfig) (controlPlaneTopolo
3030
}
3131

3232
switch numOfWorkers {
33-
case 0:
33+
case 0, 1:
3434
// Two node deployments with 0 workers mean that the control plane nodes are treated as workers
3535
// in that situation we have decided that it is appropriate to set the infrastructureTopology to HA.
3636
// All other configuration for different worker count are respected with the original intention.
@@ -39,8 +39,6 @@ func determineTopologies(installConfig *types.InstallConfig) (controlPlaneTopolo
3939
} else {
4040
infrastructureTopology = controlPlaneTopology
4141
}
42-
case 1:
43-
infrastructureTopology = configv1.SingleReplicaTopologyMode
4442
default:
4543
infrastructureTopology = configv1.HighlyAvailableTopologyMode
4644
}

pkg/asset/manifests/topologies_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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)