[Draft] - bumping with o/api changes for new capabilities#10618
Open
miyadav wants to merge 2 commits into
Open
[Draft] - bumping with o/api changes for new capabilities#10618miyadav wants to merge 2 commits into
miyadav wants to merge 2 commits into
Conversation
openshift-ci
Bot
added
the
do-not-merge/hold
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR redirects Go module resolution for ChangesModule Redirect and Test Alignment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
needs-rebase
miyadav
force-pushed
the
ocpcloud3368inst
branch
from
765040c to
6a3bb87
Compare
openshift-ci
Bot
removed
the
needs-rebase
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@go.mod`:
- Line 385: Replace the personal fork reference (miyadav/api) in the replace
directive on line 385 of go.mod with the official upstream
github.com/openshift/api, or if the fork is truly necessary, add a detailed
inline comment immediately above the replace directive that documents: (1) the
specific reason the fork is required with reference to OCPCLOUD-3368 and
openshift/api PR `#2884`, (2) the planned timeline for upstreaming this change or
confirmation of permanent fork maintenance, and (3) verification that the fork's
API remains compatible with all downstream consumers such as the operator/v1
types used in pkg/types/conversion/installconfig.go. Additionally, replace the
pseudo-version (v0.0.0-20260610140604-...) with a stable, production-ready
version tag from the fork once a decision is made to proceed, or reconsider
using a feature gate or temporary branch instead of a permanent replace
directive if this is experimental work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: a598b649-d9e9-4199-9421-eb25679ce0f3
⛔ Files ignored due to path filters (4)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (1)
go.mod
| sigs.k8s.io/cluster-api-provider-azure => github.com/mboersma/cluster-api-provider-azure v0.3.1-0.20251030205607-3161b9cc8d3e | ||
| ) | ||
|
|
||
| replace github.com/openshift/api => github.com/miyadav/api v0.0.0-20260610140604-79a53623c27b |
There was a problem hiding this comment.
Replace directive redirects to personal fork at pseudo-version; blocks production merge.
Line 385 introduces three critical supply chain concerns:
-
Personal fork (miyadav/api) instead of upstream: This breaks the installer's dependency contract and introduces supply chain attack risk. Why not use the official
github.com/openshift/api? -
Pseudo-version (v0.0.0-20260610140604-...) in production code: Per coding guidelines, no pre-release versions should be used in production. Pseudo-versions are development-only and cannot be reliably reproduced by consumers. The version predates the original (140604 vs 192510), suggesting the fork is behind upstream.
-
Missing justification: Coding guidelines require "justify need" for new deps. Only the PR description mentions OCPCLOUD-3368 and openshift/api PR
#2884. This must be documented in a code comment explaining (a) why the fork is necessary, (b) when/whether it will be upstreamed, and (c) how API compatibility is maintained.
Before this PR merges from draft:
- Add an inline comment explaining the fork's purpose and timeline.
- Verify the fork API is compatible with downstream consumers (e.g.,
github.com/openshift/api/operator/v1usage inpkg/types/conversion/installconfig.go). - Confirm whether this fork will be permanently maintained or upstreamed.
- If this is experimental, consider using a feature branch or feature gate rather than a permanent replace directive.
🤖 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 `@go.mod` at line 385, Replace the personal fork reference (miyadav/api) in the
replace directive on line 385 of go.mod with the official upstream
github.com/openshift/api, or if the fork is truly necessary, add a detailed
inline comment immediately above the replace directive that documents: (1) the
specific reason the fork is required with reference to OCPCLOUD-3368 and
openshift/api PR `#2884`, (2) the planned timeline for upstreaming this change or
confirmation of permanent fork maintenance, and (3) verification that the fork's
API remains compatible with all downstream consumers such as the operator/v1
types used in pkg/types/conversion/installconfig.go. Additionally, replace the
pseudo-version (v0.0.0-20260610140604-...) with a stable, production-ready
version tag from the fork once a decision is made to proceed, or reconsider
using a feature gate or temporary branch instead of a permanent replace
directive if this is experimental work.
Source: Coding guidelines
miyadav
force-pushed
the
ocpcloud3368inst
branch
from
6a3bb87 to
948421b
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
miyadav
force-pushed
the
ocpcloud3368inst
branch
2 times, most recently
from
54ff6e2 to
aa44a67
Compare
Member
Author
|
/retest-required |
Member
Author
|
/test unit |
Contributor
|
@miyadav: The following tests failed, say
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. |
… which gates CompatibilityRequirements and ClusterAPI capabilities behind CRDCompatibilityRequirementOperator using FeatureGateAwareEnum. Regenerate the install-config CRD accordingly. vendor changes osstream defaultfeature test revert generated by Claude
Contributor
|
@coderabbitai why are unit tests (ci/prow/unit) failing? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/installer"
pr="10618"
echo "== PR head SHA =="
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')
echo "$head_sha"
echo
echo "== Combined status contexts =="
gh api "repos/$repo/commits/$head_sha/status" --jq '
.statuses[]
| {context, state, description, target_url, updated_at}
'
echo
echo "== ci/prow/unit status =="
gh api "repos/$repo/commits/$head_sha/status" --jq '
.statuses[]
| select(.context == "ci/prow/unit")
| {context, state, description, target_url, updated_at, creator: .creator.login}
'
echo
echo "== Check suites / check runs on head commit =="
gh api "repos/$repo/commits/$head_sha/check-runs" --jq '
.check_runs[]
| {name, status, conclusion, details_url, started_at, completed_at}
'Length of output: 6627 🏁 Script executed: #!/bin/bash
set -euo pipefail
url="https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688"
echo "== Prow page title / artifact hints =="
curl -fsSL "$url" | rg -n "Artifacts|build-log|junit|spyglass|gcsweb|test-platform-results" -C2 || true
echo
echo "== Extract candidate artifact URLs from page =="
curl -fsSL "$url" \
| rg -o 'https://gcsweb\.ci\.openshift\.org/gcs/test-platform-results[^"]+|/view/gs/test-platform-results[^"]+|/gs/test-platform-results[^"]+' \
| head -n 50 || true
base="https://gcsweb.ci.openshift.org/gcs/test-platform-results/pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/artifacts"
echo
echo "== Try common artifact names =="
for path in \
"build-log.txt" \
"junit_operator.xml" \
"junit.xml" \
"junit_unit.xml" \
"junit_runner.xml" \
"junit/junit.xml"
do
echo "--- $base/$path"
code=$(curl -s -o /tmp/prow_artifact.$$ -w "%{http_code}" "$base/$path" || true)
echo "HTTP $code"
if [ "$code" = "200" ]; then
echo "First 80 lines:"
sed -n '1,80p' /tmp/prow_artifact.$$
fi
done
echo
echo "== If build-log exists, grep failures =="
code=$(curl -s -o /tmp/build-log.$$ -w "%{http_code}" "$base/build-log.txt" || true)
if [ "$code" = "200" ]; then
rg -n "FAIL|--- FAIL:|panic:|Error:|fatal|undefined:|cannot use|mismatch|expected|got:" /tmp/build-log.$$ -C3 | head -n 200 || true
fiLength of output: 5348 🏁 Script executed: #!/bin/bash
set -euo pipefail
paths=(
"pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/build-log.txt"
"pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/artifacts/build-log.txt"
"pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/artifacts/junit_operator.xml"
"pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/prowjob_junit.xml"
)
bases=(
"https://prow.ci.openshift.org/view/gs/test-platform-results"
"https://prow.ci.openshift.org/gs/test-platform-results"
"https://storage.googleapis.com/test-platform-results"
"https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results"
)
for base in "${bases[@]}"; do
echo "== BASE: $base =="
for rel in "${paths[@]}"; do
url="$base/$rel"
echo "--- $url"
code=$(curl -L -sS -o /tmp/fetch.$$ -w "%{http_code}" "$url" || true)
ctype=$(file -b --mime-type /tmp/fetch.$$ 2>/dev/null || true)
echo "HTTP $code MIME ${ctype:-unknown}"
if [[ "$code" == "200" ]]; then
echo "First 40 lines:"
sed -n '1,40p' /tmp/fetch.$$ | sed 's/\r$//'
echo
fi
done
echo
done
echo "== Probe artifact directory listing via listing hosts =="
for url in \
"https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/" \
"https://storage.googleapis.com/test-platform-results/pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/" \
"https://storage.googleapis.com/test-platform-results/pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/index.html"
do
echo "--- $url"
code=$(curl -L -sS -o /tmp/list.$$ -w "%{http_code}" "$url" || true)
echo "HTTP $code"
if [[ "$code" == "200" ]]; then
rg -n "build-log|junit|artifacts|xml|txt|FAIL|failure" /tmp/list.$$ -C1 || sed -n '1,80p' /tmp/list.$$
fi
doneLength of output: 36389 🏁 Script executed: #!/bin/bash
set -euo pipefail
url="https://storage.googleapis.com/test-platform-results/pr-logs/pull/openshift_installer/10618/pull-ci-openshift-installer-main-unit/2069808096678514688/build-log.txt"
tmp=$(mktemp)
curl -L -fsSL "$url" -o "$tmp"
echo "== Failure excerpts =="
rg -n -C4 --color=never '--- FAIL:|FAIL\tgithub\.com/openshift/installer/pkg/types/validation|pkg/types/validation|expected|got:|Error:|panic:' "$tmp" | head -n 200 || true
echo
echo "== Last 200 lines of build log =="
tail -n 200 "$tmp"Length of output: 27261
What the artifacts show:
The warnings in the log about:
are present, but the actual fatal condition recorded by the job is still the failed test package above. So the short answer is: the PR currently breaks tests under If you want, I can dig one step further and identify the exact failing test case/assertion inside that package. |
Contributor
|
@coderabbitai yes please explain the exact failing test |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/installer"
pr="10618"
echo "== PR head SHA =="
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')
echo "$head_sha"
echo
echo "== ci/prow/unit status entry =="
gh api "repos/$repo/commits/$head_sha/status" --jq '
.statuses[]
| select(.context == "ci/prow/unit")
| {context, state, description, target_url, updated_at}
'
job_url=$(gh api "repos/$repo/commits/$head_sha/status" --jq '
.statuses[]
| select(.context == "ci/prow/unit")
| .target_url
' | head -n1)
echo
echo "== ci/prow/unit target URL =="
echo "$job_url"
job_id=$(printf '%s\n' "$job_url" | rg -o '[0-9]{10,}' | tail -n1 || true)
echo
echo "== Parsed job ID =="
echo "${job_id:-<none>}"
if [[ -z "${job_id:-}" ]]; then
echo "Could not determine job ID from target URL" >&2
exit 1
fi
base="https://storage.googleapis.com/test-platform-results/pr-logs/pull/openshift_installer/${pr}/pull-ci-openshift-installer-main-unit/${job_id}"
log="$(mktemp)"
curl -L -fsSL "$base/build-log.txt" -o "$log"
echo
echo "== Failure excerpts mentioning pkg/types/validation =="
rg -n -C6 --color=never \
'--- FAIL:|FAIL\tgithub\.com/openshift/installer/pkg/types/validation|pkg/types/validation|panic:|expected|actual|got:' \
"$log" | head -n 250 || true
echo
echo "== Last 120 lines of build-log.txt =="
tail -n 120 "$log"
echo
echo "== Test files under pkg/types/validation =="
fd -t f . pkg/types/validation || true
echo
echo "== AST outline for candidate test files =="
fd -t f '.*_test\.go' pkg/types/validation -x sh -c 'echo "--- $1"; ast-grep outline "$1" --view expanded || true' sh {}
echo
echo "== Search for failing test names in source =="
# Extract likely test names from the log and search for them in the repo.
rg -o '^--- FAIL: ([^(]+)' "$log" | sed 's/^--- FAIL: //' | sort -u | while read -r testname; do
[[ -z "$testname" ]] && continue
echo "--- searching for $testname"
rg -n -C2 --fixed-strings "func ${testname}" pkg/types/validation || true
doneLength of output: 23003 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/installer"
pr="10618"
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')
job_url=$(gh api "repos/$repo/commits/$head_sha/status" --jq '
.statuses[] | select(.context == "ci/prow/unit") | .target_url
' | head -n1)
job_id=$(printf '%s\n' "$job_url" | rg -o '[0-9]{10,}' | tail -n1)
base="https://storage.googleapis.com/test-platform-results/pr-logs/pull/openshift_installer/${pr}/pull-ci-openshift-installer-main-unit/${job_id}"
log="$(mktemp)"
curl -L -fsSL "$base/build-log.txt" -o "$log"
echo "== Failure lines around TestFeatureGates in build log =="
grep -n -C8 -- 'TestFeatureGates\|--- FAIL:\|FAIL[[:space:]]\+github.com/openshift/installer/pkg/types/validation\|panic:\|expected\|actual\|got:' "$log" | head -n 200 || true
echo
echo "== Candidate junit artifacts =="
for path in \
"artifacts/junit_operator.xml" \
"artifacts/junit.xml" \
"artifacts/junit_unit.xml" \
"artifacts/junit_runner.xml" \
"prowjob_junit.xml"
do
url="$base/$path"
code=$(curl -L -s -o /tmp/junit.$$ -w "%{http_code}" "$url" || true)
echo "--- $path HTTP $code"
if [[ "$code" == "200" ]]; then
echo "Top failing testcases from $path:"
python - <<'PY'
import xml.etree.ElementTree as ET
from pathlib import Path
p = Path('/tmp/junit.$$')
try:
root = ET.parse(p).getroot()
except Exception as e:
print(f"Could not parse XML: {e}")
raise SystemExit(0)
for tc in root.iter('testcase'):
failure = tc.find('failure')
err = tc.find('error')
if failure is not None or err is not None:
node = failure if failure is not None else err
print({
"classname": tc.attrib.get("classname"),
"name": tc.attrib.get("name"),
"message": node.attrib.get("message"),
"text": (node.text or "").strip()[:1000],
})
PY
fi
done
echo
echo "== Source: pkg/types/validation/featuregate_test.go =="
sed -n '1,260p' pkg/types/validation/featuregate_test.goLength of output: 18476
Failure details from the unit job: What that means:
So the regression is: Relevant source:
In short: this is a real validation behavior mismatch, not a Prow problem. |
miyadav
force-pushed
the
ocpcloud3368inst
branch
from
aa44a67 to
75c65d4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bumping changes from my fork to add new capabilities
WIP - OCPCLOUD-3368
/hold
Generated by - claude-opus-4-6(2.1.169)
Summary by CodeRabbit
replacedirective to redirect a referenced API package to an alternate source.