Skip to content

e2e: Fix broken assertions #1571

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.

Sign up for GitHub

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

Open
oblau wants to merge 3 commits into
base: main
Choose a base branch
from
+79 −73
Open

e2e: Fix broken assertions #1571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
daa09ea
E2E: add missing matcher to offlined CPU Expect() calls (5 sites)
oblau Jul 23, 2026
4ecdb76
E2E: mixedcpus Get() checks, SMT-safe CPU requests, and shared-set ex…
oblau Jul 23, 2026
3cafd46
E2E: PPC: replace no-op log-on-match with actual assertions (4 sites)
oblau Jul 29, 2026
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
88 changes: 55 additions & 33 deletions test/e2e/performanceprofile/functests/10_performance_ppc/ppc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ package __performance_ppc
import (
"fmt"
"os/exec"
"regexp"
"path/filepath"
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/gexec"
performancev2 "github.com/openshift/cluster-node-tuning-operator/pkg/apis/performanceprofile/v2"
"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/profilecreator"
testutils "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils"
"github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/label"
testlog "github.com/openshift/cluster-node-tuning-operator/test/e2e/performanceprofile/functests/utils/log"
"k8s.io/utils/cpuset"
"sigs.k8s.io/yaml"
)
Expand Down Expand Up @@ -89,18 +89,21 @@ var _ = Describe("[rfe_id: 38968] PerformanceProfile setup helper and platform a
podmanArgs := append(defaultArgs, cmdArgs...)
session, err := ppcIntgTest.PodmanAsUserBase(podmanArgs, false, false)
Expect(err).ToNot(HaveOccurred(), "Podman command failed")

output := session.Wait(20).Out.Contents()
Expect(session).Should(gexec.Exit(0))

err = yaml.Unmarshal(output, pp)
Expect(err).ToNot(HaveOccurred(), "Unable to marshal the ppc output")
Expect(err).ToNot(HaveOccurred(), "Unable to unmarshal the ppc output")
reservedCpus, err := cpuset.Parse(string(*pp.Spec.CPU.Reserved))
Expect(err).ToNot(HaveOccurred(), "Unable to parse cpus")
totalReservedCpus := reservedCpus.Size()
Expect(totalReservedCpus).To(Equal(reservedCpuCount))
Expect(*pp.Spec.RealTimeKernel.Enabled).To(BeTrue())
Expect(*pp.Spec.WorkloadHints.RealTime).To(BeTrue())
Expect(*pp.Spec.NUMA.TopologyPolicy).To(Equal("restricted"))
Eventually(session).Should(gexec.Exit(0))
})

It("[test_id:41405] Verify PPC script fails when the splitting of reserved cpus and single numa-node policy is specified", func() {
cmdArgs := []string{
fmt.Sprintf("%s:%s:z", mustgatherDir, mustgatherDir),
Expand All @@ -116,14 +119,12 @@ var _ = Describe("[rfe_id: 38968] PerformanceProfile setup helper and platform a
podmanArgs := append(defaultArgs, cmdArgs...)
session, err := ppcIntgTest.PodmanAsUserBase(podmanArgs, false, false)
Expect(err).ToNot(HaveOccurred(), "Podman command failed")

output := session.Wait(20).Err.Contents()
errString := "Error: failed to obtain data from flags not appropriate to split reserved CPUs in case of topology-manager-policy: single-numa-node"
ok, err := regexp.MatchString(errString, string(output))
Expect(err).ToNot(HaveOccurred())
if ok {

@Tal-or Tal-or Jul 26, 2026

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.

That's a change in the behavior. it makes the test more restrict. is that what we want here?

@oblau oblau Jul 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is changing behavior - but isn't this the intention here?
If we are not asserting on ok then i fail to see why we need the errString
and regexp.MatchString(errString, string(output)) to begin with.
Maybe im missing something here.

Is the goal to make sure the correct error message appears, or that PPC script fails in general?

@Tal-or Tal-or Jul 27, 2026

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.

After reevaluation I think this change is ok.
the only change I would add is in the message to make it more clear:

Expect(ok).To(BeTrue(), "expected error %q to be found in output: %s", errString, output)

testlog.Info(errString)
}
Eventually(session).Should(gexec.Exit(1))
Expect(session).Should(gexec.Exit(1))

errString := "not appropriate to split reserved CPUs in case of topology-manager-policy: single-numa-node"
Expect(string(output)).To(ContainSubstring(errString), "expected error:\n%q\ngot:\n%s", errString, output)
})

It("[test_id:41419] Verify PPC script fails when reserved cpu count is 2 and requires to split across numa nodes", func() {
Expand All @@ -140,14 +141,12 @@ var _ = Describe("[rfe_id: 38968] PerformanceProfile setup helper and platform a
podmanArgs := append(defaultArgs, cmdArgs...)
session, err := ppcIntgTest.PodmanAsUserBase(podmanArgs, false, false)
Expect(err).ToNot(HaveOccurred(), "Podman command failed")

output := session.Wait(20).Err.Contents()
errString := "Error: failed to compute the reserved and isolated CPUs: can't allocate odd number of CPUs from a NUMA Node"
ok, err := regexp.MatchString(errString, string(output))
Expect(err).ToNot(HaveOccurred(), "did not fail with Expected:%s failure", errString)
if ok {
testlog.Info(errString)
}
Eventually(session).Should(gexec.Exit(1))
Expect(session).Should(gexec.Exit(1))

errString := "can't allocate odd number of CPUs from a NUMA Node"
Expect(string(output)).To(ContainSubstring(errString), "expected error:\n%q\ngot:\n%s", errString, output)
})

It("[test_id:41420] Verify PPC script fails when reserved cpu count is more than available cpus", func() {
Expand All @@ -165,14 +164,13 @@ var _ = Describe("[rfe_id: 38968] PerformanceProfile setup helper and platform a
podmanArgs := append(defaultArgs, cmdArgs...)
session, err := ppcIntgTest.PodmanAsUserBase(podmanArgs, false, false)
Expect(err).ToNot(HaveOccurred(), "Podman command failed")

output := session.Wait(20).Err.Contents()
errString := "Error: failed to compute the reserved and isolated CPUs: please specify the reserved CPU count in the range [1,3]"
ok, err := regexp.MatchString(errString, string(output))
Expect(err).ToNot(HaveOccurred(), "did not fail with Expected:%s failure", errString)
if ok {
testlog.Info(errString)
}
Eventually(session).Should(gexec.Exit(1))
Expect(session).Should(gexec.Exit(1))

errString := fmt.Sprintf("please specify the reserved CPU count in the range [1,%d]",
maxReservedCPUCountFromMustGather(mustgatherDir, mcpName))
Expect(string(output)).To(ContainSubstring(errString), "expected error:\n%q\ngot:\n%s", errString, output)
})

It("[test_id: 54187] PPC generates profile with PerPodPowerManagement workload hint", func() {
Expand All @@ -191,9 +189,12 @@ var _ = Describe("[rfe_id: 38968] PerformanceProfile setup helper and platform a
podmanArgs := append(defaultArgs, cmdArgs...)
session, err := ppcIntgTest.PodmanAsUserBase(podmanArgs, false, false)
Expect(err).ToNot(HaveOccurred(), "Podman command failed")

output := session.Wait(20).Out.Contents()
Expect(session).Should(gexec.Exit(0))

err = yaml.Unmarshal(output, pp)
Expect(err).ToNot(HaveOccurred(), "Unable to marshal the ppc output")
Expect(err).ToNot(HaveOccurred(), "Unable to unmarshal the ppc output")
Expect(*pp.Spec.WorkloadHints.PerPodPowerManagement).To(BeTrue())
Expect(*pp.Spec.WorkloadHints.HighPowerConsumption).To(BeFalse())
})
Expand All @@ -213,14 +214,35 @@ var _ = Describe("[rfe_id: 38968] PerformanceProfile setup helper and platform a
podmanArgs := append(defaultArgs, cmdArgs...)
session, err := ppcIntgTest.PodmanAsUserBase(podmanArgs, false, false)
Expect(err).ToNot(HaveOccurred(), "Podman command failed")

output := session.Wait(20).Err.Contents()
errString := `please use one of \[default low-latency\] power consumption modes together with the perPodPowerManagement`
ok, err := regexp.MatchString(errString, string(output))
Expect(err).ToNot(HaveOccurred(), "did not fail with Expected:%s failure", errString)
if ok {
testlog.Info(errString)
}
Eventually(session).Should(gexec.Exit(1))
Expect(session).Should(gexec.Exit(1))

errString := "please use one of [default low-latency] power consumption modes together with the perPodPowerManagement"
Expect(string(output)).To(ContainSubstring(errString), "expected error:\n%q\ngot:\n%s", errString, output)
})
})
})

// maxReservedCPUCountFromMustGather returns TotalThreads-1 from one node in mcpName
// (the upper bound in PPC's "reserved CPU count in the range [1,%d]" error).
func maxReservedCPUCountFromMustGather(mustGatherDir, mcpName string) int {
GinkgoHelper()
dir, err := filepath.Abs(mustGatherDir)
Expect(err).ToNot(HaveOccurred())
nodes, err := profilecreator.GetNodeList(dir)
Expect(err).ToNot(HaveOccurred())
mcps, err := profilecreator.GetMCPList(dir)
Expect(err).ToNot(HaveOccurred())
mcp, err := profilecreator.GetMCP(dir, mcpName)
Expect(err).ToNot(HaveOccurred())
poolNodes, err := profilecreator.GetNodesForPool(mcp, mcps, nodes)
Expect(err).ToNot(HaveOccurred())
Expect(poolNodes).ToNot(BeEmpty())
h, err := profilecreator.NewGHWHandler(dir, poolNodes[0])
Expect(err).ToNot(HaveOccurred())
DeferCleanup(h.Cleanup)
cpu, err := h.CPU()
Expect(err).ToNot(HaveOccurred())
return int(cpu.TotalThreads) - 1
}
54 changes: 19 additions & 35 deletions test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,10 @@ const (
kubeletMixedCPUsConfigFile = "/etc/kubernetes/openshift-workload-mixed-cpus"
crioRuntimesConfigFile = "/etc/crio/crio.conf.d/99-runtimes.conf"
sharedCpusResource = "workload.openshift.io/enable-shared-cpus"
// the minimal number of cores for running the test is as follows:
// reserved = one core, shared = one core, infra workload = one core, test pod = one core - 4 in total
// smt alignment won't allow us to run the test pod with a single core, hence we should cancel it.
numberOfCoresThatRequiredCancelingSMTAlignment = 4
restartCooldownTime = 1 * time.Minute
isolatedCpusEnv = "OPENSHIFT_ISOLATED_CPUS"
sharedCpusEnv = "OPENSHIFT_SHARED_CPUS"
// DeploymentName contains the name of the deployment
DeploymentName = "test-deployment"
restartCooldownTime = 1 * time.Minute
isolatedCpusEnv = "OPENSHIFT_ISOLATED_CPUS"
sharedCpusEnv = "OPENSHIFT_SHARED_CPUS"
DeploymentName = "test-deployment" // DeploymentName contains the name of the deployment
)

var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
Expand Down Expand Up @@ -151,7 +146,7 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
When("workloads requests access for shared cpus", func() {
It("verify cpu load balancing still works with mixed cpus", func() {
rl := &corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceCPU: resource.MustParse("2"),

@coderabbitai coderabbitai Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go --items all

rg -n -C 5 \
  'ResourceCPU:\s+resource\.MustParse\("2"\)|func setup\(|CPU\.Isolated|isolated.*Size\(\)|createPod\(' \
  test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go

fd -e go . test/e2e/performanceprofile | while IFS= read -r file; do
  rg -n -C 3 'MixedCPUs|CPU\.Isolated|isolated.*cpus|isolated.*CPU' "$file" || true
done

Repository: openshift/cluster-node-tuning-operator

Length of output: 50394

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- test profile and suite setup ---'
sed -n '55,155p' test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
sed -n '810,875p' test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go

printf '%s\n' '--- all direct uses of the seven workloads ---'
sed -n '140,270p' test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
sed -n '286,355p' test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
sed -n '428,470p' test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go

printf '%s\n' '--- resource and scheduling helpers ---'
sed -n '900,1035p' test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
rg -n -C 4 'enable-shared-cpus|sharedCpusResource|MixedCpus|mixed cpus|mixed CPUs|ResourceCPU' \
  pkg test/e2e/performanceprofile/functests/utils | head -n 300

Repository: openshift/cluster-node-tuning-operator

Length of output: 50394

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- profile construction and test entry points ---'
sed -n '95,155p' test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
rg -n -C 5 'testProfile\(\)|MixedCPUs|NodeSelectorLabels|ProfileNotFound' \
  test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go \
  test/e2e/performanceprofile/functests/utils

printf '%s\n' '--- mixed-CPU resource implementation ---'
rg -n -C 8 --glob '*.go' \
  'enable-shared-cpus|shared_cpus|shared cpus|SharedCPUs|MixedCpus|MixedCPUs' \
  . | head -n 500

printf '%s\n' '--- current change context for mixedcpus.go ---'
git diff --stat -- test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
git diff --unified=12 -- test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go | sed -n '1,260p'

Repository: openshift/cluster-node-tuning-operator

Length of output: 50394

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- files defining the shared CPU resource ---'
git ls-files | rg -i 'mixed|shared|cpu.*manager|kubelet|crio'
printf '%s\n' '--- exact resource-name references ---'
rg -n --glob '*.go' --glob '*.yaml' --glob '*.yml' --glob '*.json' \
  'workload\.openshift\.io/enable-shared-cpus' . || true

printf '%s\n' '--- test profile defaults and discovery behavior ---'
rg -n -C 8 --glob '*.go' \
  'IsolatedCPUs|SharedCPUs|NewPerformanceProfile|GetDiscoveryPerformanceProfile|PerformanceProfileName' \
  pkg test e2e | head -n 400

printf '%s\n' '--- concise static invariant check ---'
python3 - <<'PY'
from pathlib import Path
import re

p = Path("test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go")
s = p.read_text()

resource_blocks = re.findall(
    r'rl := &corev1\.ResourceList\{(.*?)\n\s*\}',
    s,
    re.S,
)
target = []
for i, block in enumerate(resource_blocks, 1):
    if 'resource.MustParse("2")' in block and 'sharedCpusResource' in block:
        target.append(i)

setup = s[s.index('func setup('):s.index('func cpuSetToPerformanceCPUSet(')]
print(f"target resource-list blocks: {len(target)}")
print(f"target blocks contain CPU=2 and shared resource: {target}")
print("setup changes isolated CPUs only under !profileutil.IsMixedCPUsEnabled(profile):",
      '!profileutil.IsMixedCPUsEnabled(profile)' in setup)
print("setup has no isolated-size guard:",
      'Size()' not in setup and 'List()' not in setup)
PY

Repository: openshift/cluster-node-tuning-operator

Length of output: 50394

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- generated mixed-CPU configuration ---'
cat assets/performanceprofile/configs/openshift-workload-mixed-cpus
sed -n '1,220p' docs/performanceprofile/kubelet-config-snippets.md | rg -n -C 8 \
  'mixed|shared|reservedSystemCPUs|cpu-shared'

printf '%s\n' '--- E2E profile defaults and discovery ---'
rg -n -C 6 --glob '*.go' \
  'IsolatedCPUs|SharedCPUs|NewPerformanceProfile|GetDiscoveryPerformanceProfile|PerformanceProfileName' \
  pkg test | head -n 300

printf '%s\n' '--- static invariant check ---'
python3 - <<'PY'
from pathlib import Path
import re

p = Path("test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go")
s = p.read_text()

blocks = re.findall(r'rl := &corev1\.ResourceList\{(.*?)\n\s*\}', s, re.S)
targets = [
    (i, 'CPU=2' in b, 'shared=1' in b)
    for i, b in enumerate(blocks, 1)
    if 'resource.MustParse("2")' in b and 'sharedCpusResource' in b
]
setup = s[s.index('func setup('):s.index('func cpuSetToPerformanceCPUSet(')]

print("target resource-list blocks:", len(targets))
print("all target blocks request CPU=2 and shared resource=1:",
      all(cpu and shared for _, cpu, shared in targets))
print("setup changes the profile only when mixed CPUs are disabled:",
      '!profileutil.IsMixedCPUsEnabled(profile)' in setup)
print("setup checks isolated CPU count:", 'Size()' in setup)
print("setup checks isolated CPU list length:", 'List()' in setup)
PY

Repository: openshift/cluster-node-tuning-operator

Length of output: 371

🏁 Script executed:

#!/bin/bash
set -u

python3 - <<'PY'
from pathlib import Path
import re

p = Path("test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go")
s = p.read_text()

blocks = re.findall(r'rl := &corev1\.ResourceList\{(.*?)\n\s*\}', s, re.S)
targets = [
    b for b in blocks
    if 'resource.MustParse("2")' in b and 'sharedCpusResource' in b
]
setup = s[s.index('func setup('):s.index('func cpuSetToPerformanceCPUSet(')]

print("target resource-list blocks:", len(targets))
print("all target blocks request CPU=2:", all('corev1.ResourceCPU' in b for b in targets))
print("all target blocks request shared resource=1:",
      all('sharedCpusResource' in b and 'resource.MustParse("1")' in b for b in targets))
print("setup changes the profile only when mixed CPUs are disabled:",
      '!profileutil.IsMixedCPUsEnabled(profile)' in setup)
print("setup checks isolated CPU count:", 'Size()' in setup or 'len(isolated.List())' in setup)
PY

Repository: openshift/cluster-node-tuning-operator

Length of output: 400

🌐 Web query:

OpenShift mixed CPUs allocation workload.openshift.io/enable-shared-cpus ordinary CPU request isolated CPUs

💡 Result:

The annotation workload.openshift.io/enable-shared-cpus is part of the Mixed-CPUs feature in OpenShift, which allows Guaranteed Quality of Service (QoS) pods to utilize both isolated and shared CPU pools [1][2][3]. Typically, Guaranteed QoS pods with integer CPU requests are assigned exclusive (isolated) CPUs, meaning they cannot access the shared CPU pool [4]. The Mixed-CPUs feature changes this behavior by permitting such pods to request access to the shared CPU pool via this specific resource annotation [1][2]. Key aspects of this functionality include: 1. Admission Validation: To use this feature, a pod must be in the Guaranteed QoS class [1][2][3]. The admission controller also verifies that the pod is deployed in a namespace explicitly allowed to use mixed-CPU workloads, typically indicated by the annotation workload.mixedcpus.openshift.io/allowed [2][3]. 2. Runtime Hinting: Because container runtimes (like CRI-O) are not natively aware of extended OpenShift resources, the admission controller injects an annotation (e.g., cpu-shared.crio.io/) into the pod definition [2][3]. This annotation informs the runtime that the specific container has been granted access to shared CPUs [2]. 3. Constraint: A pod can request no more than a single instance of the workload.openshift.io/enable-shared-cpus resource [1][3]. This feature is distinct from standard workload partitioning (enabled via cpuPartitioningMode: AllNodes), which separates cluster management pods into a reserved CPU pool while isolating customer workloads on separate cores [5][6][7]. The Mixed-CPUs feature specifically addresses the flexibility for customer workloads that require both isolation and access to shared resources within a Guaranteed QoS context [1][3].

Citations:

Guard the two-isolated-CPU requirement.

These workloads request two whole CPUs. The shared-CPU resource only grants shared-CPU access. setup does not check isolated CPU capacity when mixed CPUs are already enabled. Skip the workloads or provision at least two isolated CPUs before creating them.

📍 Affects 1 file
  • test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go#L149-L149 (this comment)
  • test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go#L181-L181
  • test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go#L201-L201
  • test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go#L218-L218
  • test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go#L249-L249
  • test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go#L325-L325
  • test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go#L437-L437
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go` at line 149,
Update setup in test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
to verify at least two isolated CPUs are available when mixed CPUs are enabled,
and skip these workloads when they are not; alternatively provision that
capacity before creation. Apply the protection to the two-CPU workload
definitions at lines 149, 181, 201, 218, 249, 325, and 437, ensuring shared-CPU
resources are not treated as isolated CPU capacity.

corev1.ResourceMemory: resource.MustParse("100Mi"),
sharedCpusResource: resource.MustParse("1"),
}
Expand Down Expand Up @@ -183,7 +178,7 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
})
It("should have the shared cpus under its cgroups", func() {
rl := &corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceCPU: resource.MustParse("2"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
sharedCpusResource: resource.MustParse("1"),
}
Expand All @@ -203,7 +198,7 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
})
It("should be able to disable cfs_quota", func() {
rl := &corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceCPU: resource.MustParse("2"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
sharedCpusResource: resource.MustParse("1"),
}
Expand All @@ -220,7 +215,7 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
})
It("should have OPENSHIFT_ISOLATED_CPUS and OPENSHIFT_SHARED_CPUS env variables under the container", func() {
rl := &corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceCPU: resource.MustParse("2"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
sharedCpusResource: resource.MustParse("1"),
}
Expand Down Expand Up @@ -251,7 +246,7 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
})
It("should contains the shared cpus after Kubelet restarts", func() {
rl := &corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceCPU: resource.MustParse("2"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
sharedCpusResource: resource.MustParse("1"),
}
Expand Down Expand Up @@ -320,14 +315,14 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
By(fmt.Sprintf("Waiting when %s finishes updates", poolName))
profilesupdate.WaitForTuningUpdated(context.TODO(), profile)

Expect(testclient.ControlPlaneClient.Get(ctx, client.ObjectKeyFromObject(profile), profile))
testlog.Infof("new isolated CPU set=%q\nnew shared CPU set=%q", string(*profile.Spec.CPU.Isolated), string(*profile.Spec.CPU.Isolated))
Expect(testclient.ControlPlaneClient.Get(ctx, client.ObjectKeyFromObject(profile), profile)).To(Succeed())
testlog.Infof("new isolated CPU set=%q\nnew shared CPU set=%q", string(*profile.Spec.CPU.Isolated), string(*profile.Spec.CPU.Shared))
// we do not bother to revert the profile at the end of the test, since its irrelevant which of the cpus are shared
})

It("should contains the updated values under the container", func() {
rl := &corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceCPU: resource.MustParse("2"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
sharedCpusResource: resource.MustParse("1"),
}
Expand Down Expand Up @@ -439,7 +434,7 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {

By("Creating a deployment with one pod asking for a shared cpu")
rl := &corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceCPU: resource.MustParse("2"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
sharedCpusResource: resource.MustParse("1"),
}
Expand Down Expand Up @@ -502,7 +497,7 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {
Expect(pod.Status.Phase).To(Equal(corev1.PodPending), "Pod %s is not in the pending state", pod.Name)

By("Reverting the cluster to previous state")
Expect(testclient.ControlPlaneClient.Get(ctx, client.ObjectKeyFromObject(profile), profile))
Expect(testclient.ControlPlaneClient.Get(ctx, client.ObjectKeyFromObject(profile), profile)).To(Succeed())
profile.Spec.CPU.Shared = cpuSetToPerformanceCPUSet(ppShared)
profile.Spec.WorkloadHints.MixedCpus = ptr.To(true)
profiles.UpdateWithRetry(profile)
Expand Down Expand Up @@ -577,13 +572,16 @@ var _ = Describe("Mixedcpus", Ordered, Label(string(label.MixedCPUs)), func() {

coreSiblings, err := nodes.GetCoreSiblings(ctx, workerRTNode)
Expect(err).ToNot(HaveOccurred())
// When Shared already has 1 CPU and we need 2, we replace Shared with a new pair from

@Tal-or Tal-or Aug 12, 2026

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.

But with the new logic we always have 2 shared CPUs by default, so why the comment says
"When Shared already has 1 CPU and we need 2"?

Besides the wrong comment, we don't even need to change anything, since now 2 CPUs is the default.

@oblau oblau Aug 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for flagging this, I think there's a small mix-up, my bad if the earlier phrasing wasn't clear.

The cpu:1 -> cpu:2 change in this PR was to the pods' resource requests, for SMT alignment reasons. It's unrelated to profile.Spec.CPU.Shared, which is set separately by setup() at the top of the file, and still defaults to 1 CPU there. So updatedShared.Size() < 2 is still true in the normal case, and this code (with the oldShared/Union fix) still runs.

Worth noting too: even if we changed setup() to assign 2 shared CPUs, that only covers the case where setup() itself enables mixed CPUs. If a cluster arrives with mixed CPUs already enabled and only 1 shared CPU, setup() skips that assignment entirely, so we'd still need this fallback to grow Shared to 2.

On the comment wording: Shared could in theory be 0 as well as 1, but that's actually unreachable in practice, since IsMixedCPUsEnabled requires Shared to already be non-empty. So the comment holds as written, only the "1 CPU" case ever really happens here.
Happy to add a short note in the comment mentioning that.

@Tal-or Tal-or Aug 13, 2026

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.

If a cluster arrives with mixed CPUs already enabled and only 1 shared CPU, setup() skips that assignment entirely, so we'd still need this fallback to grow Shared to 2.

This is a well defined testing environment, we're not expecting prior state and fully control it.

IOW, if we're moving from a single CPU for testing mixed-cpus feature, lets do it properly and get rid of the other ugly workarounds we had in the code, (which increasing from 1 -> 2 was one of them).

// Isolated. Put the old shared CPU back into Isolated so it is not left unassigned.
oldShared := updatedShared
updatedShared, err = nodes.GetTwoSiblingsFromCPUSet(coreSiblings, updatedIsolated)
if err != nil {
testlog.Info("no two siblings found in the given CPU set, looks like the initial profile does not respect hyperthreading; proceed then with this state and pick first two isolated CPUs as the shared CPUs")
updatedShared = cpuset.New(updatedIsolated.List()[0], updatedIsolated.List()[1])
}

updatedIsolated = updatedIsolated.Difference(updatedShared)
updatedIsolated = updatedIsolated.Difference(updatedShared).Union(oldShared)

testlog.Infof("CPU update:shared cpu %q isolated cpus %q", updatedShared.String(), updatedIsolated.String())
profile.Spec.CPU.Isolated = cpuSetToPerformanceCPUSet(&updatedIsolated)
Expand Down Expand Up @@ -843,20 +841,6 @@ func setup(ctx context.Context) func(ctx2 context.Context) {
testlog.Infof("mixed cpus already enabled for profile %q", profile.Name)
}

workers, err := nodes.GetByLabels(testutils.NodeSelectorLabels)
Expect(err).ToNot(HaveOccurred())
for _, worker := range workers {
//node cpu numbers are integral
numOfCores, _ := worker.Status.Capacity.Cpu().AsInt64()
if numOfCores <= numberOfCoresThatRequiredCancelingSMTAlignment {
profile.Annotations = map[string]string{
"kubeletconfig.experimental": "{\"cpuManagerPolicyOptions\": {\"full-pcpus-only\": \"false\"}}",
}
testlog.Infof("canceling SMT alignment for nodes under profile %q", profile.Name)
updateNeeded = true
}
}

if !updateNeeded {
return func(ctx context.Context) {
By(fmt.Sprintf("skipping teardown - no changes to profile %q were applied", profile.Name))
Expand All @@ -872,7 +856,7 @@ func setup(ctx context.Context) func(ctx2 context.Context) {

teardown := func(ctx2 context.Context) {
By(fmt.Sprintf("executing teardown - revert profile %q back to its initial state", profile.Name))
Expect(testclient.ControlPlaneClient.Get(ctx2, client.ObjectKeyFromObject(initialProfile), profile))
Expect(testclient.ControlPlaneClient.Get(ctx2, client.ObjectKeyFromObject(initialProfile), profile)).To(Succeed())
profiles.UpdateWithRetry(initialProfile)

// do not wait if nothing has changed
Expand Down
10 changes: 5 additions & 5 deletions test/e2e/performanceprofile/functests/2_performance_update/updating_profile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
Expect(err).ToNot(HaveOccurred())
offlinedCPUSetProfile, err := cpuset.Parse(string(offlined))
Expect(err).ToNot(HaveOccurred())
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile))
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile)).To(BeTrue(), "offlined CPUs mismatch: expected %q, got %q", offlinedCPUSetProfile, offlinedCPUSet)
}
})

Expand Down Expand Up @@ -707,7 +707,7 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
Expect(err).ToNot(HaveOccurred())
offlinedCPUSetProfile, err := cpuset.Parse(string(offlinedSet))
Expect(err).ToNot(HaveOccurred())
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile))
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile)).To(BeTrue(), "offlined CPUs mismatch: expected %q, got %q", offlinedCPUSetProfile, offlinedCPUSet)
}
})

Expand Down Expand Up @@ -770,7 +770,7 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
Expect(err).ToNot(HaveOccurred())
offlinedCPUSetProfile, err := cpuset.Parse(string(offlinedSet))
Expect(err).ToNot(HaveOccurred())
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile))
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile)).To(BeTrue(), "offlined CPUs mismatch: expected %q, got %q", offlinedCPUSetProfile, offlinedCPUSet)
}
})

Expand Down Expand Up @@ -840,7 +840,7 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
Expect(err).ToNot(HaveOccurred())
offlinedCPUSetProfile, err := cpuset.Parse(string(offlinedSet))
Expect(err).ToNot(HaveOccurred())
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile))
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile)).To(BeTrue(), "offlined CPUs mismatch: expected %q, got %q", offlinedCPUSetProfile, offlinedCPUSet)
}
})

Expand Down Expand Up @@ -966,7 +966,7 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance
Expect(err).ToNot(HaveOccurred())
offlinedCPUSetProfile, err := cpuset.Parse(string(offlinedSet))
Expect(err).ToNot(HaveOccurred())
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile))
Expect(offlinedCPUSet.Equals(offlinedCPUSetProfile)).To(BeTrue(), "offlined CPUs mismatch: expected %q, got %q", offlinedCPUSetProfile, offlinedCPUSet)
}
})

Expand Down