Automated - Update config ./config/openshift.yaml#3349
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds many new periodic job entries for OpenShift releases 4.20–4.22: updates Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: openshift-bot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/variantregistry/snapshot.yaml`:
- Around line 66900-66937: The ROSA nightly variants are missing
Release/ReleaseMajor/ReleaseMinor and ContainerRuntime because setRelease()
cannot extract a version from job names; update pkg/variantregistry logic so
either setRelease() can fall back to a configured default for ROSA nightlies or
populate explicit release fields in snapshot.yaml for the jobs; specifically,
either modify setRelease() to detect the "rosa" platform/nightly pattern and
assign a stable release (and then let setContainerRuntime() derive runtime), or
add explicit Release/ReleaseMajor/ReleaseMinor and ContainerRuntime keys to the
periodic-ci-openshift-online-rosa-regional-platform-main-nightly-* entries in
snapshot.yaml so setContainerRuntime() and setNetwork() can fill the variant
correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 270b1b88-2f33-44a7-ade2-d0973c28e267
📒 Files selected for processing (2)
config/openshift.yamlpkg/variantregistry/snapshot.yaml
| periodic-ci-openshift-online-rosa-regional-platform-main-nightly-ephemeral: | ||
| Aggregation: none | ||
| Architecture: amd64 | ||
| CGroupMode: v2 | ||
| FeatureSet: default | ||
| Installer: ipi | ||
| JobTier: candidate | ||
| LayeredProduct: none | ||
| NetworkAccess: default | ||
| NetworkStack: ipv4 | ||
| OS: rhcos9 | ||
| Owner: service-delivery | ||
| Platform: rosa | ||
| Procedure: none | ||
| Scheduler: default | ||
| SecurityMode: default | ||
| Suite: unknown | ||
| Topology: ha | ||
| Upgrade: none | ||
| periodic-ci-openshift-online-rosa-regional-platform-main-nightly-integration: | ||
| Aggregation: none | ||
| Architecture: amd64 | ||
| CGroupMode: v2 | ||
| FeatureSet: default | ||
| Installer: ipi | ||
| JobTier: candidate | ||
| LayeredProduct: none | ||
| NetworkAccess: default | ||
| NetworkStack: ipv4 | ||
| OS: rhcos9 | ||
| Owner: service-delivery | ||
| Platform: rosa | ||
| Procedure: none | ||
| Scheduler: default | ||
| SecurityMode: default | ||
| Suite: unknown | ||
| Topology: ha | ||
| Upgrade: none |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect variant registry parsing/validation logic =="
rg -n -C3 'ContainerRuntime|Network|ReleaseMajor|ReleaseMinor|required|validate|snapshot' pkg/variantregistry
echo
echo "== Show ROSA nightly main entries and key fields =="
rg -n -C25 '^periodic-ci-openshift-online-rosa-regional-platform-main-nightly-(ephemeral|integration):|^\s{4}(ContainerRuntime|Network|Release|ReleaseMajor|ReleaseMinor):' pkg/variantregistry/snapshot.yaml
echo
echo "== Report missing dimensions for all ROSA nightly main entries =="
python - <<'PY'
import re
from pathlib import Path
text = Path("pkg/variantregistry/snapshot.yaml").read_text()
pattern = re.compile(
r'^(periodic-ci-openshift-online-rosa-regional-platform-main-nightly-[^:]+):\n'
r'((?:\s{4}[A-Za-z][A-Za-z0-9]*:.*\n)+)',
re.M
)
required = ["ContainerRuntime", "Network", "Release", "ReleaseMajor", "ReleaseMinor"]
for name, body in pattern.findall(text):
keys = set(re.findall(r'^\s{4}([A-Za-z][A-Za-z0-9]*):', body, re.M))
missing = [k for k in required if k not in keys]
print(f"{name}: missing={','.join(missing) if missing else 'none'}")
PYRepository: openshift/sippy
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check snapshot.yaml documentation/comments ==="
head -50 pkg/variantregistry/snapshot.yaml
echo
echo "=== Check how snapshot.yaml is loaded and used ==="
rg -n "snapshot" pkg/variantregistry/ocp.go | head -20
echo
echo "=== Check if there's validation or schema for variants ==="
rg -n "validate|schema|required|omitempty" pkg/variantregistry/ocp.go | grep -i variant | head -20
echo
echo "=== Show a few complete non-ROSA periodic entries for comparison ==="
rg -n -A 20 '^periodic-ci-openshift-release-master-nightly-4\.[0-9]+-e2e-aws-ovn-' pkg/variantregistry/snapshot.yaml | head -80Repository: openshift/sippy
Length of output: 1405
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Extract the two ROSA nightly entries from lines 66900-66937 ==="
sed -n '66900,66937p' pkg/variantregistry/snapshot.yaml
echo
echo "=== Count fields in those ROSA entries ==="
sed -n '66900,66937p' pkg/variantregistry/snapshot.yaml | grep -c '^\s\{4\}[A-Za-z]'
echo
echo "=== Extract a standard AWS entry for comparison ==="
rg -A 25 '^periodic-ci-openshift-release-master-nightly-4\.16-e2e-aws-ovn-single-node-serial:' pkg/variantregistry/snapshot.yaml | head -30
echo
echo "=== Count fields in standard AWS entry ==="
rg -A 25 '^periodic-ci-openshift-release-master-nightly-4\.16-e2e-aws-ovn-single-node-serial:' pkg/variantregistry/snapshot.yaml | grep '^\s\{4\}[A-Za-z]' | wc -l
echo
echo "=== Check for Release fields in ROSA nightly entries ==="
sed -n '66900,66937p' pkg/variantregistry/snapshot.yaml | grep -E 'Release|Network|ContainerRuntime'Repository: openshift/sippy
Length of output: 1162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find all ROSA entries in snapshot.yaml ==="
rg -n '^periodic-ci-openshift-online-rosa' pkg/variantregistry/snapshot.yaml | head -20
echo
echo "=== Extract one complete ROSA entry (regional platform) ==="
rg -A 18 '^periodic-ci-openshift-online-rosa-regional-platform-main' pkg/variantregistry/snapshot.yaml | head -40
echo
echo "=== Check for Network field in ANY ROSA entry ==="
rg -B 1 'Network:' pkg/variantregistry/snapshot.yaml | rg -A 1 'rosa' | head -20
echo
echo "=== Check if Release fields are present in any ROSA entry ==="
rg -B 3 'Release.*:' pkg/variantregistry/snapshot.yaml | grep -i rosa | head -10
echo
echo "=== Show what dimensions ARE present in one AWS nightly entry ==="
rg -A 20 '^periodic-ci-openshift-release-master-nightly-4\.16-e2e-aws-ovn:' pkg/variantregistry/snapshot.yaml | sed -n '1,22p'Repository: openshift/sippy
Length of output: 1319
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check snapshot.yaml role: is it input data or reference/schema? ==="
rg -n "snapshot" pkg/variantregistry/*.go | grep -i "load\|read\|parse\|open" | head -20
echo
echo "=== Look for how variants from snapshot are used ==="
rg -n "variantsFile\|snapshot" pkg/variantregistry/ocp.go -A 3 -B 3 | head -50
echo
echo "=== Compare ROSA test case expected variants vs snapshot ==="
rg -A 30 'job.*periodic-ci-openshift-online-rosa.*4\.19' pkg/variantregistry/ocp_test.go | head -35
echo
echo "=== Check if Release/Network are conditionally optional or computed ==="
rg -n "Variant.*=.*\"\"" pkg/variantregistry/ocp.goRepository: openshift/sippy
Length of output: 273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find snapshot.Load and understand its purpose ==="
rg -n "func.*Load\|Load(" pkg/variantregistry/*.go | grep -i snapshot
echo
echo "=== Check the snapshot package/file ==="
fd snapshot pkg/variantregistry
echo
echo "=== Look at the test context around snapshot.Load ==="
rg -B 10 -A 10 'snapshot\.Load.*snapshot\.yaml' pkg/variantregistry/ocp_test.go
echo
echo "=== Check if snapshot.yaml fields must match ocp.go variant setters ==="
rg -n "variantsFile\[" pkg/variantregistry/ocp.go | head -20
echo
echo "=== Show the merge/override logic between snapshot and computed variants ==="
rg -B 5 -A 15 "for k, v := range variantFile" pkg/variantregistry/ocp.goRepository: openshift/sippy
Length of output: 214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find snapshot.Load definition ==="
rg -n "Load" pkg/variantregistry/*.go | grep -i snapshot | head -10
echo
echo "=== Find snapshot package/file ==="
find pkg/variantregistry -name "*snapshot*" -type f
echo
echo "=== Check test context around snapshot.Load ==="
rg -B 10 -A 10 'snapshot.Load' pkg/variantregistry/ocp_test.go | head -50
echo
echo "=== Check variantFile usage in ocp.go ==="
rg -n 'variantFile\[' pkg/variantregistry/ocp.go
echo
echo "=== Show merge/override logic between snapshot and computed variants ==="
rg -B 5 -A 15 'for k, v := range variantFile' pkg/variantregistry/ocp.go | head -60Repository: openshift/sippy
Length of output: 1153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Show snapshot.go structure and Load function ==="
cat pkg/variantregistry/snapshot.go
echo
echo "=== Show the variant merge/override logic in ocp.go (lines around variantFile) ==="
sed -n '260,330p' pkg/variantregistry/ocp.goRepository: openshift/sippy
Length of output: 4555
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify: Do ROSA nightly job names contain release versions? ==="
echo "Checking the two ROSA nightly job names:"
echo "1. periodic-ci-openshift-online-rosa-regional-platform-main-nightly-ephemeral"
echo "2. periodic-ci-openshift-online-rosa-regional-platform-main-nightly-integration"
echo ""
echo "Looking for release version patterns (e.g., '4.16', '4.19') in setRelease() logic:"
rg -A 30 'func.*setRelease' pkg/variantregistry/ocp.go | head -40
echo
echo "=== Compare with standard release job that HAS release version ==="
echo "Job: periodic-ci-openshift-release-master-nightly-4.16-e2e-aws-ovn-single-node-serial"
echo "(This DOES have '4.16' in the name, so Release/ReleaseMajor/ReleaseMinor should populate)"
echo ""
echo "=== Check if missing fields is expected for non-release-versioned jobs ==="
rg -B 5 -A 10 'if len.*releasesInJobName' pkg/variantregistry/ocp.goRepository: openshift/sippy
Length of output: 2046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check setNetwork and setContainerRuntime logic for jobs without Release ==="
rg -A 40 'func setNetwork' pkg/variantregistry/ocp.go | head -50
echo
echo "=== Check if Network/ContainerRuntime get default values ==="
rg -A 40 'func setContainerRuntime' pkg/variantregistry/ocp.go | head -50
echo
echo "=== Final confirmation: Do the ROSA entries OMIT Network and ContainerRuntime? ==="
sed -n '66900,66937p' pkg/variantregistry/snapshot.yaml | grep -E 'Network|ContainerRuntime' || echo "CONFIRMED: Neither Network nor ContainerRuntime present in ROSA entries"Repository: openshift/sippy
Length of output: 3039
ROSA nightly entries intentionally lack release versions—verify if release mapping should be explicit.
These ROSA entries omit Network and ContainerRuntime because the job names (periodic-ci-openshift-online-rosa-regional-platform-main-nightly-*) contain no release version (e.g., "4.16", "4.19"). The variant identification logic depends on extracting release versions from job names: without them, setRelease() produces no Release/ReleaseMajor/ReleaseMinor, and setContainerRuntime() cannot determine the runtime version (it logs a warning and skips setting the variant). The sparse shape reflects this design, but if ROSA nightly jobs should track a stable release, consider adding explicit release entries to snapshot.yaml or ensuring setNetwork() populates the Network field for these jobs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/variantregistry/snapshot.yaml` around lines 66900 - 66937, The ROSA
nightly variants are missing Release/ReleaseMajor/ReleaseMinor and
ContainerRuntime because setRelease() cannot extract a version from job names;
update pkg/variantregistry logic so either setRelease() can fall back to a
configured default for ROSA nightlies or populate explicit release fields in
snapshot.yaml for the jobs; specifically, either modify setRelease() to detect
the "rosa" platform/nightly pattern and assign a stable release (and then let
setContainerRuntime() derive runtime), or add explicit
Release/ReleaseMajor/ReleaseMinor and ContainerRuntime keys to the
periodic-ci-openshift-online-rosa-regional-platform-main-nightly-* entries in
snapshot.yaml so setContainerRuntime() and setNetwork() can fill the variant
correctly.
|
Scheduling required tests: |
9d29d43 to
ddfdb47
Compare
|
Scheduling required tests: |
|
@openshift-bot: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary by CodeRabbit