Downstream container-libs vendoring - #2066
Conversation
…roposal Describe how downstream CRI-O will vendor a downstream fork of container-libs using Go module replace directives, ensuring alignment with the container-libs version shipped in the target RHEL release. Assisted-by: Claude Code <https://claude.com/claude-code>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
|
||
| ### Workflow Description | ||
|
|
||
| **CRI-O downstream maintainer** is a developer responsible for maintaining `openshift/cri-o`. |
There was a problem hiding this comment.
Can we change the persona? Instead of "CRI-O downstream maintainer" can we say "OpenShift Node Team member". Then it cover a wider range of folks in the team and anyone can do it.
| 2. The `replace` directives in `openshift/cri-o`'s `go.mod` are updated to reference the new version. TODO: Evaluate automating this step (e.g., a bot that opens a PR when a new downstream container-libs release is tagged). In the interim, a CRI-O downstream maintainer performs this manually. | ||
| 3. The maintainer runs `go mod tidy` and `go mod vendor`. | ||
| 4. The maintainer submits a PR to `openshift/cri-o` with the updated vendor directory and `go.mod`/`go.sum`. | ||
| 5. CI validates the build and runs tests. |
There was a problem hiding this comment.
Can we add a test in origin that will fail once RHEL updates the container-libs in podman? That will help to trigger the process of updating cri-o downstream?
There was a problem hiding this comment.
Thanks!
IMHO we should test semantic correctness (it behaves intentionally), rather than syntax correctness (it has the same version as podman), but it's worth mentioning here.
…ment details Fill in TODOs and resolve inconsistencies in the enhancement proposal: - Add downstream container-libs repo link and concrete go.mod replace directives - Expand user stories with specific personas, workflows, and rationale - Add Dual Stream section covering RHEL 9/10 parity and future divergence handling - Add Bug Fix / Feature Backport Workflow section - Add RHEL Version Switch subsection to Upgrade/Downgrade Strategy - Address version skew for dual-stream RHEL transitions - Add container-libs version divergence detection test plan - Fix Goal openshift#3 to not contradict dual-stream build tag/spec patching - Clarify that MachineConfig-delivered custom configs may need updates on RHEL switch - Correct upstream module paths to go.podman.io vanity imports - Remove resolved open question about Go module proxy (already rejected in Alternatives) Assisted-by: Claude Code <https://claude.com/claude-code>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe enhancement proposes downstream CRI-O vendoring of a container-libs fork through Go module ChangesCRI-O container-libs vendoring
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 10❌ Failed checks (10 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@enhancements/node/downstream-container-libs-vendoring-for-cri-o.md`:
- Around line 78-85: Reconcile the “Non-Goals” statement with the
feature-backport workflow described near the fork scope and user story:
explicitly state whether feature backports are supported. If supported, update
the scope and non-goal to allow them; otherwise remove the feature-backport
workflow and its associated user story while retaining bug-fix-only
expectations.
- Around line 154-168: Narrow the “No impact” statements in the Hypershift /
Hosted Control Planes, Standalone Clusters, Single-node Deployments or
MicroShift, and OpenShift Kubernetes Engine sections to clarify that no
automatic cluster-level configuration edit is required. Explicitly note that
operators must review custom persisted storage.conf and containers.conf settings
when container-libs semantics change.
- Around line 127-137: Expand the “Security Fix Workflow” section to define the
embargoed-CVE release path before approval: identify the responsible owner,
turnaround target, private module/repository access, internal build and test
process, fast-track handling, and the post-embargo synchronization steps for the
public mirror and openshift/cri-o.
- Around line 88-104: Update the downstream CRI-O vendoring section to select
one dependency-source design instead of presenting an unresolved private-module
example. Document the exact replacement module paths and versions, whether
public CI resolves upstream dependencies or private forks, and how credentials
are supplied for private repositories; ensure the design is consistent with the
test plan and the RPM-patching alternative.
- Around line 60-64: Add an authoritative RHEL-stream version matrix to the
document, defining immutable downstream-container-libs versions or commits,
per-stream Go module replace inputs, and the expected vendored outputs. Specify
CI checks that validate the selected inputs and generated vendor contents; do
not imply build tags select go.mod replace directives. If stream-specific module
selection requires automation, document that separate step and update the
no-custom-tooling claim accordingly.
- Around line 368-372: Expand the “Version Skew Strategy” section to define the
storage migration contract for each supported RHEL/container-libs transition,
add mixed-version storage compatibility tests covering those transitions within
the same OCP release, and document explicit node recovery and rollback
procedures for migration failures, including the existing rollback TODO.
- Around line 5-15: Replace the TODO placeholder values in the reviewers,
approvers, and tracking-link fields with actual content before merging. For the
reviewers and approvers fields, substitute the TODO comments with a list of
actual GitHub usernames who should review and approve this enhancement. For the
tracking-link field, replace the TODO comment with a valid tracking ticket URL.
The api-approvers field is already correctly configured and does not require
changes.
- Around line 221-225: Define explicit replacement precedence rules for the
three Go modules (go.podman.io/common, go.podman.io/storage, and
go.podman.io/image/v5) in the document to establish which replacement directive
is authoritative. Add CI validation checks that verify the effective
replacements applied in go.mod match the intended precedence and that the
resulting vendor/modules.txt reflects the correct resolved replacements,
preventing text merge conflicts from introducing valid but incorrect module
versions.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 96eb04f2-3028-45f2-9a1f-86584b2e7eaf
📒 Files selected for processing (1)
enhancements/node/downstream-container-libs-vendoring-for-cri-o.md
| reviewers: | ||
| - TODO: Identify reviewers | ||
| approvers: | ||
| - TODO: Identify approvers | ||
| api-approvers: | ||
| - None | ||
| creation-date: 2026-07-23 | ||
| last-updated: 2026-07-23 | ||
| status: informational | ||
| tracking-link: | ||
| - TODO: Add tracking link |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# Find and read the enhancement file
cd enhancements/node && head -20 downstream-container-libs-vendoring-for-cri-o.mdRepository: openshift/enhancements
Length of output: 509
🏁 Script executed:
# Find the enhancement schema definition in the repository
find . -name "*.yaml" -o -name "*.yml" | xargs grep -l "reviewers\|approvers" 2>/dev/null | head -5Repository: openshift/enhancements
Length of output: 160
🏁 Script executed:
# Look for schema files that validate enhancements
find . -type f \( -name "*.md" -o -name "*.yaml" -o -name "*.yml" \) | xargs grep -l "enhancement.*schema\|kep.*template" 2>/dev/null | head -5Repository: openshift/enhancements
Length of output: 404
🏁 Script executed:
# Test YAML parsing of the pattern in question
python3 - <<'PY'
import yaml
# Test 1: Current pattern with TODO: (space after colon)
yaml_with_todo_space = """
reviewers:
- TODO: Identify reviewers
approvers:
- TODO: Identify approvers
"""
# Test 2: Quoted version
yaml_quoted = """
reviewers:
- "TODO: Identify reviewers"
approvers:
- "TODO: Identify approvers"
"""
# Test 3: Plain TODO without colon
yaml_plain = """
reviewers:
- TODO
approvers:
- TODO
"""
try:
result1 = yaml.safe_load(yaml_with_todo_space)
print("Pattern 1 (TODO: with space) parsed as:")
print(result1)
print("\nType of reviewers list item:", type(result1['reviewers'][0]), repr(result1['reviewers'][0]))
except Exception as e:
print(f"Pattern 1 failed: {e}")
print("\n" + "="*50 + "\n")
try:
result2 = yaml.safe_load(yaml_quoted)
print("Pattern 2 (quoted) parsed as:")
print(result2)
print("\nType of reviewers list item:", type(result2['reviewers'][0]), repr(result2['reviewers'][0]))
except Exception as e:
print(f"Pattern 2 failed: {e}")
print("\n" + "="*50 + "\n")
try:
result3 = yaml.safe_load(yaml_plain)
print("Pattern 3 (plain TODO) parsed as:")
print(result3)
print("\nType of reviewers list item:", type(result3['reviewers'][0]), repr(result3['reviewers'][0]))
except Exception as e:
print(f"Pattern 3 failed: {e}")
PYRepository: openshift/enhancements
Length of output: 277
🏁 Script executed:
# Install PyYAML and test YAML parsing
pip install PyYAML -q && python3 - <<'PY'
import yaml
# Test 1: Current pattern with TODO: (space after colon)
yaml_with_todo_space = """
reviewers:
- TODO: Identify reviewers
approvers:
- TODO: Identify approvers
"""
# Test 2: Quoted version
yaml_quoted = """
reviewers:
- "TODO: Identify reviewers"
approvers:
- "TODO: Identify approvers"
"""
# Test 3: Plain TODO without colon
yaml_plain = """
reviewers:
- TODO
approvers:
- TODO
"""
try:
result1 = yaml.safe_load(yaml_with_todo_space)
print("Pattern 1 (TODO: with space) parsed as:")
print(result1)
print("Type of reviewers list item:", type(result1['reviewers'][0]), "Value:", repr(result1['reviewers'][0]))
except Exception as e:
print(f"Pattern 1 failed: {e}")
print("\n" + "="*50 + "\n")
try:
result2 = yaml.safe_load(yaml_quoted)
print("Pattern 2 (quoted) parsed as:")
print(result2)
print("Type of reviewers list item:", type(result2['reviewers'][0]), "Value:", repr(result2['reviewers'][0]))
except Exception as e:
print(f"Pattern 2 failed: {e}")
print("\n" + "="*50 + "\n")
try:
result3 = yaml.safe_load(yaml_plain)
print("Pattern 3 (plain TODO) parsed as:")
print(result3)
print("Type of reviewers list item:", type(result3['reviewers'][0]), "Value:", repr(result3['reviewers'][0]))
except Exception as e:
print(f"Pattern 3 failed: {e}")
PYRepository: openshift/enhancements
Length of output: 1001
🏁 Script executed:
# Search for validation scripts or CI checks that process enhancement files
find . -type f \( -name "*.sh" -o -name "*.py" -o -name "Makefile" \) | xargs grep -l "enhancement\|kep" 2>/dev/null | head -10Repository: openshift/enhancements
Length of output: 284
🏁 Script executed:
# Look for any README or documentation about enhancement format
find . -maxdepth 2 -type f -name "README*" | xargs cat 2>/dev/null | head -100Repository: openshift/enhancements
Length of output: 3439
🏁 Script executed:
# Find and read the enhancement template
find . -name "enhancement_template.md" -o -name "*template*" -type f | head -10Repository: openshift/enhancements
Length of output: 221
🏁 Script executed:
# Look for the template file more broadly
find . -path ./tools -prune -o -type f -name "*.md" -print | xargs grep -l "template" | head -5Repository: openshift/enhancements
Length of output: 321
🏁 Script executed:
# Check the template-lint.sh script to see how enhancements are validated
cat ./hack/template-lint.shRepository: openshift/enhancements
Length of output: 1640
🏁 Script executed:
# Use standard YAML knowledge to test the pattern
# In YAML: "TODO: Identify reviewers" at list item level is parsed as a string
# The colon syntax only creates a mapping if there's a key-value pair on the same/nested level
cat > /tmp/test_yaml.txt <<'EOF'
Test 1 - Current format (TODO: with space):
reviewers:
- TODO: Identify reviewers
This is valid YAML and parses as:
- reviewers is a key
- Its value is a list with one string item: "TODO: Identify reviewers"
- The string is NOT a mapping; "TODO:" and "Identify reviewers" are part of one string value
Test 2 - If parsed as mapping (which it wouldn't be):
This would only happen if written as:
- TODO: Identify reviewers
(as a nested structure with proper indentation for a key-value pair)
But that's not the case here.
The YAML spec says: in a list item, "TODO: ..." is a string unless it's
properly formatted as a mapping with indentation indicating a nested key-value.
Conclusion: The current pattern IS valid YAML and parses correctly as a string.
EOF
cat /tmp/test_yaml.txtRepository: openshift/enhancements
Length of output: 924
🏁 Script executed:
# Read the enhancement template to see what the schema expects
head -50 ./guidelines/enhancement_template.mdRepository: openshift/enhancements
Length of output: 3549
🏁 Script executed:
# Check the full front matter section of the template
sed -n '1,40p' ./guidelines/enhancement_template.mdRepository: openshift/enhancements
Length of output: 2997
🏁 Script executed:
# Look for any YAML schema validation files
find . -type f \( -name "*.json" -o -name "*.schema" -o -name "*schema*" \) 2>/dev/null | head -10Repository: openshift/enhancements
Length of output: 160
Replace incomplete placeholder values with actual reviewer and approver names.
The fields reviewers, approvers, and tracking-link use TODO: placeholders that must be filled before this enhancement is merged. Use the template convention: assign actual GitHub usernames for reviewers and approvers, and provide a tracking ticket URL for tracking-link. The api-approvers field is correctly set to None.
🤖 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 `@enhancements/node/downstream-container-libs-vendoring-for-cri-o.md` around
lines 5 - 15, Replace the TODO placeholder values in the reviewers, approvers,
and tracking-link fields with actual content before merging. For the reviewers
and approvers fields, substitute the TODO comments with a list of actual GitHub
usernames who should review and approve this enhancement. For the tracking-link
field, replace the TODO comment with a valid tracking ticket URL. The
api-approvers field is already correctly configured and does not require
changes.
| - As a Node team member, I want downstream CRI-O to vendor the downstream container-libs so that the container libraries match those shipped with the target RHEL release, avoiding incompatibilities between the container runtime and RHEL host configuration files (e.g., `storage.conf`, `containers.conf`, `registries.conf`, `policy.json`). | ||
|
|
||
| - As a Node team member or release engineer, I want the vendoring relationship managed via Go module `replace` directives so the build process is straightforward and reproducible, with no custom build tooling required beyond standard `go mod tidy` and `go mod vendor`. | ||
|
|
||
| - As a Node team member, I want a clear process for updating container-libs versions in downstream CRI-O so that when RHEL ships a new container-libs version, I know exactly which steps to follow — update `replace` directives, run `go mod tidy`/`go mod vendor`, submit a PR — and can do so without deep knowledge of the container-libs internals. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/node/downstream-container-libs-vendoring-for-cri-o.md"
printf '%s\n' '--- file outline ---'
ast-grep outline "$file" 2>/dev/null || true
printf '%s\n' '--- relevant sections ---'
sed -n '1,280p' "$file"
printf '%s\n' '--- repository references ---'
rg -n -S 'container-libs|RHEL 9\.8|RHEL 10\.2|go mod tidy|go mod vendor|replace directive|build tag|special handling' . --glob '!vendor/**' --glob '!node_modules/**' | head -300Repository: openshift/enhancements
Length of output: 46431
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/node/downstream-container-libs-vendoring-for-cri-o.md"
printf '%s\n' '--- CI, alternatives, and conclusions ---'
sed -n '280,410p' "$file"
printf '%s\n' '--- targeted structural checks ---'
python3 - "$file" <<'PY'
from pathlib import Path
import re, sys
text = Path(sys.argv[1]).read_text()
checks = {
"explicit RHEL mapping TODO": r"TODO: Add an explicit downstream container-libs version to RHEL version mapping table",
"second mapping TODO": r"TODO: Determine the RHEL version to container-libs version mapping",
"dual-stream same-version assertion": r"OCP 5\.0–5\.2 targets both RHEL 9\.8 and RHEL 10\.2.*?same container-libs version",
"go.mod patching option": r"\*\*`go\.mod` patching:\*\*.*?replace.*?patched and re-vendored",
"build-tags option": r"\*\*Go build tags:\*\*.*?go build -tags el10",
"standard commands claim": r"no custom build tooling required beyond standard `go mod tidy` and `go mod vendor`",
"CI source-of-truth gap": r"Requires a reliable source of truth for Podman's container-libs version",
}
for name, pattern in checks.items():
match = re.search(pattern, text, re.S)
print(f"{name}: {'present' if match else 'absent'}")
PYRepository: openshift/enhancements
Length of output: 8760
🌐 Web query:
Go command documentation: whether build tags select or alter go.mod module replace directives, and how module replacement is chosen during go mod operations
💡 Result:
Build tags (also known as build constraints) do not affect, select, or alter the replace directives in a go.mod file [1][2]. The go.mod file and its dependency graph, including all require, replace, and exclude directives, are resolved by the Go command before and independently of the build process that evaluates build tags [3][4]. Build tags are used during the compilation phase to determine which source files are included in a package for the target platform or configuration [5][6], but they do not influence which versions of modules are loaded into the build list or how module replacements are applied [2][3]. Regarding how module replacement is chosen during go mod operations: 1. Scope: replace directives are only honored when they appear in the main module's go.mod file or a go.work file [7][8]. They are explicitly ignored in the go.mod files of dependencies [7][8]. 2. Resolution: A replace directive does not independently add a module to the module graph; it only takes effect if the module path (and optionally the version) on the left side of the directive is already required by the build [7][8][9]. 3. Precedence: When go.work files are used, a replace directive in the go.work file will override any version-specific replace directive in a go.mod file [8]. 4. Mechanism: Go uses Minimal Version Selection (MVS) to build the dependency graph [3][4]. The go command resolves the required versions first, and if a module is flagged for replacement, the Go toolchain simply redirects the search for that module's source code to the specified local directory or alternative module path [7][10]. This substitution happens at the file system or module resolution level, not based on build-time tags [7][8]. Because build tags are not recorded in go.mod [2], the go command cannot automatically know which tags might be required to build a dependency correctly [1][11]. Consequently, build-specific configuration—such as requiring different dependencies based on build tags—cannot be managed through go.mod directives [2].
Citations:
- 1: https://forum.golangbridge.org/t/go-mod-when-dependencies-depend-on-build-tags/26728
- 2: proposal: cmd/go: record default build tags in go.mod golang/go#43288
- 3: https://golang.design/under-the-hood/en/part5toolchain/ch17modules/minimum/
- 4: https://go.googlesource.com/proposal/%2B/master/design/24301-versioned-go.md
- 5: https://go.dev/src/go/build/doc.go
- 6: https://pkg.go.dev/go/build
- 7: https://go.dev/doc/modules/gomod-ref
- 8: https://go.dev/ref/mod?id=go-work-file-replace
- 9: https://go.dev/ref/mod
- 10: https://go.dev/doc/modules/managing-dependencies
- 11: https://go.dev/blog/using-go-modules
Add an authoritative RHEL-stream version matrix and define the build inputs.
The document leaves the RHEL-to-downstream-container-libs mapping as TODO and provides no immutable version or commit source of truth. Add per-stream replace inputs, expected vendor outputs, and CI checks. Build tags select source files; they do not select go.mod replace directives. If streams diverge, document and automate a separate module-selection step, or remove the no-custom-tooling claim.
🧰 Tools
🪛 LanguageTool
[style] ~64-~64: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d go mod tidy and go mod vendor. - As a Node team member, I want a clear proc...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~64-~64: Consider a different adjective to strengthen your wording.
Context: ...r`, submit a PR — and can do so without deep knowledge of the container-libs interna...
(DEEP_PROFOUND)
🤖 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 `@enhancements/node/downstream-container-libs-vendoring-for-cri-o.md` around
lines 60 - 64, Add an authoritative RHEL-stream version matrix to the document,
defining immutable downstream-container-libs versions or commits, per-stream Go
module replace inputs, and the expected vendored outputs. Specify CI checks that
validate the selected inputs and generated vendor contents; do not imply build
tags select go.mod replace directives. If stream-specific module selection
requires automation, document that separate step and update the
no-custom-tooling claim accordingly.
| #### Security Fix Workflow | ||
|
|
||
| When a CVE or security fix is needed in container-libs: | ||
|
|
||
| 1. The fix is applied to the downstream container-libs fork (backported from upstream if applicable). | ||
| 2. A new downstream container-libs version is tagged. | ||
| 3. The container-libs version update workflow (above) is followed to pull the fix into downstream CRI-O. | ||
|
|
||
| For embargoed CVEs, the fix cannot be pushed to any public repository (including a GitHub mirror of container-libs, if used) until the embargo is lifted. During the embargo period, the fix is built and tested through internal processes only. Once the embargo is lifted, the public mirror and `openshift/cri-o` are updated following the standard workflow. | ||
|
|
||
| TODO: Clarify the expected turnaround time for security fixes and whether there is a fast-track process. Define the specific embargo workflow with the security response team. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/node/downstream-container-libs-vendoring-for-cri-o.md"
printf '%s\n' '--- file outline ---'
ast-grep outline "$file" 2>/dev/null || true
printf '%s\n' '--- requested sections ---'
sed -n '100,155p' "$file"
sed -n '210,245p' "$file"
printf '%s\n' '--- related workflow and embargo references ---'
rg -n -i -C 3 'embargo|security fix|container-libs|vendoring|private module|fast.track|turnaround|disclosure' enhancements .github 2>/dev/null | head -n 300Repository: openshift/enhancements
Length of output: 40184
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository files related to the design ---'
git ls-files | rg -i 'container.libs|cri.o|security|vendoring|embargo|release'
printf '%s\n' '--- document headings and references ---'
rg -n '^#{1,6} |container-libs|CRI-O|CVE|embargo' enhancements/node/downstream-container-libs-vendoring-for-cri-o.mdRepository: openshift/enhancements
Length of output: 22138
Define the embargoed-CVE release path before approval.
Document the owner, turnaround target, private module access, internal build and test path, fast-track process, and post-embargo synchronization between the public mirror and openshift/cri-o.
🤖 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 `@enhancements/node/downstream-container-libs-vendoring-for-cri-o.md` around
lines 127 - 137, Expand the “Security Fix Workflow” section to define the
embargoed-CVE release path before approval: identify the responsible owner,
turnaround target, private module/repository access, internal build and test
process, fast-track handling, and the post-embargo synchronization steps for the
public mirror and openshift/cri-o.
| #### Hypershift / Hosted Control Planes | ||
|
|
||
| No impact. This is a build-time vendoring change. CRI-O runs on worker nodes regardless of control plane topology. | ||
|
|
||
| #### Standalone Clusters | ||
|
|
||
| No impact beyond the CRI-O binary shipping with updated container-libs. | ||
|
|
||
| #### Single-node Deployments or MicroShift | ||
|
|
||
| No impact. The change is limited to which container-libs version is compiled into the CRI-O binary. | ||
|
|
||
| #### OpenShift Kubernetes Engine | ||
|
|
||
| No impact. CRI-O is part of the base platform available to both OCP and OKE. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Narrow the “no impact” statements.
The document says that upgrades require no configuration changes, but it also states that persisted storage.conf and containers.conf may need updates when container-libs semantics change. State that no automatic cluster-level edit is required, while custom configuration still requires operator review.
Also applies to: 358-364
🤖 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 `@enhancements/node/downstream-container-libs-vendoring-for-cri-o.md` around
lines 154 - 168, Narrow the “No impact” statements in the Hypershift / Hosted
Control Planes, Standalone Clusters, Single-node Deployments or MicroShift, and
OpenShift Kubernetes Engine sections to clarify that no automatic cluster-level
configuration edit is required. Explicitly note that operators must review
custom persisted storage.conf and containers.conf settings when container-libs
semantics change.
| **Risk:** A container-libs update introduces a regression in downstream CRI-O. | ||
| **Mitigation:** Revert to the previous container-libs version via a rollback patch and release a new CRI-O build. TODO: Define the specific rollback process. | ||
|
|
||
| **Risk:** The `replace` directives conflict with other downstream patches in `openshift/cri-o` that also modify `go.mod`. | ||
| **Mitigation:** Go module `replace` directives are additive and well-understood. Conflicts are resolved through standard `go.mod` merge practices. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/node/downstream-container-libs-vendoring-for-cri-o.md"
printf '%s\n' '--- target file context ---'
sed -n '1,280p' "$file"
printf '%s\n' '--- container-libs and replacement references ---'
rg -n -i -C 3 'container-libs|replace|vendor|go\.mod|cri-o' --glob '!vendor/**' --glob '!node_modules/**' .
printf '%s\n' '--- repository files related to CI and vendoring ---'
git ls-files | rg -i '(^|/)(go\.mod|go\.sum|vendor/|\.github/|ci|test|container-libs|cri-o)' | head -300Repository: openshift/enhancements
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/node/downstream-container-libs-vendoring-for-cri-o.md"
printf '%s\n' '--- target section ---'
sed -n '190,245p' "$file"
printf '%s\n' '--- exact references in target file ---'
rg -n -i -C 4 'container-libs|replace|vendor|go\.mod|module' "$file"
printf '%s\n' '--- repository-level exact references ---'
rg -n -i 'container-libs|github.com/.*/(containers|storage|image|ocicrypt)|go\.mod|vendor/modules\.txt' --glob '!enhancements/**' --glob '!vendor/**' --glob '!node_modules/**' . | head -300
printf '%s\n' '--- tracked CI and policy files ---'
git ls-files | rg '^(\.github/|hack/|Makefile|Makefile\.)' | head -300Repository: openshift/enhancements
Length of output: 50380
🌐 Web query:
Official Go documentation for replace directives with duplicate/conflicting replacements, and for vendor/modules.txt consistency checks when using -mod=vendor
💡 Result:
In Go, the replace directive in a go.mod file is used to redirect module resolution, allowing you to substitute a module path and version with another module or a local filesystem path [1][2]. Duplicate or Conflicting Replace Directives When multiple replace directives target the same module, the Go toolchain does not strictly forbid them in the syntax, but it applies specific rules for resolution. - A replace directive with a version (e.g., example.com/mod v1.0.0 =>...) is more specific than one without a version (example.com/mod =>...). The specific, versioned replacement takes precedence when the corresponding version is selected by the module solver [3]. - If you have multiple conflicting directives (such as replacing the same version with different targets), the Go toolchain typically processes them sequentially, but rely on them being deterministic is not advised; generally, the most specific match for the required version is used [1][3]. - In a workspace context (go.work), a replace directive in the go.work file overrides any replace directive for the same module or version in the individual module's go.mod file [1][4]. Consistency Checks for vendor/modules.txt When using -mod=vendor, the Go command requires the vendor/modules.txt file to be strictly consistent with the go.mod (and go.work) files [5]. This consistency check ensures that the dependencies recorded in vendor/modules.txt accurately reflect the dependency graph, including all replacements [6][5]. The Go command will report an error if: - A module is required in go.mod but not marked as explicit in vendor/modules.txt [7]. - A replacement exists in go.mod but is missing from vendor/modules.txt [7]. - A replacement target in vendor/modules.txt differs from the replacement specified in go.mod [7]. - A replacement is recorded in vendor/modules.txt but does not exist in go.mod [7]. If inconsistencies are detected, the go command will fail the build with an error message indicating the discrepancy and suggesting that you run 'go mod vendor' (or 'go work vendor' in a workspace) to synchronize the vendor directory with the current module configuration [7][5].
Citations:
- 1: https://go.dev/ref/mod
- 2: https://go.dev/doc/modules/gomod-ref
- 3: proposal: cmd/go: pinning replacements in require golang/go#68638
- 4: cmd/go: go.work:
go buildrequires versioned replace directive when replacing a used module with a local path golang/go#54264 - 5: https://groups.google.com/g/golang-nuts/c/G_00oU9tkQ4
- 6: https://archive.ph/0gYJ2
- 7: https://tip.golang.org/src/cmd/go/testdata/script/work_vendor_modules_txt_consistent.txt
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/node/downstream-container-libs-vendoring-for-cri-o.md"
printf '%s\n' '--- document invariants ---'
python3 - "$file" <<'PY'
import re, sys
from pathlib import Path
text = Path(sys.argv[1]).read_text()
expected = [
"go.podman.io/common",
"go.podman.io/storage",
"go.podman.io/image/v5",
]
block = re.search(r"```go\nreplace \(\n(.*?)\n\)\n```", text, re.S)
assert block, "replace block not found"
found = re.findall(r"^\s*(go\.podman\.io/\S+)\s+=>", block.group(1), re.M)
print("replace module paths:", found)
print("all three present:", found == expected)
print("mitigation says additive:", "replace` directives are additive" in text)
print("CI/vendor metadata requirement present:",
"vendor/modules.txt" in text and "effective replacement" in text)
printf '%s\n' '--- synthetic duplicate replacement parse ---'
if command -v go >/dev/null 2>&1; then
d="$(mktemp -d)"
trap 'rm -rf "$d"' EXIT
cat >"$d/duplicate.mod" <<'EOF'
module example.com/main
go 1.20
require example.com/a v1.0.0
replace example.com/a => ../a
replace example.com/a => ../b
EOF
if go mod edit -json -modfile="$d/duplicate.mod" >/tmp/duplicate-replace.json 2>/tmp/duplicate-replace.err; then
echo "duplicate unversioned replacements: accepted"
cat /tmp/duplicate-replace.json
else
echo "duplicate unversioned replacements: rejected"
cat /tmp/duplicate-replace.err
fi
else
echo "go tool: unavailable"
fiRepository: openshift/enhancements
Length of output: 408
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/node/downstream-container-libs-vendoring-for-cri-o.md"
python3 - "$file" <<'PY'
import re
import shutil
import subprocess
import sys
import tempfile
from pathlib import Path
text = Path(sys.argv[1]).read_text()
expected = [
"go.podman.io/common",
"go.podman.io/storage",
"go.podman.io/image/v5",
]
block = re.search(r"```go\nreplace \(\n(.*?)\n\)\n```", text, re.S)
assert block, "replace block not found"
found = re.findall(r"^\s*(go\.podman\.io/\S+)\s+=>", block.group(1), re.M)
print("replace module paths:", found)
print("all three present:", found == expected)
print("mitigation says additive:", "replace` directives are additive" in text)
print("CI/vendor metadata requirement present:",
"vendor/modules.txt" in text and "effective replacement" in text)
go = shutil.which("go")
if not go:
print("go tool: unavailable")
raise SystemExit
with tempfile.TemporaryDirectory() as d:
modfile = Path(d) / "duplicate.mod"
modfile.write_text(
"module example.com/main\n\n"
"go 1.20\n\n"
"require example.com/a v1.0.0\n\n"
"replace example.com/a => ../a\n"
"replace example.com/a => ../b\n"
)
result = subprocess.run(
[go, "mod", "edit", "-json", f"-modfile={modfile}"],
text=True,
capture_output=True,
check=False,
)
print("duplicate unversioned replacements accepted:", result.returncode == 0)
if result.returncode:
print(result.stderr.strip())
else:
print(result.stdout.strip())
PYRepository: openshift/enhancements
Length of output: 517
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import shutil
import subprocess
import tempfile
from pathlib import Path
go = shutil.which("go")
if not go:
print("go tool: unavailable")
raise SystemExit
cases = {
"duplicate unversioned replacements": (
"replace example.com/a => ../a\n"
"replace example.com/a => ../b\n"
),
"version-scoped replacements": (
"replace example.com/a v1.0.0 => ../a\n"
"replace example.com/a v1.0.0 => ../b\n"
),
}
for name, replacements in cases.items():
with tempfile.TemporaryDirectory() as d:
root = Path(d)
(root / "duplicate.mod").write_text(
"module example.com/main\n\n"
"go 1.20\n\n"
"require example.com/a v1.0.0\n\n"
+ replacements
)
result = subprocess.run(
[go, "mod", "edit", "-json", "-modfile=duplicate.mod"],
cwd=d,
text=True,
capture_output=True,
check=False,
)
print(f"{name} accepted:", result.returncode == 0)
if result.returncode:
print(result.stderr.strip())
else:
print(result.stdout.strip())
PYRepository: openshift/enhancements
Length of output: 475
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import shutil
import subprocess
import tempfile
from pathlib import Path
go = shutil.which("go")
if not go:
print("go tool: unavailable")
raise SystemExit
cases = {
"duplicate unversioned replacements": (
"replace example.com/a => ../a\n"
"replace example.com/a => ../b\n"
),
"version-scoped replacements": (
"replace example.com/a v1.0.0 => ../a\n"
"replace example.com/a v1.0.0 => ../b\n"
),
}
for name, replacements in cases.items():
with tempfile.TemporaryDirectory() as d:
root = Path(d)
(root / "go.mod").write_text("module example.com/main\n\ngo 1.20\n")
(root / "duplicate.mod").write_text(
"module example.com/main\n\n"
"go 1.20\n\n"
"require example.com/a v1.0.0\n\n"
+ replacements
)
result = subprocess.run(
[go, "mod", "edit", "-json", "-modfile=duplicate.mod"],
cwd=d,
text=True,
capture_output=True,
check=False,
)
print(f"{name} accepted:", result.returncode == 0)
if result.returncode:
print(result.stderr.strip())
else:
print(result.stdout.strip())
PYRepository: openshift/enhancements
Length of output: 912
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import shutil
import subprocess
import tempfile
from pathlib import Path
go = shutil.which("go")
if not go:
print("go tool: unavailable")
raise SystemExit
modules = {
"go.podman.io/common": "common",
"go.podman.io/storage": "storage",
"go.podman.io/image/v5": "image",
}
with tempfile.TemporaryDirectory() as d:
root = Path(d)
replacements = []
imports = []
for index, (path, package) in enumerate(modules.items(), 1):
source = root / f"source-{index}"
source.mkdir()
(source / "go.mod").write_text(f"module {path}\n\ngo 1.20\n")
(source / f"{package}.go").write_text(
f"package {package}\n\nfunc Value() string {{ return \"{package}\" }}\n"
)
replacements.append(f" {path} => {source}\n")
imports.append(f' "{path}"\n')
(root / "go.mod").write_text(
"module example.com/main\n\n"
"go 1.20\n\n"
+ "".join(f"require {path} v0.0.0\n" for path in modules)
+ "\nreplace (\n"
+ "".join(replacements)
+ ")\n"
)
(root / "main.go").write_text(
"package main\n\nimport (\n"
+ "".join(imports)
+ ")\n\nfunc main() { _ = common.Value; _ = storage.Value; _ = image.Value }\n"
)
vendor = subprocess.run(
[go, "mod", "vendor"],
cwd=root,
text=True,
capture_output=True,
check=False,
)
print("go mod vendor succeeded:", vendor.returncode == 0)
if vendor.returncode:
print(vendor.stderr.strip())
raise SystemExit
metadata = (root / "vendor" / "modules.txt").read_text()
print("vendor/modules.txt records all three modules:",
all(path in metadata for path in modules))
print("--- vendor replacement metadata ---")
print("\n".join(line for line in metadata.splitlines()
if any(path in line for path in modules) or line.startswith("# =>")))
# Point one replacement at a different source without regenerating vendor/.
altered = (root / "go.mod").read_text()
altered = altered.replace(
f"=> {root / 'source-1'}",
f"=> {root / 'source-2'}",
1,
)
(root / "go.mod").write_text(altered)
check = subprocess.run(
[go, "list", "-mod=vendor", "./..."],
cwd=root,
text=True,
capture_output=True,
check=False,
)
print("stale vendor metadata rejected:", check.returncode != 0)
print(check.stderr.strip())
PYRepository: openshift/enhancements
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import shutil
import subprocess
import tempfile
from pathlib import Path
go = shutil.which("go")
if not go:
print("go tool: unavailable")
raise SystemExit
modules = {
"go.podman.io/common": ("common", "v0.0.0"),
"go.podman.io/storage": ("storage", "v0.0.0"),
"go.podman.io/image/v5": ("image", "v5.0.0"),
}
with tempfile.TemporaryDirectory() as d:
root = Path(d)
replacements = []
imports = []
for index, (path, (package, version)) in enumerate(modules.items(), 1):
source = root / f"source-{index}"
source.mkdir()
(source / "go.mod").write_text(f"module {path}\n\ngo 1.20\n")
(source / f"{package}.go").write_text(
f"package {package}\n\nfunc Value() string {{ return \"{package}\" }}\n"
)
replacements.append(f" {path} => {source}\n")
imports.append(f' "{path}"\n')
(root / "go.mod").write_text(
"module example.com/main\n\n"
"go 1.20\n\n"
+ "".join(f"require {path} {version}\n" for path, (_, version) in modules.items())
+ "\nreplace (\n"
+ "".join(replacements)
+ ")\n"
)
(root / "main.go").write_text(
"package main\n\nimport (\n"
+ "".join(imports)
+ ")\n\nfunc main() { _ = common.Value; _ = storage.Value; _ = image.Value }\n"
)
vendor = subprocess.run(
[go, "mod", "vendor"],
cwd=root,
text=True,
capture_output=True,
check=False,
)
print("go mod vendor succeeded:", vendor.returncode == 0)
if vendor.returncode:
print(vendor.stderr.strip())
raise SystemExit
metadata = (root / "vendor" / "modules.txt").read_text()
print("vendor/modules.txt records all three modules:",
all(path in metadata for path in modules))
print("--- vendor replacement metadata ---")
print("\n".join(line for line in metadata.splitlines()
if any(path in line for path in modules) or line.startswith("# =>")))
altered = (root / "go.mod").read_text()
altered = altered.replace(
f"=> {root / 'source-1'}",
f"=> {root / 'source-2'}",
1,
)
(root / "go.mod").write_text(altered)
check = subprocess.run(
[go, "list", "-mod=vendor", "./..."],
cwd=root,
text=True,
capture_output=True,
check=False,
)
print("stale vendor metadata rejected:", check.returncode != 0)
print(check.stderr.strip())
PYRepository: openshift/enhancements
Length of output: 1023
Define replacement precedence and enforce it in CI.
Specify the authoritative replacement for go.podman.io/common, go.podman.io/storage, and go.podman.io/image/v5. Add CI checks for the effective replacements and matching vendor/modules.txt; a text merge can retain a valid but incorrect replacement.
🤖 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 `@enhancements/node/downstream-container-libs-vendoring-for-cri-o.md` around
lines 221 - 225, Define explicit replacement precedence rules for the three Go
modules (go.podman.io/common, go.podman.io/storage, and go.podman.io/image/v5)
in the document to establish which replacement directive is authoritative. Add
CI validation checks that verify the effective replacements applied in go.mod
match the intended precedence and that the resulting vendor/modules.txt reflects
the correct resolved replacements, preventing text merge conflicts from
introducing valid but incorrect module versions.
| ## Version Skew Strategy | ||
|
|
||
| CRI-O is a node-level component. During upgrades, nodes may temporarily run different CRI-O versions. This is unchanged by this proposal — container-libs vendoring is internal to the CRI-O binary and does not affect inter-component communication or APIs. | ||
|
|
||
| During dual-stream RHEL transitions, nodes may run CRI-O binaries built with different container-libs versions. This does not introduce inter-node version skew concerns because container-libs is statically linked into CRI-O and does not affect inter-component APIs. On-disk container storage formats managed by the `storage` library are local to each node; if a container-libs update changes the on-disk format, the impact is limited to the upgraded node and handled by CRI-O's existing storage migration logic. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
cat -n enhancements/node/downstream-container-libs-vendoring-for-cri-o.md | head -400 | tail -100Repository: openshift/enhancements
Length of output: 8399
🏁 Script executed:
rg -n "storage|migration|test|rollback|recovery" enhancements/node/downstream-container-libs-vendoring-for-cri-o.md -iRepository: openshift/enhancements
Length of output: 4660
🏁 Script executed:
sed -n '360,400p' enhancements/node/downstream-container-libs-vendoring-for-cri-o.mdRepository: openshift/enhancements
Length of output: 3230
Define storage migration contract and add mixed-version storage tests before declaring dual-stream upgrades safe.
The proposal delegates on-disk format changes to "CRI-O's existing storage migration logic" (line 372) but provides no definition of that migration contract, no test coverage for the dual-stream scenario (nodes running different container-libs versions simultaneously), and no explicit recovery procedure. The rollback process is already flagged as incomplete on line 222 (TODO: Define the specific rollback process).
Before merge, add:
- Documented storage migration contract specifying supported format transitions for each RHEL/container-libs pair.
- Mixed-version storage compatibility tests covering at least the supported RHEL transitions in the same OCP release.
- Explicit recovery and rollback procedures if migration fails on a node.
🤖 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 `@enhancements/node/downstream-container-libs-vendoring-for-cri-o.md` around
lines 368 - 372, Expand the “Version Skew Strategy” section to define the
storage migration contract for each supported RHEL/container-libs transition,
add mixed-version storage compatibility tests covering those transitions within
the same OCP release, and document explicit node recovery and rollback
procedures for migration failures, including the existing rollback TODO.
…ture details - Add CI Access to gitlab.cee section describing the intranet capability and GOPRIVATE configuration for Prow jobs - Replace manual workflow with automated Prow job for container-libs vendoring updates - Add presubmit divergence check to test plan - Resolve open questions on automation and gitlab.cee accessibility - Simplify upstream rebase workflow to use carry patch model - Simplify dual-stream RHEL mitigation - Update Infrastructure Needed with specific Prow job requirements Assisted-by: Claude Code <https://claude.com/claude-code>
2 participants
Describe how downstream CRI-O will vendor a downstream fork of container-libs using Go module replace directives, ensuring alignment with the container-libs version shipped in the target RHEL release.
Assisted-by: Claude Code https://claude.com/claude-code
Summary by CodeRabbit