-
Notifications
You must be signed in to change notification settings - Fork 42
Switch containers/[image,storage,common] to go.podman.io. #353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
These go packages were migrated to a monorepo, as stated in the https://blog.podman.io/2025/08/migration-to-the-container-libs-monorepo-is-complete/. This commit updates the crc package to use these packages from new locations. It was generated using following commands: ``` $ find . -type f -name '*.go' -exec sed -i -e 's,"github.com/containers/image,"go.podman.io/image,g' {} \; $ find . -type f -name '*.go' -exec sed -i -e 's,"github.com/containers/common,"go.podman.io/common,g' {} \; $ find . -type f -name '*.go' -exec sed -i -e 's,"github.com/containers/storage,"go.podman.io/storage,g' {} \; $ goimports -v -w . $ git checkout vendor/ $ vi go.mod # to change the storage, image and common imports $ go mod tidy $ go mod vendor ``` Signed-off-by: Jan Kaluza <jkaluza@redhat.com>
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughUpdated module dependencies in go.mod and switched a strongunits import path in pkg/config/config.go from github.com/containers/common to go.podman.io/common. No functional or public API changes indicated. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Hi @jankaluza. Thanks for your PR. I'm waiting for a crc-org member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod
(3 hunks)pkg/config/config.go
(1 hunks)
🔇 Additional comments (5)
go.mod (5)
22-22
: Good: added go.podman.io/commonThis aligns with the monorepo migration. Consider pinning to a tagged release if/when available for reproducibility (pseudo-versions are fine short-term).
20-20
: Testify upgrade looks fineNo concerns; incremental test-only dependency bump.
72-72
: x/net bump observedIncidental to the migration; low risk but keep an eye on any net/http2 behavior changes in CI.
37-37
: Confirm no direct legacygithub.com/containers/...
imports remain. Therg
search produced no matches; please manually verify that none are hidden in vendored or generated files.
16-16
: Confirmed direct import of gopsutil—no change needed
The packagegithub.com/shirou/gopsutil/v4/process
is imported inpkg/process/process.go
(line 27), so the direct require ingo.mod
is valid and should remain.
"strings" | ||
|
||
"github.com/containers/common/pkg/strongunits" | ||
"go.podman.io/common/pkg/strongunits" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Public API break: exported strongunits type changed module path
Switching strongunits
to go.podman.io/common
changes the type identity of strongunits.B
used in the exported VirtualMachine.Memory
field. Downstream users constructing config.VirtualMachine
with the old github.com/containers/common/pkg/strongunits.B
will no longer compile.
- If vfkit’s Go API is considered stable, mitigate or call this out as a breaking change:
- Option A: stop exporting
strongunits
in the public surface (e.g., store memory internally asuint64
bytes; provide constructors/getters likeNewVirtualMachine(vcpus, memoryMiB)
andSetMemoryMiB
), keeping JSON stable. - Option B: accept the break with a major version bump and a release note.
- Option A: stop exporting
I can draft a minimal refactor to internalize the unit type while preserving behavior and JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment needs a closer look as podman/crc/… are using this code
/hold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@cfergeau I'll investigate the downstream usage impact of this change. Let me analyze how this package is consumed and what the actual breakage risk is.
🏁 Script executed:
#!/bin/bash
# Check if there are any examples or tests that show how VirtualMachine is used
echo "=== Searching for VirtualMachine usage patterns ==="
rg -n -C3 -t go "VirtualMachine" --exclude-dir=vendor
echo -e "\n=== Checking for any consumers of the Memory field specifically ==="
rg -n -C3 -t go "\.Memory" --exclude-dir=vendor
echo -e "\n=== Looking for any go.mod files that might depend on this package ==="
fd -t f "go.mod" -x cat {} \;
echo -e "\n=== Checking for any existing imports of strongunits in the codebase ==="
rg -n -t go "strongunits" --exclude-dir=vendor
Length of output: 15455
🏁 Script executed:
#!/bin/bash
# Fix the ripgrep syntax and search for usage patterns
echo "=== Searching for VirtualMachine usage patterns ==="
rg -n -C3 -t go "VirtualMachine" -g '!vendor/**'
echo -e "\n=== Checking for any consumers of the Memory field specifically ==="
rg -n -C3 -t go "\.Memory" -g '!vendor/**'
echo -e "\n=== Checking for any existing imports of strongunits in the codebase ==="
rg -n -t go "strongunits" -g '!vendor/**'
echo -e "\n=== Looking for any test files that might show usage patterns ==="
rg -n -C3 -t go -g '*test*.go' "VirtualMachine\|strongunits"
Length of output: 708
🏁 Script executed:
#!/bin/bash
# Let's try a simpler approach without complex glob patterns
echo "=== Listing Go files to understand structure ==="
find . -name "*.go" -type f | head -20
echo -e "\n=== Searching for VirtualMachine usage patterns ==="
rg -n -C3 "VirtualMachine"
echo -e "\n=== Checking for any consumers of the Memory field specifically ==="
rg -n -C3 "\.Memory"
echo -e "\n=== Checking for any existing imports of strongunits in the codebase ==="
rg -n "strongunits"
echo -e "\n=== Looking for external dependencies on this package ==="
# Check if there are any GitHub references or import statements that would indicate external usage
rg -n "github.com/crc-org/vfkit"
Length of output: 54928
🏁 Script executed:
#!/bin/bash
# Let's search for external usage of this vfkit package to understand the downstream impact
echo "=== Searching for potential external dependencies using GitHub search simulation ==="
echo "This would show projects that might depend on github.com/crc-org/vfkit/pkg/config"
echo -e "\n=== Let's check if there are any examples of strongunits.B being used in exported APIs ==="
rg -n -C2 "strongunits\.B"
echo -e "\n=== Let's check what strongunits types are actually exported from the VirtualMachine struct ==="
echo "Looking at the VirtualMachine struct definition:"
rg -n -A10 -B2 "type VirtualMachine struct"
echo -e "\n=== Let's check if the JSON marshaling preserves compatibility ==="
rg -n -C3 "memoryBytes"
Length of output: 10869
@cfergeau You're absolutely right to flag this for closer examination. This change does introduce a Go API breaking change for downstream consumers. Let me investigate the actual usage patterns to understand the full impact.
🌐 Web query:
How does crc-org/crc project use vfkit VirtualMachine Memory field strongunits
💡 Result:
Short answer: vfkit's VirtualMachine.Memory is typed as strongunits.B (a bytes type) and crc passes the configured VM memory (in MiB) into vfkit, which stores/serializes it as that strongunits.B value (i.e., raw bytes). strongunits provides typed byte/size helpers and conversions used by vfkit's config layer.
Sources:
- vfkit config shows VirtualMachine.Memory is strongunits.B. [1]
- vfkit CLI/REST accepts a --memory value (MiB) and vfkit builds a VirtualMachine config from options. [2]
- strongunits.B is a uint64 bytes type with conversion helpers (ToBytes / GiB/MiB types). [3]
- CRC documentation notes it uses vfkit on Apple Silicon (crc passes VM resource values to vfkit). [4]
References:
[1] vfkit pkg config (VirtualMachine.Memory = strongunits.B).
[2] vfkit usage (--memory option, memory in MiB).
[3] containers/common/pkg/strongunits (type B represents bytes).
[4] crc Apple Silicon / vfkit docs.
🌐 Web query:
crc-org/crc vfkit import usage "github.com/crc-org/vfkit/pkg/config" VirtualMachine
💡 Result:
Use the config package to build a config.VirtualMachine, then call vm.Cmd(vfkitPath) to get an *exec.Cmd to run vfkit. Example (minimal):
Imports:
- github.com/crc-org/vfkit/pkg/config
- os/exec, fmt, os
Example snippet:
package main
import (
"fmt"
"os"
"os/exec"
"github.com/crc-org/vfkit/pkg/config"
)
func main() {
// create a Linux bootloader (kernel path, cmdline, optional initrd)
boot := config.NewLinuxBootloader("/path/to/vmlinuz", "console=ttyAMA0", "/path/to/initrd")
// create VM with 4 vCPUs and 4096 MiB RAM
vm := config.NewVirtualMachine(4, 4096, boot)
// add devices as needed, e.g. virtio block / network (use provided constructors)
// err := vm.AddDevice(config.VirtioBlockNew("disk0", "/path/to/disk.img"))
// handle err...
// produce exec.Cmd for vfkit binary (path to vfkit)
cmd, err := vm.Cmd("/usr/local/bin/vfkit")
if err != nil {
fmt.Fprintf(os.Stderr, "cmd: %v\n", err)
os.Exit(1)
}
// run vfkit
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
if err := cmd.Run(); err != nil {
fmt.Fprintf(os.Stderr, "run: %v\n", err)
os.Exit(1)
}
}
Notes:
- Use NewEFIBootloader or NewLinuxBootloader for Bootloader types; there are helpers to add devices (VirtioGPUNew, VirtioNetNew, VirtioBlockNew, VirtioVsockNew, etc.) and methods ToCmdLine/Cmd. See package docs for device constructors and additional helpers. [1][2]
Sources:
[1] crc-org/vfkit GitHub repository (README & usage).
[2] pkg.go.dev: github.com/crc-org/vfkit/pkg/config (VirtualMachine, NewVirtualMachine, Cmd, bootloader helpers).
🌐 Web query:
podman crc vfkit "strongunits.B" "Memory" field usage breaking change
💡 Result:
Short answer: Yes — Podman 5's rewrite of the podman machine backend introduced breaking changes in machine config / unit handling that can affect how memory is interpreted (causing silent downgrades). See the Podman issue tracking the "silently downgrade memory" problem and the recent Podman v5 breaking-change notes; vfkit is also referenced as the Apple hypervisor provider involved. [1][2][3]
Sources
- Podman issue tracking silent memory downgrade (podman machine init can silently downgrade the amount of memory containers/podman#25803). [1]
- Podman v5 release notes / breaking changes (podman machine config/VM format changes). [2]
- vfkit (crc-org/vfkit) — Mac virtualization backend referenced by Podman/CRC. [3]
If you want, I can fetch the specific issue/PR that links vfkit (#297) and the exact commit or workaround steps.
Based on my analysis of the code and web research, I can now provide you with a comprehensive assessment of this breaking change:
@cfergeau You're absolutely correct to be concerned about this change. This is a Go API breaking change that will affect downstream consumers like CRC and Podman. Here's the detailed impact:
Breaking Change Analysis
What's Breaking:
- The
VirtualMachine.Memory
field changes fromgithub.com/containers/common/pkg/strongunits.B
togo.podman.io/common/pkg/strongunits.B
- While these might be functionally equivalent, Go treats them as different types due to different import paths
- Any code directly constructing
config.VirtualMachine{Memory: strongunits.B(...)}
will break at compile time
What's Safe:
- JSON serialization/deserialization remains compatible (uses
memoryBytes
numeric field) - Code using only
config.NewVirtualMachine(vcpus, memoryMiB, bootloader)
constructor should continue working - Code that doesn't directly access the
Memory
field type should be unaffected
Downstream Impact:
- CRC: Uses vfkit as the virtualization backend on Apple Silicon, likely through the public API
- Podman: Machine backend references vfkit for macOS virtualization
- Any other projects importing
github.com/crc-org/vfkit/pkg/config
and working with theVirtualMachine
type
Recommendations
Since this affects multiple downstream projects, I'd suggest:
- Coordinate the migration - Ensure CRC, Podman, and other consumers update their imports simultaneously
- Document the breaking change in release notes with migration guidance
- Consider a major version bump to signal the API break
- Provide migration period - Could temporarily support both import paths if feasible
Would you like me to:
- Search for the specific usage patterns in CRC's codebase to understand exactly what needs updating?
- Help draft migration guidance for downstream consumers?
- Investigate if there's a way to maintain backwards compatibility during a transition period?
The migration itself is straightforward (update import paths), but the coordination across the ecosystem is the key challenge here.
PR needs rebase. Instructions 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. |
These go packages were migrated to a monorepo, as stated in the https://blog.podman.io/2025/08/migration-to-the-container-libs-monorepo-is-complete/.
This commit updates the crc package to use these packages from new locations.
It was generated using following commands:
Summary by CodeRabbit