Skip to content
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

Merged
merged 3 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
20 changes: 20 additions & 0 deletions pkg/kubelet/cm/cpumanager/policy_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ import (

"k8s.io/apimachinery/pkg/util/sets"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/klog/v2"
kubefeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/topology"
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager"
"k8s.io/kubernetes/pkg/kubelet/llcalign"
)

// Names of the options, as part of the user interface.
Expand Down Expand Up @@ -56,6 +58,14 @@ func CheckPolicyOptionAvailable(option string) error {
return fmt.Errorf("unknown CPU Manager Policy option: %q", option)
}

// 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) {
Copy link
Member

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?

Copy link
Author

@ffromani ffromani Dec 11, 2024

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)

// note that we override the decision and shortcut exit with success
// all other cases exit early with failure.
return nil
}
if alphaOptions.Has(option) && !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.CPUManagerPolicyAlphaOptions) {
return fmt.Errorf("CPU Manager Policy Alpha-level Options not enabled, but option %q provided", option)
}
Expand Down Expand Up @@ -173,3 +183,13 @@ func ValidateStaticPolicyOptions(opts StaticPolicyOptions, topology *topology.CP
}
return nil
}

func checkPolicyOptionHasEnablementFile(option string) bool {
switch option {
case PreferAlignByUnCoreCacheOption:
val := llcalign.IsEnabled()
klog.InfoS("policy option enablement file check", "option", option, "enablementFile", val)
return val
}
return false
}
63 changes: 63 additions & 0 deletions pkg/kubelet/cm/cpumanager/policy_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
pkgfeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/topology"
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager"
"k8s.io/kubernetes/pkg/kubelet/llcalign"
)

type optionAvailTest struct {
Expand Down Expand Up @@ -246,3 +247,65 @@ func TestPolicyOptionsCompatibility(t *testing.T) {
})
}
}

func TestPolicyOptionsAvailableWithEnablement(t *testing.T) {

type optionAvailEnabTest struct {
name string
option string
featureGate featuregate.Feature
featureGateEnable bool
featureEnablementFlag bool
expectedAvailable bool
}

testCases := []optionAvailEnabTest{
{
name: "all disabled",
option: PreferAlignByUnCoreCacheOption,
featureGate: pkgfeatures.CPUManagerPolicyAlphaOptions,
featureGateEnable: false, // expected standard case
featureEnablementFlag: false,
expectedAvailable: false,
},
{
name: "all enabled",
option: PreferAlignByUnCoreCacheOption,
featureGate: pkgfeatures.CPUManagerPolicyAlphaOptions,
featureGateEnable: true, // this should not be allowed by OCP profiles
featureEnablementFlag: true,
expectedAvailable: true,
},
{
name: "enabled by feature gate",
option: PreferAlignByUnCoreCacheOption,
featureGate: pkgfeatures.CPUManagerPolicyAlphaOptions,
featureGateEnable: true, // this should not be allowed by OCP profiles, makes no sense either
featureEnablementFlag: false,
expectedAvailable: true,
},
{
name: "enabled by enablement file",
option: PreferAlignByUnCoreCacheOption,
featureGate: pkgfeatures.CPUManagerPolicyAlphaOptions,
featureGateEnable: false,
featureEnablementFlag: true,
expectedAvailable: true,
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, testCase.featureGate, testCase.featureGateEnable)
oldEnablementFlag := llcalign.TestOnlySetEnabled(testCase.featureEnablementFlag)

err := CheckPolicyOptionAvailable(testCase.option)

_ = llcalign.TestOnlySetEnabled(oldEnablementFlag)

isEnabled := (err == nil)
if isEnabled != testCase.expectedAvailable {
t.Errorf("option %q available got=%v expected=%v", testCase.option, isEnabled, testCase.expectedAvailable)
}
})
}
}
46 changes: 46 additions & 0 deletions pkg/kubelet/llcalign/llcalign.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package llcalign

import (
"os"
)

var (
llcAlignmentEnabled bool
llcAlignmentFilename = "/etc/kubernetes/openshift-llc-alignment"
)

func init() {
readEnablementFile()
}

func readEnablementFile() {
if _, err := os.Stat(llcAlignmentFilename); err == nil {
llcAlignmentEnabled = true
}
}

func IsEnabled() bool {
return llcAlignmentEnabled
}

func TestOnlySetEnabled(enabled bool) bool {
oldEnabled := llcAlignmentEnabled
llcAlignmentEnabled = enabled
return oldEnabled
}