Skip to content

WIP: Rework feature gate test filtering to cover component owned jobs #3731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dgoodwin wants to merge 1 commit into
base: main
Choose a base branch
from
+521 −24
Open

WIP: Rework feature gate test filtering to cover component owned jobs #3731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions cmd/sippy/seed_data.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
}

Expand All @@ -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})
Expand Down Expand Up @@ -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
}
17 changes: 9 additions & 8 deletions pkg/apis/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
28 changes: 28 additions & 0 deletions pkg/sippyserver/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net"
"net/http"
"net/http/httptest"
"net/url"
"os"
"os/signal"
"regexp"
Expand Down Expand Up @@ -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))
}
Comment on lines +690 to +710

@coderabbitai coderabbitai Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C5 '"variants"' --type=go pkg/filter pkg/db/query 2>/dev/null
rg -n -C5 'columnField.*variants|case "variants"' --type=go

Repository: openshift/sippy

Length of output: 153

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and search for feature-gate link construction and filter handling.
ast-grep outline pkg/sippyserver/server.go --view expanded || true
rg -n -C4 'injectFeatureGateHATEOASLinks|tests_by_capability|tests_by_annotation|FeatureGate:' pkg/sippyserver/server.go
rg -n -C4 'columnField.*variants|variants.*contains|case "variants"|operatorValue.*contains' pkg --type=go

Repository: openshift/sippy

Length of output: 18733

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect any filter/query code that may define "contains" semantics.
rg -n -C4 'contains' pkg --type=go
rg -n -C4 '"variants"' pkg --type=go
rg -n -C4 'FeatureGate|feature gate|feature_gate' pkg --type=go

Repository: openshift/sippy

Length of output: 50372

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the filter implementation to determine how "contains" behaves for array fields.
ast-grep outline pkg/filter --view expanded || true
rg -n -C4 'func .*contains|Operator: "contains"|strings.Contains|Contains' pkg/filter --type=go
sed -n '1,260p' pkg/filter/filterable.go

Repository: openshift/sippy

Length of output: 20233

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect how feature-gate annotations are represented and whether the trailing bracket is intentional.
rg -n -C3 'FeatureGate:' pkg --type=go
rg -n -C3 'FeatureGate.*\]' pkg --type=go
rg -n -C3 'annotations|annotation' pkg/apis pkg/filter pkg/sippyserver --type=go | head -n 200

Repository: openshift/sippy

Length of output: 12971

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the feature-gate response type to see how links and gate names are represented.
sed -n '1040,1105p' pkg/apis/api/types.go

Repository: openshift/sippy

Length of output: 1594

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for existing feature-gate names that might already share prefixes.
rg -n 'FeatureGate:' pkg --type=go

Repository: openshift/sippy

Length of output: 562

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check the in-memory array filter semantics for the exact operator we should use here.
sed -n '640,660p' pkg/filter/filterable.go

Repository: openshift/sippy

Length of output: 587

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the FeatureGate API type to see how its name/value fields are used.
sed -n '1070,1100p' pkg/apis/api/types.go

Repository: openshift/sippy

Length of output: 713

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for OCPFeatureGate usage to understand whether it should be included in the link filter.
rg -n 'OCPFeatureGate' .

Repository: openshift/sippy

Length of output: 983

Include both feature-gate tag forms

  • tests_by_annotation only matches FeatureGate:%s], but the data also carries OCPFeatureGate:%s] tags, so this link can miss valid tests.
  • tests_by_capability should use exact array membership (has entry) instead of contains; array filtering here is substring-based, so prefix collisions can pull in unrelated tests.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/sippyserver/server.go` around lines 690 - 710, The HATEOAS link builder
in injectFeatureGateHATEOASLinks is too narrow and can return incorrect test
links. Update tests_by_annotation to match both feature-gate tag forms,
including OCPFeatureGate alongside FeatureGate, and adjust tests_by_capability
to use exact array membership semantics (“has entry”) rather than
substring-based contains so unrelated tests are not matched by prefix
collisions.


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 == "" {
Expand Down
15 changes: 8 additions & 7 deletions sippy-ng/src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
findFirstNonGARelease,
getReportStartDate,
getUrlWithoutParams,
pathForTestSubstringByVariant,
relativeTime,
} from './helpers'
import { JobAnalysis } from './jobs/JobAnalysis'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -259,16 +259,17 @@ const JobsWrapper = () => {
)
}

const FeatureGateRedirectWrapper = () => {
const FeatureGateDetailWrapper = () => {
let { release, feature_gate } = useParams()
if (release === 'latest') {
release = findFirstNonGARelease(React.useContext(ReleasesContext))
}

return (
<Navigate
to={pathForTestSubstringByVariant(release, 'FeatureGate:' + feature_gate)}
replace
return RedirectLatestReleaseWrapper(
<FeatureGateDetail
key={'fg-detail-' + release + '-' + feature_gate}
release={release}
featureGate={feature_gate}
/>
)
}
Comment on lines +262 to 275

@coderabbitai coderabbitai Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect other usages of RedirectLatestReleaseWrapper on multi-segment routes
# to confirm whether this relative-navigation pattern is already known/handled elsewhere.
rg -n -B3 -A15 'RedirectLatestReleaseWrapper' sippy-ng/src/App.js

Repository: openshift/sippy

Length of output: 5644

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the feature gate routes and the wrapper implementation in context.
sed -n '150,310p' sippy-ng/src/App.js

printf '\n--- package version ---\n'
jq -r '.dependencies["react-router-dom"] // .devDependencies["react-router-dom"] // empty' sippy-ng/package.json

Repository: openshift/sippy

Length of output: 4466

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the feature gate route definitions and any navigation helpers tied to them.
rg -n -A4 -B4 'FeatureGateDetailWrapper|FeatureGatesWrapper|feature_gate|feature-gates|feature_gates' sippy-ng/src/App.js

Repository: openshift/sippy

Length of output: 1476

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for the actual feature-gate route path in the router config.
rg -n -A3 -B3 'FeatureGateDetail|featureGate|feature-gates|feature_gates' sippy-ng/src/App.js

Repository: openshift/sippy

Length of output: 1486

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the router table around the feature gate routes.
rg -n -A8 -B8 'FeatureGateDetailWrapper|FeatureGatesWrapper|Route path=.*feature|<Route' sippy-ng/src/App.js

Repository: openshift/sippy

Length of output: 7265

🌐 Web query:

React Router v6 useNavigate relative: "path" "../" path segment resolution docs

💡 Result:

In React Router v6, the useNavigate hook (and Link component) provides the relative option to control how relative paths, such as "../", are resolved [1][2]. By default, React Router v6 uses relative: "route" [1]. In this mode, ".." navigates up one level in the route hierarchy, effectively traversing the nested route tree rather than the URL string segments [2][3]. When you specify relative: "path", React Router switches to path-based resolution [1]. In this mode, ".." behaves similarly to a command-line "cd" operation, removing exactly one segment from the current URL path [1][2]. Key points regarding this behavior: - Scope: The relative: "path" option only impacts how the relative path string is resolved [1]. It does not change the "starting" location, which is always the route where the hook is called (or where the Link is rendered) [1][2]. - Difference from standard URL resolution: Because React Router's default behavior is based on the route hierarchy, it can sometimes produce results that differ from standard browser link behavior if your route structure does not align perfectly with your URL segments [1][3]. - Usage Example: navigate("..", { relative: "path" }); If you require resolution against the literal current URL path regardless of the route hierarchy, the recommended approach is to use the current location and the browser's URL constructor [1][2]: let location = useLocation; new URL("..", window.origin + location.pathname);

Citations:

🌐 Web query:

React Router v6.30.3 useNavigate relative path dot segments resolvePath "../" feature_gate

💡 Result:

In React Router v6.30.3, the useNavigate hook supports relative path segments (such as "../") for navigation [1][2]. There is no "feature_gate" required or present to enable this base functionality, as it is a core feature of the library [1]. Key behavioral details for relative paths in v6.30.3: Default Behavior (Route Hierarchy): By default, navigation is relative to the route hierarchy [1]. Using ".." in the navigate function will move the user up one level in the defined route structure, not necessarily one segment in the URL path [1][2]. Path-Relative Routing: If you want ".." to resolve relative to the actual URL path segments (e.g., to navigate to a sibling route), you can opt into this behavior by passing the relative: "path" option to the navigate function [1][3]: navigate("..", { relative: "path" }); Important Considerations: 1. The relative: "path" option only impacts how the relative path is resolved; it does not change the starting location, which remains tied to the route hierarchy where the hook is called [1][4]. 2. Splat Paths: While there is no feature gate for general "../" functionality, React Router does include a future flag named future.v7_relativeSplatPath, which affects how relative paths (including "..") resolve when inside splat routes [3][5]. This flag was introduced to fix legacy, buggy behavior regarding splat path resolution and is recommended for preparing applications for v7, but it is not a gate for the core ability to use "../" in navigation [5]. resolvePath: The resolvePath utility function follows similar resolution logic and is used internally by React Router components and hooks (like useResolvedPath) to resolve relative paths [6][7][8].

Citations:

latest is rewritten in the wrong URL segment. navigate('../' + defaultRelease, { relative: 'path' }) from /feature_gates/:release/:feature_gate changes :feature_gate, not :release, so the URL stays on latest and the redirect keeps firing. Navigate to /feature_gates/${defaultRelease}/${feature_gate} instead.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sippy-ng/src/App.js` around lines 262 - 275, The redirect logic in
FeatureGateDetailWrapper is updating the wrong path segment for the “latest”
release case. When useParams() returns release === 'latest', change the
navigation in RedirectLatestReleaseWrapper usage so it targets the full
/feature_gates/${defaultRelease}/${feature_gate} URL instead of a relative ../
redirect. Keep the existing release and feature_gate values from
FeatureGateDetailWrapper/FeatureGateDetail, but ensure the redirect writes the
release segment so the loop stops.

Expand Down Expand Up @@ -713,7 +714,7 @@ function App(props) {

<Route
path="/feature_gates/:release/:feature_gate"
element={<FeatureGateRedirectWrapper />}
element={<FeatureGateDetailWrapper />}
/>

<Route
Expand Down
Loading