UI update for variant cross-comparison

pkg/api/componentreadiness/component_report.go

@@ -413,15 +413,24 @@ func (c *componentReportGenerator) getCommonTestStatusQuery(allJobVariants crtyp
 		queryString += ` AND NOT 'Disruption' in UNNEST(capabilities)`
 	}
 
-	for k, vs := range c.IncludeVariants {
+	variantGroups := c.IncludeVariants
+	// potentially cross-compare variants for the sample
+	if isSample && len(c.VariantCrossCompare) > 0 {
+		variantGroups = c.CompareVariants
+	}
+	if variantGroups == nil { // server-side view definitions may omit a variants map
+		variantGroups = map[string][]string{}
+	}
+
+	for group, variants := range variantGroups {
 		queryString += " AND ("
 		first := true
-		for _, v := range vs {
+		for _, variant := range variants {
 			if first {
-				queryString += fmt.Sprintf(`jv_%s.variant_value = '%s'`, k, v)
+				queryString += fmt.Sprintf(`jv_%s.variant_value = '%s'`, group, variant)
 				first = false
 			} else {
-				queryString += fmt.Sprintf(` OR jv_%s.variant_value = '%s'`, k, v)
+				queryString += fmt.Sprintf(` OR jv_%s.variant_value = '%s'`, group, variant)
 			}
 		}
 		queryString += ")"
@@ -534,7 +543,10 @@ func (c *componentReportGenerator) getSampleQueryStatus(
 		ComponentReportGenerator: c,
 	}
 
-	componentReportTestStatus, errs := api.GetDataFromCacheOrGenerate[crtype.ReportTestStatus](c.client.Cache, c.cacheOption, api.GetPrefixedCacheKey("SampleTestStatus~", generator), generator.queryTestStatus, crtype.ReportTestStatus{})
+	componentReportTestStatus, errs := api.GetDataFromCacheOrGenerate[crtype.ReportTestStatus](
+		c.client.Cache, c.cacheOption,
+		api.GetPrefixedCacheKey("SampleTestStatus~", generator),
+		generator.queryTestStatus, crtype.ReportTestStatus{})
 
 	if len(errs) > 0 {
 		return nil, errs
@@ -1322,8 +1334,12 @@ func (c *componentReportGenerator) generateComponentTestReport(baseStatus map[st
 		if !ok {
 			testStats.ReportStatus = crtype.MissingSample
 		} else {
-			approvedRegression := regressionallowances.IntentionalRegressionFor(c.SampleRelease.Release, testID.ColumnIdentification, testID.TestID)
-			resolvedIssueCompensation, triagedIncidents = c.triagedIncidentsFor(testID)
+			var approvedRegression *regressionallowances.IntentionalRegression
+			if len(c.VariantCrossCompare) == 0 { // only really makes sense when not cross-comparing variants:
+				// look for corresponding regressions we can account for in the analysis
+				approvedRegression = regressionallowances.IntentionalRegressionFor(c.SampleRelease.Release, testID.ColumnIdentification, testID.TestID)
+				resolvedIssueCompensation, triagedIncidents = c.triagedIncidentsFor(testID)
+			}
 			requiredConfidence := c.getRequiredConfidence(testID.TestID, testID.Variants)
 			testStats = c.assessComponentStatus(requiredConfidence, sampleStats.TotalCount, sampleStats.SuccessCount,
 				sampleStats.FlakeCount, baseStats.TotalCount, baseStats.SuccessCount,

pkg/api/componentreadiness/queryparamparser.go

@@ -158,29 +158,56 @@ func parsePROptions(req *http.Request) *crtype.PullRequestOptions {
 	return &pro
 }
 
-func parseVariantOptions(req *http.Request, allJobVariants crtype.JobVariants) (crtype.RequestVariantOptions, error) {
-	var err error
-	variantOption := crtype.RequestVariantOptions{}
+func parseVariantOptions(req *http.Request, allJobVariants crtype.JobVariants) (opts crtype.RequestVariantOptions, err error) {
 	columnGroupBy := req.URL.Query().Get("columnGroupBy")
-	variantOption.ColumnGroupBy, err = api.VariantsStringToSet(allJobVariants, columnGroupBy)
+	opts.ColumnGroupBy, err = api.VariantsStringToSet(allJobVariants, columnGroupBy)
 	if err != nil {
-		return variantOption, err
+		return
 	}
 	dbGroupBy := req.URL.Query().Get("dbGroupBy")
-	variantOption.DBGroupBy, err = api.VariantsStringToSet(allJobVariants, dbGroupBy)
+	opts.DBGroupBy, err = api.VariantsStringToSet(allJobVariants, dbGroupBy)
 	if err != nil {
-		return variantOption, err
+		return
 	}
-	variantOption.RequestedVariants = map[string]string{}
+
+	opts.RequestedVariants = map[string]string{}
 	// Only the dbGroupBy variants can be specifically requested
-	for _, variant := range variantOption.DBGroupBy.List() {
+	for _, variant := range opts.DBGroupBy.List() {
 		if value := req.URL.Query().Get(variant); value != "" {
-			variantOption.RequestedVariants[variant] = value
+			opts.RequestedVariants[variant] = value
 		}
 	}
 	includeVariants := req.URL.Query()["includeVariant"]
-	variantOption.IncludeVariants, err = api.IncludeVariantsToMap(allJobVariants, includeVariants)
-	return variantOption, err
+	opts.IncludeVariants, err = api.VariantListToMap(allJobVariants, includeVariants)
+	if err != nil {
+		return
+	}
+	compareVariants, err := api.VariantListToMap(allJobVariants, req.URL.Query()["compareVariant"])
+	if err != nil {
+		return
+	}
+
+	opts.VariantCrossCompare = req.URL.Query()["variantCrossCompare"]
+	if len(opts.VariantCrossCompare) > 0 {
+		// when we are cross-comparing variants, we need to construct the compareVariants map from the parameters.
+		// the resulting compareVariants map is includeVariants...
+		opts.CompareVariants = map[string][]string{}
+		for group, variants := range opts.IncludeVariants {
+			opts.CompareVariants[group] = variants
+		}
+
+		// ...with overrides from compareVariant parameters.
+		for _, group := range opts.VariantCrossCompare {
+			if variants := compareVariants[group]; len(variants) > 0 {
+				opts.CompareVariants[group] = variants
+			} else {
+				// a group override without any variants listed means not to restrict the variants in this group.
+				// in that case we don't want any where clause for the group, so we just omit it from the map.
+				delete(opts.CompareVariants, group)
+			}
+		}
+	}
+	return
 }
 
 func ParseIntArg(req *http.Request, name string, defaultVal int, validator func(int) bool) (int, error) {

pkg/api/componentreadiness/queryparamparser_test.go

@@ -25,7 +25,7 @@ var (
 func TestParseComponentReportRequest(t *testing.T) {
 
 	allJobVariants := crtype.JobVariants{Variants: map[string][]string{
-		"Architecture": {"amd64", "arm64", "heterogeneous"},
+		"Architecture": {"amd64", "arm64", "s390x", "ppc64le", "heterogeneous"},
 		"FeatureSet":   {"default", "techpreview"},
 		"Installer":    {"ipi", "upi"},
 		"Network":      {"ovn", "sdn"},
@@ -60,8 +60,29 @@ func TestParseComponentReportRequest(t *testing.T) {
 			IgnoreDisruption: true,
 		},
 	}
+	// would like to test with a view that does define cross-compare variants
+	view417cross := view417main
+	view417cross.Name = "4.17-cross"
+	view417cross.VariantOptions = crtype.RequestVariantOptions{
+		VariantCrossCompare: []string{"Topology"},
+		IncludeVariants: map[string][]string{
+			"Architecture": []string{"amd64"},
+			"Installer":    []string{"ipi", "upi"},
+			"Topology":     []string{"ha"},
+		},
+		CompareVariants: map[string][]string{
+			"Architecture": []string{"amd64"},
+			"Installer":    []string{"ipi", "upi"},
+			"Topology":     []string{"single"},
+		},
+		// also remove Topology from columnGroupBy and dbGroupBy
+		ColumnGroupBy: sets.NewString("Platform", "Architecture", "Network"),
+		DBGroupBy:     sets.NewString("Platform", "Architecture", "Network", "Suite", "FeatureSet", "Upgrade", "Installer"),
+	}
+
 	views := []crtype.View{
 		view417main,
+		view417cross,
 	}
 
 	now := time.Now().UTC()
@@ -92,7 +113,6 @@ func TestParseComponentReportRequest(t *testing.T) {
 				{"baseRelease", "4.15"},
 				{"baseStartTime", "2024-02-01T00:00:00Z"},
 				{"confidence", "95"},
-				{"groupBy", "cloud,arch,network"},
 				{"columnGroupBy", "Platform,Architecture,Network"},
 				{"dbGroupBy", "Platform,Architecture,Network,Topology,FeatureSet,Upgrade,Installer"},
 				{"ignoreDisruption", "true"},
@@ -146,7 +166,6 @@ func TestParseComponentReportRequest(t *testing.T) {
 				{"baseRelease", "4.15"},
 				{"baseStartTime", "ga-30d"},
 				{"confidence", "95"},
-				{"groupBy", "cloud,arch,network"},
 				{"columnGroupBy", "Platform,Architecture,Network"},
 				{"dbGroupBy", "Platform,Architecture,Network,Topology,FeatureSet,Upgrade,Installer"},
 				{"ignoreDisruption", "true"},
@@ -206,6 +225,7 @@ func TestParseComponentReportRequest(t *testing.T) {
 					"FeatureSet":   {"default", "techpreview"},
 					"Installer":    {"ipi", "upi"},
 				},
+				CompareVariants: nil, // the view is likely not to specify compare variants at all
 			},
 			baseRelease: crtype.RequestReleaseOptions{
 				Release: "4.16",
@@ -244,6 +264,111 @@ func TestParseComponentReportRequest(t *testing.T) {
 			},
 			errMessage: "params cannot be combined with view",
 		},
+		{
+			name: "normal query params but with variant cross-compare",
+			queryParams: [][]string{
+				{"baseEndTime", "2024-02-28T23:59:59Z"},
+				{"baseRelease", "4.15"},
+				{"baseStartTime", "2024-02-01T00:00:00Z"},
+				{"columnGroupBy", "Platform,Network"},
+				{"dbGroupBy", "Platform,Network,FeatureSet,Upgrade,Installer"},
+				{"sampleEndTime", "2024-04-11T23:59:59Z"},
+				{"sampleRelease", "4.16"},
+				{"sampleStartTime", "2024-04-04T00:00:05Z"},
+				{"includeVariant", "Architecture:amd64"},
+				{"includeVariant", "Architecture:arm64"},
+				{"includeVariant", "Topology:ha"},
+				{"includeVariant", "FeatureSet:default"},
+				{"includeVariant", "Installer:ipi"},
+				{"includeVariant", "Installer:upi"},
+				{"variantCrossCompare", "Architecture"},
+				{"variantCrossCompare", "Topology"},
+				{"compareVariant", "Architecture:s390x"},
+				{"compareVariant", "Architecture:ppc64le"},
+				{"compareVariant", "Topology:single"},
+			},
+			variantOption: crtype.RequestVariantOptions{
+				ColumnGroupBy: sets.NewString("Platform", "Network"),
+				DBGroupBy:     sets.NewString("Platform", "Network", "FeatureSet", "Upgrade", "Installer"),
+				IncludeVariants: map[string][]string{
+					"Architecture": {"amd64", "arm64"},
+					"Topology":     {"ha"},
+					"FeatureSet":   {"default"},
+					"Installer":    {"ipi", "upi"},
+				},
+				CompareVariants: map[string][]string{
+					"Architecture": {"s390x", "ppc64le"},
+					"Topology":     {"single"},
+					"FeatureSet":   {"default"},
+					"Installer":    {"ipi", "upi"},
+				},
+				VariantCrossCompare: []string{"Architecture", "Topology"},
+				RequestedVariants:   map[string]string{},
+			},
+			baseRelease: crtype.RequestReleaseOptions{
+				Release: "4.15",
+				Start:   time.Date(2024, time.February, 1, 0, 0, 0, 0, time.UTC),
+				End:     time.Date(2024, time.February, 28, 23, 59, 59, 0, time.UTC),
+			},
+			sampleRelease: crtype.RequestReleaseOptions{
+				Release: "4.16",
+				Start:   time.Date(2024, time.April, 4, 0, 0, 5, 0, time.UTC),
+				End:     time.Date(2024, time.April, 11, 23, 59, 59, 0, time.UTC),
+			},
+			testIDOption: crtype.RequestTestIdentificationOptions{},
+			advancedOption: crtype.RequestAdvancedOptions{
+				MinimumFailure:   3,
+				Confidence:       95,
+				PityFactor:       5,
+				IgnoreMissing:    false,
+				IgnoreDisruption: true,
+			},
+			cacheOption: cache.RequestOptions{
+				ForceRefresh: false,
+			},
+		},
+		{
+			name: "cross-compare view",
+			queryParams: [][]string{
+				{"view", "4.17-cross"},
+			},
+			variantOption: crtype.RequestVariantOptions{
+				ColumnGroupBy: sets.NewString("Platform", "Architecture", "Network"),
+				DBGroupBy:     sets.NewString("Platform", "Architecture", "Network", "Suite", "FeatureSet", "Upgrade", "Installer"),
+				IncludeVariants: map[string][]string{
+					"Architecture": {"amd64"},
+					"Installer":    {"ipi", "upi"},
+					"Topology":     {"ha"},
+				},
+				VariantCrossCompare: []string{"Topology"},
+				CompareVariants: map[string][]string{
+					"Architecture": {"amd64"},
+					"Installer":    {"ipi", "upi"},
+					"Topology":     {"single"},
+				},
+			},
+			baseRelease: crtype.RequestReleaseOptions{
+				Release: "4.16",
+				Start:   time.Date(2024, time.May, 28, 0, 0, 0, 0, time.UTC),
+				End:     time.Date(2024, time.June, 27, 23, 59, 59, 0, time.UTC),
+			},
+			sampleRelease: crtype.RequestReleaseOptions{
+				Release: "4.17",
+				Start:   time.Date(nowMinus7Days.Year(), nowMinus7Days.Month(), nowMinus7Days.Day(), 0, 0, 0, 0, time.UTC),
+				End:     nowRoundUp,
+			},
+			testIDOption: crtype.RequestTestIdentificationOptions{},
+			advancedOption: crtype.RequestAdvancedOptions{
+				MinimumFailure:   3,
+				Confidence:       95,
+				PityFactor:       5,
+				IgnoreMissing:    false,
+				IgnoreDisruption: true,
+			},
+			cacheOption: cache.RequestOptions{
+				ForceRefresh: false,
+			},
+		},
 	}
 
 	for _, tc := range tests {