From 0a32b7d6a25228f344b1dcb5b0cce59a462b0ad5 Mon Sep 17 00:00:00 2001 From: "Niranjan M.R" Date: Thu, 18 Jun 2026 18:04:38 +0530 Subject: [PATCH 1/2] E2E: Fix tuned active profile check for wrapped performance profiles Test 37127 read /etc/tuned/active_profile on the node and expected the performance profile name. That fails when a higher-priority Tuned CR wraps the PP via include= (e.g. ran-du-performance), because the active profile name is the wrapper, not the PP name. Detect wrapper profiles by scanning Tuned CRs for include= and assert Profile.status.tunedProfile via the API instead of pod exec. Traverse the tuned profile chain In this commit we are traversing the Tuned profile that contains includes= and verifying if we can traverse back to Performance Profile AI Attribution: AIA Human-AI blend, New content, Human-initiated, Reviewed, Cursor 3.0.16 v1.0 Signed-off-by: Niranjan M.R --- .../functests/1_performance/performance.go | 178 +++++++++++++++++- 1 file changed, 173 insertions(+), 5 deletions(-) diff --git a/test/e2e/performanceprofile/functests/1_performance/performance.go b/test/e2e/performanceprofile/functests/1_performance/performance.go index 6d68c9cfa..d82f88488 100644 --- a/test/e2e/performanceprofile/functests/1_performance/performance.go +++ b/test/e2e/performanceprofile/functests/1_performance/performance.go @@ -11,6 +11,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "gopkg.in/ini.v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -105,13 +106,41 @@ var _ = Describe("[rfe_id:27368][performance]", Ordered, func() { "tuned CR name owned by a performance profile CR should only be %q", tunedExpectedName) }) + // When a higher-priority Tuned profile wraps the performance profile via include=, + // the active profile name is the wrapper (e.g. ran-du-performance), not the performance profile name. It("[test_id:37127] Node should point to right tuned profile", func() { + ctx := context.TODO() + performanceProfileName := components.GetComponentName(testutils.PerformanceProfileName, components.ProfileNamePerformance) + + // Get all Tuned profiles once (for lookup) + tunedList := &tunedv1.TunedList{} + Expect(testclient.ControlPlaneClient.List(ctx, tunedList)).To(Succeed(), + "failed to list Tuned profiles") + for _, node := range workerRTNodes { - tuned := nodes.TunedForNode(&node, RunningOnSingleNode) - activeProfile, err := pods.WaitForPodOutput(context.TODO(), testclient.K8sClient, tuned, []string{"cat", "/etc/tuned/active_profile"}) - Expect(err).ToNot(HaveOccurred(), "Error getting the tuned active profile") - activeProfileName := string(activeProfile) - Expect(strings.TrimSpace(activeProfileName)).To(Equal(tunedExpectedName), "active profile name mismatch got %q expected %q", activeProfileName, tunedExpectedName) + By(fmt.Sprintf("Checking tuned profile on node %s", node.Name)) + key := types.NamespacedName{ + Name: node.Name, + Namespace: components.NamespaceNodeTuningOperator, + } + + Eventually(func(g Gomega) { + tunedProfile := &tunedv1.Profile{} + g.Expect(testclient.DataPlaneClient.Get(ctx, key, tunedProfile)).To(Succeed(), + "failed to get Tuned profile for node %s", node.Name) + + activeProfile := tunedProfile.Status.TunedProfile + testlog.Infof("Node %s has active tuned profile: %q", node.Name, activeProfile) + + // Check if this node's active profile traces back to the performance profile + tracesToPerformanceProfile := profileIncludesTarget(activeProfile, performanceProfileName, tunedList) + + g.Expect(tracesToPerformanceProfile).To(BeTrue(), + "node %s active profile %q does not include performance profile %q", + node.Name, activeProfile, performanceProfileName) + }).WithTimeout(cluster.ComputeTestTimeout(120*time.Second, RunningOnSingleNode)). + WithPolling(testPollInterval * time.Second). + To(Succeed()) } }) @@ -1408,3 +1437,142 @@ func findCondition(conditions []tunedv1.StatusCondition, conditionType string) * } return nil } + +// profileIncludesTarget checks if startProfile includes targetProfile, +// either directly or through a chain of includes. +// +// Example: if "wrapper-b" includes "wrapper-a" and "wrapper-a" includes "perf-profile", +// then profileIncludesTarget("wrapper-b", "perf-profile", ...) returns true. +// +// This walks the include chain step by step until it finds the target or hits a dead end. +func profileIncludesTarget(startProfile, targetProfile string, tunedList *tunedv1.TunedList) bool { + // Build a lookup map: profile name -> list of profiles it includes + includesMap := buildProfileIncludesMap(tunedList) + + // Walk the include chain, starting from startProfile + current := startProfile + visited := make(map[string]bool) // Prevent infinite loops + maxHops := len(includesMap) + chain := []string{startProfile} // Track the chain for logging + + for hop := 0; hop < maxHops; hop++ { + // Found it? + if current == targetProfile { + testlog.Infof("Profile chain validated: %s", strings.Join(chain, " → ")) + return true + } + + // Already visited? (cycle detection) + if visited[current] { + testlog.Warningf("Cycle detected in profile chain at %q", current) + return false + } + visited[current] = true + + // What does the current profile include? + includedProfiles, exists := includesMap[current] + if !exists { + testlog.Warningf("Profile %q not found in cluster", current) + return false // Profile not found + } + + // Check if it directly includes the target + for _, included := range includedProfiles { + if included == targetProfile { + chain = append(chain, targetProfile) + testlog.Infof("Profile chain validated: %s", strings.Join(chain, " → ")) + return true + } + } + + // Find the next profile in the chain (first include that exists in our map) + nextProfile := "" + for _, included := range includedProfiles { + if _, exists := includesMap[included]; exists { + nextProfile = included + break + } + } + + if nextProfile == "" { + testlog.Warningf("Dead end in profile chain at %q (no valid includes found)", current) + return false // Dead end - no more includes + } + + // Move to the next profile in the chain + chain = append(chain, nextProfile) + current = nextProfile + } + + // Shouldn't reach here unless chain is unreasonably deep + testlog.Warningf("Profile chain exceeded maximum depth of %d hops", maxHops) + return false +} + +// buildProfileIncludesMap creates a map of profile name -> list of profiles it includes +// This extracts only the include= statements, not the entire profile data +func buildProfileIncludesMap(tunedList *tunedv1.TunedList) map[string][]string { + result := make(map[string][]string) + + for _, tuned := range tunedList.Items { + for _, profile := range tuned.Spec.Profile { + if profile.Name == nil || profile.Data == nil { + continue + } + + profileName := *profile.Name + includes := extractIncludedProfiles(*profile.Data) + result[profileName] = includes + } + } + return result +} + +// extractIncludedProfiles parses the profile data using the gopkg.in/ini.v1 library +// (same library used by the Tuned controller) and extracts profile names from include= statements. +// For example, "include=profile-a,profile-b" returns ["profile-a", "profile-b"] +// Tuned variables like ${f:...} are skipped since they require runtime expansion. +func extractIncludedProfiles(profileData string) []string { + cfg, err := ini.Load([]byte(profileData)) + if err != nil || cfg == nil { + // Invalid INI data - return empty list + return []string{} + } + + // Check if the [main] section has an "include" key + if !cfg.Section("main").HasKey("include") { + return []string{} + } + + // Get the include value + includeValue := cfg.Section("main").Key("include").String() + if includeValue == "" { + return []string{} + } + + // Split by comma and filter out invalid entries + parts := strings.Split(includeValue, ",") + var includes []string + + for _, part := range parts { + part = strings.TrimSpace(part) + + // Skip empty parts + if part == "" { + continue + } + + // Skip if it contains variable syntax (${...}) - these need runtime expansion + if strings.Contains(part, "${") { + continue + } + + // Skip conditional loading prefixes (parts starting with -) + if strings.HasPrefix(part, "-") { + continue + } + + includes = append(includes, part) + } + return includes +} From d8e98529d886e0f8a6a23f951cec8e8431c79906 Mon Sep 17 00:00:00 2001 From: Niranjan MR Date: Mon, 29 Jun 2026 09:30:47 +0000 Subject: [PATCH 2/2] Return bool and error on function signatures - Fix Hypershift compatibility: Use DataPlaneClient for listing Tuned CRs On Hypershift clusters, tuned objects are on the data plane, not control plane. - Add error handling: Change function signatures to return (bool, error) Return errors for unexpected conditions (cycles, missing profiles, parsing failures) while returning (false, nil) for valid "not found" cases. Error cases: * Cycle detected in profile chain * Profile referenced but not found in cluster * Chain exceeded maximum depth * INI parsing failed Valid "not found" case: * Profile chain ends without reaching target (false, nil) - Add logging for better observability: * Log when INI parsing fails with the error details * Log when profiles have no 'include' key * Log when 'include' value is empty * Changed dead-end from Warning to Info (it's a normal case) --- .../functests/1_performance/performance.go | 64 ++++++++++++------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/test/e2e/performanceprofile/functests/1_performance/performance.go b/test/e2e/performanceprofile/functests/1_performance/performance.go index d82f88488..f866cd24f 100644 --- a/test/e2e/performanceprofile/functests/1_performance/performance.go +++ b/test/e2e/performanceprofile/functests/1_performance/performance.go @@ -113,8 +113,9 @@ var _ = Describe("[rfe_id:27368][performance]", Ordered, func() { performanceProfileName := components.GetComponentName(testutils.PerformanceProfileName, components.ProfileNamePerformance) // Get all Tuned profiles once (for lookup) + // Note: Tuned objects are on DataPlaneClient for Hypershift clusters tunedList := &tunedv1.TunedList{} - Expect(testclient.ControlPlaneClient.List(ctx, tunedList)).To(Succeed(), + Expect(testclient.DataPlaneClient.List(ctx, tunedList)).To(Succeed(), "failed to list Tuned profiles") for _, node := range workerRTNodes { @@ -133,7 +134,9 @@ var _ = Describe("[rfe_id:27368][performance]", Ordered, func() { testlog.Infof("Node %s has active tuned profile: %q", node.Name, activeProfile) // Check if this node's active profile traces back to the performance profile - tracesToPerformanceProfile := profileIncludesTarget(activeProfile, performanceProfileName, tunedList) + tracesToPerformanceProfile, err := profileIncludesTarget(activeProfile, performanceProfileName, tunedList) + g.Expect(err).ToNot(HaveOccurred(), + "failed to trace profile chain for node %s: %v", node.Name, err) g.Expect(tracesToPerformanceProfile).To(BeTrue(), "node %s active profile %q does not include performance profile %q", @@ -1445,9 +1448,15 @@ func findCondition(conditions []tunedv1.StatusCondition, conditionType string) * // then profileIncludesTarget("wrapper-b", "perf-profile", ...) returns true. // // This walks the include chain step by step until it finds the target or hits a dead end. -func profileIncludesTarget(startProfile, targetProfile string, tunedList *tunedv1.TunedList) bool { +// +// Returns (true, nil) if the chain is found, (false, nil) if no valid chain exists, +// or (false, error) if an error occurs during processing. +func profileIncludesTarget(startProfile, targetProfile string, tunedList *tunedv1.TunedList) (bool, error) { // Build a lookup map: profile name -> list of profiles it includes - includesMap := buildProfileIncludesMap(tunedList) + includesMap, err := buildProfileIncludesMap(tunedList) + if err != nil { + return false, fmt.Errorf("failed to build profile includes map: %w", err) + } // Walk the include chain, starting from startProfile current := startProfile @@ -1459,21 +1468,23 @@ func profileIncludesTarget(startProfile, targetProfile string, tunedList *tunedv // Found it? if current == targetProfile { testlog.Infof("Profile chain validated: %s", strings.Join(chain, " → ")) - return true + return true, nil } // Already visited? (cycle detection) if visited[current] { - testlog.Warningf("Cycle detected in profile chain at %q", current) - return false + cycleErr := fmt.Errorf("cycle detected in profile chain at %q", current) + testlog.Warningf("Profile chain traversal error: %v", cycleErr) + return false, cycleErr } visited[current] = true // What does the current profile include? includedProfiles, exists := includesMap[current] if !exists { - testlog.Warningf("Profile %q not found in cluster", current) - return false // Profile not found + missingErr := fmt.Errorf("profile %q not found in cluster", current) + testlog.Warningf("Profile lookup error: %v", missingErr) + return false, missingErr } // Check if it directly includes the target @@ -1481,7 +1492,7 @@ func profileIncludesTarget(startProfile, targetProfile string, tunedList *tunedv if included == targetProfile { chain = append(chain, targetProfile) testlog.Infof("Profile chain validated: %s", strings.Join(chain, " → ")) - return true + return true, nil } } @@ -1495,8 +1506,9 @@ func profileIncludesTarget(startProfile, targetProfile string, tunedList *tunedv } if nextProfile == "" { - testlog.Warningf("Dead end in profile chain at %q (no valid includes found)", current) - return false // Dead end - no more includes + // Dead end - profile doesn't include the target (this is a valid "not found" case) + testlog.Infof("Profile chain ends at %q (no path to target %q)", current, targetProfile) + return false, nil } // Move to the next profile in the chain @@ -1505,13 +1517,14 @@ func profileIncludesTarget(startProfile, targetProfile string, tunedList *tunedv } // Shouldn't reach here unless chain is unreasonably deep - testlog.Warningf("Profile chain exceeded maximum depth of %d hops", maxHops) - return false + depthErr := fmt.Errorf("profile chain exceeded maximum depth of %d hops", maxHops) + testlog.Warningf("Profile chain depth error: %v", depthErr) + return false, depthErr } // buildProfileIncludesMap creates a map of profile name -> list of profiles it includes // This extracts only the include= statements, not the entire profile data -func buildProfileIncludesMap(tunedList *tunedv1.TunedList) map[string][]string { +func buildProfileIncludesMap(tunedList *tunedv1.TunedList) (map[string][]string, error) { result := make(map[string][]string) for _, tuned := range tunedList.Items { @@ -1521,33 +1534,38 @@ func buildProfileIncludesMap(tunedList *tunedv1.TunedList) map[string][]string { } profileName := *profile.Name - includes := extractIncludedProfiles(*profile.Data) + includes, err := extractIncludedProfiles(*profile.Data) + if err != nil { + return nil, fmt.Errorf("failed to extract includes from profile %q: %w", profileName, err) + } result[profileName] = includes } } - return result + return result, nil } // extractIncludedProfiles parses the profile data using the gopkg.in/ini.v1 library // (same library used by the Tuned controller) and extracts profile names from include= statements. // For example, "include=profile-a,profile-b" returns ["profile-a", "profile-b"] // Tuned variables like ${f:...} are skipped since they require runtime expansion. -func extractIncludedProfiles(profileData string) []string { +func extractIncludedProfiles(profileData string) ([]string, error) { cfg, err := ini.Load([]byte(profileData)) if err != nil || cfg == nil { - // Invalid INI data - return empty list - return []string{} + testlog.Warningf("Failed to parse profile INI data: %v", err) + return nil, fmt.Errorf("invalid INI data: %w", err) } // Check if the [main] section has an "include" key if !cfg.Section("main").HasKey("include") { - return []string{} + testlog.Infof("Profile has no 'include' key in [main] section") + return []string{}, nil } // Get the include value includeValue := cfg.Section("main").Key("include").String() if includeValue == "" { - return []string{} + testlog.Infof("Profile 'include' key is empty") + return []string{}, nil } // Split by comma and filter out invalid entries @@ -1574,5 +1592,5 @@ func extractIncludedProfiles(profileData string) []string { includes = append(includes, part) } - return includes + return includes, nil }