-
Notifications
You must be signed in to change notification settings - Fork 108
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-44786: add support for the LLC alignment cpumanager policy option #2136
OCPBUGS-44786: add support for the LLC alignment cpumanager policy option #2136
Conversation
@ffromani: This pull request references Jira Issue OCPBUGS-44786, 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. |
@ffromani: the contents of this pull request could not be automatically validated. The following commits are valid:
The following commits could not be validated and must be approved by a top-level approver:
Comment |
/retest-required |
From what I remember, we don't really have a lot of e2e tests on this feature in upstream. How are we going to verify that this works on previous releases? |
we will have e2e tests in the telco testsuites; and the telco testsuites will run on split-LLC hardware (high end AMD CPUs). |
cfa2d28
to
054924a
Compare
@ffromani: the contents of this pull request could not be automatically validated. The following commits are valid:
The following commits could not be validated and must be approved by a top-level approver:
Comment |
054924a
to
4e9cffd
Compare
@ffromani: the contents of this pull request could not be automatically validated. The following commits are valid:
The following commits could not be validated and must be approved by a top-level approver:
Comment |
/retest-required |
/jira refresh |
@ffromani: This pull request references Jira Issue OCPBUGS-44786, which is invalid:
Comment 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 |
@ffromani: This pull request references Jira Issue OCPBUGS-44786, which is invalid:
Comment 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 |
@ffromani: This pull request references Jira Issue OCPBUGS-44786, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request. 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. |
pkg/kubelet/llcalign/llcalign.go
Outdated
// TestOnlySetEnabled allows changing the state of management partition enablement | ||
// This method MUST NOT be used outside of test code | ||
func TestOnlySetEnabled(enabled bool) { | ||
llcAlignmentEnabled = enabled | ||
} |
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.
what're the plans for using this?
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.
no real plans yet - too eager copy-paste from managed
code. I will remove and re-add later if needed.
4e9cffd
to
61a8b58
Compare
@ffromani: the contents of this pull request could not be automatically validated. The following commits are valid:
The following commits could not be validated and must be approved by a top-level approver:
Comment |
add feature gate to enable selected users to consume cpumanager policy options of alfpha maturtiy. Needs to be merged alongside openshift/kubernetes#2136 which enables per-option granularity for more details: openshift/enhancements#1724 Signed-off-by: Francesco Romani <[email protected]>
@ffromani: the contents of this pull request could not be automatically validated. The following commits are valid:
The following commits could not be validated and must be approved by a top-level approver:
Comment |
/retest |
@haircommander @mrunalp could you PTAL again? the PR implements now the suggested approach I outlined and we agreed upon in openshift/enhancements#1724 |
/retest-required |
2 similar comments
/retest-required |
/retest-required |
/test okd-scos-e2e-aws-ovn
|
/lgtm |
/test okd-scos-e2e-aws-ovn |
// must override the base feature gate check. Relevant only for alpha (disabled by default). | ||
// for beta options are enabled by default and we totally want to keep the possibility to | ||
// disable them explicitly. | ||
if alphaOptions.Has(option) && checkPolicyOptionHasEnablementFile(option) { |
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.
wouldn't this put the cluster in TPNU?
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 the internal check: alphaOptions
is the internal set of known cpumanager policy options, and this specific guard is meant to bypass the upstream alpha FG check for features which have the enablement file. It was implemented this way (vs extending the existing guard below) to make it as easy as possible to remove later on when the carry is no longer needed. It is meant to be equivalent to:
// CheckPolicyOptionAvailable verifies if the given option can be used depending on the Feature Gate Settings.
// returns nil on success, or an error describing the failure on error.
func CheckPolicyOptionAvailable(option string) error {
if !alphaOptions.Has(option) && !betaOptions.Has(option) && !stableOptions.Has(option) {
return fmt.Errorf("unknown CPU Manager Policy option: %q", option)
}
if alphaOptions.Has(option) {
if checkPolicyOptionHasEnablementFile(option) {
return nil
}
if !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.CPUManagerPolicyAlphaOptions) {
return fmt.Errorf("CPU Manager Policy Alpha-level Options not enabled, but option %q provided", option)
}
}
if betaOptions.Has(option) && !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.CPUManagerPolicyBetaOptions) {
return fmt.Errorf("CPU Manager Policy Beta-level Options not enabled, but option %q provided", option)
}
return nil
}
(note that the feature is enabled if either the enablement file or the FG are enabled)
/test e2e-agnostic-ovn-cmd
|
|
/test okd-scos-e2e-aws-ovn |
@ffromani: The following tests failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: ffromani, haircommander 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 |
3c62f73
into
openshift:master
@ffromani: Jira Issue OCPBUGS-44786: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-44786 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. |
/cherry-pick release-4.18 |
@ffromani: new pull request created: #2162 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-sigs/prow repository. |
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-hyperkube |
[ART PR BUILD NOTIFIER] Distgit: kube-proxy |
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-pod |
[ART PR BUILD NOTIFIER] Distgit: ose-installer-kube-apiserver-artifacts |
add support for the LLC alignment by pulling ahead of time u/s PR 126750.