MULTIARCH-3964: Power VS: Control SMT level with machineconfig#7704
MULTIARCH-3964: Power VS: Control SMT level with machineconfig#7704openshift-merge-bot[bot] merged 1 commit intoopenshift:masterfrom
Conversation
|
/retest-required |
mjturek
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@mjturek: This pull request references MULTIARCH-3419 which is a valid jira issue. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
mjturek
changed the title
|
@mjturek: This pull request references MULTIARCH-3964 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
mjturek
force-pushed
the
smt
branch
2 times, most recently
from
754ebe9 to
fc14290
Compare
|
/test okd-e2e-aws-ovn-upgrade |
|
/lgtm |
|
/test e2e-aws-ovn |
|
/assign @r4f4 |
mjturek
force-pushed
the
smt
branch
2 times, most recently
from
a16638b to
a2fc9b0
Compare
Co-authored-by: Rafael F. <r4f4rfs@gmail.com>
|
/jira refresh |
|
@mjturek: This pull request references MULTIARCH-3964 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
patrickdillon
left a comment
patrickdillon
left a comment
There was a problem hiding this comment.
Normally machine configuration like this would be laid down by the machine config operator—not the installer.
if there are install-time user-supplied values we could put them in the api or elsewhere in the cluster for the mco to reference.
One thing that’s not clear to me with this API is whether all compute nodes should have the same SMT level or whether the level could be set at different levels for different sets of compute nodes. If it’s the former, that would work well with the MCO.
| // SMTLevel specifies the level of SMT to set the control plane and worker nodes to. | ||
| // | ||
| // +optional | ||
| SMTLevel string `json:"smtLevel,omitempty"` |
There was a problem hiding this comment.
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.)
|
@patrickdillon would you be okay with instead a |
|
@mjturek: This pull request references MULTIARCH-3964 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@mjturek: This pull request references MULTIARCH-3964 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
after a slack discussion with Patrick, I've added more details as to why we're pursing this method and would like some input from someone on the MCO team. |
It is fine with me to have these machine configs live in the installer while waiting for full support |
| "fmt" | ||
|
|
||
| ignutil "github.com/coreos/ignition/v2/config/util" | ||
| igntypes "github.com/coreos/ignition/v2/config/v3_2/types" |
There was a problem hiding this comment.
is 3.2 right here? I know the MCO just added support for 3.4 recently I believe.
There was a problem hiding this comment.
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!!
| }, | ||
| } | ||
|
|
||
| rawExt, err := ignition.ConvertToRawExtension(ignConfig) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
All other machineconfigs in the installer do this, but I could add test in a follow up if you'd like!
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
Just a note, this will work logically, but much like the sshkey machineconfig, this will be interpreted as a user-provided config, making it harder to remove at a later time. Just curious, how early do you need this karg to take effect? If you provide it via the MCO during install time, it is true that the ignition kargs won't take effect, but the MCD-firstboot service will handle this before updating the OS image and rebooting, so essentially the first time the node joins the cluster it will already have the karg. Does this have to be present for the node to boot the first time? |
|
@mjturek: This pull request references MULTIARCH-3964 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/lgtm |
|
/label acknowledge-critical-fixes-only |
openshift-ci
Bot
added
the
acknowledge-critical-fixes-only
|
@mjturek: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
[ART PR BUILD NOTIFIER] This PR has been included in build ose-installer-altinfra-container-v4.15.0-202311231531.p0.ge2d6013.assembly.stream for distgit ose-installer-altinfra. |
8 participants
Through the installer, you can currently disable SMT. Power hardware allows levels of control that cannot currently be accessed through the installer (1-8). Until we can control SMT via kargs [0], and until igniton kargs support [1] is available in the MCO, we believe this is the best option for SMT control at install time.
[0] https://lore.kernel.org/all/20230705145143.40545-8-ldufour@linux.ibm.com/
[1] #5439