From 086d0744254a42f4b357da186eca87fa597e3a35 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Fri, 3 Jul 2026 14:19:45 -0300 Subject: [PATCH] Rework feature gate test filtering to cover component owned jobs - Centralizes the logic for how to find test results for a featuregate for re-use in the openshift/api verification job. - Feature Gate API now returns hateoas links clients can follow. - Includes a link to the tests tagged for the feature gate name. - Includes a link to the synthetic "openshift-tests should work" as an indicator the job fully passed, for any jobs tied to a Capability. - Replaces the hack for featuregates containing Install to automatically route to all install results and exclude actual tests for the FG. - Adds feature gate seed data and e2e tests. --- cmd/sippy/seed_data.go | 59 +++++ pkg/apis/api/types.go | 17 +- pkg/sippyserver/server.go | 28 +++ sippy-ng/src/App.js | 15 +- sippy-ng/src/tests/FeatureGateDetail.js | 274 ++++++++++++++++++++++++ sippy-ng/src/tests/FeatureGates.js | 21 +- test/e2e/feature_gates_test.go | 125 +++++++++++ test/e2e/util/e2erequest.go | 6 +- 8 files changed, 521 insertions(+), 24 deletions(-) create mode 100644 sippy-ng/src/tests/FeatureGateDetail.js create mode 100644 test/e2e/feature_gates_test.go diff --git a/cmd/sippy/seed_data.go b/cmd/sippy/seed_data.go index adc6817ed..06a949da6 100644 --- a/cmd/sippy/seed_data.go +++ b/cmd/sippy/seed_data.go @@ -176,6 +176,15 @@ var syntheticJobs = []syntheticJobDef{ "Suite": "unknown", "Upgrade": "micro", "LayeredProduct": "none", }, }, + { + nameTemplate: "periodic-ci-openshift-release-master-ci-%s-e2e-aws-ovn-amd64-capability-networksegmentation", + variants: map[string]string{ + "Platform": "aws", "Architecture": "amd64", "Network": "ovn", + "Topology": "ha", "Installer": "ipi", "FeatureSet": "default", + "Suite": "parallel", "Upgrade": "none", "LayeredProduct": "none", + "Capability": "NetworkSegmentation", + }, + }, } // Job template constants for referencing specific jobs in test specs. @@ -331,6 +340,29 @@ var syntheticTests = []syntheticTestSpec{ }, }, + // --- Feature gate annotated tests --- + { + testID: "test-fg-network-segmentation", testName: "[sig-network] [FeatureGate:NetworkSegmentation] pods should communicate across segments", + component: "Networking / ovn-kubernetes", capabilities: []string{"networking"}, + jobCounts: map[string]map[string]testCount{ + awsAmd64Parallel: {"4.21": {100, 95, 0}, "4.22": {100, 93, 0}}, + gcpAmd64Parallel: {"4.21": {100, 97, 0}, "4.22": {100, 95, 0}}, + }, + }, + { + testID: "test-fg-network-segmentation-2", testName: "[sig-network] [FeatureGate:NetworkSegmentation] network policy should enforce segmentation", + component: "Networking / ovn-kubernetes", capabilities: []string{"networking"}, + jobCounts: map[string]map[string]testCount{ + awsAmd64Parallel: {"4.21": {100, 94, 0}, "4.22": {100, 92, 0}}, + }, + }, + { + testID: "test-fg-aws-dual-stack-install", testName: "[sig-installer] [FeatureGate:AWSDualStackInstall] dual stack install should succeed", + component: "Installer / openshift-installer", capabilities: []string{"AWSDualStackInstall"}, + jobCounts: map[string]map[string]testCount{ + awsAmd64Parallel: {"4.22": {50, 48, 0}}, + }, + }, // --- Install / health indicator tests: run on every job, every release --- { testID: "test-install-overall", testName: "install should succeed: overall", @@ -408,6 +440,10 @@ func seedSyntheticData(dbc *db.DB) error { } if count > 0 { log.Infof("Database already contains %d ProwJobs, skipping seed. Drop and recreate the database to re-seed (e.g. docker compose down -v).", count) + // Feature gates use FirstOrCreate and are safe to re-run on an existing DB. + if err := seedFeatureGates(dbc); err != nil { + return errors.WithMessage(err, "failed to seed feature gates") + } return nil } @@ -433,6 +469,10 @@ func seedSyntheticData(dbc *db.DB) error { return errors.WithMessage(err, "failed to create labels and symptoms") } + if err := seedFeatureGates(dbc); err != nil { + return errors.WithMessage(err, "failed to seed feature gates") + } + log.Info("Refreshing materialized views...") seedStart := time.Now().Add(-190 * 24 * time.Hour) sippyserver.RefreshData(dbc, nil, false, dailysummary.Options{StartOverride: &seedStart}) @@ -908,3 +948,22 @@ func createLabelsAndSymptoms(dbc *db.DB) error { return nil } + +func seedFeatureGates(dbc *db.DB) error { + featureGates := []models.FeatureGate{ + {Release: "4.22", Topology: "SelfManagedHA", FeatureSet: "TechPreviewNoUpgrade", FeatureGate: "NetworkSegmentation", Status: "enabled"}, + {Release: "4.22", Topology: "SelfManagedHA", FeatureSet: "TechPreviewNoUpgrade", FeatureGate: "AWSDualStackInstall", Status: "enabled"}, + } + + for _, fg := range featureGates { + var existing models.FeatureGate + if err := dbc.DB.Where( + "release = ? AND topology = ? AND feature_set = ? AND feature_gate = ?", + fg.Release, fg.Topology, fg.FeatureSet, fg.FeatureGate, + ).FirstOrCreate(&existing, fg).Error; err != nil { + return fmt.Errorf("failed to create feature gate %s/%s: %w", fg.Release, fg.FeatureGate, err) + } + } + log.Infof("Created %d feature gate records", len(featureGates)) + return nil +} diff --git a/pkg/apis/api/types.go b/pkg/apis/api/types.go index 66201c52f..8f25cc85c 100644 --- a/pkg/apis/api/types.go +++ b/pkg/apis/api/types.go @@ -1032,14 +1032,15 @@ type SippyViews struct { } type FeatureGate struct { - ID int `json:"id"` - FeatureGate string `json:"feature_gate"` - Release string `json:"release"` - UniqueTestCount int64 `json:"unique_test_count"` - FirstSeenIn string `json:"first_seen_in"` - FirstSeenInMajor int64 `json:"first_seen_in_major"` - FirstSeenInMinor int64 `json:"first_seen_in_minor"` - Enabled pq.StringArray `json:"enabled" gorm:"type:text[]"` + ID int `json:"id"` + FeatureGate string `json:"feature_gate"` + Release string `json:"release"` + UniqueTestCount int64 `json:"unique_test_count"` + FirstSeenIn string `json:"first_seen_in"` + FirstSeenInMajor int64 `json:"first_seen_in_major"` + FirstSeenInMinor int64 `json:"first_seen_in_minor"` + Enabled pq.StringArray `json:"enabled" gorm:"type:text[]"` + Links map[string]string `json:"links,omitempty" gorm:"-"` } func (fg FeatureGate) GetFieldType(param string) ColumnType { diff --git a/pkg/sippyserver/server.go b/pkg/sippyserver/server.go index 266d756a5..3934f7c6c 100644 --- a/pkg/sippyserver/server.go +++ b/pkg/sippyserver/server.go @@ -10,6 +10,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "os" "os/signal" "regexp" @@ -677,10 +678,37 @@ func (s *Server) jsonFeatureGates(w http.ResponseWriter, req *http.Request) { failureResponse(w, http.StatusInternalServerError, "couldn't parse filter opts: "+err.Error()) return } + baseAPIURL := api.GetBaseURL(req) + baseFrontendURL := api.GetBaseFrontendURL(req) + for i := range gates { + injectFeatureGateHATEOASLinks(&gates[i], release, baseAPIURL, baseFrontendURL) + } api.RespondWithJSON(http.StatusOK, w, gates) } } +func injectFeatureGateHATEOASLinks(fg *apitype.FeatureGate, release, baseAPIURL, baseFrontendURL string) { + fg.Links = make(map[string]string, 3) + + annotationFilter := url.QueryEscape(fmt.Sprintf( + `{"items":[{"columnField":"name","operatorValue":"contains","value":"FeatureGate:%s]"}]}`, + fg.FeatureGate)) + fg.Links["tests_by_annotation"] = fmt.Sprintf( + "%s/api/tests?release=%s&filter=%s", + baseAPIURL, release, annotationFilter) + + capabilityFilter := url.QueryEscape(fmt.Sprintf( + `{"items":[{"columnField":"name","operatorValue":"contains","value":"openshift-tests should work"},{"columnField":"variants","operatorValue":"contains","value":"Capability:%s"}],"linkOperator":"and"}`, + fg.FeatureGate)) + fg.Links["tests_by_capability"] = fmt.Sprintf( + "%s/api/tests?release=%s&filter=%s", + baseAPIURL, release, capabilityFilter) + + fg.Links["ui_detail"] = fmt.Sprintf( + "%s/feature_gates/%s/%s", + baseFrontendURL, release, url.PathEscape(fg.FeatureGate)) +} + func (s *Server) jsonTestAnalysis(w http.ResponseWriter, req *http.Request, dbFN func(*db.DB, *filter.Filter, string, string, time.Time) (map[string][]api.CountByDate, error)) { testName := s.getParamOrFail(w, req, "test") if testName == "" { diff --git a/sippy-ng/src/App.js b/sippy-ng/src/App.js index 988a5883a..25b42779a 100644 --- a/sippy-ng/src/App.js +++ b/sippy-ng/src/App.js @@ -15,7 +15,6 @@ import { findFirstNonGARelease, getReportStartDate, getUrlWithoutParams, - pathForTestSubstringByVariant, relativeTime, } from './helpers' import { JobAnalysis } from './jobs/JobAnalysis' @@ -46,6 +45,7 @@ import CollapsibleChatDrawer from './chat/CollapsibleChatDrawer' import ComponentReadiness from './component_readiness/ComponentReadiness' import Drawer from '@mui/material/Drawer' import EventsChart from './prow_job_runs/EventsChart' +import FeatureGateDetail from './tests/FeatureGateDetail' import FeatureGates from './tests/FeatureGates' import IconButton from '@mui/material/IconButton' import Install from './releases/Install' @@ -259,16 +259,17 @@ const JobsWrapper = () => { ) } -const FeatureGateRedirectWrapper = () => { +const FeatureGateDetailWrapper = () => { let { release, feature_gate } = useParams() if (release === 'latest') { release = findFirstNonGARelease(React.useContext(ReleasesContext)) } - return ( - ) } @@ -713,7 +714,7 @@ function App(props) { } + element={} /> { + if (!apiUrl) return + setLoaded(false) + setFetchError('') + + fetch(apiUrl) + .then((response) => { + if (response.status !== 200) { + throw new Error('server returned ' + response.status) + } + return response.json() + }) + .then((json) => { + setRows(json || []) + setLoaded(true) + }) + .catch((error) => { + setFetchError('Could not retrieve ' + title + ': ' + error) + setLoaded(true) + }) + }, [apiUrl, title]) + + const columns = [ + { + field: 'name', + headerName: 'Test Name', + flex: 4, + renderCell: (params) => ( + + {params.value} + + ), + }, + { + field: 'current_successes', + headerName: 'Current Successes', + type: 'number', + flex: 1, + }, + { + field: 'current_failures', + headerName: 'Current Failures', + type: 'number', + flex: 1, + }, + { + field: 'current_flakes', + headerName: 'Current Flakes', + type: 'number', + flex: 1, + }, + { + field: 'current_pass_percentage', + headerName: 'Current Pass %', + type: 'number', + flex: 1, + renderCell: (params) => + params.value != null ? params.value.toFixed(2) + '%' : '', + }, + { + field: 'current_runs', + headerName: 'Current Runs', + type: 'number', + flex: 1, + }, + ] + + if (fetchError) { + return {fetchError} + } + + return ( + + + {title} ({isLoaded ? rows.length : '...'} tests) + + row.name} + getRowHeight={() => 'auto'} + autoHeight={true} + rowsPerPageOptions={[10, 25, 50]} + pageSize={25} + sortModel={[{ field: 'current_pass_percentage', sort: 'asc' }]} + disableSelectionOnClick + /> + + ) +} + +TestResultsSection.propTypes = { + title: PropTypes.string.isRequired, + apiUrl: PropTypes.string, + release: PropTypes.string.isRequired, +} + +export default function FeatureGateDetail(props) { + const { release, featureGate } = props + + const [gate, setGate] = React.useState(null) + const [isLoaded, setLoaded] = React.useState(false) + const [fetchError, setFetchError] = React.useState('') + const [activeTab, setActiveTab] = React.useState(0) + + useEffect(() => { + document.title = `Sippy > ${release} > Feature Gates > ${featureGate}` + setLoaded(false) + setFetchError('') + + const filterParam = safeEncodeURIComponent( + JSON.stringify({ + items: [ + { + columnField: 'feature_gate', + operatorValue: 'equals', + value: featureGate, + }, + ], + }) + ) + + fetch( + process.env.REACT_APP_API_URL + + '/api/feature_gates?release=' + + release + + '&filter=' + + filterParam + ) + .then((response) => { + if (response.status !== 200) { + throw new Error('server returned ' + response.status) + } + return response.json() + }) + .then((json) => { + if (json && json.length > 0) { + setGate(json[0]) + } + setLoaded(true) + }) + .catch((error) => { + setFetchError('Could not retrieve feature gate: ' + error) + setLoaded(true) + }) + }, [release, featureGate]) + + if (fetchError) { + return ( + + {fetchError} + + ) + } + + if (!isLoaded) { + return ( + + + + ) + } + + if (!gate) { + return ( + + + Feature gate "{featureGate}" not found in release {release}. + + + ) + } + + return ( + + + + + {featureGate} + + + + + + Release: {gate.release} + + + First Seen In: {gate.first_seen_in} + + + Total Test Count: {gate.unique_test_count} + + + Enabled:{' '} + {gate.enabled && + gate.enabled.map((e) => ( + + ))} + + + + + + setActiveTab(v)} + aria-label="feature gate test sections" + > + + + + + + {activeTab === 0 && gate.links && ( + + )} + + {activeTab === 1 && gate.links && ( + + )} + + + ) +} + +FeatureGateDetail.propTypes = { + release: PropTypes.string.isRequired, + featureGate: PropTypes.string.isRequired, +} diff --git a/sippy-ng/src/tests/FeatureGates.js b/sippy-ng/src/tests/FeatureGates.js index 31ac07999..c366d2f74 100644 --- a/sippy-ng/src/tests/FeatureGates.js +++ b/sippy-ng/src/tests/FeatureGates.js @@ -4,7 +4,6 @@ import { Link, useNavigate } from 'react-router-dom' import { NumberParam, StringParam, useQueryParam } from 'use-query-params' import { parseVersion, - pathForTestSubstringByVariant, safeEncodeURIComponent, useStableJSONQueryParam, } from '../helpers' @@ -259,7 +258,7 @@ export default function FeatureGates(props) { type: 'number', flex: 2, renderCell: (params) => { - return {params.value} + return {params.value} }, }, { @@ -293,12 +292,18 @@ export default function FeatureGates(props) { }, ] - const linkForFGTests = (params) => { - let fgAnnotation = `FeatureGate:${params.row.feature_gate}]` - if (params.row.feature_gate.includes('Install')) { - fgAnnotation = 'install should succeed' + const linkForFeatureGateDetail = (params) => { + if (params.row.links && params.row.links.ui_detail) { + try { + const url = new URL(params.row.links.ui_detail) + return url.pathname + } catch (e) { + // fall through + } } - return pathForTestSubstringByVariant(props.release, fgAnnotation) + return `/feature_gates/${props.release}/${encodeURIComponent( + params.row.feature_gate + )}` } const fetchData = () => { @@ -343,7 +348,7 @@ export default function FeatureGates(props) { const onRowClick = (params) => { console.log('clicked') - navigate(linkForFGTests(params)) + navigate(linkForFeatureGateDetail(params)) } useEffect(() => { diff --git a/test/e2e/feature_gates_test.go b/test/e2e/feature_gates_test.go new file mode 100644 index 000000000..f622bbb5f --- /dev/null +++ b/test/e2e/feature_gates_test.go @@ -0,0 +1,125 @@ +package e2e + +import ( + "testing" + + "github.com/openshift/sippy/pkg/apis/api" + "github.com/openshift/sippy/test/e2e/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFeatureGatesAPI(t *testing.T) { + var gates []api.FeatureGate + err := util.SippyGet("/api/feature_gates?release="+util.Release, &gates) + require.NoError(t, err, "error fetching feature gates") + require.Greater(t, len(gates), 0, "no feature gates returned") + t.Logf("found %d feature gates", len(gates)) + + gatesByName := make(map[string]api.FeatureGate) + for _, g := range gates { + gatesByName[g.FeatureGate] = g + } + + t.Run("NetworkSegmentation gate exists with correct data", func(t *testing.T) { + fg, ok := gatesByName["NetworkSegmentation"] + require.True(t, ok, "NetworkSegmentation feature gate not found") + assert.Equal(t, util.Release, fg.Release) + assert.Greater(t, fg.UniqueTestCount, int64(0), "expected tests for NetworkSegmentation") + assert.NotEmpty(t, fg.Enabled, "expected enabled topologies") + }) + + t.Run("AWSDualStackInstall gate exists", func(t *testing.T) { + fg, ok := gatesByName["AWSDualStackInstall"] + require.True(t, ok, "AWSDualStackInstall feature gate not found") + assert.Equal(t, util.Release, fg.Release) + }) +} + +func TestFeatureGatesHATEOASLinks(t *testing.T) { + var gates []api.FeatureGate + err := util.SippyGet("/api/feature_gates?release="+util.Release, &gates) + require.NoError(t, err, "error fetching feature gates") + require.Greater(t, len(gates), 0, "no feature gates returned") + + for _, fg := range gates { + t.Run(fg.FeatureGate+" has HATEOAS links", func(t *testing.T) { + require.NotNil(t, fg.Links, "Links map should not be nil") + + testsAnnotation, ok := fg.Links["tests_by_annotation"] + assert.True(t, ok, "missing tests_by_annotation link") + assert.Contains(t, testsAnnotation, "/api/tests?release="+util.Release) + assert.Contains(t, testsAnnotation, "FeatureGate%3A"+fg.FeatureGate) + + testsCapability, ok := fg.Links["tests_by_capability"] + assert.True(t, ok, "missing tests_by_capability link") + assert.Contains(t, testsCapability, "/api/tests?release="+util.Release) + assert.Contains(t, testsCapability, "openshift-tests+should+work") + assert.Contains(t, testsCapability, "Capability%3A"+fg.FeatureGate) + + uiDetail, ok := fg.Links["ui_detail"] + assert.True(t, ok, "missing ui_detail link") + assert.Contains(t, uiDetail, "/feature_gates/"+util.Release+"/"+fg.FeatureGate) + }) + } +} + +func TestFeatureGatesAnnotationLinkFollowable(t *testing.T) { + var gates []api.FeatureGate + err := util.SippyGet("/api/feature_gates?release="+util.Release, &gates) + require.NoError(t, err) + + gatesByName := make(map[string]api.FeatureGate) + for _, g := range gates { + gatesByName[g.FeatureGate] = g + } + + fg, ok := gatesByName["NetworkSegmentation"] + require.True(t, ok, "NetworkSegmentation not found") + + link := fg.Links["tests_by_annotation"] + require.NotEmpty(t, link) + + var tests []api.Test + err = util.SippyGetAbsolute(link, &tests) + require.NoError(t, err, "failed to follow tests_by_annotation link") + assert.Greater(t, len(tests), 0, "expected tests when following tests_by_annotation link for NetworkSegmentation") + for _, test := range tests { + assert.Contains(t, test.Name, "FeatureGate:NetworkSegmentation", "test name should contain the feature gate annotation") + } +} + +func TestFeatureGatesCapabilityLinkFollowable(t *testing.T) { + var gates []api.FeatureGate + err := util.SippyGet("/api/feature_gates?release="+util.Release, &gates) + require.NoError(t, err) + + gatesByName := make(map[string]api.FeatureGate) + for _, g := range gates { + gatesByName[g.FeatureGate] = g + } + + fg, ok := gatesByName["NetworkSegmentation"] + require.True(t, ok, "NetworkSegmentation not found") + + link := fg.Links["tests_by_capability"] + require.NotEmpty(t, link) + + var tests []api.Test + err = util.SippyGetAbsolute(link, &tests) + require.NoError(t, err, "failed to follow tests_by_capability link") + assert.Greater(t, len(tests), 0, "expected tests when following tests_by_capability link for NetworkSegmentation") + for _, test := range tests { + assert.Contains(t, test.Name, "openshift-tests should work", "test name should contain openshift-tests should work") + } +} + +func TestFeatureGatesFilterByName(t *testing.T) { + var gates []api.FeatureGate + filterJSON := `{"items":[{"columnField":"feature_gate","operatorValue":"equals","value":"NetworkSegmentation"}]}` + err := util.SippyGet("/api/feature_gates?release="+util.Release+"&filter="+filterJSON, &gates) + require.NoError(t, err, "error filtering feature gates") + require.Len(t, gates, 1, "expected exactly one feature gate") + assert.Equal(t, "NetworkSegmentation", gates[0].FeatureGate) + assert.NotNil(t, gates[0].Links) +} diff --git a/test/e2e/util/e2erequest.go b/test/e2e/util/e2erequest.go index 1e2ece14c..6a3c917ae 100644 --- a/test/e2e/util/e2erequest.go +++ b/test/e2e/util/e2erequest.go @@ -40,8 +40,12 @@ func BuildE2EURL(apiPath string) string { } func SippyGet(path string, data interface{}) error { + return SippyGetAbsolute(BuildE2EURL(path), data) +} + +func SippyGetAbsolute(url string, data interface{}) error { client := &http.Client{Timeout: 30 * time.Second} - req, err := client.Get(BuildE2EURL(path)) //nolint:gosec // G704: URL is constructed from test helper's hardcoded localhost base URL + req, err := client.Get(url) //nolint:gosec // G704: URL is constructed from test helper's hardcoded localhost base URL if err != nil { return err }