Skip to content

Fix the system compressible test case to use drop-in files #31332

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

Draft
ngopalak-redhat wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −18
Draft

Fix the system compressible test case to use drop-in files #31332

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 17 additions & 18 deletions test/extended/node/system_compressible.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import (
o "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
"k8s.io/kubernetes/test/e2e/framework"
"sigs.k8s.io/yaml"

mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
machineconfigclient "github.com/openshift/client-go/machineconfiguration/clientset/versioned"
Expand Down Expand Up @@ -55,24 +57,21 @@ var _ = g.Describe("[Suite:openshift/disruptive-longrunning][sig-node][Disruptiv
o.Expect(isSystemCompressibleEnabled(config)).To(o.BeTrue(),
"System compressible should be enabled by default")

// Read SYSTEM_RESERVED_CPU from /etc/node-sizing.env
g.By("Reading SYSTEM_RESERVED_CPU from /etc/node-sizing.env")
nodeSizingOutput, err := ExecOnNodeWithChroot(oc, nodeName, "cat", "/etc/node-sizing.env")
o.Expect(err).NotTo(o.HaveOccurred(), "Should be able to read /etc/node-sizing.env")
framework.Logf("/etc/node-sizing.env contents:\n%s", nodeSizingOutput)

// Parse SYSTEM_RESERVED_CPU value (e.g., "0.5" means 500m)
var systemReservedCPU float64
for _, line := range strings.Split(nodeSizingOutput, "\n") {
if strings.HasPrefix(line, "SYSTEM_RESERVED_CPU=") {
cpuStr := strings.TrimPrefix(line, "SYSTEM_RESERVED_CPU=")
systemReservedCPU, err = strconv.ParseFloat(cpuStr, 64)
o.Expect(err).NotTo(o.HaveOccurred(), "Should be able to parse SYSTEM_RESERVED_CPU value: %s", cpuStr)
break
}
}
o.Expect(systemReservedCPU).To(o.BeNumerically(">", 0), "SYSTEM_RESERVED_CPU should be set")
framework.Logf("SYSTEM_RESERVED_CPU: %.2f (%.0f millicores)", systemReservedCPU, systemReservedCPU*1000)
g.By("Reading systemReserved.cpu from /etc/openshift/kubelet.conf.d/20-auto-sizing.conf")
autoSizingOutput, err := ExecOnNodeWithChroot(oc, nodeName, "cat", "/etc/openshift/kubelet.conf.d/20-auto-sizing.conf")
o.Expect(err).NotTo(o.HaveOccurred(), "Should be able to read /etc/openshift/kubelet.conf.d/20-auto-sizing.conf")
framework.Logf("/etc/openshift/kubelet.conf.d/20-auto-sizing.conf contents:\n%s", autoSizingOutput)

var autoSizingConfig kubeletconfigv1beta1.KubeletConfiguration
err = yaml.Unmarshal([]byte(autoSizingOutput), &autoSizingConfig)
o.Expect(err).NotTo(o.HaveOccurred(), "Should be able to parse auto-sizing config")

cpuQuantity, ok := autoSizingConfig.SystemReserved["cpu"]
o.Expect(ok).To(o.BeTrue(), "systemReserved.cpu should be set")
cpuResource := resource.MustParse(cpuQuantity)
systemReservedCPU := float64(cpuResource.MilliValue()) / 1000.0
o.Expect(systemReservedCPU).To(o.BeNumerically(">", 0), "systemReserved.cpu should be greater than 0")
Comment on lines +71 to +73

@coderabbitai coderabbitai Bot Jun 24, 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.

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file and the specific lines in question
cat -n test/extended/node/system_compressible.go | sed -n '60,85p'

Repository: openshift/origin

Length of output: 2035

🏁 Script executed:

# Let's also check a broader context to understand where cpuQuantity comes from
cat -n test/extended/node/system_compressible.go | sed -n '1,100p'

Repository: openshift/origin

Length of output: 5316

🏁 Script executed:

# Let's search for other uses of MustParse in this test file to see if there's a pattern
rg "MustParse|ParseQuantity" test/extended/node/system_compressible.go -n

Repository: openshift/origin

Length of output: 206

Avoid panic-prone parsing for node config values.

Line 71 uses resource.MustParse(cpuQuantity), which panics on malformed config and aborts the test. Even though this value comes from a system-controlled configuration file, defensive parsing ensures test stability and provides clear error messages. Use resource.ParseQuantity() with explicit error handling.

Suggested fix 
-		cpuResource := resource.MustParse(cpuQuantity)
+		cpuResource, err := resource.ParseQuantity(cpuQuantity)
+		o.Expect(err).NotTo(o.HaveOccurred(), "systemReserved.cpu must be a valid resource quantity")
 		systemReservedCPU := float64(cpuResource.MilliValue()) / 1000.0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cpuResource := resource.MustParse(cpuQuantity)
systemReservedCPU := float64(cpuResource.MilliValue()) / 1000.0
o.Expect(systemReservedCPU).To(o.BeNumerically(">", 0), "systemReserved.cpu should be greater than 0")
cpuResource, err := resource.ParseQuantity(cpuQuantity)
o.Expect(err).NotTo(o.HaveOccurred(), "systemReserved.cpu must be a valid resource quantity")
systemReservedCPU := float64(cpuResource.MilliValue()) / 1000.0
o.Expect(systemReservedCPU).To(o.BeNumerically(">", 0), "systemReserved.cpu should be greater than 0")
🤖 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/extended/node/system_compressible.go` around lines 71 - 73, The
`resource.MustParse(cpuQuantity)` call on line 71 will panic if the
configuration value is malformed, causing the test to abort without a clear
error message. Replace `resource.MustParse(cpuQuantity)` with
`resource.ParseQuantity(cpuQuantity)` and add explicit error handling to check
if parsing fails, then use `o.Expect()` or a similar test assertion to fail
gracefully with a descriptive error message explaining that the CPU quantity
could not be parsed.

framework.Logf("systemReserved.cpu: %.2f (%.0f millicores)", systemReservedCPU, systemReservedCPU*1000)

// Convert to cpuShares: cpuShares = systemReservedCPU * 1024
cpuShares := uint64(systemReservedCPU * 1024)
Expand Down