diff --git a/pkg/asset/installconfig/installconfig_test.go b/pkg/asset/installconfig/installconfig_test.go index 5c02065c5bd..d58728e23ad 100644 --- a/pkg/asset/installconfig/installconfig_test.go +++ b/pkg/asset/installconfig/installconfig_test.go @@ -76,13 +76,15 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) { }, }, ControlPlane: &types.MachinePool{ - Name: "master", - Replicas: pointer.Int64Ptr(3), + Name: "master", + Replicas: pointer.Int64Ptr(3), + Hyperthreading: types.HyperthreadingEnabled, }, Compute: []types.MachinePool{ { - Name: "worker", - Replicas: pointer.Int64Ptr(3), + Name: "worker", + Replicas: pointer.Int64Ptr(3), + Hyperthreading: types.HyperthreadingEnabled, }, }, Platform: types.Platform{ @@ -135,13 +137,15 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}" }, }, ControlPlane: &types.MachinePool{ - Name: "master", - Replicas: pointer.Int64Ptr(3), + Name: "master", + Replicas: pointer.Int64Ptr(3), + Hyperthreading: types.HyperthreadingEnabled, }, Compute: []types.MachinePool{ { - Name: "worker", - Replicas: pointer.Int64Ptr(3), + Name: "worker", + Replicas: pointer.Int64Ptr(3), + Hyperthreading: types.HyperthreadingEnabled, }, }, Platform: types.Platform{ @@ -214,13 +218,15 @@ network: }, }, ControlPlane: &types.MachinePool{ - Name: "master", - Replicas: pointer.Int64Ptr(3), + Name: "master", + Replicas: pointer.Int64Ptr(3), + Hyperthreading: types.HyperthreadingEnabled, }, Compute: []types.MachinePool{ { - Name: "worker", - Replicas: pointer.Int64Ptr(3), + Name: "worker", + Replicas: pointer.Int64Ptr(3), + Hyperthreading: types.HyperthreadingEnabled, }, }, Platform: types.Platform{ diff --git a/pkg/asset/machines/machineconfig/hyperthreading.go b/pkg/asset/machines/machineconfig/hyperthreading.go new file mode 100644 index 00000000000..ef3662e88ef --- /dev/null +++ b/pkg/asset/machines/machineconfig/hyperthreading.go @@ -0,0 +1,40 @@ +package machineconfig + +import ( + "fmt" + + igntypes "github.com/coreos/ignition/config/v2_2/types" + mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openshift/installer/pkg/asset/ignition" +) + +// ForHyperthreadingDisabled creates the MachineConfig to enable hyperthreading. +// RHCOS ships with pivot.service that uses the `/etc/pivot/kernel-args` to override the kernel arguments for hosts. +func ForHyperthreadingDisabled(role string) *mcfgv1.MachineConfig { + return &mcfgv1.MachineConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "machineconfiguration.openshift.io/v1", + Kind: "MachineConfig", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("99-%s-hyperthreading", role), + Labels: map[string]string{ + "machineconfiguration.openshift.io/role": role, + }, + }, + Spec: mcfgv1.MachineConfigSpec{ + Config: igntypes.Config{ + Ignition: igntypes.Ignition{ + Version: igntypes.MaxVersion.String(), + }, + Storage: igntypes.Storage{ + Files: []igntypes.File{ + ignition.FileFromString("/etc/pivot/kernel-args", "root", 0600, "ADD nosmt"), + }, + }, + }, + }, + } +} diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 372dbe46e9f..c3f5ad5f5f2 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -9,6 +9,15 @@ import ( libvirtapi "github.com/openshift/cluster-api-provider-libvirt/pkg/apis" libvirtprovider "github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1alpha1" machineapi "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1" + mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + awsapi "sigs.k8s.io/cluster-api-provider-aws/pkg/apis" + awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1beta1" + openstackapi "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis" + openstackprovider "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1" + "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/ignition/machine" "github.com/openshift/installer/pkg/asset/installconfig" @@ -17,20 +26,13 @@ import ( "github.com/openshift/installer/pkg/asset/machines/machineconfig" "github.com/openshift/installer/pkg/asset/machines/openstack" "github.com/openshift/installer/pkg/asset/rhcos" + "github.com/openshift/installer/pkg/types" awstypes "github.com/openshift/installer/pkg/types/aws" awsdefaults "github.com/openshift/installer/pkg/types/aws/defaults" libvirttypes "github.com/openshift/installer/pkg/types/libvirt" nonetypes "github.com/openshift/installer/pkg/types/none" openstacktypes "github.com/openshift/installer/pkg/types/openstack" vspheretypes "github.com/openshift/installer/pkg/types/vsphere" - mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" - awsapi "sigs.k8s.io/cluster-api-provider-aws/pkg/apis" - awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1beta1" - openstackapi "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis" - openstackprovider "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1" ) // Master generates the machines for the `master` machine pool. @@ -91,6 +93,7 @@ func (m *Master) Generate(dependencies asset.Parents) error { dependencies.Get(clusterID, installconfig, rhcosImage, mign) ic := installconfig.Config + pool := ic.ControlPlane var err error machines := []machineapi.Machine{} @@ -150,6 +153,9 @@ func (m *Master) Generate(dependencies asset.Parents) error { } machineConfigs := []*mcfgv1.MachineConfig{} + if pool.Hyperthreading == types.HyperthreadingDisabled { + machineConfigs = append(machineConfigs, machineconfig.ForHyperthreadingDisabled("master")) + } if ic.SSHKey != "" { machineConfigs = append(machineConfigs, machineconfig.ForAuthorizedKeys(ic.SSHKey, "master")) } diff --git a/pkg/asset/machines/master_test.go b/pkg/asset/machines/master_test.go index 99232f98fb2..19c02e52efa 100644 --- a/pkg/asset/machines/master_test.go +++ b/pkg/asset/machines/master_test.go @@ -19,17 +19,116 @@ func TestMasterGenerateMachineConfigs(t *testing.T) { cases := []struct { name string key string + hyperthreading types.HyperthreadingMode expectedMachineConfig string }{ { - name: "no key", + name: "no key hyperthreading enabled", + hyperthreading: types.HyperthreadingEnabled, }, { - name: "key present", - key: "ssh-rsa: dummy-key", + name: "key present hyperthreading enabled", + key: "ssh-rsa: dummy-key", + hyperthreading: types.HyperthreadingEnabled, expectedMachineConfig: `--- apiVersion: machineconfiguration.openshift.io/v1 kind: MachineConfig +metadata: + creationTimestamp: null + labels: + machineconfiguration.openshift.io/role: master + name: 99-master-ssh +spec: + config: + ignition: + config: {} + security: + tls: {} + timeouts: {} + version: 2.2.0 + networkd: {} + passwd: + users: + - name: core + sshAuthorizedKeys: + - 'ssh-rsa: dummy-key' + storage: {} + systemd: {} + osImageURL: "" +`, + }, + { + name: "no key hyperthreading disabled", + hyperthreading: types.HyperthreadingDisabled, + expectedMachineConfig: `--- +apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig +metadata: + creationTimestamp: null + labels: + machineconfiguration.openshift.io/role: master + name: 99-master-hyperthreading +spec: + config: + ignition: + config: {} + security: + tls: {} + timeouts: {} + version: 2.2.0 + networkd: {} + passwd: {} + storage: + files: + - contents: + source: data:text/plain;charset=utf-8;base64,QUREIG5vc210 + verification: {} + filesystem: root + mode: 384 + path: /etc/pivot/kernel-args + user: + name: root + systemd: {} + osImageURL: "" +`, + }, + { + name: "key present hyperthreading disabled", + key: "ssh-rsa: dummy-key", + hyperthreading: types.HyperthreadingDisabled, + expectedMachineConfig: `--- +apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig +metadata: + creationTimestamp: null + labels: + machineconfiguration.openshift.io/role: master + name: 99-master-hyperthreading +spec: + config: + ignition: + config: {} + security: + tls: {} + timeouts: {} + version: 2.2.0 + networkd: {} + passwd: {} + storage: + files: + - contents: + source: data:text/plain;charset=utf-8;base64,QUREIG5vc210 + verification: {} + filesystem: root + mode: 384 + path: /etc/pivot/kernel-args + user: + name: root + systemd: {} + osImageURL: "" +--- +apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig metadata: creationTimestamp: null labels: @@ -76,7 +175,8 @@ spec: }, }, ControlPlane: &types.MachinePool{ - Replicas: pointer.Int64Ptr(1), + Hyperthreading: tc.hyperthreading, + Replicas: pointer.Int64Ptr(1), Platform: types.MachinePoolPlatform{ AWS: &awstypes.MachinePool{ Zones: []string{"us-east-1a"}, diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index 968174a5cfd..7384da49605 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -26,6 +26,7 @@ import ( "github.com/openshift/installer/pkg/asset/machines/machineconfig" "github.com/openshift/installer/pkg/asset/machines/openstack" "github.com/openshift/installer/pkg/asset/rhcos" + "github.com/openshift/installer/pkg/types" awstypes "github.com/openshift/installer/pkg/types/aws" awsdefaults "github.com/openshift/installer/pkg/types/aws/defaults" libvirttypes "github.com/openshift/installer/pkg/types/libvirt" @@ -110,11 +111,16 @@ func (w *Worker) Generate(dependencies asset.Parents) error { machineConfigs := []*mcfgv1.MachineConfig{} machineSets := []runtime.Object{} + var err error ic := installconfig.Config for _, pool := range ic.Compute { + if pool.Hyperthreading == types.HyperthreadingDisabled { + machineConfigs = append(machineConfigs, machineconfig.ForHyperthreadingDisabled("worker")) + } if ic.SSHKey != "" { machineConfigs = append(machineConfigs, machineconfig.ForAuthorizedKeys(ic.SSHKey, "worker")) } + switch ic.Platform.Name() { case awstypes.Name: mpool := defaultAWSMachinePoolPlatform() @@ -196,7 +202,6 @@ func (w *Worker) Generate(dependencies asset.Parents) error { Data: data, } } - return nil } diff --git a/pkg/asset/machines/worker_test.go b/pkg/asset/machines/worker_test.go index 17e03f30e09..6eee5bde722 100644 --- a/pkg/asset/machines/worker_test.go +++ b/pkg/asset/machines/worker_test.go @@ -19,17 +19,116 @@ func TestWorkerGenerate(t *testing.T) { cases := []struct { name string key string + hyperthreading types.HyperthreadingMode expectedMachineConfig string }{ { - name: "no key", + name: "no key hyperthreading enabled", + hyperthreading: types.HyperthreadingEnabled, }, { - name: "key present", - key: "ssh-rsa: dummy-key", + name: "key present hyperthreading enabled", + key: "ssh-rsa: dummy-key", + hyperthreading: types.HyperthreadingEnabled, expectedMachineConfig: `--- apiVersion: machineconfiguration.openshift.io/v1 kind: MachineConfig +metadata: + creationTimestamp: null + labels: + machineconfiguration.openshift.io/role: worker + name: 99-worker-ssh +spec: + config: + ignition: + config: {} + security: + tls: {} + timeouts: {} + version: 2.2.0 + networkd: {} + passwd: + users: + - name: core + sshAuthorizedKeys: + - 'ssh-rsa: dummy-key' + storage: {} + systemd: {} + osImageURL: "" +`, + }, + { + name: "no key hyperthreading disabled", + hyperthreading: types.HyperthreadingDisabled, + expectedMachineConfig: `--- +apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig +metadata: + creationTimestamp: null + labels: + machineconfiguration.openshift.io/role: worker + name: 99-worker-hyperthreading +spec: + config: + ignition: + config: {} + security: + tls: {} + timeouts: {} + version: 2.2.0 + networkd: {} + passwd: {} + storage: + files: + - contents: + source: data:text/plain;charset=utf-8;base64,QUREIG5vc210 + verification: {} + filesystem: root + mode: 384 + path: /etc/pivot/kernel-args + user: + name: root + systemd: {} + osImageURL: "" +`, + }, + { + name: "key present hyperthreading disabled", + key: "ssh-rsa: dummy-key", + hyperthreading: types.HyperthreadingDisabled, + expectedMachineConfig: `--- +apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig +metadata: + creationTimestamp: null + labels: + machineconfiguration.openshift.io/role: worker + name: 99-worker-hyperthreading +spec: + config: + ignition: + config: {} + security: + tls: {} + timeouts: {} + version: 2.2.0 + networkd: {} + passwd: {} + storage: + files: + - contents: + source: data:text/plain;charset=utf-8;base64,QUREIG5vc210 + verification: {} + filesystem: root + mode: 384 + path: /etc/pivot/kernel-args + user: + name: root + systemd: {} + osImageURL: "" +--- +apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig metadata: creationTimestamp: null labels: @@ -77,7 +176,8 @@ spec: }, Compute: []types.MachinePool{ { - Replicas: pointer.Int64Ptr(1), + Replicas: pointer.Int64Ptr(1), + Hyperthreading: tc.hyperthreading, Platform: types.MachinePoolPlatform{ AWS: &awstypes.MachinePool{ Zones: []string{"us-east-1a"}, diff --git a/pkg/types/defaults/installconfig.go b/pkg/types/defaults/installconfig.go index fca1eefbdda..097c1a8eb7a 100644 --- a/pkg/types/defaults/installconfig.go +++ b/pkg/types/defaults/installconfig.go @@ -42,28 +42,16 @@ func SetInstallConfigDefaults(c *types.InstallConfig) { }, } } - defaultReplicaCount := int64(3) - if c.Platform.Libvirt != nil { - defaultReplicaCount = 1 - } if c.ControlPlane == nil { - c.ControlPlane = &types.MachinePool{ - Replicas: &defaultReplicaCount, - } + c.ControlPlane = &types.MachinePool{} } c.ControlPlane.Name = "master" + SetMachinePoolDefaults(c.ControlPlane, c.Platform.Name()) if len(c.Compute) == 0 { - c.Compute = []types.MachinePool{ - { - Name: "worker", - Replicas: &defaultReplicaCount, - }, - } + c.Compute = []types.MachinePool{{Name: "worker"}} } - for i, p := range c.Compute { - if p.Replicas == nil { - c.Compute[i].Replicas = &defaultReplicaCount - } + for i := range c.Compute { + SetMachinePoolDefaults(&c.Compute[i], c.Platform.Name()) } switch { case c.Platform.AWS != nil: diff --git a/pkg/types/defaults/installconfig_test.go b/pkg/types/defaults/installconfig_test.go index ac6587f6460..2bbff685559 100644 --- a/pkg/types/defaults/installconfig_test.go +++ b/pkg/types/defaults/installconfig_test.go @@ -31,16 +31,8 @@ func defaultInstallConfig() *types.InstallConfig { }, }, }, - ControlPlane: &types.MachinePool{ - Name: "master", - Replicas: pointer.Int64Ptr(3), - }, - Compute: []types.MachinePool{ - { - Name: "worker", - Replicas: pointer.Int64Ptr(3), - }, - }, + ControlPlane: defaultMachinePool("master"), + Compute: []types.MachinePool{*defaultMachinePool("worker")}, } } @@ -169,6 +161,13 @@ func TestSetInstallConfigDefaults(t *testing.T) { return c }(), }, + { + name: "control plane present", + config: &types.InstallConfig{ + ControlPlane: &types.MachinePool{}, + }, + expected: defaultInstallConfig(), + }, { name: "Compute present", config: &types.InstallConfig{ @@ -176,12 +175,7 @@ func TestSetInstallConfigDefaults(t *testing.T) { }, expected: func() *types.InstallConfig { c := defaultInstallConfig() - c.Compute = []types.MachinePool{ - { - Name: "test-compute", - Replicas: pointer.Int64Ptr(3), - }, - } + c.Compute = []types.MachinePool{*defaultMachinePool("test-compute")} return c }(), }, diff --git a/pkg/types/defaults/machinepools.go b/pkg/types/defaults/machinepools.go new file mode 100644 index 00000000000..e9810c2e9c5 --- /dev/null +++ b/pkg/types/defaults/machinepools.go @@ -0,0 +1,20 @@ +package defaults + +import ( + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/libvirt" +) + +// SetMachinePoolDefaults sets the defaults for the machine pool. +func SetMachinePoolDefaults(p *types.MachinePool, platform string) { + defaultReplicaCount := int64(3) + if platform == libvirt.Name { + defaultReplicaCount = 1 + } + if p.Replicas == nil { + p.Replicas = &defaultReplicaCount + } + if p.Hyperthreading == "" { + p.Hyperthreading = types.HyperthreadingEnabled + } +} diff --git a/pkg/types/defaults/machinepools_test.go b/pkg/types/defaults/machinepools_test.go new file mode 100644 index 00000000000..4746ca72df5 --- /dev/null +++ b/pkg/types/defaults/machinepools_test.go @@ -0,0 +1,80 @@ +package defaults + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/utils/pointer" + + "github.com/openshift/installer/pkg/types" +) + +func defaultMachinePool(name string) *types.MachinePool { + return &types.MachinePool{ + Name: name, + Replicas: pointer.Int64Ptr(3), + Hyperthreading: types.HyperthreadingEnabled, + } +} + +func TestSetMahcinePoolDefaults(t *testing.T) { + cases := []struct { + name string + pool *types.MachinePool + platform string + expected *types.MachinePool + }{ + { + name: "empty", + pool: &types.MachinePool{}, + expected: defaultMachinePool(""), + }, + { + name: "default", + pool: defaultMachinePool("test-name"), + expected: defaultMachinePool("test-name"), + }, + { + name: "non-default replicas", + pool: func() *types.MachinePool { + p := defaultMachinePool("test-name") + p.Replicas = pointer.Int64Ptr(5) + return p + }(), + expected: func() *types.MachinePool { + p := defaultMachinePool("test-name") + p.Replicas = pointer.Int64Ptr(5) + return p + }(), + }, + { + name: "libvirt replicas", + pool: &types.MachinePool{}, + platform: "libvirt", + expected: func() *types.MachinePool { + p := defaultMachinePool("") + p.Replicas = pointer.Int64Ptr(1) + return p + }(), + }, + { + name: "non-default hyperthreading", + pool: func() *types.MachinePool { + p := defaultMachinePool("test-name") + p.Hyperthreading = types.HyperthreadingMode("test-hyperthreading") + return p + }(), + expected: func() *types.MachinePool { + p := defaultMachinePool("test-name") + p.Hyperthreading = types.HyperthreadingMode("test-hyperthreading") + return p + }(), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + SetMachinePoolDefaults(tc.pool, tc.platform) + assert.Equal(t, tc.expected, tc.pool, "unexpected machine pool") + }) + } +} diff --git a/pkg/types/machinepools.go b/pkg/types/machinepools.go index eb61485f430..b6844d4aee3 100644 --- a/pkg/types/machinepools.go +++ b/pkg/types/machinepools.go @@ -7,6 +7,16 @@ import ( "github.com/openshift/installer/pkg/types/vsphere" ) +// HyperthreadingMode is the mode of hyperthreading for a machine. +type HyperthreadingMode string + +const ( + // HyperthreadingEnabled indicates that hyperthreading is enabled. + HyperthreadingEnabled HyperthreadingMode = "Enabled" + // HyperthreadingDisabled indicates that hyperthreading is disabled. + HyperthreadingDisabled HyperthreadingMode = "Disabled" +) + // MachinePool is a pool of machines to be installed. type MachinePool struct { // Name is the name of the machine pool. @@ -17,8 +27,14 @@ type MachinePool struct { // Replicas is the count of machines for this machine pool. Replicas *int64 `json:"replicas,omitempty"` - // Platform is configuration for machine pool specific to the platfrom. + // Platform is configuration for machine pool specific to the platform. Platform MachinePoolPlatform `json:"platform"` + + // Hyperthreading determines the mode of hyperthreading that machines in this + // pool will utilize. + // +optional + // Default is for hyperthreading to be enabled. + Hyperthreading HyperthreadingMode `json:"hyperthreading,omitempty"` } // MachinePoolPlatform is the platform-specific configuration for a machine diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index b8241881daf..af5a8d5918f 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -39,16 +39,8 @@ func validInstallConfig() *types.InstallConfig { }, }, }, - ControlPlane: &types.MachinePool{ - Name: "master", - Replicas: pointer.Int64Ptr(3), - }, - Compute: []types.MachinePool{ - { - Name: "worker", - Replicas: pointer.Int64Ptr(3), - }, - }, + ControlPlane: validMachinePool("master"), + Compute: []types.MachinePool{*validMachinePool("worker")}, Platform: types.Platform{ AWS: validAWSPlatform(), }, @@ -302,14 +294,8 @@ func TestValidateInstallConfig(t *testing.T) { installConfig: func() *types.InstallConfig { c := validInstallConfig() c.Compute = []types.MachinePool{ - { - Name: "worker", - Replicas: pointer.Int64Ptr(1), - }, - { - Name: "worker", - Replicas: pointer.Int64Ptr(2), - }, + *validMachinePool("worker"), + *validMachinePool("worker"), } return c }(), @@ -320,10 +306,11 @@ func TestValidateInstallConfig(t *testing.T) { installConfig: func() *types.InstallConfig { c := validInstallConfig() c.Compute = []types.MachinePool{ - { - Name: "worker", - Replicas: pointer.Int64Ptr(0), - }, + func() types.MachinePool { + p := *validMachinePool("worker") + p.Replicas = pointer.Int64Ptr(0) + return p + }(), } return c }(), @@ -333,13 +320,13 @@ func TestValidateInstallConfig(t *testing.T) { installConfig: func() *types.InstallConfig { c := validInstallConfig() c.Compute = []types.MachinePool{ - { - Name: "worker", - Replicas: pointer.Int64Ptr(3), - Platform: types.MachinePoolPlatform{ + func() types.MachinePool { + p := *validMachinePool("worker") + p.Platform = types.MachinePoolPlatform{ OpenStack: &openstack.MachinePool{}, - }, - }, + } + return p + }(), } return c }(), diff --git a/pkg/types/validation/machinepools.go b/pkg/types/validation/machinepools.go index aad285fca28..2196e666100 100644 --- a/pkg/types/validation/machinepools.go +++ b/pkg/types/validation/machinepools.go @@ -14,6 +14,21 @@ import ( openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation" ) +var ( + validHyperthreadingModes = map[types.HyperthreadingMode]bool{ + types.HyperthreadingDisabled: true, + types.HyperthreadingEnabled: true, + } + + validHyperthreadingModeValues = func() []string { + v := make([]string, 0, len(validHyperthreadingModes)) + for m := range validHyperthreadingModes { + v = append(v, string(m)) + } + return v + }() +) + // ValidateMachinePool checks that the specified machine pool is valid. func ValidateMachinePool(platform *types.Platform, p *types.MachinePool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -24,6 +39,9 @@ func ValidateMachinePool(platform *types.Platform, p *types.MachinePool, fldPath } else { allErrs = append(allErrs, field.Required(fldPath.Child("replicas"), "replicas is required")) } + if !validHyperthreadingModes[p.Hyperthreading] { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("hyperthreading"), p.Hyperthreading, validHyperthreadingModeValues)) + } allErrs = append(allErrs, validateMachinePoolPlatform(platform, &p.Platform, fldPath.Child("platform"))...) return allErrs } diff --git a/pkg/types/validation/machinepools_test.go b/pkg/types/validation/machinepools_test.go index 572c4de3522..6c0e3b55808 100644 --- a/pkg/types/validation/machinepools_test.go +++ b/pkg/types/validation/machinepools_test.go @@ -13,10 +13,11 @@ import ( "github.com/openshift/installer/pkg/types/openstack" ) -func validMachinePool() *types.MachinePool { +func validMachinePool(name string) *types.MachinePool { return &types.MachinePool{ - Name: "test-pool", - Replicas: pointer.Int64Ptr(1), + Name: name, + Replicas: pointer.Int64Ptr(1), + Hyperthreading: types.HyperthreadingDisabled, } } @@ -30,14 +31,14 @@ func TestValidateMachinePool(t *testing.T) { { name: "minimal", platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, - pool: validMachinePool(), + pool: validMachinePool("test-name"), valid: true, }, { name: "missing replicas", platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Replicas = nil return p }(), @@ -47,7 +48,7 @@ func TestValidateMachinePool(t *testing.T) { name: "invalid replicas", platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Replicas = pointer.Int64Ptr(-1) return p }(), @@ -57,7 +58,7 @@ func TestValidateMachinePool(t *testing.T) { name: "valid aws", platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Platform = types.MachinePoolPlatform{ AWS: &aws.MachinePool{}, } @@ -69,7 +70,7 @@ func TestValidateMachinePool(t *testing.T) { name: "invalid aws", platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Platform = types.MachinePoolPlatform{ AWS: &aws.MachinePool{ EC2RootVolume: aws.EC2RootVolume{ @@ -85,7 +86,7 @@ func TestValidateMachinePool(t *testing.T) { name: "valid libvirt", platform: &types.Platform{Libvirt: &libvirt.Platform{}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Platform = types.MachinePoolPlatform{ Libvirt: &libvirt.MachinePool{}, } @@ -97,7 +98,7 @@ func TestValidateMachinePool(t *testing.T) { name: "valid openstack", platform: &types.Platform{OpenStack: &openstack.Platform{}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Platform = types.MachinePoolPlatform{ OpenStack: &openstack.MachinePool{}, } @@ -109,7 +110,7 @@ func TestValidateMachinePool(t *testing.T) { name: "mis-matched platform", platform: &types.Platform{Libvirt: &libvirt.Platform{}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Platform = types.MachinePoolPlatform{ AWS: &aws.MachinePool{}, } @@ -121,7 +122,7 @@ func TestValidateMachinePool(t *testing.T) { name: "multiple platforms", platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Platform = types.MachinePoolPlatform{ AWS: &aws.MachinePool{}, Libvirt: &libvirt.MachinePool{},