New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS-27948: Tuned rendering and MCP detection improvements #928
OCPBUGS-27948: Tuned rendering and MCP detection improvements #928
Conversation
render-bootcmd-mc can only compute the kernel args for a MachineConfigPool that matches the node it is executed on, so we add the MachineConfigPool name as input parameter to filter the applicable PerformanceProfiles. Signed-off-by: Jose Luis Ojosnegros Manchón <jojosneg@redhat.com>
* feat: update render command to create default MCPs added default MCP creation to help correctly render out the resources for performance profile to use Signed-off-by: ehila <ehila@redhat.com> upkeep: fix spelling Signed-off-by: ehila <ehila@redhat.com> * feat: add default mcpools to tuned renderer Signed-off-by: ehila <ehila@redhat.com> --------- Signed-off-by: ehila <ehila@redhat.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MarSik The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@MarSik: This pull request references Jira Issue OCPBUGS-27948, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In 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 openshift-eng/jira-lifecycle-plugin repository. |
/jira refresh |
@MarSik: This pull request references Jira Issue OCPBUGS-27948, which is valid. The bug has been moved to the POST state. 6 validation(s) were run on this bug
Requesting review from QA contact: In 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 openshift-eng/jira-lifecycle-plugin repository. |
@jlojosnegros @vitus133 @eggfoobar Took a me a while to untangle the patch ordering, but this should be it. |
@MarSik: This pull request references Jira Issue OCPBUGS-27948, which is valid. 6 validation(s) were run on this bug
Requesting review from QA contact: In 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 openshift-eng/jira-lifecycle-plugin repository. |
/label backport-risk-assessed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
a bit too clever, but somehow self-testing. I guess there's no simple way to test before to commit.
done | ||
rm -r "${ARTIFACT_DIR}" | ||
|
||
function join_by { local IFS="$1"; shift; echo "$*"; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is clever! My gut feeling is this is a bit too clever, but I don't have strong arguments against.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not invent this one actually. But it uses bash features cleverly indeed.
Actually there is a way. I executed the code and it generated the exact same manifests we already had in the repo. git reported no changes. I also executed the e2e local tests that validate the rendering and all passed. That probably means the output is still valid and matches the rendered yamls. |
/retest-required |
pkg/tuned/cmd/render/render.go
Outdated
return fmt.Errorf("Unable to get PerformanceProfile to apply using MachineConfigPool %s. error : %w", mcpName, err) | ||
} | ||
|
||
if len(filteredPerformanceProfiles) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had just changed this behaviour in master with #924 so the absence of a performance profile is not considered an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added that patch here too! Thanks for noticing that.
This command should render a MC from the available sources. PerformanceProfile is a source, but it is not mandatory. So, as there are scenarios where this command should work with no additional PP let's the command proceed even if there is no PP Signed-off-by: Jose Luis Ojosnegros Manchón <jojosneg@redhat.com>
@MarSik: This pull request references Jira Issue OCPBUGS-27948, which is valid. 6 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In 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 openshift-eng/jira-lifecycle-plugin repository. |
/retest |
1 similar comment
/retest |
/label cherry-pick-approved |
/retest-required |
/lgtm |
@MarSik: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
a8d42a7
into
openshift:release-4.15
@MarSik: Jira Issue OCPBUGS-27948: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-27948 has been moved to the MODIFIED state. In 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 openshift-eng/jira-lifecycle-plugin repository. |
[ART PR BUILD NOTIFIER] This PR has been included in build cluster-node-tuning-operator-container-v4.15.0-202402051538.p0.ga8d42a7.assembly.stream for distgit cluster-node-tuning-operator. |
Fix included in accepted release 4.15.0-0.nightly-2024-02-05-163411 |
/cherry-pick release-4.14 |
@yanirq: #928 failed to apply on top of branch "release-4.14":
In 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. |
Fix included in accepted release 4.15.0-0.nightly-2024-02-12-213938 |
…ift#928) * NTO: Add MCP name to filter PP (openshift#878) render-bootcmd-mc can only compute the kernel args for a MachineConfigPool that matches the node it is executed on, so we add the MachineConfigPool name as input parameter to filter the applicable PerformanceProfiles. Signed-off-by: Jose Luis Ojosnegros Manchón <jojosneg@redhat.com> * OCPBUGS-22095: Add default MCP objects for rendering (openshift#833) * feat: update render command to create default MCPs added default MCP creation to help correctly render out the resources for performance profile to use Signed-off-by: ehila <ehila@redhat.com> upkeep: fix spelling Signed-off-by: ehila <ehila@redhat.com> * feat: add default mcpools to tuned renderer Signed-off-by: ehila <ehila@redhat.com> --------- Signed-off-by: ehila <ehila@redhat.com> * Enhance render sync to include bootstrap rendering tests * Render MC without additional PP This command should render a MC from the available sources. PerformanceProfile is a source, but it is not mandatory. So, as there are scenarios where this command should work with no additional PP let's the command proceed even if there is no PP Signed-off-by: Jose Luis Ojosnegros Manchón <jojosneg@redhat.com> * Render sync --------- Signed-off-by: Jose Luis Ojosnegros Manchón <jojosneg@redhat.com> Signed-off-by: ehila <ehila@redhat.com> Co-authored-by: Jose Luis Ojosnegros <jojosneg@redhat.com> Co-authored-by: E Hila <ehila@redhat.com>
This is a backport of three related PRs: #878, #833 and #924 that improve the rendering mode during the installation bootstrap phase.
The most notable change is the added assumption that basic master and worker MachineConfigPools with default labels will always exist even though they are not yet available in the input manifest folder.