diff --git a/data/data/rhcos.json b/data/data/rhcos.json index 60878c249ca..8596410b9fa 100644 --- a/data/data/rhcos.json +++ b/data/data/rhcos.json @@ -1,94 +1,111 @@ { "amis": { "ap-northeast-1": { - "hvm": "ami-0fbc0018a8310e53e" + "hvm": "ami-09d3305f4d4a32aab" }, "ap-northeast-2": { - "hvm": "ami-0ba6edf20991dee91" + "hvm": "ami-05000ed15563794d4" }, "ap-south-1": { - "hvm": "ami-0acf1668760f8f8e9" + "hvm": "ami-085b9dd31ce1d0c47" }, "ap-southeast-1": { - "hvm": "ami-0bcecfdf9ff5fd5ca" + "hvm": "ami-0b32c19c0e043fa25" }, "ap-southeast-2": { - "hvm": "ami-0480abd0220d56ae2" + "hvm": "ami-0fa580c9d5f47044d" }, "ca-central-1": { - "hvm": "ami-0de4e822461cb671b" + "hvm": "ami-0e7d27d33a5f83576" }, "eu-central-1": { - "hvm": "ami-056c9291dce6d5023" + "hvm": "ami-0fb5771d5798d7999" }, "eu-west-1": { - "hvm": "ami-0f0159b00648b0bf4" + "hvm": "ami-046291643f08c3c4b" }, "eu-west-2": { - "hvm": "ami-044f299a3abcb4d96" + "hvm": "ami-05076ca846cebfed7" }, "eu-west-3": { - "hvm": "ami-095776c2e71c62b2f" + "hvm": "ami-037a1a3dbe2087b95" }, "sa-east-1": { - "hvm": "ami-00aefabf6d653ff5d" + "hvm": "ami-0e045e16599cf5732" }, "us-east-1": { - "hvm": "ami-07f604c5f18a1e7a9" + "hvm": "ami-035b0b988ae903258" }, "us-east-2": { - "hvm": "ami-0eef624367320ec26" + "hvm": "ami-0ccc42f7973195c2a" }, "us-west-1": { - "hvm": "ami-0f89dba68d747846b" + "hvm": "ami-09c22dc55423861b8" }, "us-west-2": { - "hvm": "ami-0eac581fbaa9fa9c6" + "hvm": "ami-0e4e79ed5d1ef2d16" } }, - "baseURI": "https://releases-rhcos.svc.ci.openshift.org/storage/releases/ootpa/410.8.20190325.0/", - "buildid": "410.8.20190325.0", + "baseURI": "https://releases-rhcos.svc.ci.openshift.org/storage/releases/ootpa/410.8.20190410.0/", + "buildid": "410.8.20190410.0", "images": { + "aws": { + "path": "rhcos-410.8.20190410.0-aws.vmdk", + "sha256": "a63f92fc88064005df2171c88053c6c339f9a326f91bb9aa6f333830a1568bb3", + "size": "754143810", + "uncompressed-sha256": "67b75cff7f650721227142a79465ab7d33c4df12d31343dd94368f864db69943", + "uncompressed-size": 788509696 + }, + "initramfs": { + "path": "rhcos-410.8.20190410.0-installer-initramfs.img", + "sha256": "7c8b27cdcf9ffc6e8bb1f058470b6d952be6fefe7e3d9c3ae8f8cef8c7465c21" + }, + "iso": { + "path": "rhcos-410.8.20190410.0-installer.iso", + "sha256": "f44282e438d38795b8ac35d76ccc9ceadafdc1edf735b2909c0e6b7e1dd1932e" + }, + "kernel": { + "path": "rhcos-410.8.20190410.0-installer-kernel", + "sha256": "370db9a3943d4f46dc079dbaeb7e0cc3910dca069f7eede66d3d7d0d5177f684" + }, "metal-bios": { - "path": "rhcos-410.8.20190325.0-metal-bios.raw", - "sha256": "3caaf8d714ac8bb820af6b31d595b8afae5ef951c68f8cee3701e236e9600f30", - "size": "735841103", - "uncompressed-sha256": "32d19b94cfb2aa45799caed2dda70f4e7ac1d0fca2b834a1cbd4459a6b0d056a", - "uncompressed-size": "17179869184" + "path": "rhcos-410.8.20190410.0-metal-bios.raw", + "sha256": "0ee2ee1535f34bc1c525025473276cc6783a54225241cd62304cc27bafecc716", + "size": "763528244", + "uncompressed-sha256": "1aedfa206accd485346fdcc536da0d7ce0f84b570c0724baebb3f467d20feca1", + "uncompressed-size": "3490709504" }, "metal-uefi": { - "path": "rhcos-410.8.20190325.0-metal-uefi.raw", - "sha256": "1686bb2e3b01873804325b50e286d858435afe1fbc3e88479d54b7a21e58d0f1", - "size": "732781221", - "uncompressed-sha256": "4c9a34f8b30ff7c5b107c87faac0f34a6b591d07a43ae729c45d98d15f3a8fa1", - "uncompressed-size": "17179869184" + "path": "rhcos-410.8.20190410.0-metal-uefi.raw", + "sha256": "37adc9d83eaf91332a91faed999a808028f6a4a3eb102aea13402d401a68ce59", + "size": "763025389", + "uncompressed-sha256": "e956e0da3c20b07acbf2c26433cff773b447c8fdf96922ae22551aa3d5a36a30", + "uncompressed-size": "3490709504" }, "openstack": { - "path": "rhcos-410.8.20190325.0-openstack.qcow2", - "sha256": "1b16214e59d38b5c1e6d291aea35f2539845232ca3300462104d022f9877fede", - "size": "721092654", - "uncompressed-sha256": "f2f1362c155d8d331387ce759a46b16c83c8a454f59d2ab5087d9781cdc29c97", - "uncompressed-size": "2014380032" + "path": "rhcos-410.8.20190410.0-openstack.qcow2", + "sha256": "707195b2dd279e10638f8bd6c46e4592f7c292d928197ce6ba9a876c8cdad1ca", + "size": "762738936", + "uncompressed-sha256": "176a6ab6206c5bbced4322cb6dc31b76b5a52842a3a39d4ef69a87198c2d3c2f", + "uncompressed-size": "2165440512" }, "qemu": { - "path": "rhcos-410.8.20190325.0-qemu.qcow2", - "sha256": "12b2db0cae8ea4019f24183dfc905609ab27e8a0fa01bf3631dc6ad7d2afe664", - "size": "721096885", - "uncompressed-sha256": "3b288cf2e02b63f8852c731836d5923a75230a836c63814a3988a96c26268257", - "uncompressed-size": "2014314496" + "path": "rhcos-410.8.20190410.0-qemu.qcow2", + "sha256": "f162c7caeb444146d28e3d0c9c8ffbc7083bb77170f3a824778b92e0c9259370", + "size": "762718951", + "uncompressed-sha256": "3a9956efb0a66906c2889ae57d2424b892314db09e157e10b4eb055af15c7089", + "uncompressed-size": "2165374976" }, "vmware": { - "path": "rhcos-410.8.20190325.0-vmware.ova", - "sha256": "d8c78e90f3a6fccd21c848a53b669c1c51080972377ad3f6bc81cdcc2cb48300", - "size": "710257080", - "uncompressed-sha256": "f73f5ef77095b0c79a648e2957a965f60544da08c78176028393948500da0dfc", - "uncompressed-size": "743802880" + "path": "rhcos-410.8.20190410.0-vmware.ova", + "sha256": "d1409d64665ce05318719f29e547e667f7183e7c50ed7a394dc462ed8e66d06b", + "size": "788520960" } }, "oscontainer": { - "digest": "sha256:007512169406dfa78f2eefdcd1553d94c399340bd1e871b4c884baf609a967ba", + "digest": "sha256:e1bd2d7b1819c84c0d7b56fa96ca40a35e92a728dce4c70a44e4c00bbea12aae", "image": "docker-registry-default.cloud.registry.upshift.redhat.com/redhat-coreos/ootpa" }, - "ostree-commit": "ed3e5f10b22db9db37e48b72e907ee60113ed259eeef5c019faa0109feeeccf8", - "ostree-version": "410.8.20190325.0" + "ostree-commit": "67a85a81e3dc38dbae236a623ddccac67a5f8bb4ddd541e50e704978d1e0cca4", + "ostree-version": "410.8.20190410.0" } \ No newline at end of file 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..60ed4a7a58d --- /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 disable 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-disable-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..cf3fc5c1440 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-disable-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-disable-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..b44d90129b3 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-disable-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-disable-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 9fd767e151e..eb131cdf1c3 100644 --- a/pkg/types/defaults/installconfig.go +++ b/pkg/types/defaults/installconfig.go @@ -44,28 +44,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 23696e00897..9f449a6a513 100644 --- a/pkg/types/defaults/installconfig_test.go +++ b/pkg/types/defaults/installconfig_test.go @@ -33,16 +33,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")}, } } @@ -187,6 +179,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{ @@ -194,12 +193,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 d3728351827..f352986faa3 100644 --- a/pkg/types/machinepools.go +++ b/pkg/types/machinepools.go @@ -8,6 +8,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. @@ -18,8 +28,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 2a3041a84cf..045801c989e 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(), }, @@ -322,14 +314,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 }(), @@ -340,10 +326,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 }(), @@ -353,13 +340,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 8d9eae4b2ed..52064f21956 100644 --- a/pkg/types/validation/machinepools.go +++ b/pkg/types/validation/machinepools.go @@ -16,6 +16,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{} @@ -26,6 +41,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 3ee1d319920..44517d78c01 100644 --- a/pkg/types/validation/machinepools_test.go +++ b/pkg/types/validation/machinepools_test.go @@ -14,10 +14,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, } } @@ -31,14 +32,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 }(), @@ -48,7 +49,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 }(), @@ -58,7 +59,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{}, } @@ -70,7 +71,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{ @@ -86,7 +87,7 @@ func TestValidateMachinePool(t *testing.T) { name: "valid azure", platform: &types.Platform{Azure: &azure.Platform{Region: "eastus"}}, pool: func() *types.MachinePool { - p := validMachinePool() + p := validMachinePool("test-name") p.Platform = types.MachinePoolPlatform{ Azure: &azure.MachinePool{}, } @@ -98,7 +99,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{}, } @@ -110,7 +111,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{}, } @@ -122,7 +123,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{}, } @@ -134,7 +135,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{},