Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,10 @@ spec:
description: Processors defines the processing units for
the instance.
x-kubernetes-int-or-string: true
smtLevel:
description: SMTLevel specifies the level of SMT to set
the control plane and worker nodes to.
type: string
sysType:
description: SysType defines the system type for instance.
type: string
Expand Down Expand Up @@ -1906,6 +1910,10 @@ spec:
description: Processors defines the processing units for the
instance.
x-kubernetes-int-or-string: true
smtLevel:
description: SMTLevel specifies the level of SMT to set the
control plane and worker nodes to.
type: string
sysType:
description: SysType defines the system type for instance.
type: string
Expand Down Expand Up @@ -4152,6 +4160,10 @@ spec:
description: Processors defines the processing units for the
instance.
x-kubernetes-int-or-string: true
smtLevel:
description: SMTLevel specifies the level of SMT to set the
control plane and worker nodes to.
type: string
sysType:
description: SysType defines the system type for instance.
type: string
Expand Down
70 changes: 70 additions & 0 deletions pkg/asset/machines/machineconfig/powersmt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package machineconfig

import (
"fmt"

ignutil "github.com/coreos/ignition/v2/config/util"
igntypes "github.com/coreos/ignition/v2/config/v3_2/types"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is 3.2 right here? I know the MCO just added support for 3.4 recently I believe.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently all other machineconfigs in the installer are still using 3.2 types. I will keep it here for now but thanks for the info!!

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/openshift/installer/pkg/asset/ignition"
mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
)

// ForPowerSMT sets the SMT level for Power machines.
func ForPowerSMT(role string, smtLevel string) (*mcfgv1.MachineConfig, error) {
powerSMTUnit, err := createPowerSMTUnit(smtLevel)
if err != nil {
return nil, err
}

ignConfig := igntypes.Config{
Ignition: igntypes.Ignition{
Version: igntypes.MaxVersion.String(),
},
Systemd: igntypes.Systemd{
Units: []igntypes.Unit{
{
Contents: &powerSMTUnit,
Name: fmt.Sprintf("99-%s-powersmt.service", role),
Enabled: ignutil.BoolToPtr(true),
},
},
},
}

rawExt, err := ignition.ConvertToRawExtension(ignConfig)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know in the MCO we also just json.Marshal ignition usually rather than ConvertToRawExtension might be worth writing some tests here to ensure this works?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other machineconfigs in the installer do this, but I could add test in a follow up if you'd like!

if err != nil {
return nil, err
}

return &mcfgv1.MachineConfig{
TypeMeta: metav1.TypeMeta{
APIVersion: "machineconfiguration.openshift.io/v1",
Kind: "MachineConfig",
},
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("99-%s-powersmt", role),
Labels: map[string]string{
"machineconfiguration.openshift.io/role": role,
},
},
Spec: mcfgv1.MachineConfigSpec{
Config: rawExt,
},
}, nil
}

func createPowerSMTUnit(smtLevel string) (string, error) {
unit := `[Unit]
Description=Set SMT
After=network-online.target
Before= crio.service
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/sbin/ppc64_cpu --smt=%s
[Install]
WantedBy=multi-user.target`
return fmt.Sprintf(unit, smtLevel), nil
}
9 changes: 9 additions & 0 deletions pkg/asset/machines/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,15 @@ func (m *Master) Generate(dependencies asset.Parents) error {
return errors.Wrap(err, "failed to create ignition for Multipath enabled for master machines")
}
machineConfigs = append(machineConfigs, ignMultipath)

// set SMT level if specified for powervs.
if pool.Platform.PowerVS.SMTLevel != "" {
ignPowerSMT, err := machineconfig.ForPowerSMT("master", pool.Platform.PowerVS.SMTLevel)
if err != nil {
return errors.Wrap(err, "failed to create ignition for Power SMT for master machines")
}
machineConfigs = append(machineConfigs, ignPowerSMT)
}
}
// The maximum number of networks supported on ServiceNetwork is two, one IPv4 and one IPv6 network.
// The cluster-network-operator handles the validation of this field.
Expand Down
9 changes: 9 additions & 0 deletions pkg/asset/machines/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,15 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
return errors.Wrap(err, "failed to create ignition for multipath enabled for worker machines")
}
machineConfigs = append(machineConfigs, ignMultipath)

// set SMT level if specified for powervs.
if pool.Platform.PowerVS != nil && pool.Platform.PowerVS.SMTLevel != "" {
ignPowerSMT, err := machineconfig.ForPowerSMT("worker", pool.Platform.PowerVS.SMTLevel)
if err != nil {
return errors.Wrap(err, "failed to create ignition for Power SMT for worker machines")
}
machineConfigs = append(machineConfigs, ignPowerSMT)
}
}
// The maximum number of networks supported on ServiceNetwork is two, one IPv4 and one IPv6 network.
// The cluster-network-operator handles the validation of this field.
Expand Down
8 changes: 8 additions & 0 deletions pkg/types/powervs/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ type MachinePool struct {
// +optional
ProcType machinev1.PowerVSProcessorType `json:"procType,omitempty"`

// SMTLevel specifies the level of SMT to set the control plane and worker nodes to.
//
// +optional
SMTLevel string `json:"smtLevel,omitempty"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Values in the machine pools would typically correspond to values set in the machinesets to be handleded by the machine api operator. (There is day 2 support for machine pools on hive in some platforms.)


// SysType defines the system type for instance.
//
// +optional
Expand All @@ -53,6 +58,9 @@ func (a *MachinePool) Set(required *MachinePool) {
if required.ProcType != "" {
a.ProcType = required.ProcType
}
if required.SMTLevel != "" {
a.SMTLevel = required.SMTLevel
}
if required.SysType != "" {
a.SysType = required.SysType
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/types/powervs/validation/machinepool.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package validation

import (
"fmt"
"regexp"
"strconv"
"strings"
Expand All @@ -13,6 +14,8 @@ import (
"github.com/openshift/installer/pkg/types/powervs"
)

var validSMTLevels = sets.New[string]("1", "2", "3", "4", "5", "6", "7", "8", "on", "off")

// ValidateMachinePool checks that the specified machine pool is valid.
func ValidateMachinePool(p *powervs.MachinePool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down Expand Up @@ -70,6 +73,11 @@ func ValidateMachinePool(p *powervs.MachinePool, fldPath *field.Path) field.Erro
}
}

// Validate SMTLevel
if p.SMTLevel != "" && !validSMTLevels.Has(p.SMTLevel) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("smtLevel"), p.SMTLevel, fmt.Sprintf("Valid SMT Levels are %s", sets.List(validSMTLevels))))
}

// Validate SysType
if p.SysType != "" {
const sysTypeRegex = `^(?:e980|s922(-.*|))$`
Expand Down