diff --git a/pkg/asset/machines/arbiter.go b/pkg/asset/machines/arbiter.go index 9b838f29e1..e1c72ddfcd 100644 --- a/pkg/asset/machines/arbiter.go +++ b/pkg/asset/machines/arbiter.go @@ -31,6 +31,7 @@ import ( "github.com/openshift/installer/pkg/asset/rhcos" "github.com/openshift/installer/pkg/types" baremetaltypes "github.com/openshift/installer/pkg/types/baremetal" + nonetypes "github.com/openshift/installer/pkg/types/none" ibmcloudapi "github.com/openshift/machine-api-provider-ibmcloud/pkg/apis" ibmcloudprovider "github.com/openshift/machine-api-provider-ibmcloud/pkg/apis/ibmcloudprovider/v1" ) @@ -115,8 +116,8 @@ func (m *Arbiter) Generate(ctx context.Context, dependencies asset.Parents) erro if ic.Arbiter == nil { return nil } - if ic.Platform.Name() != baremetaltypes.Name { - return fmt.Errorf("only BareMetal platform is supported for Arbiter deployments") + if ic.Platform.Name() != baremetaltypes.Name && ic.Platform.Name() != nonetypes.Name { + return fmt.Errorf("only BareMetal and None platforms are supported for Arbiter deployments") } pool := *ic.Arbiter @@ -125,43 +126,47 @@ func (m *Arbiter) Generate(ctx context.Context, dependencies asset.Parents) erro var ipClaims []ipamv1.IPAddressClaim var ipAddrs []ipamv1.IPAddress - mpool := defaultBareMetalMachinePoolPlatform() - mpool.Set(ic.Platform.BareMetal.DefaultMachinePlatform) - mpool.Set(pool.Platform.BareMetal) - pool.Platform.BareMetal = &mpool - // Use managed user data secret, since we always have up to date images // available in the cluster arbiterUserDataSecretName := "arbiter-user-data-managed" // #nosec G101 - enabledCaps := installConfig.Config.GetEnabledCapabilities() - if enabledCaps.Has(configv1.ClusterVersionCapabilityMachineAPI) { - machines, err = baremetal.Machines(clusterID.InfraID, ic, &pool, "arbiter", arbiterUserDataSecretName) - if err != nil { - return fmt.Errorf("failed to create arbiter machine objects: %w", err) - } - hostSettings, err := baremetal.ArbiterHosts(ic, machines, arbiterUserDataSecretName) - if err != nil { - return fmt.Errorf("failed to assemble host data: %w", err) - } - - hosts, err := createHostAssetFiles(hostSettings.Hosts, arbiterHostFileName) - if err != nil { - return err - } - m.HostFiles = append(m.HostFiles, hosts...) - - secrets, err := createSecretAssetFiles(hostSettings.Secrets, arbiterSecretFileName) - if err != nil { - return err - } - m.SecretFiles = append(m.SecretFiles, secrets...) - - networkSecrets, err := createSecretAssetFiles(hostSettings.NetworkConfigSecrets, arbiterNetworkConfigSecretFileName) - if err != nil { - return err + // BareMetal-specific: configure machine pool and generate Machine API objects + if ic.Platform.BareMetal != nil { + mpool := defaultBareMetalMachinePoolPlatform() + mpool.Set(ic.Platform.BareMetal.DefaultMachinePlatform) + mpool.Set(pool.Platform.BareMetal) + pool.Platform.BareMetal = &mpool + + enabledCaps := installConfig.Config.GetEnabledCapabilities() + if enabledCaps.Has(configv1.ClusterVersionCapabilityMachineAPI) { + machines, err = baremetal.Machines(clusterID.InfraID, ic, &pool, "arbiter", arbiterUserDataSecretName) + if err != nil { + return fmt.Errorf("failed to create arbiter machine objects: %w", err) + } + + hostSettings, err := baremetal.ArbiterHosts(ic, machines, arbiterUserDataSecretName) + if err != nil { + return fmt.Errorf("failed to assemble host data: %w", err) + } + + hosts, err := createHostAssetFiles(hostSettings.Hosts, arbiterHostFileName) + if err != nil { + return err + } + m.HostFiles = append(m.HostFiles, hosts...) + + secrets, err := createSecretAssetFiles(hostSettings.Secrets, arbiterSecretFileName) + if err != nil { + return err + } + m.SecretFiles = append(m.SecretFiles, secrets...) + + networkSecrets, err := createSecretAssetFiles(hostSettings.NetworkConfigSecrets, arbiterNetworkConfigSecretFileName) + if err != nil { + return err + } + m.NetworkConfigSecretFiles = append(m.NetworkConfigSecretFiles, networkSecrets...) } - m.NetworkConfigSecretFiles = append(m.NetworkConfigSecretFiles, networkSecrets...) } data, err := UserDataSecret(arbiterUserDataSecretName, mign.File.Data) diff --git a/pkg/asset/machines/arbiter_test.go b/pkg/asset/machines/arbiter_test.go index d2edfd037b..559a1ca4ef 100644 --- a/pkg/asset/machines/arbiter_test.go +++ b/pkg/asset/machines/arbiter_test.go @@ -15,6 +15,7 @@ import ( "github.com/openshift/installer/pkg/types" awstypes "github.com/openshift/installer/pkg/types/aws" "github.com/openshift/installer/pkg/types/baremetal" + "github.com/openshift/installer/pkg/types/none" ) func TestArbiterGenerateMachineConfigs(t *testing.T) { @@ -219,7 +220,51 @@ func TestArbiterInstallOnlyForBaremetal(t *testing.T) { arbiter := &Arbiter{} err := arbiter.Generate(context.Background(), parents) assert.NotNil(t, err, "expected arbiter generate to fail for non baremetal platforms") - assert.Contains(t, err.Error(), "only BareMetal platform is supported for Arbiter deployments") + assert.Contains(t, err.Error(), "only BareMetal and None platforms are supported for Arbiter deployments") +} + +func TestArbiterInstallNonePlatform(t *testing.T) { + parents := asset.Parents{} + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + SSHKey: "ssh-rsa: dummy-key", + BaseDomain: "test-domain", + Platform: types.Platform{ + None: &none.Platform{}, + }, + Arbiter: &types.MachinePool{ + Hyperthreading: types.HyperthreadingDisabled, + Replicas: ptr.To(int64(1)), + }, + }) + + parents.Add( + &installconfig.ClusterID{ + UUID: "test-uuid", + InfraID: "test-infra-id", + }, + installConfig, + rhcos.MakeAsset("test-image"), + (*rhcos.Release)(ptr.To("412.86.202208101040-0")), + &machine.Arbiter{ + File: &asset.File{ + Filename: "arbiter-ignition", + Data: []byte("test-ignition"), + }, + }, + ) + arbiter := &Arbiter{} + err := arbiter.Generate(context.Background(), parents) + assert.Nil(t, err, "expected arbiter generate to succeed for None platform") + // For None platform, no Machine API objects or BareMetalHost CRs should be generated + assert.Equal(t, 0, len(arbiter.MachineFiles), "expected no machine files for None platform") + assert.Equal(t, 0, len(arbiter.HostFiles), "expected no host files for None platform") + assert.Equal(t, 0, len(arbiter.SecretFiles), "expected no secret files for None platform") + // MachineConfigs should still be generated (hyperthreading disabled + SSH key) + assert.Greater(t, len(arbiter.MachineConfigFiles), 0, "expected machine config files for None platform") } func TestArbiterIsNotModified(t *testing.T) { diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index 1d9af71289..fa7b9954ba 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -39,6 +39,7 @@ import ( gcpvalidation "github.com/openshift/installer/pkg/types/gcp/validation" "github.com/openshift/installer/pkg/types/ibmcloud" ibmcloudvalidation "github.com/openshift/installer/pkg/types/ibmcloud/validation" + "github.com/openshift/installer/pkg/types/none" "github.com/openshift/installer/pkg/types/nutanix" nutanixvalidation "github.com/openshift/installer/pkg/types/nutanix/validation" "github.com/openshift/installer/pkg/types/openstack" @@ -778,8 +779,8 @@ func validateControlPlane(installConfig *types.InstallConfig, fldPath *field.Pat func validateArbiter(platform *types.Platform, arbiterPool, masterPool *types.MachinePool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if platform != nil && platform.BareMetal == nil { - allErrs = append(allErrs, field.NotSupported(fldPath.Child("platform"), platform.Name(), []string{baremetal.Name})) + if platform != nil && platform.BareMetal == nil && platform.None == nil { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("platform"), platform.Name(), []string{baremetal.Name, none.Name})) } if arbiterPool.Name != types.MachinePoolArbiterRoleName { allErrs = append(allErrs, field.NotSupported(fldPath.Child("name"), arbiterPool.Name, []string{types.MachinePoolArbiterRoleName})) diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index 8bcca22985..3cdba4b3d8 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -3198,6 +3198,105 @@ func TestValidateTNF(t *testing.T) { } } +func TestValidateArbiter(t *testing.T) { + cases := []struct { + name string + config *types.InstallConfig + expected string + }{ + { + config: installConfig(). + PlatformNone(). + MachinePoolArbiter( + machinePool(). + Name("arbiter"). + Hyperthreading(types.HyperthreadingEnabled). + Architecture(types.ArchitectureAMD64)). + MachinePoolCP(machinePool()). + ArbiterReplicas(1). + CpReplicas(2).build(), + name: "valid_platform_none", + expected: "", + }, + { + config: installConfig(). + PlatformBMWithHosts(). + MachinePoolArbiter( + machinePool(). + Name("arbiter"). + Hyperthreading(types.HyperthreadingEnabled). + Architecture(types.ArchitectureAMD64)). + MachinePoolCP(machinePool()). + ArbiterReplicas(1). + CpReplicas(2).build(), + name: "valid_platform_baremetal", + expected: "", + }, + { + config: installConfig(). + PlatformAWS(). + MachinePoolArbiter(machinePool(). + Name("arbiter"). + Hyperthreading(types.HyperthreadingEnabled). + Architecture(types.ArchitectureAMD64)). + MachinePoolCP(machinePool()). + ArbiterReplicas(1). + CpReplicas(2).build(), + name: "invalid_platform", + expected: `supported values: "baremetal", "none"`, + }, + { + config: installConfig(). + PlatformNone(). + MachinePoolArbiter(machinePool(). + Hyperthreading(types.HyperthreadingEnabled). + Architecture(types.ArchitectureAMD64)). + MachinePoolCP(machinePool()). + ArbiterReplicas(1). + CpReplicas(2).build(), + name: "invalid_arbiter_machine_pool_name", + expected: `arbiter.name: Unsupported value:`, + }, + { + config: installConfig(). + PlatformNone(). + MachinePoolArbiter(machinePool(). + Name("arbiter"). + Hyperthreading(types.HyperthreadingEnabled). + Architecture(types.ArchitectureAMD64)). + MachinePoolCP(machinePool()). + ArbiterReplicas(0). + CpReplicas(2).build(), + name: "invalid_arbiter_machine_pool_size", + expected: `arbiter.replicas: Invalid value:`, + }, + { + config: installConfig(). + PlatformNone(). + MachinePoolArbiter(machinePool(). + Name("arbiter"). + Hyperthreading(types.HyperthreadingEnabled). + Architecture(types.ArchitectureAMD64)). + MachinePoolCP(machinePool()). + ArbiterReplicas(1). + CpReplicas(1).build(), + name: "invalid_master_machine_pool_size", + expected: `number of controlPlane replicas must be at least 2`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateArbiter(&tc.config.Platform, tc.config.Arbiter, tc.config.ControlPlane, field.NewPath("arbiter")).ToAggregate() + + if tc.expected == "" { + assert.NoError(t, err) + } else { + assert.Regexp(t, tc.expected, err) + } + }) + } +} + type credentialBuilder struct { types.Credential }