Skip to content

Commit 248e2de

Browse files
committed
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.
1 parent 72066cc commit 248e2de

3 files changed

Lines changed: 499 additions & 21 deletions

File tree

tools/codegen/cmd/featuregate-test-analyzer.go

Lines changed: 217 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ const (
3535
// required pass rate.
3636
// nearly all current tests pass 99% of the time, but in a two week window we lack enough data to say.
3737
requiredPassRateOfTestsPerVariant = 0.95
38+
39+
// required pass rate for "install should succeed" test
40+
requiredPassRateForInstallTest = 1.0
3841
)
3942

4043
type FeatureGateTestAnalyzerOptions struct {
@@ -181,7 +184,7 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error {
181184
clusterProfiles := recentlyEnabledFeatureGatesToClusterProfiles[enabledFeatureGate]
182185
md.Title(1, enabledFeatureGate)
183186

184-
testingResults, err := listTestResultFor(enabledFeatureGate, clusterProfiles)
187+
testingResults, installTestLevelData, err := listTestResultFor(enabledFeatureGate, clusterProfiles)
185188
if err != nil {
186189
return err
187190
}
@@ -190,7 +193,7 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error {
190193

191194
validationResults := checkIfTestingIsSufficient(enabledFeatureGate, testingResults)
192195

193-
// Separate warnings from blocking errors
196+
// Separate warnings and blocking errors
194197
blockingErrors := []error{}
195198
warnings := []error{}
196199
for _, vr := range validationResults {
@@ -201,24 +204,62 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error {
201204
}
202205
}
203206

207+
// For Install feature gates, report "install should succeed: overall" test statistics first
208+
if strings.Contains(enabledFeatureGate, "Install") {
209+
md.Text("")
210+
fmt.Fprintf(o.Out, "\n")
211+
md.Textf("**Install test statistics for \"install should succeed: overall\":**\n")
212+
fmt.Fprintf(o.Out, "Install test statistics for \"install should succeed: overall\":\n")
213+
jobVariants := make([]JobVariant, 0, len(testingResults))
214+
for jobVariant := range testingResults {
215+
jobVariants = append(jobVariants, jobVariant)
216+
}
217+
sort.Slice(jobVariants, func(i, j int) bool {
218+
return jobVariants[i].String() < jobVariants[j].String()
219+
})
220+
for _, jobVariant := range jobVariants {
221+
installTest := installTestLevelData[jobVariant]
222+
if installTest == nil {
223+
md.Textf(" - %v: test not found\n", jobVariant)
224+
fmt.Fprintf(o.Out, " %v: test not found\n", jobVariant)
225+
} else if installTest.TotalRuns > 0 {
226+
passPercent := float32(installTest.SuccessfulRuns) / float32(installTest.TotalRuns)
227+
displayActual := int(passPercent * 100)
228+
md.Textf(" - %v: passed %d%% (%d/%d runs)\n", jobVariant, displayActual, installTest.SuccessfulRuns, installTest.TotalRuns)
229+
fmt.Fprintf(o.Out, " %v: passed %d%% (%d/%d runs)\n", jobVariant, displayActual, installTest.SuccessfulRuns, installTest.TotalRuns)
230+
} else {
231+
md.Textf(" - %v: 0 runs\n", jobVariant)
232+
fmt.Fprintf(o.Out, " %v: 0 runs\n", jobVariant)
233+
}
234+
}
235+
md.Text("")
236+
fmt.Fprintf(o.Out, "\n")
237+
}
238+
204239
if len(validationResults) == 0 {
205240
md.Textf("Sufficient CI testing for %q.\n", enabledFeatureGate)
206241
fmt.Fprintf(o.Out, "Sufficient CI testing for %q.\n", enabledFeatureGate)
207242
} else {
208243
if len(blockingErrors) > 0 {
209244
md.Textf("INSUFFICIENT CI testing for %q.\n", enabledFeatureGate)
210245
fmt.Fprintf(o.Out, "INSUFFICIENT CI testing for %q.\n", enabledFeatureGate)
211-
} else {
246+
} else if len(warnings) > 0 {
212247
md.Textf("CI testing issues found for %q (non-blocking warnings).\n", enabledFeatureGate)
213248
fmt.Fprintf(o.Out, "CI testing issues found for %q (non-blocking warnings).\n", enabledFeatureGate)
249+
} else {
250+
md.Textf("Sufficient CI testing for %q.\n", enabledFeatureGate)
251+
fmt.Fprintf(o.Out, "Sufficient CI testing for %q.\n", enabledFeatureGate)
214252
}
215253

216-
md.Textf("* At least five tests are expected for a feature\n")
217-
md.Textf("* Tests must be be run on every TechPreview platform (ask for an exception if your feature doesn't support a variant)")
218-
md.Textf("* All tests must run at least 14 times on every platform")
219-
md.Textf("* All tests must pass at least 95%% of the time")
220-
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")
221-
md.Text("")
254+
if len(blockingErrors) > 0 || len(warnings) > 0 {
255+
md.Textf("* At least five tests are expected for a feature\n")
256+
md.Textf("* Tests must be be run on every TechPreview platform (ask for an exception if your feature doesn't support a variant)")
257+
md.Textf("* All tests must run at least 14 times on every platform")
258+
md.Textf("* All tests must pass at least 95%% of the time")
259+
md.Textf("* For Install feature gates, the \"install should succeed: overall\" test must pass at least 100%% of the time")
260+
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")
261+
md.Text("")
262+
}
222263

223264
if len(warnings) > 0 {
224265
md.Textf("**Non-blocking warnings (optional variants):**\n")
@@ -394,6 +435,11 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian
394435
}
395436

396437
for _, testResults := range testedVariant.TestResults {
438+
// Skip "install should succeed: overall" for Install feature gates - it has special validation below
439+
if strings.Contains(featureGate, "Install") && testResults.TestName == "install should succeed: overall" {
440+
continue
441+
}
442+
397443
if testResults.TotalRuns < requiredNumberOfTestRunsPerVariant {
398444
results = append(results, ValidationResult{
399445
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
415461
})
416462
}
417463
}
464+
465+
// For Install feature gates, validate "install should succeed: overall" test
466+
if strings.Contains(featureGate, "Install") {
467+
installTest := testResultByName(testedVariant.TestResults, "install should succeed: overall")
468+
if installTest == nil {
469+
results = append(results, ValidationResult{
470+
Error: fmt.Errorf("warning: \"install should succeed: overall\" test not found for Install feature gate %q on %v",
471+
featureGate, jobVariant),
472+
IsWarning: true,
473+
})
474+
} else {
475+
if installTest.TotalRuns < requiredNumberOfTestRunsPerVariant {
476+
results = append(results, ValidationResult{
477+
Error: fmt.Errorf("error: \"install should succeed: overall\" only has %d runs, need at least %d runs for %q on %v",
478+
installTest.TotalRuns, requiredNumberOfTestRunsPerVariant, featureGate, jobVariant),
479+
IsWarning: isOptional,
480+
})
481+
}
482+
if installTest.TotalRuns > 0 {
483+
passPercent := float32(installTest.SuccessfulRuns) / float32(installTest.TotalRuns)
484+
if passPercent < requiredPassRateForInstallTest {
485+
displayExpected := int(requiredPassRateForInstallTest * 100)
486+
displayActual := int(passPercent * 100)
487+
results = append(results, ValidationResult{
488+
Error: fmt.Errorf("error: \"install should succeed: overall\" only passed %d%%, need at least %d%% for %q on %v",
489+
displayActual, displayExpected, featureGate, jobVariant),
490+
IsWarning: isOptional,
491+
})
492+
}
493+
}
494+
}
495+
}
418496
}
419497

420498
return results
@@ -622,6 +700,20 @@ type JobVariant struct {
622700
Optional bool // If true, validation failures for this variant are non-blocking warnings
623701
}
624702

703+
func (jv JobVariant) String() string {
704+
result := fmt.Sprintf("cloud=%s arch=%s topology=%s", jv.Cloud, jv.Architecture, jv.Topology)
705+
if jv.NetworkStack != "" {
706+
result += fmt.Sprintf(" network=%s", jv.NetworkStack)
707+
}
708+
if jv.OS != "" {
709+
result += fmt.Sprintf(" os=%s", jv.OS)
710+
}
711+
if jv.Optional {
712+
result += " optional=true"
713+
}
714+
return result
715+
}
716+
625717
type OrderedJobVariants []JobVariant
626718

627719
func (a OrderedJobVariants) Len() int { return len(a) }
@@ -694,6 +786,7 @@ type TestResults struct {
694786
type ValidationResult struct {
695787
Error error
696788
IsWarning bool // if true, this is a non-blocking warning (for optional variants)
789+
IsInfo bool // if true, this is informational telemetry (e.g., install test pass percentage)
697790
}
698791

699792
func testResultByName(results []TestResults, testName string) *TestResults {
@@ -736,10 +829,11 @@ func validateJobTiers(jobVariant JobVariant) error {
736829
return nil
737830
}
738831

739-
func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (map[JobVariant]*TestingResults, error) {
832+
func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (map[JobVariant]*TestingResults, map[JobVariant]*TestResults, error) {
740833
fmt.Printf("Query sippy for all test run results for feature gate %q on clusterProfile %q\n", featureGate, sets.List(clusterProfiles))
741834

742835
results := map[JobVariant]*TestingResults{}
836+
installTestLevelData := map[JobVariant]*TestResults{}
743837

744838
var jobVariantsToCheck []JobVariant
745839
if clusterProfiles.Has("Hypershift") && !nonHypershiftPlatforms.MatchString(featureGate) {
@@ -760,19 +854,28 @@ func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (ma
760854
// Validate all variants before making expensive API calls
761855
for _, jobVariant := range jobVariantsToCheck {
762856
if err := validateJobTiers(jobVariant); err != nil {
763-
return nil, err
857+
return nil, nil, err
764858
}
765859
}
766860

767861
for _, jobVariant := range jobVariantsToCheck {
768862
jobVariantResults, err := listTestResultForVariant(featureGate, jobVariant)
769863
if err != nil {
770-
return nil, err
864+
return nil, nil, err
771865
}
772866
results[jobVariant] = jobVariantResults
867+
868+
// For Install feature gates, also get test-level data for "install should succeed: overall"
869+
if strings.Contains(featureGate, "Install") {
870+
installTestData, err := getInstallTestLevelData(featureGate, jobVariant)
871+
if err != nil {
872+
return nil, nil, err
873+
}
874+
installTestLevelData[jobVariant] = installTestData
875+
}
773876
}
774877

775-
return results, nil
878+
return results, installTestLevelData, nil
776879
}
777880

778881
func filterVariants(featureGate string, variantsList ...[]JobVariant) []JobVariant {
@@ -862,15 +965,111 @@ func getRelease() (string, error) {
862965
return getLatestRelease()
863966
}
864967

865-
func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*TestingResults, error) {
866-
// Substring here matches for both [OCPFeatureGate:...] and [FeatureGate:...]
867-
testPattern := fmt.Sprintf("FeatureGate:%s]", featureGate)
968+
func getInstallTestLevelData(featureGate string, jobVariant JobVariant) (*TestResults, error) {
969+
testPattern := "install should succeed: overall"
970+
queries := sippy.QueriesForWithCapability(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology,
971+
jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern, featureGate)
972+
973+
defaultTransport := &http.Transport{
974+
Proxy: http.ProxyFromEnvironment,
975+
ForceAttemptHTTP2: true,
976+
MaxIdleConns: 100,
977+
IdleConnTimeout: 90 * time.Second,
978+
TLSHandshakeTimeout: 10 * time.Second,
979+
ExpectContinueTimeout: 1 * time.Second,
980+
TLSClientConfig: &tls.Config{
981+
InsecureSkipVerify: true,
982+
},
983+
}
984+
985+
sippyClient := &http.Client{
986+
Timeout: 2 * time.Minute,
987+
Transport: defaultTransport,
988+
}
989+
990+
release, err := getRelease()
991+
if err != nil {
992+
return nil, fmt.Errorf("couldn't fetch latest release version: %w", err)
993+
}
994+
995+
var installTestResult *TestResults
996+
for _, currQuery := range queries {
997+
currURL := &url.URL{
998+
Scheme: "https",
999+
Host: "sippy.dptools.openshift.org",
1000+
Path: "api/tests",
1001+
}
1002+
queryParams := currURL.Query()
1003+
queryParams.Add("release", release)
1004+
queryParams.Add("period", "default")
1005+
filterJSON, err := json.Marshal(currQuery)
1006+
if err != nil {
1007+
return nil, err
1008+
}
1009+
queryParams.Add("filter", string(filterJSON))
1010+
currURL.RawQuery = queryParams.Encode()
1011+
1012+
req, err := http.NewRequest(http.MethodGet, currURL.String(), nil)
1013+
if err != nil {
1014+
return nil, err
1015+
}
1016+
1017+
response, err := sippyClient.Do(req)
1018+
if err != nil {
1019+
return nil, err
1020+
}
1021+
if response.StatusCode < 200 || response.StatusCode > 299 {
1022+
return nil, fmt.Errorf("error getting sippy results (status=%d) for: %v", response.StatusCode, currURL.String())
1023+
}
1024+
queryResultBytes, err := io.ReadAll(response.Body)
1025+
if err != nil {
1026+
return nil, err
1027+
}
1028+
response.Body.Close()
8681029

1030+
testInfos := []sippy.SippyTestInfo{}
1031+
if err := json.Unmarshal(queryResultBytes, &testInfos); err != nil {
1032+
return nil, err
1033+
}
1034+
1035+
for _, currTest := range testInfos {
1036+
if installTestResult == nil {
1037+
installTestResult = &TestResults{
1038+
TestName: currTest.Name,
1039+
}
1040+
}
1041+
1042+
// Accumulate results across multiple JobTier queries
1043+
if currTest.CurrentRuns >= requiredNumberOfTestRunsPerVariant {
1044+
installTestResult.TotalRuns += currTest.CurrentRuns
1045+
installTestResult.SuccessfulRuns += currTest.CurrentSuccesses
1046+
installTestResult.FailedRuns += currTest.CurrentFailures
1047+
installTestResult.FlakedRuns += currTest.CurrentFlakes
1048+
} else {
1049+
installTestResult.TotalRuns += currTest.CurrentRuns + currTest.PreviousRuns
1050+
installTestResult.SuccessfulRuns += currTest.CurrentSuccesses + currTest.PreviousSuccesses
1051+
installTestResult.FailedRuns += currTest.CurrentFailures + currTest.PreviousFailures
1052+
installTestResult.FlakedRuns += currTest.CurrentFlakes + currTest.PreviousFlakes
1053+
}
1054+
}
1055+
}
1056+
1057+
return installTestResult, nil
1058+
}
1059+
1060+
func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*TestingResults, error) {
8691061
// Feature gates used by the installer don't need separate tests, use the overall install tests
8701062
if strings.Contains(featureGate, "Install") {
8711063
return verifyJobBasedFeatureGatePromotion(featureGate, jobVariant)
8721064
}
8731065

1066+
var testPattern string
1067+
var queries []*sippy.SippyQueryStruct
1068+
1069+
// Substring here matches for both [OCPFeatureGate:...] and [FeatureGate:...]
1070+
testPattern = fmt.Sprintf("FeatureGate:%s]", featureGate)
1071+
queries = sippy.QueriesFor(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology,
1072+
jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern)
8741073
fmt.Printf("Query sippy for all test run results for pattern %q on variant %#v\n", testPattern, jobVariant)
8751074

8761075
defaultTransport := &http.Transport{
@@ -892,12 +1091,10 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi
8921091

8931092
testNameToResults := map[string]*TestResults{}
8941093
hasCandidateTierResults := false
895-
queries := sippy.QueriesFor(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology, jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern)
8961094
release, err := getRelease()
8971095
if err != nil {
8981096
return nil, fmt.Errorf("couldn't fetch latest release version: %w", err)
8991097
}
900-
fmt.Printf("Querying sippy release %s for test run results\n", release)
9011098

9021099
for _, currQuery := range queries {
9031100
currURL := &url.URL{
@@ -1139,7 +1336,7 @@ func getJobsForFeatureGateFromSippy(client *http.Client, release, featureGate st
11391336
defer resp.Body.Close()
11401337

11411338
if resp.StatusCode != http.StatusOK {
1142-
return nil, fmt.Errorf("expected a 200 OK status code but got %s", resp.StatusCode)
1339+
return nil, fmt.Errorf("expected a 200 OK status code but got %d", resp.StatusCode)
11431340
}
11441341

11451342
body, err := io.ReadAll(resp.Body)
@@ -1165,7 +1362,7 @@ func getJobRunsFromSippy(client *http.Client, release, jobName string) ([]sippy.
11651362
defer resp.Body.Close()
11661363

11671364
if resp.StatusCode != http.StatusOK {
1168-
return nil, fmt.Errorf("expected a 200 OK status code but got %s", resp.StatusCode)
1365+
return nil, fmt.Errorf("expected a 200 OK status code but got %d", resp.StatusCode)
11691366
}
11701367

11711368
body, err := io.ReadAll(resp.Body)

0 commit comments

Comments
 (0)