+
Skip to content

Conversation

jankaluza
Copy link

@jankaluza jankaluza commented Sep 1, 2025

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

Summary by CodeRabbit

  • New Features
    • No user-facing feature changes in this release.
  • Refactor
    • Aligned internal imports with upstream modules to maintain compatibility; no behavior changes.
  • Chores
    • Refreshed third-party dependencies to improve stability, compatibility, and future maintainability.
    • Brought network-related libraries up to the latest patch levels.
    • Updated testing dependencies for improved reliability.
  • Notes
    • No changes to public interfaces; existing workflows continue to function as before.

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>
@openshift-ci openshift-ci bot requested review from cfergeau and lstocchi September 1, 2025 13:37
Copy link

openshift-ci bot commented Sep 1, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign praveenkumar for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

coderabbitai bot commented Sep 1, 2025

Walkthrough

Updated 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

Cohort / File(s) Summary
Dependencies
go.mod
Direct deps: removed github.com/containers/common v0.64.1; added github.com/shirou/gopsutil/v4 v4.25.6, go.podman.io/common v0.0.0-20250826054041-6e4bed3c9118; updated github.com/stretchr/testify v1.11.0. Indirect deps: added github.com/containers/common v0.62.2, upgraded golang.org/x/net v0.43.0.
Config import path
pkg/config/config.go
Updated import from github.com/containers/common/pkg/strongunits to go.podman.io/common/pkg/strongunits. No code or API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers at tidy deps,
Hop-hop, the imports take lighter steps.
From old burrow to Podman glen,
Strong units march in files again.
Carrots aligned, tests in tow—
A gentle update, and off we go! 🥕🐇

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

openshift-ci bot commented Sep 1, 2025

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

Copy link

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between ec2c483 and 3fa3255.

⛔ 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/common

This 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 fine

No concerns; incremental test-only dependency bump.


72-72: x/net bump observed

Incidental to the migration; low risk but keep an eye on any net/http2 behavior changes in CI.


37-37: Confirm no direct legacy github.com/containers/... imports remain. The rg 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 package github.com/shirou/gopsutil/v4/process is imported in pkg/process/process.go (line 27), so the direct require in go.mod is valid and should remain.

"strings"

"github.com/containers/common/pkg/strongunits"
"go.podman.io/common/pkg/strongunits"
Copy link

@coderabbitai coderabbitai bot Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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 as uint64 bytes; provide constructors/getters like NewVirtualMachine(vcpus, memoryMiB) and SetMemoryMiB), keeping JSON stable.
    • Option B: accept the break with a major version bump and a release note.

I can draft a minimal refactor to internalize the unit type while preserving behavior and JSON.

Copy link
Collaborator

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

Copy link

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

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 from github.com/containers/common/pkg/strongunits.B to go.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 the VirtualMachine type

Recommendations

Since this affects multiple downstream projects, I'd suggest:

  1. Coordinate the migration - Ensure CRC, Podman, and other consumers update their imports simultaneously
  2. Document the breaking change in release notes with migration guidance
  3. Consider a major version bump to signal the API break
  4. Provide migration period - Could temporarily support both import paths if feasible

Would you like me to:

  1. Search for the specific usage patterns in CRC's codebase to understand exactly what needs updating?
  2. Help draft migration guidance for downstream consumers?
  3. 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.

@openshift-merge-robot
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载