diff --git a/pkg/asset/manifests/infrastructure.go b/pkg/asset/manifests/infrastructure.go index bc1ac019e31..efa5d87a3bb 100644 --- a/pkg/asset/manifests/infrastructure.go +++ b/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 diff --git a/pkg/asset/manifests/infrastructure_test.go b/pkg/asset/manifests/infrastructure_test.go index f3fcbd69693..71ccc448f32 100644 --- a/pkg/asset/manifests/infrastructure_test.go +++ b/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 } diff --git a/pkg/asset/manifests/ingress.go b/pkg/asset/manifests/ingress.go index 1458732dafe..f88ce3e57cb 100644 --- a/pkg/asset/manifests/ingress.go +++ b/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 diff --git a/pkg/asset/manifests/ingress_test.go b/pkg/asset/manifests/ingress_test.go index 1d76c91a1fe..98f6d7d6f65 100644 --- a/pkg/asset/manifests/ingress_test.go +++ b/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) diff --git a/pkg/asset/manifests/scheduler.go b/pkg/asset/manifests/scheduler.go index 4222ed3e2ab..e724f1e6395 100644 --- a/pkg/asset/manifests/scheduler.go +++ b/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 } diff --git a/pkg/asset/manifests/topologies.go b/pkg/asset/manifests/topologies.go index 5d837e83d1d..6ffad62ee44 100644 --- a/pkg/asset/manifests/topologies.go +++ b/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 diff --git a/pkg/asset/manifests/topologies_test.go b/pkg/asset/manifests/topologies_test.go index 4c96fdc7533..0a868063c0a 100644 --- a/pkg/asset/manifests/topologies_test.go +++ b/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) }