OCPBUGS-64793: use master schedulability for topology calculation

pkg/asset/manifests/infrastructure.go

@@ -60,6 +60,7 @@ func (*Infrastructure) Dependencies() []asset.Asset {
 		&installconfig.InstallConfig{},
 		&CloudProviderConfig{},
 		&AdditionalTrustBundleConfig{},
+		&Scheduler{},
 	}
 }
 
@@ -72,7 +73,8 @@ func (i *Infrastructure) Generate(ctx context.Context, dependencies asset.Parent
 	installConfig := &installconfig.InstallConfig{}
 	cloudproviderconfig := &CloudProviderConfig{}
 	trustbundleconfig := &AdditionalTrustBundleConfig{}
-	dependencies.Get(clusterID, installConfig, cloudproviderconfig, trustbundleconfig)
+	scheduler := &Scheduler{}
+	dependencies.Get(clusterID, installConfig, cloudproviderconfig, trustbundleconfig, scheduler)
 
 	config := &configv1.Infrastructure{
 		TypeMeta: metav1.TypeMeta{
@@ -94,7 +96,7 @@ func (i *Infrastructure) Generate(ctx context.Context, dependencies asset.Parent
 		},
 	}
 
-	controlPlaneTopology, infrastructureTopology := determineTopologies(installConfig.Config)
+	controlPlaneTopology, infrastructureTopology := determineTopologies(installConfig.Config, scheduler.MastersSchedulable)
 
 	config.Status.InfrastructureTopology = infrastructureTopology
 	config.Status.ControlPlaneTopology = controlPlaneTopology

pkg/asset/manifests/infrastructure_test.go

@@ -284,6 +284,15 @@ func TestGenerateInfrastructure(t *testing.T) {
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
+			// Generate Scheduler asset
+			scheduler := &Scheduler{}
+			schedulerParents := asset.Parents{}
+			schedulerParents.Add(installconfig.MakeAsset(tc.installConfig))
+			err := scheduler.Generate(context.Background(), schedulerParents)
+			if !assert.NoError(t, err, "failed to generate scheduler") {
+				return
+			}
+
 			parents := asset.Parents{}
 			parents.Add(
 				&installconfig.ClusterID{
@@ -293,9 +302,10 @@ func TestGenerateInfrastructure(t *testing.T) {
 				installconfig.MakeAsset(tc.installConfig),
 				&CloudProviderConfig{},
 				&AdditionalTrustBundleConfig{},
+				scheduler,
 			)
 			infraAsset := &Infrastructure{}
-			err := infraAsset.Generate(context.Background(), parents)
+			err = infraAsset.Generate(context.Background(), parents)
 			if !assert.NoError(t, err, "failed to generate asset") {
 				return
 			}

pkg/asset/manifests/ingress.go

@@ -79,7 +79,8 @@ func (ing *Ingress) Generate(_ context.Context, dependencies asset.Parents) erro
 }
 
 func (ing *Ingress) generateClusterConfig(config *types.InstallConfig) ([]byte, error) {
-	controlPlaneTopology, _ := determineTopologies(config)
+	// mastersSchedulable doesn't affect controlPlaneTopology determination, so we pass false
+	controlPlaneTopology, _ := determineTopologies(config, false)
 
 	isSingleControlPlaneNode := controlPlaneTopology == configv1.SingleReplicaTopologyMode
 

pkg/asset/manifests/ingress_test.go

@@ -46,8 +46,17 @@ func installConfigFromTopologies(t *testing.T, options []icOption,
 		}
 	}
 
+	// Compute mastersSchedulable based on worker count (same logic as Scheduler)
+	computeReplicas := int64(0)
+	for _, pool := range installConfig.Compute {
+		if pool.Replicas != nil {
+			computeReplicas += *pool.Replicas
+		}
+	}
+	mastersSchedulable := computeReplicas == 0
+
 	// Assert that this function actually works
-	generatedControlPlaneTopology, generatedInfrastructureTopology := determineTopologies(installConfig)
+	generatedControlPlaneTopology, generatedInfrastructureTopology := determineTopologies(installConfig, mastersSchedulable)
 	assert.Equal(t, generatedControlPlaneTopology, controlPlaneTopology)
 	assert.Equal(t, generatedInfrastructureTopology, infrastructureTopology)
 

pkg/asset/manifests/scheduler.go

@@ -2,6 +2,7 @@ package manifests
 
 import (
 	"context"
+	"os"
 	"path"
 
 	"github.com/pkg/errors"
@@ -21,7 +22,8 @@ var (
 
 // Scheduler generates the cluster-scheduler-*.yml files.
 type Scheduler struct {
-	FileList []*asset.File
+	FileList           []*asset.File
+	MastersSchedulable bool
 }
 
 var _ asset.WritableAsset = (*Scheduler)(nil)
@@ -86,6 +88,7 @@ func (s *Scheduler) Generate(_ context.Context, dependencies asset.Parents) erro
 			Data:     configData,
 		},
 	}
+	s.MastersSchedulable = config.Spec.MastersSchedulable
 
 	return nil
 }
@@ -95,7 +98,23 @@ func (s *Scheduler) Files() []*asset.File {
 	return s.FileList
 }
 
-// Load returns false since this asset is not written to disk by the installer.
+// Load loads the Scheduler manifest from disk if it exists.
 func (s *Scheduler) Load(f asset.FileFetcher) (bool, error) {
-	return false, nil
+	file, err := f.FetchByName(SchedulerCfgFilename)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
+		return false, err
+	}
+
+	config := &configv1.Scheduler{}
+	if err := yaml.Unmarshal(file.Data, config); err != nil {
+		return false, errors.Wrapf(err, "failed to unmarshal %s", SchedulerCfgFilename)
+	}
+
+	s.FileList = []*asset.File{file}
+	s.MastersSchedulable = config.Spec.MastersSchedulable
+
+	return true, nil
 }

pkg/asset/manifests/topologies.go

@@ -9,7 +9,7 @@ import (
 
 // determineTopologies determines the Infrastructure CR's
 // infrastructureTopology and controlPlaneTopology given an install config file
-func determineTopologies(installConfig *types.InstallConfig) (controlPlaneTopology configv1.TopologyMode, infrastructureTopology configv1.TopologyMode) {
+func determineTopologies(installConfig *types.InstallConfig, mastersSchedulable bool) (controlPlaneTopology configv1.TopologyMode, infrastructureTopology configv1.TopologyMode) {
 	controlPlaneReplicas := ptr.Deref(installConfig.ControlPlane.Replicas, 3)
 	switch controlPlaneReplicas {
 	case 1:
@@ -29,17 +29,16 @@ func determineTopologies(installConfig *types.InstallConfig) (controlPlaneTopolo
 		numOfWorkers += ptr.Deref(mp.Replicas, 0)
 	}
 
-	switch numOfWorkers {
-	case 0:
-		// Two node deployments with 0 workers mean that the control plane nodes are treated as workers
-		// in that situation we have decided that it is appropriate to set the infrastructureTopology to HA.
-		// All other configuration for different worker count are respected with the original intention.
-		if controlPlaneTopology == configv1.DualReplicaTopologyMode || controlPlaneTopology == configv1.HighlyAvailableArbiterMode {
-			infrastructureTopology = configv1.HighlyAvailableTopologyMode
-		} else {
-			infrastructureTopology = controlPlaneTopology
-		}
-	case 1:
+	switch {
+	case numOfWorkers == 0 && controlPlaneTopology == configv1.DualReplicaTopologyMode:
+		infrastructureTopology = configv1.HighlyAvailableTopologyMode
+	case numOfWorkers == 0 && controlPlaneTopology == configv1.HighlyAvailableArbiterMode:
+		infrastructureTopology = configv1.HighlyAvailableTopologyMode
+	case numOfWorkers == 0:
+		infrastructureTopology = controlPlaneTopology
+	case numOfWorkers == 1 && mastersSchedulable:
+		infrastructureTopology = configv1.HighlyAvailableTopologyMode
+	case numOfWorkers == 1:
 		infrastructureTopology = configv1.SingleReplicaTopologyMode
 	default:
 		infrastructureTopology = configv1.HighlyAvailableTopologyMode

pkg/asset/manifests/topologies_test.go

@@ -13,6 +13,7 @@ func Test_DetermineTopologies(t *testing.T) {
 	testCases := []struct {
 		desc                 string
 		installConfig        *types.InstallConfig
+		mastersSchedulable   bool
 		expectedControlPlane configv1.TopologyMode
 		expectedInfra        configv1.TopologyMode
 	}{
@@ -24,6 +25,7 @@ func Test_DetermineTopologies(t *testing.T) {
 				},
 				Compute: []types.MachinePool{},
 			},
+			mastersSchedulable:   true,
 			expectedControlPlane: configv1.HighlyAvailableTopologyMode,
 			expectedInfra:        configv1.HighlyAvailableTopologyMode,
 		},
@@ -35,6 +37,7 @@ func Test_DetermineTopologies(t *testing.T) {
 				},
 				Compute: []types.MachinePool{},
 			},
+			mastersSchedulable:   true,
 			expectedControlPlane: configv1.SingleReplicaTopologyMode,
 			expectedInfra:        configv1.SingleReplicaTopologyMode,
 		},
@@ -50,6 +53,7 @@ func Test_DetermineTopologies(t *testing.T) {
 					},
 				},
 			},
+			mastersSchedulable:   false,
 			expectedControlPlane: configv1.SingleReplicaTopologyMode,
 			expectedInfra:        configv1.HighlyAvailableTopologyMode,
 		},
@@ -65,6 +69,7 @@ func Test_DetermineTopologies(t *testing.T) {
 					},
 				},
 			},
+			mastersSchedulable:   false,
 			expectedControlPlane: configv1.SingleReplicaTopologyMode,
 			expectedInfra:        configv1.SingleReplicaTopologyMode,
 		},
@@ -76,6 +81,7 @@ func Test_DetermineTopologies(t *testing.T) {
 				},
 				Compute: []types.MachinePool{},
 			},
+			mastersSchedulable:   true,
 			expectedControlPlane: configv1.DualReplicaTopologyMode,
 			expectedInfra:        configv1.HighlyAvailableTopologyMode,
 		},
@@ -90,6 +96,7 @@ func Test_DetermineTopologies(t *testing.T) {
 				},
 				Compute: []types.MachinePool{},
 			},
+			mastersSchedulable:   true,
 			expectedControlPlane: configv1.HighlyAvailableArbiterMode,
 			expectedInfra:        configv1.HighlyAvailableTopologyMode,
 		},
@@ -104,6 +111,7 @@ func Test_DetermineTopologies(t *testing.T) {
 				},
 				Compute: []types.MachinePool{},
 			},
+			mastersSchedulable:   true,
 			expectedControlPlane: configv1.HighlyAvailableArbiterMode,
 			expectedInfra:        configv1.HighlyAvailableTopologyMode,
 		},
@@ -122,13 +130,26 @@ func Test_DetermineTopologies(t *testing.T) {
 					},
 				},
 			},
+			mastersSchedulable:   false,
+			expectedControlPlane: configv1.HighlyAvailableTopologyMode,
+			expectedInfra:        configv1.HighlyAvailableTopologyMode,
+		},
+		{
+			desc: "should default infra to HA when control plane is HA, 0 workers, and masters are not schedulable",
+			installConfig: &types.InstallConfig{
+				ControlPlane: &types.MachinePool{
+					Replicas: ptr.To[int64](3),
+				},
+				Compute: []types.MachinePool{},
+			},
+			mastersSchedulable:   false,
 			expectedControlPlane: configv1.HighlyAvailableTopologyMode,
 			expectedInfra:        configv1.HighlyAvailableTopologyMode,
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			controlPlane, infra := determineTopologies(tc.installConfig)
+			controlPlane, infra := determineTopologies(tc.installConfig, tc.mastersSchedulable)
 			if controlPlane != tc.expectedControlPlane {
 				t.Fatalf("expected control plane topology to be %s but got %s", tc.expectedControlPlane, controlPlane)
 			}