[SANDBOX-1705] Upgrade to OCP 4.21 deps#1266
[SANDBOX-1705] Upgrade to OCP 4.21 deps#1266metlos wants to merge 5 commits intocodeready-toolchain:masterfrom
Conversation
- Update openshift/api to release-4.21 - Update k8s.io dependencies to v0.34.3 - Update controller-runtime to v0.22.5 - Add replace directives for api and toolchain-common forks - Fix prometheus TextParser: use NewTextParser with UTF8Validation (common v0.66.1 changed ValidationScheme enum) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated Go module dependency versions and added two replace directives in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 16-17: Update the stale comment above the openshift dependency to
reference the correct branch: change the text that currently says "using latest
commit from 'github.com/openshift/api branch release-4.19'" to mention
"release-4.21" so it matches the upgraded dependency line for
github.com/openshift/api v0.0.0-20260304172252-b0658d22beea; ensure only the
comment text is updated (not the module line).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1745fbb5-d30a-433d-a349-bbed687e34c5
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modtestsupport/metrics/metrics.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go.mod (1)
20-140:⚠️ Potential issue | 🔴 CriticalRemove replace directives pointing to personal forks before merging.
The go.mod file contains two replace directives that redirect dependencies to personal GitHub forks instead of canonical repositories:
github.com/codeready-toolchain/api→github.com/metlos/apigithub.com/codeready-toolchain/toolchain-common→github.com/metlos/toolchain-commonThese must be removed before this change is merged to production. Ensure companion PRs in the api and toolchain-common repositories are merged to their canonical locations first.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 20 - 140, Remove the temporary replace directives that point the modules github.com/codeready-toolchain/api -> github.com/metlos/api and github.com/codeready-toolchain/toolchain-common -> github.com/metlos/toolchain-common from go.mod; instead restore/leave the canonical module requirements (github.com/codeready-toolchain/api and github.com/codeready-toolchain/toolchain-common) and ensure the upstream PRs in those repositories are merged first so CI resolves the official packages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 150-152: The go.mod contains replace directives pointing to
personal forks (github.com/metlos/api and github.com/metlos/toolchain-common);
remove or replace them with canonical module paths and appropriate versions/tags
before merging. Either (1) change the replace lines to point at official
codeready-toolchain modules with released or official pre-release tags, (2)
remove the replace directives entirely if those forked changes are no longer
required, or (3) keep them only temporarily and update them to the canonical
repositories once the companion PRs are merged; ensure any types referenced
(e.g., UserSignup, MasterUserRecord, Space, and utilities used from
toolchain-common) are compatible with the chosen canonical versions.
---
Outside diff comments:
In `@go.mod`:
- Around line 20-140: Remove the temporary replace directives that point the
modules github.com/codeready-toolchain/api -> github.com/metlos/api and
github.com/codeready-toolchain/toolchain-common ->
github.com/metlos/toolchain-common from go.mod; instead restore/leave the
canonical module requirements (github.com/codeready-toolchain/api and
github.com/codeready-toolchain/toolchain-common) and ensure the upstream PRs in
those repositories are merged first so CI resolves the official packages.
| replace github.com/codeready-toolchain/api => github.com/metlos/api v0.0.0-20260318102359-237e681a639a | ||
|
|
||
| replace github.com/codeready-toolchain/toolchain-common => github.com/metlos/toolchain-common v0.0.0-20260318113116-fbd4bba9beb1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check companion PR status and verify no breaking changes
echo "=== Checking companion PR status ==="
gh pr view 501 --repo codeready-toolchain/api --json state,mergeable,title
gh pr view 522 --repo codeready-toolchain/toolchain-common --json state,mergeable,title
echo -e "\n=== Checking if forked commits match companion PRs ==="
# Check if the commit hashes in replace directives match the companion PRs
gh pr view 501 --repo codeready-toolchain/api --json commits --jq '.commits[-1].oid' | grep -q "237e681a639a" && echo "api commit matches" || echo "api commit MISMATCH"
gh pr view 522 --repo codeready-toolchain/toolchain-common --json commits --jq '.commits[-1].oid' | grep -q "fbd4bba9beb1" && echo "toolchain-common commit matches" || echo "toolchain-common commit MISMATCH"Repository: codeready-toolchain/toolchain-e2e
Length of output: 635
🏁 Script executed:
# First, examine the go.mod file to confirm the replace directives
wc -l go.modRepository: codeready-toolchain/toolchain-e2e
Length of output: 87
🏁 Script executed:
# Look at the specific lines mentioned (150-152) and surrounding context
sed -n '140,160p' go.modRepository: codeready-toolchain/toolchain-e2e
Length of output: 535
🏁 Script executed:
# Search for any comments or documentation about these replace directives
rg -i "metlos|replace" go.mod -B 2 -A 2Repository: codeready-toolchain/toolchain-e2e
Length of output: 376
🏁 Script executed:
# Check if there are any other replace directives in the file
grep "^replace " go.modRepository: codeready-toolchain/toolchain-e2e
Length of output: 308
🏁 Script executed:
# Search the codebase for any references to these forked packages or companion PRs
rg "metlos|companion.*pr|pending.*merge" -i --type-not=binary | head -20Repository: codeready-toolchain/toolchain-e2e
Length of output: 107
🏁 Script executed:
# Check git log for changes to go.mod, specifically the replace directives
git log --oneline --all -S "metlos" -- go.mod | head -20Repository: codeready-toolchain/toolchain-e2e
Length of output: 163
🏁 Script executed:
# Get the full commit that added these replace directives
git log -p -S "metlos" -- go.mod | head -100Repository: codeready-toolchain/toolchain-e2e
Length of output: 59
🏁 Script executed:
# Check if there's a PR description or commit message explaining this
git log --format="%H %s %b" -S "metlos" -- go.mod | head -50Repository: codeready-toolchain/toolchain-e2e
Length of output: 176
🏁 Script executed:
# Try to get more recent git history with simpler command
git log --oneline -20 go.modRepository: codeready-toolchain/toolchain-e2e
Length of output: 142
🏁 Script executed:
# Check if there's a PR or branch context
git branch -a --contains HEAD | head -10Repository: codeready-toolchain/toolchain-e2e
Length of output: 108
🏁 Script executed:
# Look for any README or documentation about replace directives or development setup
find . -maxdepth 2 -type f \( -name "README*" -o -name "CONTRIBUTING*" -o -name "*.md" \) -exec grep -l "replace\|fork\|metlos" {} \;Repository: codeready-toolchain/toolchain-e2e
Length of output: 90
🏁 Script executed:
# Check for any .github or documentation about this
ls -la .github/ 2>/dev/null || echo "No .github directory"Repository: codeready-toolchain/toolchain-e2e
Length of output: 310
🏁 Script executed:
# Look for any CI/build configuration that might reference these
find . -maxdepth 2 -type f \( -name "*.yml" -o -name "*.yaml" \) -exec grep -l "metlos\|replace" {} \;Repository: codeready-toolchain/toolchain-e2e
Length of output: 59
🏁 Script executed:
# Check the README for any context about replace directives or forks
cat README.adoc | grep -i -A 5 -B 5 "replace\|metlos\|fork\|dependency"Repository: codeready-toolchain/toolchain-e2e
Length of output: 4443
🏁 Script executed:
# Check if there's recent git history showing when replace directives were added
git log --all --format="%H %ai %s" go.mod | head -5Repository: codeready-toolchain/toolchain-e2e
Length of output: 676
🏁 Script executed:
# Get the actual commit diff that introduced metlos
git show 2b578400b082dd05b509660877c2999ad9f4a212:go.mod 2>/dev/null | grep -A 2 -B 2 "metlos" || echo "Commit doesn't reference metlos"Repository: codeready-toolchain/toolchain-e2e
Length of output: 376
🏁 Script executed:
# Check if metlos forks are actually accessible/valid
python3 << 'EOF'
import subprocess
import json
# Try to check if the commit hashes exist in the metlos repos
commits = {
"api": "237e681a639a",
"toolchain-common": "fbd4bba9beb1"
}
for repo_name, commit in commits.items():
repo_url = f"https://github.com/metlos/{repo_name}"
try:
result = subprocess.run(
["git", "ls-remote", repo_url, commit],
capture_output=True,
timeout=5,
text=True
)
if result.returncode == 0 and result.stdout.strip():
print(f"✓ {repo_name}: Commit {commit} exists and is accessible")
else:
print(f"✗ {repo_name}: Commit {commit} not found or repo not accessible")
except Exception as e:
print(f"✗ {repo_name}: Error checking - {e}")
EOFRepository: codeready-toolchain/toolchain-e2e
Length of output: 209
🏁 Script executed:
# Double-check by trying to fetch the actual commits directly from metlos repos
git ls-remote https://github.com/metlos/api refs/heads/* refs/tags/* 2>&1 | grep -E "237e681a639a|HEAD" || echo "Repository or commits not found"Repository: codeready-toolchain/toolchain-e2e
Length of output: 145
🏁 Script executed:
# Check if these versions exist as tags or releases
curl -s https://api.github.com/repos/metlos/api/tags 2>&1 | head -20 || echo "Cannot reach GitHub API"Repository: codeready-toolchain/toolchain-e2e
Length of output: 356
🏁 Script executed:
# The first commit exists! Let's verify the second one
git ls-remote https://github.com/metlos/toolchain-common refs/heads/* refs/tags/* 2>&1 | grep -E "fbd4bba9beb1|HEAD" || echo "Checking second repo..."Repository: codeready-toolchain/toolchain-e2e
Length of output: 145
🏁 Script executed:
# Check if these are in development branches by looking at the metlos repos
git ls-remote https://github.com/metlos/api 2>&1 | head -20Repository: codeready-toolchain/toolchain-e2e
Length of output: 930
🏁 Script executed:
# Check metlos/toolchain-common as well
git ls-remote https://github.com/metlos/toolchain-common 2>&1 | head -20Repository: codeready-toolchain/toolchain-e2e
Length of output: 1592
🏁 Script executed:
# Now let's check what the actual purpose is - look for any documentation or PR references
# that explain these forks are for development
find . -type f -name "*.md" -o -name "*.adoc" -o -name "*.txt" | xargs grep -l "upgrade-ocp\|metlos" 2>/dev/null | head -5Repository: codeready-toolchain/toolchain-e2e
Length of output: 59
🏁 Script executed:
# Check the current branch and what it's tracking
git branch -vvRepository: codeready-toolchain/toolchain-e2e
Length of output: 289
🏁 Script executed:
# Check if there's a PR description or branch name that explains this
git log --oneline -1Repository: codeready-toolchain/toolchain-e2e
Length of output: 142
🏁 Script executed:
# See if there's any documentation about this specific upgrade work
find . -type f \( -name "*.md" -o -name "*.adoc" \) -exec grep -l "4.21\|OCP.*4" {} \;Repository: codeready-toolchain/toolchain-e2e
Length of output: 96
🏁 Script executed:
# Check the setup/README.adoc for context about OCP 4.21
cat ./setup/README.adoc | grep -A 10 -B 10 "4.21\|upgrade\|OCP"Repository: codeready-toolchain/toolchain-e2e
Length of output: 1029
🏁 Script executed:
# Look at the actual commit that's being reviewed
git show 2b57840 --statRepository: codeready-toolchain/toolchain-e2e
Length of output: 14309
🏁 Script executed:
# Get the commit message in full
git show 2b57840 --format=fullerRepository: codeready-toolchain/toolchain-e2e
Length of output: 50393
Replace directives point to personal forks — must be resolved before merge.
The replace directives reference personal forks (github.com/metlos/api and github.com/metlos/toolchain-common) instead of the canonical codeready-toolchain repositories. These appear to be part of companion work on the upgrade-ocp-4.21 branch but cannot be merged as-is:
Options to resolve:
- Wait for companion PRs to merge to canonical repositories and update to point to official releases
- Remove the replace directives if the changes are no longer needed
- Update to point to official pre-release tags if available
Risks of merging with personal forks:
- Bypasses official code review and release process
- Commits could be force-pushed or deleted
- Potential for incompatible API changes affecting types like
UserSignup,MasterUserRecord,Space, and utilities fromtoolchain-common
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 150 - 152, The go.mod contains replace directives
pointing to personal forks (github.com/metlos/api and
github.com/metlos/toolchain-common); remove or replace them with canonical
module paths and appropriate versions/tags before merging. Either (1) change the
replace lines to point at official codeready-toolchain modules with released or
official pre-release tags, (2) remove the replace directives entirely if those
forked changes are no longer required, or (3) keep them only temporarily and
update them to the canonical repositories once the companion PRs are merged;
ensure any types referenced (e.g., UserSignup, MasterUserRecord, Space, and
utilities used from toolchain-common) are compatible with the chosen canonical
versions.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MatousJobanek, metlos The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…the newer versions)
|
|
@metlos: The following test 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. |
3 participants
Related PRs:
Summary by CodeRabbit