diff --git a/tools/codegen/cmd/featuregate-test-analyzer.go b/tools/codegen/cmd/featuregate-test-analyzer.go index 9eda83a226b..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" @@ -35,6 +34,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 { @@ -170,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) } @@ -181,24 +185,65 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error { clusterProfiles := recentlyEnabledFeatureGatesToClusterProfiles[enabledFeatureGate] md.Title(1, enabledFeatureGate) - testingResults, err := listTestResultFor(enabledFeatureGate, clusterProfiles) + testingResults, installTestLevelData, err := listTestResultFor(sippyCtx, enabledFeatureGate, clusterProfiles) if err != nil { return err } writeTestingMarkDown(testingResults, md) - validationResults := checkIfTestingIsSufficient(enabledFeatureGate, testingResults) + validationResults := checkIfTestingIsSufficient(enabledFeatureGate, testingResults, installTestLevelData) - // Separate warnings from blocking errors + // 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) + } + } + + // 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() + }) + 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) + 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 { @@ -208,38 +253,41 @@ 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") - for _, warn := range warnings { - md.Textf(" - %s\n", warn.Error()) - } + md.Textf("**Non-blocking warnings and informational results:**\n") + 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...) + if len(blockingErrors) > 0 { + errs = append(errs, blockingErrors...) + } featureGateHTMLData = append(featureGateHTMLData, buildHTMLFeatureGateData(enabledFeatureGate, testingResults, blockingErrors, release)) } @@ -257,7 +305,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 { @@ -365,7 +416,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 { @@ -382,23 +433,45 @@ 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, - }) + 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 { + // 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", testResults.TestName, testResults.TotalRuns, requiredNumberOfTestRunsPerVariant, featureGate, jobVariant), - IsWarning: isOptional, + IsWarning: isOptional, + Category: CategoryInsufficientRuns, }) } if testResults.TotalRuns == 0 { @@ -412,7 +485,43 @@ 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" using test-level data from Sippy + if strings.Contains(featureGate, "Install") { + installTest := installTestLevelData[jobVariant] + if installTest == nil { + 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: isOptional, + Category: CategoryInstallTest, }) + } 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, + Category: CategoryInstallTest, + }) + } + 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, + Category: CategoryInstallTest, + }) + } + } } } } @@ -420,6 +529,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() @@ -599,6 +745,18 @@ var ( NetworkStack: "dual", JobTiers: "candidate,standard,informing,blocking", }, + { + Cloud: "metal", + Architecture: "amd64", + Topology: "single", + Optional: true, + }, + { + Cloud: "metal", + Architecture: "amd64", + Topology: "compact", + Optional: true, + }, } nonHypershiftPlatforms = regexp.MustCompile("(?i)nutanix|metal|vsphere|openstack|azure|gcp") @@ -622,6 +780,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) } @@ -690,10 +862,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) + 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 { @@ -736,10 +920,11 @@ func validateJobTiers(jobVariant JobVariant) error { return nil } -func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (map[JobVariant]*TestingResults, 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{} + installTestLevelData := map[JobVariant]*TestResults{} var jobVariantsToCheck []JobVariant if clusterProfiles.Has("Hypershift") && !nonHypershiftPlatforms.MatchString(featureGate) { @@ -755,24 +940,41 @@ 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 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) + jobVariantResults, err := listTestResultForVariant(ctx, featureGate, jobVariant) if err != nil { - return nil, err + return nil, nil, err } results[jobVariant] = jobVariantResults + + // 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(ctx, 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 { @@ -795,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) } @@ -851,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-") { @@ -859,45 +1065,172 @@ func getRelease() (string, error) { return strings.TrimPrefix(currentRelease, "release-"), nil } // means its main branch - return getLatestRelease() + return getLatestRelease(ctx) } -func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*TestingResults, error) { - // Substring here matches for both [OCPFeatureGate:...] and [FeatureGate:...] - testPattern := fmt.Sprintf("FeatureGate:%s]", featureGate) +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, + }, + } +} - // 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) +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) } - fmt.Printf("Query sippy for all test run results for pattern %q on variant %#v\n", testPattern, jobVariant) + sippyClient := newSippyClient() - 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, - }, + jobs, err := getJobsForFeatureGateFromSippy(ctx, sippyClient, release, featureGate, jobVariant) + if err != nil { + return nil, fmt.Errorf("getting jobs for install test results: %w", err) } - sippyClient := &http.Client{ - Timeout: 2 * time.Minute, - Transport: defaultTransport, + testResults := &TestResults{ + TestName: "install should succeed: overall", } + for _, job := range jobs { + jobRuns, err := getJobRunsFromSippy(ctx, 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(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) + + sippyClient := newSippyClient() + + release, err := getRelease(ctx) + 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.NewRequestWithContext(ctx, http.MethodGet, currURL.String(), nil) + if err != nil { + return nil, err + } + + response, err := sippyClient.Do(req) + 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()) + } + queryResultBytes, err := io.ReadAll(response.Body) + if err != nil { + return nil, err + } + + 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(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(ctx, 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) + + sippyClient := newSippyClient() + 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() + release, err := getRelease(ctx) 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{ @@ -915,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 } @@ -924,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()) } @@ -931,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 { @@ -990,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) } @@ -1021,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) } @@ -1035,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 @@ -1086,22 +1404,30 @@ 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(ctx, 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), } - triagedTestFailures, err := getTriagedTestFailuresFromSippy(client, release, variant) - if err != nil { - return nil, fmt.Errorf("getting triaged test failures from sippy: %w", err) - } - for _, jobRun := range jobRuns { + if jobRun.OverallResult == "N" || jobRun.OverallResult == "n" { + infraFailures++ + continue + } + if jobRun.OverallResult == "F" && !jobRun.KnownFailure { untriagedTestFailures := []string{} @@ -1128,18 +1454,24 @@ func verifyJobPassRate(client *http.Client, release string, job sippy.SippyJob, testResults.SuccessfulRuns++ } + testResults.TotalRuns -= infraFailures + 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) } 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) @@ -1157,15 +1489,19 @@ 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) } 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) @@ -1183,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) @@ -1203,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 24d2bad6c20..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 @@ -481,6 +482,205 @@ func Test_checkIfTestingIsSufficient_OptionalVariants(t *testing.T) { } } +func Test_checkIfTestingIsSufficient_InstallFeatureGates(t *testing.T) { + tests := []struct { + name string + featureGate string + testingResults map[JobVariant]*TestingResults + installTestData map[JobVariant]*TestResults + 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}, + }, + }, + }, + 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, + }, + { + 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}, + }, + }, + }, + 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", + 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}, + }, + }, + }, + installTestData: nil, + 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}, + }, + }, + }, + installTestData: map[JobVariant]*TestResults{ + {Cloud: "aws", Architecture: "amd64", Topology: "ha"}: {TestName: "install should succeed: overall", TotalRuns: 20, SuccessfulRuns: 20}, + }, + 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}, + }, + }, + }, + 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, + }, + { + 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}, + }, + }, + }, + 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, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults, tt.installTestData) + + 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/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) } } 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{ {