From 4411fd6b2c52846a312560d6968dc31d6f1972db Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Thu, 16 Jul 2026 14:48:02 -0400 Subject: [PATCH 1/5] Update featuregate-test-analyzer output for Install features Augment `verify-feature-promotion` output to indicate pass percentage for `install should succeed` tests for featuregates that include "Install" in their name. This update gives a better indication of whether Install features are failing at installation or later during execution of e2e conformance tests. This update does not change the criteria for reporting success but adds more information in the output for easier analysis of feature state. --- .../codegen/cmd/featuregate-test-analyzer.go | 237 ++++++++++++++++-- .../cmd/featuregate-test-analyzer_test.go | 185 +++++++++++++- tools/codegen/pkg/sippy/json_types.go | 98 ++++++++ 3 files changed, 499 insertions(+), 21 deletions(-) diff --git a/tools/codegen/cmd/featuregate-test-analyzer.go b/tools/codegen/cmd/featuregate-test-analyzer.go index 9eda83a226b..175fcebae7b 100644 --- a/tools/codegen/cmd/featuregate-test-analyzer.go +++ b/tools/codegen/cmd/featuregate-test-analyzer.go @@ -35,6 +35,9 @@ const ( // required pass rate. // nearly all current tests pass 99% of the time, but in a two week window we lack enough data to say. requiredPassRateOfTestsPerVariant = 0.95 + + // required pass rate for "install should succeed" test + requiredPassRateForInstallTest = 1.0 ) type FeatureGateTestAnalyzerOptions struct { @@ -181,7 +184,7 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { clusterProfiles := recentlyEnabledFeatureGatesToClusterProfiles[enabledFeatureGate] md.Title(1, enabledFeatureGate) - testingResults, err := listTestResultFor(enabledFeatureGate, clusterProfiles) + testingResults, installTestLevelData, err := listTestResultFor(enabledFeatureGate, clusterProfiles) if err != nil { return err } @@ -190,7 +193,7 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { validationResults := checkIfTestingIsSufficient(enabledFeatureGate, testingResults) - // Separate warnings from blocking errors + // Separate warnings and blocking errors blockingErrors := []error{} warnings := []error{} for _, vr := range validationResults { @@ -201,6 +204,38 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { } } + // For Install feature gates, report "install should succeed: overall" test statistics first + if strings.Contains(enabledFeatureGate, "Install") { + md.Text("") + fmt.Fprintf(o.Out, "\n") + md.Textf("**Install test statistics for \"install should succeed: overall\":**\n") + fmt.Fprintf(o.Out, "Install test statistics for \"install should succeed: overall\":\n") + jobVariants := make([]JobVariant, 0, len(testingResults)) + for jobVariant := range testingResults { + jobVariants = append(jobVariants, jobVariant) + } + sort.Slice(jobVariants, func(i, j int) bool { + return jobVariants[i].String() < jobVariants[j].String() + }) + for _, jobVariant := range jobVariants { + installTest := installTestLevelData[jobVariant] + if installTest == nil { + md.Textf(" - %v: test not found\n", jobVariant) + fmt.Fprintf(o.Out, " %v: test not found\n", jobVariant) + } else if installTest.TotalRuns > 0 { + passPercent := float32(installTest.SuccessfulRuns) / float32(installTest.TotalRuns) + displayActual := int(passPercent * 100) + md.Textf(" - %v: passed %d%% (%d/%d runs)\n", jobVariant, displayActual, installTest.SuccessfulRuns, installTest.TotalRuns) + fmt.Fprintf(o.Out, " %v: passed %d%% (%d/%d runs)\n", jobVariant, displayActual, installTest.SuccessfulRuns, installTest.TotalRuns) + } else { + md.Textf(" - %v: 0 runs\n", jobVariant) + fmt.Fprintf(o.Out, " %v: 0 runs\n", jobVariant) + } + } + md.Text("") + fmt.Fprintf(o.Out, "\n") + } + if len(validationResults) == 0 { md.Textf("Sufficient CI testing for %q.\n", enabledFeatureGate) fmt.Fprintf(o.Out, "Sufficient CI testing for %q.\n", enabledFeatureGate) @@ -208,17 +243,23 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { if len(blockingErrors) > 0 { md.Textf("INSUFFICIENT CI testing for %q.\n", enabledFeatureGate) fmt.Fprintf(o.Out, "INSUFFICIENT CI testing for %q.\n", enabledFeatureGate) - } else { + } else if len(warnings) > 0 { md.Textf("CI testing issues found for %q (non-blocking warnings).\n", enabledFeatureGate) fmt.Fprintf(o.Out, "CI testing issues found for %q (non-blocking warnings).\n", enabledFeatureGate) + } else { + md.Textf("Sufficient CI testing for %q.\n", enabledFeatureGate) + fmt.Fprintf(o.Out, "Sufficient CI testing for %q.\n", enabledFeatureGate) } - md.Textf("* At least five tests are expected for a feature\n") - md.Textf("* Tests must be be run on every TechPreview platform (ask for an exception if your feature doesn't support a variant)") - md.Textf("* All tests must run at least 14 times on every platform") - md.Textf("* All tests must pass at least 95%% of the time") - md.Textf("* JobTier must be one of: standard, informing, blocking, candidate (candidate is allowed but produces a warning as it is not covered by Component Readiness)\n") - md.Text("") + if len(blockingErrors) > 0 || len(warnings) > 0 { + md.Textf("* At least five tests are expected for a feature\n") + md.Textf("* Tests must be be run on every TechPreview platform (ask for an exception if your feature doesn't support a variant)") + md.Textf("* All tests must run at least 14 times on every platform") + md.Textf("* All tests must pass at least 95%% of the time") + md.Textf("* For Install feature gates, the \"install should succeed: overall\" test must pass at least 100%% of the time") + md.Textf("* JobTier must be one of: standard, informing, blocking, candidate (candidate is allowed but produces a warning as it is not covered by Component Readiness)\n") + md.Text("") + } if len(warnings) > 0 { md.Textf("**Non-blocking warnings (optional variants):**\n") @@ -394,6 +435,11 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian } for _, testResults := range testedVariant.TestResults { + // Skip "install should succeed: overall" for Install feature gates - it has special validation below + if strings.Contains(featureGate, "Install") && testResults.TestName == "install should succeed: overall" { + continue + } + if testResults.TotalRuns < requiredNumberOfTestRunsPerVariant { results = append(results, ValidationResult{ Error: fmt.Errorf("error: %q only has %d runs, need at least %d runs for %q on %v", @@ -415,6 +461,38 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian }) } } + + // For Install feature gates, validate "install should succeed: overall" test + if strings.Contains(featureGate, "Install") { + installTest := testResultByName(testedVariant.TestResults, "install should succeed: overall") + if installTest == nil { + results = append(results, ValidationResult{ + Error: fmt.Errorf("warning: \"install should succeed: overall\" test not found for Install feature gate %q on %v", + featureGate, jobVariant), + IsWarning: true, + }) + } else { + if installTest.TotalRuns < requiredNumberOfTestRunsPerVariant { + results = append(results, ValidationResult{ + Error: fmt.Errorf("error: \"install should succeed: overall\" only has %d runs, need at least %d runs for %q on %v", + installTest.TotalRuns, requiredNumberOfTestRunsPerVariant, featureGate, jobVariant), + IsWarning: isOptional, + }) + } + if installTest.TotalRuns > 0 { + passPercent := float32(installTest.SuccessfulRuns) / float32(installTest.TotalRuns) + if passPercent < requiredPassRateForInstallTest { + displayExpected := int(requiredPassRateForInstallTest * 100) + displayActual := int(passPercent * 100) + results = append(results, ValidationResult{ + Error: fmt.Errorf("error: \"install should succeed: overall\" only passed %d%%, need at least %d%% for %q on %v", + displayActual, displayExpected, featureGate, jobVariant), + IsWarning: isOptional, + }) + } + } + } + } } return results @@ -622,6 +700,20 @@ type JobVariant struct { Optional bool // If true, validation failures for this variant are non-blocking warnings } +func (jv JobVariant) String() string { + result := fmt.Sprintf("cloud=%s arch=%s topology=%s", jv.Cloud, jv.Architecture, jv.Topology) + if jv.NetworkStack != "" { + result += fmt.Sprintf(" network=%s", jv.NetworkStack) + } + if jv.OS != "" { + result += fmt.Sprintf(" os=%s", jv.OS) + } + if jv.Optional { + result += " optional=true" + } + return result +} + type OrderedJobVariants []JobVariant func (a OrderedJobVariants) Len() int { return len(a) } @@ -694,6 +786,7 @@ type TestResults struct { type ValidationResult struct { Error error IsWarning bool // if true, this is a non-blocking warning (for optional variants) + IsInfo bool // if true, this is informational telemetry (e.g., install test pass percentage) } func testResultByName(results []TestResults, testName string) *TestResults { @@ -736,10 +829,11 @@ func validateJobTiers(jobVariant JobVariant) error { return nil } -func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (map[JobVariant]*TestingResults, error) { +func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (map[JobVariant]*TestingResults, map[JobVariant]*TestResults, error) { fmt.Printf("Query sippy for all test run results for feature gate %q on clusterProfile %q\n", featureGate, sets.List(clusterProfiles)) results := map[JobVariant]*TestingResults{} + installTestLevelData := map[JobVariant]*TestResults{} var jobVariantsToCheck []JobVariant if clusterProfiles.Has("Hypershift") && !nonHypershiftPlatforms.MatchString(featureGate) { @@ -760,19 +854,28 @@ func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (ma // Validate all variants before making expensive API calls for _, jobVariant := range jobVariantsToCheck { if err := validateJobTiers(jobVariant); err != nil { - return nil, err + return nil, nil, err } } for _, jobVariant := range jobVariantsToCheck { jobVariantResults, err := listTestResultForVariant(featureGate, jobVariant) if err != nil { - return nil, err + return nil, nil, err } results[jobVariant] = jobVariantResults + + // For Install feature gates, also get test-level data for "install should succeed: overall" + if strings.Contains(featureGate, "Install") { + installTestData, err := getInstallTestLevelData(featureGate, jobVariant) + if err != nil { + return nil, nil, err + } + installTestLevelData[jobVariant] = installTestData + } } - return results, nil + return results, installTestLevelData, nil } func filterVariants(featureGate string, variantsList ...[]JobVariant) []JobVariant { @@ -862,15 +965,111 @@ func getRelease() (string, error) { return getLatestRelease() } -func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*TestingResults, error) { - // Substring here matches for both [OCPFeatureGate:...] and [FeatureGate:...] - testPattern := fmt.Sprintf("FeatureGate:%s]", featureGate) +func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestResults, error) { + testPattern := "install should succeed: overall" + queries := sippy.QueriesForWithCapability(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology, + jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern, featureGate) + + defaultTransport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + } + + sippyClient := &http.Client{ + Timeout: 2 * time.Minute, + Transport: defaultTransport, + } + + release, err := getRelease() + if err != nil { + return nil, fmt.Errorf("couldn't fetch latest release version: %w", err) + } + + var installTestResult *TestResults + for _, currQuery := range queries { + currURL := &url.URL{ + Scheme: "https", + Host: "sippy.dptools.openshift.org", + Path: "api/tests", + } + queryParams := currURL.Query() + queryParams.Add("release", release) + queryParams.Add("period", "default") + filterJSON, err := json.Marshal(currQuery) + if err != nil { + return nil, err + } + queryParams.Add("filter", string(filterJSON)) + currURL.RawQuery = queryParams.Encode() + + req, err := http.NewRequest(http.MethodGet, currURL.String(), nil) + if err != nil { + return nil, err + } + + response, err := sippyClient.Do(req) + if err != nil { + return nil, err + } + if response.StatusCode < 200 || response.StatusCode > 299 { + return nil, fmt.Errorf("error getting sippy results (status=%d) for: %v", response.StatusCode, currURL.String()) + } + queryResultBytes, err := io.ReadAll(response.Body) + if err != nil { + return nil, err + } + response.Body.Close() + testInfos := []sippy.SippyTestInfo{} + if err := json.Unmarshal(queryResultBytes, &testInfos); err != nil { + return nil, err + } + + for _, currTest := range testInfos { + if installTestResult == nil { + installTestResult = &TestResults{ + TestName: currTest.Name, + } + } + + // Accumulate results across multiple JobTier queries + if currTest.CurrentRuns >= requiredNumberOfTestRunsPerVariant { + installTestResult.TotalRuns += currTest.CurrentRuns + installTestResult.SuccessfulRuns += currTest.CurrentSuccesses + installTestResult.FailedRuns += currTest.CurrentFailures + installTestResult.FlakedRuns += currTest.CurrentFlakes + } else { + installTestResult.TotalRuns += currTest.CurrentRuns + currTest.PreviousRuns + installTestResult.SuccessfulRuns += currTest.CurrentSuccesses + currTest.PreviousSuccesses + installTestResult.FailedRuns += currTest.CurrentFailures + currTest.PreviousFailures + installTestResult.FlakedRuns += currTest.CurrentFlakes + currTest.PreviousFlakes + } + } + } + + return installTestResult, nil +} + +func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*TestingResults, error) { // Feature gates used by the installer don't need separate tests, use the overall install tests if strings.Contains(featureGate, "Install") { return verifyJobBasedFeatureGatePromotion(featureGate, jobVariant) } + var testPattern string + var queries []*sippy.SippyQueryStruct + + // Substring here matches for both [OCPFeatureGate:...] and [FeatureGate:...] + testPattern = fmt.Sprintf("FeatureGate:%s]", featureGate) + queries = sippy.QueriesFor(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology, + jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern) fmt.Printf("Query sippy for all test run results for pattern %q on variant %#v\n", testPattern, jobVariant) defaultTransport := &http.Transport{ @@ -892,12 +1091,10 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi testNameToResults := map[string]*TestResults{} hasCandidateTierResults := false - queries := sippy.QueriesFor(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology, jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern) release, err := getRelease() if err != nil { return nil, fmt.Errorf("couldn't fetch latest release version: %w", err) } - fmt.Printf("Querying sippy release %s for test run results\n", release) for _, currQuery := range queries { currURL := &url.URL{ @@ -1139,7 +1336,7 @@ func getJobsForFeatureGateFromSippy(client *http.Client, release, featureGate st defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("expected a 200 OK status code but got %s", resp.StatusCode) + return nil, fmt.Errorf("expected a 200 OK status code but got %d", resp.StatusCode) } body, err := io.ReadAll(resp.Body) @@ -1165,7 +1362,7 @@ func getJobRunsFromSippy(client *http.Client, release, jobName string) ([]sippy. defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("expected a 200 OK status code but got %s", resp.StatusCode) + return nil, fmt.Errorf("expected a 200 OK status code but got %d", resp.StatusCode) } body, err := io.ReadAll(resp.Body) diff --git a/tools/codegen/cmd/featuregate-test-analyzer_test.go b/tools/codegen/cmd/featuregate-test-analyzer_test.go index 24d2bad6c20..2c22e5da4e9 100644 --- a/tools/codegen/cmd/featuregate-test-analyzer_test.go +++ b/tools/codegen/cmd/featuregate-test-analyzer_test.go @@ -65,7 +65,7 @@ func Test_listTestResultFor(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Skip("this is for ease of manual testing") - got, err := listTestResultFor(tt.args.featureGate, sets.New[string](tt.args.clusterProfile)) + got, _, err := listTestResultFor(tt.args.featureGate, sets.New[string](tt.args.clusterProfile)) if (err != nil) != tt.wantErr { t.Errorf("listTestResultFor() error = %v, wantErr %v", err, tt.wantErr) return @@ -481,6 +481,189 @@ func Test_checkIfTestingIsSufficient_OptionalVariants(t *testing.T) { } } +func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { + tests := []struct { + name string + featureGate string + testingResults map[JobVariant]*TestingResults + wantBlockingErrors int + wantWarnings int + }{ + { + name: "Install feature gate with install should succeed: overall test failing 100% requirement", + featureGate: "FakeInstallFeature", + testingResults: map[JobVariant]*TestingResults{ + { + Cloud: "metal", + Architecture: "amd64", + Topology: "ha", + }: { + TestResults: []TestResults{ + {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 19}, + {TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15}, + }, + }, + }, + wantBlockingErrors: 1, // Blocking error: install should succeed: overall must pass at 100% + wantWarnings: 0, + }, + { + name: "Install feature gate without install should succeed: overall test - warning reported", + featureGate: "MockInstallGate", + testingResults: map[JobVariant]*TestingResults{ + { + Cloud: "metal", + Architecture: "amd64", + Topology: "ha", + }: { + TestResults: []TestResults{ + {TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test5", TotalRuns: 15, SuccessfulRuns: 15}, + }, + }, + }, + wantBlockingErrors: 0, + wantWarnings: 1, // Warning about missing "install should succeed: overall" test + }, + { + name: "Non-Install feature gate with install should succeed: overall test - no special validation", + featureGate: "SomeOtherFeature", + testingResults: map[JobVariant]*TestingResults{ + { + Cloud: "aws", + Architecture: "amd64", + Topology: "ha", + }: { + TestResults: []TestResults{ + {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 19}, + {TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15}, + }, + }, + }, + wantBlockingErrors: 0, + wantWarnings: 0, + }, + { + name: "Install feature gate with 100% pass rate - no errors", + featureGate: "FakeInstallFeature", + testingResults: map[JobVariant]*TestingResults{ + { + Cloud: "aws", + Architecture: "amd64", + Topology: "ha", + }: { + TestResults: []TestResults{ + {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 20}, + {TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15}, + }, + }, + }, + wantBlockingErrors: 0, + wantWarnings: 0, + }, + { + name: "Install feature gate with multiple variants - one fails 100% requirement", + featureGate: "FakeInstallFeature", + testingResults: map[JobVariant]*TestingResults{ + { + Cloud: "metal", + Architecture: "amd64", + Topology: "ha", + }: { + TestResults: []TestResults{ + {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 19}, + {TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15}, + }, + }, + { + Cloud: "metal", + Architecture: "amd64", + Topology: "single", + }: { + TestResults: []TestResults{ + {TestName: "install should succeed: overall", TotalRuns: 18, SuccessfulRuns: 18}, + {TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15}, + }, + }, + }, + wantBlockingErrors: 1, // One variant (ha) fails 100% requirement + wantWarnings: 0, + }, + { + name: "Install feature gate with insufficient runs for install test - blocking error", + featureGate: "MockInstallGate", + testingResults: map[JobVariant]*TestingResults{ + { + Cloud: "aws", + Architecture: "amd64", + Topology: "ha", + }: { + TestResults: []TestResults{ + {TestName: "install should succeed: overall", TotalRuns: 10, SuccessfulRuns: 10}, + {TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15}, + {TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15}, + }, + }, + }, + wantBlockingErrors: 1, // Blocking error for insufficient runs (< 14) + wantWarnings: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults) + + blockingErrors := 0 + warnings := 0 + for _, result := range results { + if result.IsWarning { + warnings++ + } else { + blockingErrors++ + } + } + + if blockingErrors != tt.wantBlockingErrors { + t.Errorf("got %d blocking errors, want %d", blockingErrors, tt.wantBlockingErrors) + for _, result := range results { + if !result.IsWarning { + t.Logf(" Blocking error: %v", result.Error) + } + } + } + if warnings != tt.wantWarnings { + t.Errorf("got %d warnings, want %d", warnings, tt.wantWarnings) + for _, result := range results { + if result.IsWarning { + t.Logf(" Warning: %v", result.Error) + } + } + } + }) + } +} + func Test_defaultQueriesIncludeCandidateTier(t *testing.T) { // When JobTiers is empty, QueriesFor should generate queries for all tiers // including candidate. This test is added to prevent regressions for candidate-tier diff --git a/tools/codegen/pkg/sippy/json_types.go b/tools/codegen/pkg/sippy/json_types.go index ad2012a3ef5..2242eab6ac9 100644 --- a/tools/codegen/pkg/sippy/json_types.go +++ b/tools/codegen/pkg/sippy/json_types.go @@ -179,6 +179,104 @@ func QueriesFor(cloud, architecture, topology, networkStack, os, jobTiers, testP return queries } +func QueriesForWithCapability(cloud, architecture, topology, networkStack, os, jobTiers, testPattern, capability string) []*SippyQueryStruct { + // Build base query items that are common to all JobTier queries + baseItems := []SippyQueryItem{ + { + ColumnField: "variants", + Not: false, + OperatorValue: "contains", + Value: fmt.Sprintf("Platform:%s", cloud), + }, + { + ColumnField: "variants", + Not: false, + OperatorValue: "contains", + Value: fmt.Sprintf("Architecture:%s", architecture), + }, + { + ColumnField: "variants", + Not: false, + OperatorValue: "contains", + Value: fmt.Sprintf("Topology:%s", topology), + }, + { + ColumnField: "name", + Not: false, + OperatorValue: "contains", + Value: testPattern, + }, + { + ColumnField: "variants", + Not: false, + OperatorValue: "contains", + Value: fmt.Sprintf("Capability:%s", capability), + }, + } + + if networkStack != "" { + baseItems = append(baseItems, SippyQueryItem{ + ColumnField: "variants", + Not: false, + OperatorValue: "contains", + Value: fmt.Sprintf("NetworkStack:%s", networkStack), + }) + } + + if os != "" { + baseItems = append(baseItems, SippyQueryItem{ + ColumnField: "variants", + Not: false, + OperatorValue: "contains", + Value: fmt.Sprintf("OS:%s", os), + }) + } + + // Parse JobTiers - comma-separated string, default to standard/informing/blocking/candidate if empty + var jobTiersList []string + if jobTiers == "" { + jobTiersList = []string{"standard", "informing", "blocking", "candidate"} + } else { + // Split by comma, trim whitespace, and deduplicate using sets + tierSet := sets.New[string]() + for _, tier := range strings.Split(jobTiers, ",") { + if trimmed := strings.TrimSpace(tier); trimmed != "" { + tierSet.Insert(trimmed) + } + } + // If all tiers were whitespace/empty after trimming, use defaults + if tierSet.Len() == 0 { + jobTiersList = []string{"standard", "informing", "blocking", "candidate"} + } else { + jobTiersList = sets.List(tierSet) + } + } + + // Generate one query per JobTier (to work around API's single LinkOperator limitation) + var queries []*SippyQueryStruct + for _, jobTier := range jobTiersList { + // Copy base items for this query + items := make([]SippyQueryItem, len(baseItems)) + copy(items, baseItems) + + // Add JobTier filter + items = append(items, SippyQueryItem{ + ColumnField: "variants", + Not: false, + OperatorValue: "contains", + Value: fmt.Sprintf("JobTier:%s", jobTier), + }) + + queries = append(queries, &SippyQueryStruct{ + Items: items, + LinkOperator: "and", + TierName: jobTier, + }) + } + + return queries +} + func BuildSippyTestAnalysisURL(release, testName, topology, cloud, architecture, networkStack, os string) string { filterItems := []SippyQueryItem{ { From 816c32e63cf12c9dbd404cc27795adf0d664d389 Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Mon, 20 Jul 2026 16:43:41 -0400 Subject: [PATCH 2/5] Always include metal single and compact variants as optional checks --- .../codegen/cmd/featuregate-test-analyzer.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tools/codegen/cmd/featuregate-test-analyzer.go b/tools/codegen/cmd/featuregate-test-analyzer.go index 175fcebae7b..5e36b50b18b 100644 --- a/tools/codegen/cmd/featuregate-test-analyzer.go +++ b/tools/codegen/cmd/featuregate-test-analyzer.go @@ -217,7 +217,13 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { sort.Slice(jobVariants, func(i, j int) bool { return jobVariants[i].String() < jobVariants[j].String() }) + prevCloud := "" for _, jobVariant := range jobVariants { + if prevCloud != "" && jobVariant.Cloud != prevCloud { + md.Text("") + fmt.Fprintf(o.Out, "\n") + } + prevCloud = jobVariant.Cloud installTest := installTestLevelData[jobVariant] if installTest == nil { md.Textf(" - %v: test not found\n", jobVariant) @@ -677,6 +683,16 @@ var ( NetworkStack: "dual", JobTiers: "candidate,standard,informing,blocking", }, + { + Cloud: "metal", + Architecture: "amd64", + Topology: "single", + }, + { + Cloud: "metal", + Architecture: "amd64", + Topology: "compact", + }, } nonHypershiftPlatforms = regexp.MustCompile("(?i)nutanix|metal|vsphere|openstack|azure|gcp") @@ -849,6 +865,13 @@ func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (ma } jobVariantsToCheck = append(jobVariantsToCheck, selfManagedPlatformVariants...) + + // Always include metal single and compact variants as optional checks + for _, v := range optionalSelfManagedPlatformVariants { + if strings.ToLower(v.Cloud) == "metal" && (strings.ToLower(v.Topology) == "single" || strings.ToLower(v.Topology) == "compact") { + jobVariantsToCheck = append(jobVariantsToCheck, v) + } + } } // Validate all variants before making expensive API calls From 13d08e25f6237e756f4e4a09f48f4b290839923f Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Wed, 22 Jul 2026 12:30:10 -0400 Subject: [PATCH 3/5] Exclude job runs with internal and external infrastrcuture failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignore runs with internal and external infrastrcuture failures while calculating pass percentages. The fix does the following: 1. Fetches all jobs for the feature gate variant 2. Fetches individual runs for each job 3. Skips any run with OverallResult == "N" or "n" (infra failures) entirely 4. For remaining runs: checks if "install should succeed: overall" is in FailedTestNames — if not, it's a success --- .../codegen/cmd/featuregate-test-analyzer.go | 178 +++++++++++++++--- 1 file changed, 155 insertions(+), 23 deletions(-) diff --git a/tools/codegen/cmd/featuregate-test-analyzer.go b/tools/codegen/cmd/featuregate-test-analyzer.go index 5e36b50b18b..f6d807dcca5 100644 --- a/tools/codegen/cmd/featuregate-test-analyzer.go +++ b/tools/codegen/cmd/featuregate-test-analyzer.go @@ -191,16 +191,19 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { writeTestingMarkDown(testingResults, md) - validationResults := checkIfTestingIsSufficient(enabledFeatureGate, testingResults) + validationResults := checkIfTestingIsSufficient(enabledFeatureGate, testingResults, installTestLevelData) // Separate warnings and blocking errors blockingErrors := []error{} warnings := []error{} + var blockingResults, warningResults []ValidationResult for _, vr := range validationResults { if vr.IsWarning { warnings = append(warnings, vr.Error) + warningResults = append(warningResults, vr) } else { blockingErrors = append(blockingErrors, vr.Error) + blockingResults = append(blockingResults, vr) } } @@ -269,24 +272,20 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { if len(warnings) > 0 { md.Textf("**Non-blocking warnings (optional variants):**\n") - for _, warn := range warnings { - md.Textf(" - %s\n", warn.Error()) - } + writeGroupedValidationResults(warningResults, md) md.Text("") } if len(blockingErrors) > 0 { md.Textf("**Blocking errors:**\n") - for _, err := range blockingErrors { - md.Textf(" - %s\n", err.Error()) - } + writeGroupedValidationResults(blockingResults, md) md.Text("") } md.Text("") } // Only add blocking errors to the error list (warnings don't fail the job) - errs = append(errs, blockingErrors...) + errs = append(errs, groupErrorsByCategory(blockingResults)...) featureGateHTMLData = append(featureGateHTMLData, buildHTMLFeatureGateData(enabledFeatureGate, testingResults, blockingErrors, release)) } @@ -412,7 +411,7 @@ func writeHTMLFromTemplate(filename string, featureGateHTMLData []utils.HTMLFeat return nil } -func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVariant]*TestingResults) []ValidationResult { +func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVariant]*TestingResults, installTestLevelData map[JobVariant]*TestResults) []ValidationResult { results := []ValidationResult{} for jobVariant, testedVariant := range testingResults { @@ -429,14 +428,20 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian Error: fmt.Errorf("warning: variant %v includes test data from candidate-tier jobs which are not covered by Component Readiness and lack standard regression protection", jobVariant), IsWarning: true, + Category: CategoryCandidateTier, }) } + if len(testedVariant.TestResults) == 0 { + continue + } + if len(testedVariant.TestResults) < requiredNumberOfTests { results = append(results, ValidationResult{ Error: fmt.Errorf("error: only %d tests found, need at least %d for %q on %v", len(testedVariant.TestResults), requiredNumberOfTests, featureGate, jobVariant), IsWarning: isOptional, + Category: CategoryInsufficientTests, }) } @@ -450,7 +455,8 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian results = append(results, ValidationResult{ Error: fmt.Errorf("error: %q only has %d runs, need at least %d runs for %q on %v", testResults.TestName, testResults.TotalRuns, requiredNumberOfTestRunsPerVariant, featureGate, jobVariant), - IsWarning: isOptional, + IsWarning: isOptional, + Category: CategoryInsufficientRuns, }) } if testResults.TotalRuns == 0 { @@ -464,18 +470,20 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian Error: fmt.Errorf("error: %q only passed %d%%, need at least %d%% for %q on %v", testResults.TestName, displayActual, displayExpected, featureGate, jobVariant), IsWarning: isOptional, + Category: CategoryPassRate, }) } } - // For Install feature gates, validate "install should succeed: overall" test + // For Install feature gates, validate "install should succeed: overall" using test-level data from Sippy if strings.Contains(featureGate, "Install") { - installTest := testResultByName(testedVariant.TestResults, "install should succeed: overall") + installTest := installTestLevelData[jobVariant] if installTest == nil { results = append(results, ValidationResult{ - Error: fmt.Errorf("warning: \"install should succeed: overall\" test not found for Install feature gate %q on %v", + Error: fmt.Errorf("error: \"install should succeed: overall\" test data not found for Install feature gate %q on %v", featureGate, jobVariant), - IsWarning: true, + IsWarning: false, + Category: CategoryInstallTest, }) } else { if installTest.TotalRuns < requiredNumberOfTestRunsPerVariant { @@ -483,6 +491,7 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian Error: fmt.Errorf("error: \"install should succeed: overall\" only has %d runs, need at least %d runs for %q on %v", installTest.TotalRuns, requiredNumberOfTestRunsPerVariant, featureGate, jobVariant), IsWarning: isOptional, + Category: CategoryInstallTest, }) } if installTest.TotalRuns > 0 { @@ -494,6 +503,7 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian Error: fmt.Errorf("error: \"install should succeed: overall\" only passed %d%%, need at least %d%% for %q on %v", displayActual, displayExpected, featureGate, jobVariant), IsWarning: isOptional, + Category: CategoryInstallTest, }) } } @@ -504,6 +514,43 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian return results } +func groupErrorsByCategory(results []ValidationResult) []error { + categoryOrder := []ValidationCategory{} + grouped := map[ValidationCategory][]ValidationResult{} + for _, vr := range results { + if _, seen := grouped[vr.Category]; !seen { + categoryOrder = append(categoryOrder, vr.Category) + } + grouped[vr.Category] = append(grouped[vr.Category], vr) + } + var errs []error + for _, cat := range categoryOrder { + for _, vr := range grouped[cat] { + errs = append(errs, vr.Error) + } + } + return errs +} + +func writeGroupedValidationResults(results []ValidationResult, md *utils.Markdown) { + categoryOrder := []ValidationCategory{} + grouped := map[ValidationCategory][]ValidationResult{} + for _, vr := range results { + if _, seen := grouped[vr.Category]; !seen { + categoryOrder = append(categoryOrder, vr.Category) + } + grouped[vr.Category] = append(grouped[vr.Category], vr) + } + for i, cat := range categoryOrder { + if i > 0 { + md.Text("") + } + for _, vr := range grouped[cat] { + md.Textf(" - %s\n", vr.Error.Error()) + } + } +} + func writeTestingMarkDown(testingResults map[JobVariant]*TestingResults, md *utils.Markdown) { jobVariantsSet := sets.KeySet(testingResults) jobVariants := jobVariantsSet.UnsortedList() @@ -798,11 +845,22 @@ type TestResults struct { FlakedRuns int } +type ValidationCategory string + +const ( + CategoryCandidateTier ValidationCategory = "candidate-tier" + CategoryInsufficientTests ValidationCategory = "insufficient-tests" + CategoryInsufficientRuns ValidationCategory = "insufficient-runs" + CategoryPassRate ValidationCategory = "pass-rate" + CategoryInstallTest ValidationCategory = "install-test" +) + // ValidationResult represents a validation error or warning type ValidationResult struct { Error error - IsWarning bool // if true, this is a non-blocking warning (for optional variants) - IsInfo bool // if true, this is informational telemetry (e.g., install test pass percentage) + IsWarning bool // if true, this is a non-blocking warning (for optional variants) + IsInfo bool // if true, this is informational telemetry (e.g., install test pass percentage) + Category ValidationCategory // groups related results together in output } func testResultByName(results []TestResults, testName string) *TestResults { @@ -888,9 +946,10 @@ func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (ma } results[jobVariant] = jobVariantResults - // For Install feature gates, also get test-level data for "install should succeed: overall" + // For Install feature gates, count "install should succeed: overall" results + // directly from individual job runs, excluding infrastructure failures entirely. if strings.Contains(featureGate, "Install") { - installTestData, err := getInstallTestLevelData(featureGate, jobVariant) + installTestData, err := getInstallTestResultsFromJobRuns(featureGate, jobVariant) if err != nil { return nil, nil, err } @@ -988,6 +1047,69 @@ func getRelease() (string, error) { return getLatestRelease() } +func getInstallTestResultsFromJobRuns(featureGate string, jobVariant JobVariant) (*TestResults, error) { + release, err := getRelease() + if err != nil { + return nil, fmt.Errorf("couldn't determine release: %w", err) + } + + defaultTransport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + } + sippyClient := &http.Client{ + Timeout: 2 * time.Minute, + Transport: defaultTransport, + } + + jobs, err := getJobsForFeatureGateFromSippy(sippyClient, release, featureGate, jobVariant) + if err != nil { + return nil, fmt.Errorf("getting jobs for install test results: %w", err) + } + + testResults := &TestResults{ + TestName: "install should succeed: overall", + } + + for _, job := range jobs { + jobRuns, err := getJobRunsFromSippy(sippyClient, release, job.Name) + if err != nil { + return nil, fmt.Errorf("getting job runs for %q: %w", job.Name, err) + } + for _, jobRun := range jobRuns { + if jobRun.OverallResult == "N" || jobRun.OverallResult == "n" { + continue + } + testResults.TotalRuns++ + installFailed := false + for _, failure := range jobRun.FailedTestNames { + if failure == "install should succeed: overall" { + installFailed = true + break + } + } + if installFailed { + testResults.FailedRuns++ + } else { + testResults.SuccessfulRuns++ + } + } + } + + if testResults.TotalRuns == 0 { + return nil, nil + } + + return testResults, nil +} + func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestResults, error) { testPattern := "install should succeed: overall" queries := sippy.QueriesForWithCapability(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology, @@ -1311,17 +1433,25 @@ func verifyJobPassRate(client *http.Client, release string, job sippy.SippyJob, return nil, fmt.Errorf("getting job %q results from sippy: %w", job.Name, err) } - testResults := &TestResults{ - TestName: job.Name, - TotalRuns: len(jobRuns), - } - triagedTestFailures, err := getTriagedTestFailuresFromSippy(client, release, variant) if err != nil { return nil, fmt.Errorf("getting triaged test failures from sippy: %w", err) } + fmt.Printf("\nIgnoring job runs that have internal or external infrastructure failures from our analysis.\n\n") + + infraFailures := 0 + testResults := &TestResults{ + TestName: job.Name, + TotalRuns: len(jobRuns), + } + for _, jobRun := range jobRuns { + if jobRun.OverallResult == "N" || jobRun.OverallResult == "n" { + infraFailures++ + continue + } + if jobRun.OverallResult == "F" && !jobRun.KnownFailure { untriagedTestFailures := []string{} @@ -1348,6 +1478,8 @@ func verifyJobPassRate(client *http.Client, release string, job sippy.SippyJob, testResults.SuccessfulRuns++ } + testResults.TotalRuns -= infraFailures + return testResults, nil } From 31330413d0192a6fb49adf6aaf06b77ce08ce004 Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Thu, 23 Jul 2026 12:41:33 -0400 Subject: [PATCH 4/5] Fix Install feature gates not needing 5 new tests Install feature gates require entirely new jobs to be created that execise the new feature. New tests may be added and existing tests are usually updated to account for the new feature. Adjust the feature gate promotion verification code to account for that. --- .../codegen/cmd/featuregate-test-analyzer.go | 32 +++++++++++++------ tools/codegen/cmd/root.go | 2 +- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/tools/codegen/cmd/featuregate-test-analyzer.go b/tools/codegen/cmd/featuregate-test-analyzer.go index f6d807dcca5..f2f9919043e 100644 --- a/tools/codegen/cmd/featuregate-test-analyzer.go +++ b/tools/codegen/cmd/featuregate-test-analyzer.go @@ -284,8 +284,9 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { md.Text("") } - // Only add blocking errors to the error list (warnings don't fail the job) - errs = append(errs, groupErrorsByCategory(blockingResults)...) + if len(blockingErrors) > 0 { + errs = append(errs, blockingErrors...) + } featureGateHTMLData = append(featureGateHTMLData, buildHTMLFeatureGateData(enabledFeatureGate, testingResults, blockingErrors, release)) } @@ -303,7 +304,10 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { } } - return errors.Join(errs...) + if len(errs) > 0 { + return fmt.Errorf("feature gate promotion validation errors:\n%w", errors.Join(errs...)) + } + return nil } func topologyDisplayName(topology string) string { @@ -437,12 +441,22 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian } if len(testedVariant.TestResults) < requiredNumberOfTests { - results = append(results, ValidationResult{ - Error: fmt.Errorf("error: only %d tests found, need at least %d for %q on %v", - len(testedVariant.TestResults), requiredNumberOfTests, featureGate, jobVariant), - IsWarning: isOptional, - Category: CategoryInsufficientTests, - }) + if strings.Contains(featureGate, "Install") { + results = append(results, ValidationResult{ + Error: fmt.Errorf("info: %d tests found for %q on %v", + len(testedVariant.TestResults), featureGate, jobVariant), + IsWarning: true, + IsInfo: true, + Category: CategoryInsufficientTests, + }) + } else { + results = append(results, ValidationResult{ + Error: fmt.Errorf("error: only %d tests found, need at least %d for %q on %v", + len(testedVariant.TestResults), requiredNumberOfTests, featureGate, jobVariant), + IsWarning: isOptional, + Category: CategoryInsufficientTests, + }) + } } for _, testResults := range testedVariant.TestResults { diff --git a/tools/codegen/cmd/root.go b/tools/codegen/cmd/root.go index 55236924461..b35479e362f 100644 --- a/tools/codegen/cmd/root.go +++ b/tools/codegen/cmd/root.go @@ -61,7 +61,7 @@ var rootCmd = &cobra.Command{ func main() { err := rootCmd.Execute() if err != nil { - klog.Fatalf("Error running codegen: %v", err) + klog.Fatalf("%v", err) } } From b218be2751382f929651e8a1b4f82e34c46b432b Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Thu, 23 Jul 2026 14:02:43 -0400 Subject: [PATCH 5/5] Address code review comments and fix unit tests --- .../codegen/cmd/featuregate-test-analyzer.go | 179 ++++++++---------- .../cmd/featuregate-test-analyzer_test.go | 29 ++- 2 files changed, 99 insertions(+), 109 deletions(-) diff --git a/tools/codegen/cmd/featuregate-test-analyzer.go b/tools/codegen/cmd/featuregate-test-analyzer.go index f2f9919043e..73071004b5b 100644 --- a/tools/codegen/cmd/featuregate-test-analyzer.go +++ b/tools/codegen/cmd/featuregate-test-analyzer.go @@ -2,7 +2,6 @@ package main import ( "context" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -173,7 +172,9 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { fmt.Fprintf(o.Out, "No new Default FeatureGates found.\n") } - release, err := getRelease() + sippyCtx := context.Background() + + release, err := getRelease(sippyCtx) if err != nil { return fmt.Errorf("couldn't determine release version: %w", err) } @@ -184,7 +185,7 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { clusterProfiles := recentlyEnabledFeatureGatesToClusterProfiles[enabledFeatureGate] md.Title(1, enabledFeatureGate) - testingResults, installTestLevelData, err := listTestResultFor(enabledFeatureGate, clusterProfiles) + testingResults, installTestLevelData, err := listTestResultFor(sippyCtx, enabledFeatureGate, clusterProfiles) if err != nil { return err } @@ -271,7 +272,7 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { } if len(warnings) > 0 { - md.Textf("**Non-blocking warnings (optional variants):**\n") + md.Textf("**Non-blocking warnings and informational results:**\n") writeGroupedValidationResults(warningResults, md) md.Text("") } @@ -496,7 +497,7 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian results = append(results, ValidationResult{ Error: fmt.Errorf("error: \"install should succeed: overall\" test data not found for Install feature gate %q on %v", featureGate, jobVariant), - IsWarning: false, + IsWarning: isOptional, Category: CategoryInstallTest, }) } else { @@ -748,11 +749,13 @@ var ( Cloud: "metal", Architecture: "amd64", Topology: "single", + Optional: true, }, { Cloud: "metal", Architecture: "amd64", Topology: "compact", + Optional: true, }, } @@ -917,7 +920,7 @@ func validateJobTiers(jobVariant JobVariant) error { return nil } -func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (map[JobVariant]*TestingResults, map[JobVariant]*TestResults, error) { +func listTestResultFor(ctx context.Context, featureGate string, clusterProfiles sets.Set[string]) (map[JobVariant]*TestingResults, map[JobVariant]*TestResults, error) { fmt.Printf("Query sippy for all test run results for feature gate %q on clusterProfile %q\n", featureGate, sets.List(clusterProfiles)) results := map[JobVariant]*TestingResults{} @@ -954,7 +957,7 @@ func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (ma } for _, jobVariant := range jobVariantsToCheck { - jobVariantResults, err := listTestResultForVariant(featureGate, jobVariant) + jobVariantResults, err := listTestResultForVariant(ctx, featureGate, jobVariant) if err != nil { return nil, nil, err } @@ -963,7 +966,7 @@ func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (ma // For Install feature gates, count "install should succeed: overall" results // directly from individual job runs, excluding infrastructure failures entirely. if strings.Contains(featureGate, "Install") { - installTestData, err := getInstallTestResultsFromJobRuns(featureGate, jobVariant) + installTestData, err := getInstallTestResultsFromJobRuns(ctx, featureGate, jobVariant) if err != nil { return nil, nil, err } @@ -994,9 +997,13 @@ func filterVariants(featureGate string, variantsList ...[]JobVariant) []JobVaria } // getLatestRelease returns the latest release from Sippy. -func getLatestRelease() (string, error) { +func getLatestRelease(ctx context.Context) (string, error) { releaseAPI := "https://sippy.dptools.openshift.org/api/releases" - resp, err := http.Get(releaseAPI) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, releaseAPI, nil) + if err != nil { + return "", fmt.Errorf("creating release API request: %v", err) + } + resp, err := http.DefaultClient.Do(req) if err != nil { return "", fmt.Errorf("error fetching data from API: %v", err) } @@ -1050,7 +1057,7 @@ func getLatestRelease() (string, error) { return latestRelease, nil } -func getRelease() (string, error) { +func getRelease(ctx context.Context) (string, error) { // if its not main branch, then use the ENV var to determine the release version currentRelease := os.Getenv("PULL_BASE_REF") if strings.Contains(currentRelease, "release-") { @@ -1058,32 +1065,32 @@ func getRelease() (string, error) { return strings.TrimPrefix(currentRelease, "release-"), nil } // means its main branch - return getLatestRelease() + return getLatestRelease(ctx) +} + +func newSippyClient() *http.Client { + return &http.Client{ + Timeout: 2 * time.Minute, + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + }, + } } -func getInstallTestResultsFromJobRuns(featureGate string, jobVariant JobVariant) (*TestResults, error) { - release, err := getRelease() +func getInstallTestResultsFromJobRuns(ctx context.Context, featureGate string, jobVariant JobVariant) (*TestResults, error) { + release, err := getRelease(ctx) if err != nil { return nil, fmt.Errorf("couldn't determine release: %w", err) } - defaultTransport := &http.Transport{ - Proxy: http.ProxyFromEnvironment, - ForceAttemptHTTP2: true, - MaxIdleConns: 100, - IdleConnTimeout: 90 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - } - sippyClient := &http.Client{ - Timeout: 2 * time.Minute, - Transport: defaultTransport, - } + sippyClient := newSippyClient() - jobs, err := getJobsForFeatureGateFromSippy(sippyClient, release, featureGate, jobVariant) + jobs, err := getJobsForFeatureGateFromSippy(ctx, sippyClient, release, featureGate, jobVariant) if err != nil { return nil, fmt.Errorf("getting jobs for install test results: %w", err) } @@ -1093,7 +1100,7 @@ func getInstallTestResultsFromJobRuns(featureGate string, jobVariant JobVariant) } for _, job := range jobs { - jobRuns, err := getJobRunsFromSippy(sippyClient, release, job.Name) + jobRuns, err := getJobRunsFromSippy(ctx, sippyClient, release, job.Name) if err != nil { return nil, fmt.Errorf("getting job runs for %q: %w", job.Name, err) } @@ -1124,29 +1131,14 @@ func getInstallTestResultsFromJobRuns(featureGate string, jobVariant JobVariant) return testResults, nil } -func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestResults, error) { +func getInstallTestLevelData(ctx context.Context, featureGate string, jobVariant JobVariant) (*TestResults, error) { testPattern := "install should succeed: overall" queries := sippy.QueriesForWithCapability(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology, jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern, featureGate) - defaultTransport := &http.Transport{ - Proxy: http.ProxyFromEnvironment, - ForceAttemptHTTP2: true, - MaxIdleConns: 100, - IdleConnTimeout: 90 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - } - - sippyClient := &http.Client{ - Timeout: 2 * time.Minute, - Transport: defaultTransport, - } + sippyClient := newSippyClient() - release, err := getRelease() + release, err := getRelease(ctx) if err != nil { return nil, fmt.Errorf("couldn't fetch latest release version: %w", err) } @@ -1168,7 +1160,7 @@ func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestRe queryParams.Add("filter", string(filterJSON)) currURL.RawQuery = queryParams.Encode() - req, err := http.NewRequest(http.MethodGet, currURL.String(), nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, currURL.String(), nil) if err != nil { return nil, err } @@ -1177,6 +1169,7 @@ func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestRe if err != nil { return nil, err } + defer response.Body.Close() if response.StatusCode < 200 || response.StatusCode > 299 { return nil, fmt.Errorf("error getting sippy results (status=%d) for: %v", response.StatusCode, currURL.String()) } @@ -1184,7 +1177,6 @@ func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestRe if err != nil { return nil, err } - response.Body.Close() testInfos := []sippy.SippyTestInfo{} if err := json.Unmarshal(queryResultBytes, &testInfos); err != nil { @@ -1216,10 +1208,10 @@ func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestRe return installTestResult, nil } -func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*TestingResults, error) { +func listTestResultForVariant(ctx context.Context, featureGate string, jobVariant JobVariant) (*TestingResults, error) { // Feature gates used by the installer don't need separate tests, use the overall install tests if strings.Contains(featureGate, "Install") { - return verifyJobBasedFeatureGatePromotion(featureGate, jobVariant) + return verifyJobBasedFeatureGatePromotion(ctx, featureGate, jobVariant) } var testPattern string @@ -1231,26 +1223,11 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern) fmt.Printf("Query sippy for all test run results for pattern %q on variant %#v\n", testPattern, jobVariant) - defaultTransport := &http.Transport{ - Proxy: http.ProxyFromEnvironment, - ForceAttemptHTTP2: true, - MaxIdleConns: 100, - IdleConnTimeout: 90 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - } - - sippyClient := &http.Client{ - Timeout: 2 * time.Minute, - Transport: defaultTransport, - } + sippyClient := newSippyClient() testNameToResults := map[string]*TestResults{} hasCandidateTierResults := false - release, err := getRelease() + release, err := getRelease(ctx) if err != nil { return nil, fmt.Errorf("couldn't fetch latest release version: %w", err) } @@ -1271,7 +1248,7 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi queryParams.Add("filter", string(filterJSON)) currURL.RawQuery = queryParams.Encode() - req, err := http.NewRequest(http.MethodGet, currURL.String(), nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, currURL.String(), nil) if err != nil { return nil, err } @@ -1280,6 +1257,7 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi if err != nil { return nil, err } + defer response.Body.Close() if response.StatusCode < 200 || response.StatusCode > 299 { return nil, fmt.Errorf("error getting sippy results (status=%d) for: %v", response.StatusCode, currURL.String()) } @@ -1287,7 +1265,6 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi if err != nil { return nil, err } - response.Body.Close() testInfos := []sippy.SippyTestInfo{} if err := json.Unmarshal(queryResultBytes, &testInfos); err != nil { @@ -1346,30 +1323,15 @@ func matchTwoNodeFeatureGates(featureGate string, topology string) bool { return false } -func verifyJobBasedFeatureGatePromotion(featureGate string, jobVariant JobVariant) (*TestingResults, error) { - ocpRelease, err := getRelease() +func verifyJobBasedFeatureGatePromotion(ctx context.Context, featureGate string, jobVariant JobVariant) (*TestingResults, error) { + ocpRelease, err := getRelease(ctx) if err != nil { return nil, fmt.Errorf("getting release version: %w", err) } - defaultTransport := &http.Transport{ - Proxy: http.ProxyFromEnvironment, - ForceAttemptHTTP2: true, - MaxIdleConns: 100, - IdleConnTimeout: 90 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - } - - sippyClient := &http.Client{ - Timeout: 2 * time.Minute, - Transport: defaultTransport, - } + sippyClient := newSippyClient() - jobs, err := getJobsForFeatureGateFromSippy(sippyClient, ocpRelease, featureGate, jobVariant) + jobs, err := getJobsForFeatureGateFromSippy(ctx, sippyClient, ocpRelease, featureGate, jobVariant) if err != nil { return nil, fmt.Errorf("getting jobs for feature-gate %q for variant %v : %w", featureGate, jobVariant, err) } @@ -1377,7 +1339,7 @@ func verifyJobBasedFeatureGatePromotion(featureGate string, jobVariant JobVarian testResults := []TestResults{} for _, job := range jobs { - results, err := verifyJobPassRate(sippyClient, ocpRelease, job, jobVariant) + results, err := verifyJobPassRate(ctx, sippyClient, ocpRelease, job, jobVariant) if err != nil { return nil, fmt.Errorf("verifying job pass rate for job %q: %w", job.Name, err) } @@ -1391,7 +1353,7 @@ func verifyJobBasedFeatureGatePromotion(featureGate string, jobVariant JobVarian }, nil } -func verifyJobPassRate(client *http.Client, release string, job sippy.SippyJob, variant JobVariant) (*TestResults, error) { +func verifyJobPassRate(ctx context.Context, client *http.Client, release string, job sippy.SippyJob, variant JobVariant) (*TestResults, error) { // Do an early check for 95% pass rate with at least 14 runs runs := job.CurrentRuns passes := job.CurrentPasses @@ -1442,12 +1404,12 @@ func verifyJobPassRate(client *http.Client, release string, job sippy.SippyJob, // job runs failed tests with the known regressions - only counting failures that have // unknown test failures as a true failure. - jobRuns, err := getJobRunsFromSippy(client, release, job.Name) + jobRuns, err := getJobRunsFromSippy(ctx, client, release, job.Name) if err != nil { return nil, fmt.Errorf("getting job %q results from sippy: %w", job.Name, err) } - triagedTestFailures, err := getTriagedTestFailuresFromSippy(client, release, variant) + triagedTestFailures, err := getTriagedTestFailuresFromSippy(ctx, client, release, variant) if err != nil { return nil, fmt.Errorf("getting triaged test failures from sippy: %w", err) } @@ -1497,8 +1459,12 @@ func verifyJobPassRate(client *http.Client, release string, job sippy.SippyJob, return testResults, nil } -func getJobsForFeatureGateFromSippy(client *http.Client, release, featureGate string, variant JobVariant) ([]sippy.SippyJob, error) { - resp, err := client.Get(sippy.BuildSippyJobsForFeatureGateURL(featureGate, release, variant.Topology, variant.Cloud, variant.Architecture, variant.NetworkStack, variant.OS)) +func getJobsForFeatureGateFromSippy(ctx context.Context, client *http.Client, release, featureGate string, variant JobVariant) ([]sippy.SippyJob, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, sippy.BuildSippyJobsForFeatureGateURL(featureGate, release, variant.Topology, variant.Cloud, variant.Architecture, variant.NetworkStack, variant.OS), nil) + if err != nil { + return nil, fmt.Errorf("creating request: %w", err) + } + resp, err := client.Do(req) if err != nil { return nil, fmt.Errorf("getting job info: %w", err) } @@ -1523,8 +1489,12 @@ func getJobsForFeatureGateFromSippy(client *http.Client, release, featureGate st return jobs, nil } -func getJobRunsFromSippy(client *http.Client, release, jobName string) ([]sippy.SippyJobRun, error) { - resp, err := client.Get(sippy.BuildSippyJobRunsForJobURL(release, jobName, time.Now().Add(-1 * 14 * 24 * time.Hour))) +func getJobRunsFromSippy(ctx context.Context, client *http.Client, release, jobName string) ([]sippy.SippyJobRun, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, sippy.BuildSippyJobRunsForJobURL(release, jobName, time.Now().Add(-1*14*24*time.Hour)), nil) + if err != nil { + return nil, fmt.Errorf("creating request: %w", err) + } + resp, err := client.Do(req) if err != nil { return nil, fmt.Errorf("getting job info: %w", err) } @@ -1549,16 +1519,21 @@ func getJobRunsFromSippy(client *http.Client, release, jobName string) ([]sippy. return runResults.Rows, nil } -func getTriagedTestFailuresFromSippy(client *http.Client, release string, variant JobVariant) (sets.Set[string], error) { +func getTriagedTestFailuresFromSippy(ctx context.Context, client *http.Client, release string, variant JobVariant) (sets.Set[string], error) { reqURL, err := url.Parse("https://sippy.dptools.openshift.org/api/component_readiness/triages") if err != nil { panic(fmt.Sprintf("couldn't parse sippy triages url: %v", err)) } - resp, err := client.Get(reqURL.String()) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("creating request: %w", err) + } + resp, err := client.Do(req) if err != nil { return nil, fmt.Errorf("getting sippy triages: %w", err) } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("expected a 200 OK status code but got %d", resp.StatusCode) @@ -1569,8 +1544,6 @@ func getTriagedTestFailuresFromSippy(client *http.Client, release string, varian return nil, fmt.Errorf("reading response body: %w", err) } - defer resp.Body.Close() - triageItems := []sippy.SippyTriageItem{} err = json.Unmarshal(body, &triageItems) if err != nil { diff --git a/tools/codegen/cmd/featuregate-test-analyzer_test.go b/tools/codegen/cmd/featuregate-test-analyzer_test.go index 2c22e5da4e9..18fa572f3b1 100644 --- a/tools/codegen/cmd/featuregate-test-analyzer_test.go +++ b/tools/codegen/cmd/featuregate-test-analyzer_test.go @@ -1,6 +1,7 @@ package main import ( + "context" "encoding/json" "fmt" "reflect" @@ -65,7 +66,7 @@ func Test_listTestResultFor(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Skip("this is for ease of manual testing") - got, _, err := listTestResultFor(tt.args.featureGate, sets.New[string](tt.args.clusterProfile)) + got, _, err := listTestResultFor(context.Background(), tt.args.featureGate, sets.New[string](tt.args.clusterProfile)) if (err != nil) != tt.wantErr { t.Errorf("listTestResultFor() error = %v, wantErr %v", err, tt.wantErr) return @@ -241,7 +242,7 @@ func Test_checkIfTestingIsSufficient_CandidateVariants(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults) + results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults, nil) blockingErrors := 0 warnings := 0 @@ -449,7 +450,7 @@ func Test_checkIfTestingIsSufficient_OptionalVariants(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults) + results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults, nil) blockingErrors := 0 warnings := 0 @@ -486,6 +487,7 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { name string featureGate string testingResults map[JobVariant]*TestingResults + installTestData map[JobVariant]*TestResults wantBlockingErrors int wantWarnings int }{ @@ -507,6 +509,9 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { }, }, }, + installTestData: map[JobVariant]*TestResults{ + {Cloud: "metal", Architecture: "amd64", Topology: "ha"}: {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 19}, + }, wantBlockingErrors: 1, // Blocking error: install should succeed: overall must pass at 100% wantWarnings: 0, }, @@ -528,8 +533,9 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { }, }, }, - wantBlockingErrors: 0, - wantWarnings: 1, // Warning about missing "install should succeed: overall" test + installTestData: map[JobVariant]*TestResults{}, + wantBlockingErrors: 1, // Blocking error: install test data not found on required variant + wantWarnings: 0, }, { name: "Non-Install feature gate with install should succeed: overall test - no special validation", @@ -549,6 +555,7 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { }, }, }, + installTestData: nil, wantBlockingErrors: 0, wantWarnings: 0, }, @@ -570,6 +577,9 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { }, }, }, + installTestData: map[JobVariant]*TestResults{ + {Cloud: "aws", Architecture: "amd64", Topology: "ha"}: {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 20}, + }, wantBlockingErrors: 0, wantWarnings: 0, }, @@ -604,6 +614,10 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { }, }, }, + installTestData: map[JobVariant]*TestResults{ + {Cloud: "metal", Architecture: "amd64", Topology: "ha"}: {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 19}, + {Cloud: "metal", Architecture: "amd64", Topology: "single"}: {TestName: "install should succeed: overall", TotalRuns: 18, SuccessfulRuns: 18}, + }, wantBlockingErrors: 1, // One variant (ha) fails 100% requirement wantWarnings: 0, }, @@ -625,6 +639,9 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { }, }, }, + installTestData: map[JobVariant]*TestResults{ + {Cloud: "aws", Architecture: "amd64", Topology: "ha"}: {TestName: "install should succeed: overall", TotalRuns: 10, SuccessfulRuns: 10}, + }, wantBlockingErrors: 1, // Blocking error for insufficient runs (< 14) wantWarnings: 0, }, @@ -632,7 +649,7 @@ func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults) + results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults, tt.installTestData) blockingErrors := 0 warnings := 0