Merge pull request #3562 from dgoodwin/remove-variant-junit-table-overrides

Delete the unused rarely run jobs / variant overrides feature

Author: openshift-merge-bot[bot]

cmd/sippy/annotatejobruns.go

@@ -180,7 +180,7 @@ Example run: sippy annotate-job-runs  --google-service-account-credential-file=f
 				return errors.WithMessage(err, "couldn't get DB client")
 			}
 
-			allVariants, errs := componentreadiness.GetJobVariants(ctx, bqprovider.NewBigQueryProvider(bigQueryClient, nil))
+			allVariants, errs := componentreadiness.GetJobVariants(ctx, bqprovider.NewBigQueryProvider(bigQueryClient))
 			if len(errs) > 0 {
 				return fmt.Errorf("failed to get job variants: %v", errs)
 			}

cmd/sippy/automatejira.go

@@ -165,12 +165,7 @@ func NewAutomateJiraCommand() *cobra.Command {
 				return fmt.Errorf("couldn't get jira client: jira auth is not configured")
 			}
 
-			config, err := f.ConfigFlags.GetConfig()
-			if err != nil {
-				log.WithError(err).Warn("error reading config file")
-			}
-
-			provider := bqprovider.NewBigQueryProvider(bigQueryClient, config.ComponentReadinessConfig.VariantJunitTableOverrides)
+			provider := bqprovider.NewBigQueryProvider(bigQueryClient)
 			allVariants, errs := componentreadiness.GetJobVariants(ctx, provider)
 			if len(errs) > 0 {
 				return fmt.Errorf("failed to get job variants: %v", errs)

cmd/sippy/component_readiness.go

@@ -187,7 +187,7 @@ func (f *ComponentReadinessFlags) runServerMode() error {
 		log.WithError(err).Warn("unable to initialize Jira client, bug filing will be disabled")
 	}
 
-	crDataProvider := bqprovider.NewBigQueryProvider(bigQueryClient, config.ComponentReadinessConfig.VariantJunitTableOverrides)
+	crDataProvider := bqprovider.NewBigQueryProvider(bigQueryClient)
 
 	server := sippyserver.NewServer(
 		sippyserver.ModeOpenShift,

cmd/sippy/load.go

@@ -223,7 +223,6 @@ func NewLoadCommand() *cobra.Command {
 						dbc, bqc, config, views.ComponentReadiness, releaseConfigs,
 						f.ComponentReadinessFlags.CRTimeRoundingFactor,
 						regressionStore,
-						config.ComponentReadinessConfig.VariantJunitTableOverrides,
 					)
 					if err != nil {
 						return errors.Wrap(err, "error creating regression cache loader")

cmd/sippy/serve.go

@@ -133,7 +133,7 @@ func NewServeCommand() *cobra.Command {
 						bigQueryClient = f.CacheFlags.DecorateBiqQueryClientWithPersistentCache(bigQueryClient)
 					}
 
-					crDataProvider = bqprovider.NewBigQueryProvider(bigQueryClient, config.ComponentReadinessConfig.VariantJunitTableOverrides)
+					crDataProvider = bqprovider.NewBigQueryProvider(bigQueryClient)
 				}
 
 			case "postgres":

config/openshift-customizations.yaml

@@ -1,12 +1,6 @@
 # This file is used as an overlay to customize the generated config, so it contains the Presubmit pseudorelease.
 prow:
   url: https://prow.ci.openshift.org/prowjobs.js
-componentReadiness:
-  variantJunitTableOverrides:
-    - variantName: JobTier
-      variantValue: rare
-      tableName: junit_rarely_run_jobs
-      relativeStart: end-90d
 releases:
   "3.11":
     jobs:

pkg/api/componentreadiness/dataprovider/bigquery/provider.go

@@ -6,7 +6,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"cloud.google.com/go/bigquery"
@@ -19,11 +18,9 @@ import (
 	"github.com/openshift/sippy/pkg/apis/api/componentreport/crtest"
 	"github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts"
 	apiCache "github.com/openshift/sippy/pkg/apis/cache"
-	configv1 "github.com/openshift/sippy/pkg/apis/config/v1"
 	v1 "github.com/openshift/sippy/pkg/apis/sippy/v1"
 	bqcachedclient "github.com/openshift/sippy/pkg/bigquery"
 	"github.com/openshift/sippy/pkg/bigquery/bqlabel"
-	"github.com/openshift/sippy/pkg/util"
 	"github.com/openshift/sippy/pkg/util/param"
 	"github.com/openshift/sippy/pkg/util/sets"
 )
@@ -33,12 +30,11 @@ var _ dataprovider.DataProvider = &BigQueryProvider{}
 // BigQueryProvider implements dataprovider.DataProvider using Google BigQuery
 // as the backing data store, wrapping the existing query generators.
 type BigQueryProvider struct {
-	client                     *bqcachedclient.Client
-	variantJunitTableOverrides []configv1.VariantJunitTableOverride
+	client *bqcachedclient.Client
 }
 
-func NewBigQueryProvider(client *bqcachedclient.Client, overrides []configv1.VariantJunitTableOverride) *BigQueryProvider {
-	return &BigQueryProvider{client: client, variantJunitTableOverrides: overrides}
+func NewBigQueryProvider(client *bqcachedclient.Client) *BigQueryProvider {
+	return &BigQueryProvider{client: client}
 }
 
 // Client returns the underlying BigQuery client for callers that still need direct access
@@ -72,92 +68,7 @@ func (p *BigQueryProvider) QuerySampleTestStatus(ctx context.Context, reqOptions
 	includeVariants map[string][]string,
 	start, end time.Time) (map[string]crstatus.TestStatus, []error) {
 
-	fLog := log.WithField("func", "QuerySampleTestStatus")
-
-	// Filter out overridden variants from the default query
-	filteredVariants, skipDefault := copyIncludeVariantsAndRemoveOverrides(p.variantJunitTableOverrides, -1, includeVariants)
-
-	type result struct {
-		status map[string]crstatus.TestStatus
-		errs   []error
-	}
-	resultCh := make(chan result)
-	var wg sync.WaitGroup
-
-	// Run default query (unless all variants were overridden)
-	if !skipDefault {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			select {
-			case <-ctx.Done():
-				return
-			default:
-				fLog.Infof("running default sample query with includeVariants: %+v", filteredVariants)
-				s, e := p.querySampleTestStatusForTable(ctx, reqOptions, allJobVariants, filteredVariants, start, end, DefaultJunitTable)
-				resultCh <- result{s, e}
-			}
-		}()
-	}
-
-	// Run override queries for applicable variants
-	for i, or := range p.variantJunitTableOverrides {
-		if !containsOverriddenVariant(includeVariants, or.VariantName, or.VariantValue) {
-			continue
-		}
-		index, override := i, or
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			select {
-			case <-ctx.Done():
-				return
-			default:
-				overrideVariants, skip := copyIncludeVariantsAndRemoveOverrides(p.variantJunitTableOverrides, index, includeVariants)
-				if skip {
-					fLog.Infof("skipping override sample query as all values for a variant were overridden")
-					return
-				}
-				overrideEnd := end
-				overrideStart, err := util.ParseCRReleaseTime([]v1.Release{}, "", override.RelativeStart,
-					true, &overrideEnd, reqOptions.CacheOption.CRTimeRoundingFactor)
-				if err != nil {
-					resultCh <- result{nil, []error{err}}
-					return
-				}
-				fLog.Infof("running override sample query for %+v with includeVariants: %+v", override, overrideVariants)
-				s, e := p.querySampleTestStatusForTable(ctx, reqOptions, allJobVariants, overrideVariants, overrideStart, overrideEnd, override.TableName)
-				resultCh <- result{s, e}
-			}
-		}()
-	}
-
-	go func() {
-		wg.Wait()
-		close(resultCh)
-	}()
-
-	merged := make(map[string]crstatus.TestStatus)
-	var allErrs []error
-	for r := range resultCh {
-		allErrs = append(allErrs, r.errs...)
-		for k, v := range r.status {
-			merged[k] = v
-		}
-	}
-	if len(allErrs) > 0 {
-		return nil, allErrs
-	}
-	return merged, nil
-}
-
-func (p *BigQueryProvider) querySampleTestStatusForTable(ctx context.Context, reqOptions reqopts.RequestOptions,
-	allJobVariants crtest.JobVariants,
-	includeVariants map[string][]string,
-	start, end time.Time,
-	junitTable string) (map[string]crstatus.TestStatus, []error) {
-
-	generator := NewSampleQueryGenerator(p.client, reqOptions, allJobVariants, includeVariants, start, end, junitTable)
+	generator := NewSampleQueryGenerator(p.client, reqOptions, allJobVariants, includeVariants, start, end)
 	result, errs := apiPkg.GetDataFromCacheOrGenerate[crstatus.ReportTestStatus](
 		ctx, p.client.Cache, reqOptions.CacheOption,
 		apiPkg.NewCacheSpec(generator, "SampleTestStatus~", &reqOptions.SampleRelease.End),
@@ -194,89 +105,7 @@ func (p *BigQueryProvider) QuerySampleJobRunTestStatus(ctx context.Context, reqO
 	includeVariants map[string][]string,
 	start, end time.Time) (map[string][]crstatus.TestJobRunRows, []error) {
 
-	fLog := log.WithField("func", "QuerySampleJobRunTestStatus")
-
-	filteredVariants, skipDefault := copyIncludeVariantsAndRemoveOverrides(p.variantJunitTableOverrides, -1, includeVariants)
-
-	type result struct {
-		status map[string][]crstatus.TestJobRunRows
-		errs   []error
-	}
-	resultCh := make(chan result)
-	var wg sync.WaitGroup
-
-	if !skipDefault {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			select {
-			case <-ctx.Done():
-				return
-			default:
-				fLog.Infof("running default sample job run query with includeVariants: %+v", filteredVariants)
-				s, e := p.querySampleJobRunTestStatusForTable(ctx, reqOptions, allJobVariants, filteredVariants, start, end, DefaultJunitTable)
-				resultCh <- result{s, e}
-			}
-		}()
-	}
-
-	for i, or := range p.variantJunitTableOverrides {
-		if !containsOverriddenVariant(includeVariants, or.VariantName, or.VariantValue) {
-			continue
-		}
-		index, override := i, or
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			select {
-			case <-ctx.Done():
-				return
-			default:
-				overrideVariants, skip := copyIncludeVariantsAndRemoveOverrides(p.variantJunitTableOverrides, index, includeVariants)
-				if skip {
-					fLog.Infof("skipping override sample job run query as all values for a variant were overridden")
-					return
-				}
-				overrideEnd := end
-				overrideStart, err := util.ParseCRReleaseTime([]v1.Release{}, "", override.RelativeStart,
-					true, &overrideEnd, reqOptions.CacheOption.CRTimeRoundingFactor)
-				if err != nil {
-					resultCh <- result{nil, []error{err}}
-					return
-				}
-				fLog.Infof("running override sample job run query for %+v with includeVariants: %+v", override, overrideVariants)
-				s, e := p.querySampleJobRunTestStatusForTable(ctx, reqOptions, allJobVariants, overrideVariants, overrideStart, overrideEnd, override.TableName)
-				resultCh <- result{s, e}
-			}
-		}()
-	}
-
-	go func() {
-		wg.Wait()
-		close(resultCh)
-	}()
-
-	merged := make(map[string][]crstatus.TestJobRunRows)
-	var allErrs []error
-	for r := range resultCh {
-		allErrs = append(allErrs, r.errs...)
-		for k, v := range r.status {
-			merged[k] = v
-		}
-	}
-	if len(allErrs) > 0 {
-		return nil, allErrs
-	}
-	return merged, nil
-}
-
-func (p *BigQueryProvider) querySampleJobRunTestStatusForTable(ctx context.Context, reqOptions reqopts.RequestOptions,
-	allJobVariants crtest.JobVariants,
-	includeVariants map[string][]string,
-	start, end time.Time,
-	junitTable string) (map[string][]crstatus.TestJobRunRows, []error) {
-
-	generator := NewSampleTestDetailsQueryGenerator(p.client, reqOptions, allJobVariants, includeVariants, start, end, junitTable)
+	generator := NewSampleTestDetailsQueryGenerator(p.client, reqOptions, allJobVariants, includeVariants, start, end)
 	result, errs := apiPkg.GetDataFromCacheOrGenerate[crstatus.TestJobRunStatuses](
 		ctx, p.client.Cache, reqOptions.CacheOption,
 		apiPkg.NewCacheSpec(generator, "SampleJobRunTestStatusV2~", &end),
@@ -568,58 +397,3 @@ func getSingleColumnResultToSlice(ctx context.Context, q *bigquery.Query) ([]str
 	}
 	return names, nil
 }
-
-// containsOverriddenVariant checks whether the given variant key/value pair
-// is present in the includeVariants map (i.e. the request actually includes
-// data for this overridden variant).
-func containsOverriddenVariant(includeVariants map[string][]string, key, value string) bool {
-	for k, v := range includeVariants {
-		if k != key {
-			continue
-		}
-		for _, vv := range v {
-			if vv == value {
-				return true
-			}
-		}
-	}
-	return false
-}
-
-// copyIncludeVariantsAndRemoveOverrides copies includeVariants and removes
-// overridden variant values. For the default query (currOverride=-1), all
-// overridden values are removed. For an override query at index i, only
-// other overrides' values are removed. Returns true if the query should be
-// skipped because all values for a variant were removed.
-func copyIncludeVariantsAndRemoveOverrides(
-	overrides []configv1.VariantJunitTableOverride,
-	currOverride int,
-	includeVariants map[string][]string) (map[string][]string, bool) {
-
-	cp := make(map[string][]string)
-	for key, values := range includeVariants {
-		var newSlice []string
-		for _, v := range values {
-			if !shouldSkipVariant(overrides, currOverride, key, v) {
-				newSlice = append(newSlice, v)
-			}
-		}
-		if len(newSlice) == 0 {
-			return cp, true
-		}
-		cp[key] = newSlice
-	}
-	return cp, false
-}
-
-func shouldSkipVariant(overrides []configv1.VariantJunitTableOverride, currOverride int, key, value string) bool {
-	for i, override := range overrides {
-		if i == currOverride {
-			return false
-		}
-		if override.VariantName == key && override.VariantValue == value {
-			return true
-		}
-	}
-	return false
-}