Skip to content
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.

Sign up for GitHub

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

libct/devices: move config to libct/cg/devices/config #4577

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

libct/devices: move config to libct/cg/devices/config #4577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9cddad
libct/devices: move config to libct/cg/devices/config
kolyshkin Jan 4, 2025
ebe7ad7
libct: switch from libct/devices to libct/cgroups/devices/config
kolyshkin Jan 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libcontainer/cgroups/config_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package cgroups

import (
systemdDbus "github.com/coreos/go-systemd/v22/dbus"
"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
)

type FreezerState string
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/devices/device.go → ...ontainer/cgroups/devices/config/device.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package devices
package config

import (
"fmt"
Expand Down
14 changes: 14 additions & 0 deletions libcontainer/cgroups/devices/config/mknod_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package config

import (
"errors"

"golang.org/x/sys/unix"
)

func mkDev(d *Rule) (uint64, error) {
if d.Major == Wildcard || d.Minor == Wildcard {
return 0, errors.New("cannot mkdev() device with wildcards")
}
return unix.Mkdev(uint32(d.Major), uint32(d.Minor)), nil
}
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/devicefilter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"strconv"

"github.com/cilium/ebpf/asm"
"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"golang.org/x/sys/unix"
)

Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/devicefilter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"strings"
"testing"

"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/specconv"
)

Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/devices_emulator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"strconv"
"strings"

"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
)

// deviceMeta is a Rule without the Allow or Permissions fields, and no
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/devices_emulator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"strings"
"testing"

"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
)

func TestDeviceEmulatorLoad(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/systemd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/sirupsen/logrus"

"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
)

// systemdProperties takes the configured device rules and generates a
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/systemd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"testing"

"github.com/opencontainers/runc/libcontainer/cgroups"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
"github.com/opencontainers/runc/libcontainer/devices"
)

// TestPodSkipDevicesUpdate checks that updating a pod having SkipDevices: true
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/v1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"github.com/moby/sys/userns"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
)

var testingSkipFinalCheck bool
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/v1_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/moby/sys/userns"

"github.com/opencontainers/runc/libcontainer/cgroups"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
"github.com/opencontainers/runc/libcontainer/devices"
)

func init() {
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/devices/v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"golang.org/x/sys/unix"

"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
)

func isRWM(perms devices.Permissions) bool {
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/configs/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"

"github.com/opencontainers/runc/libcontainer/devices"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runtime-spec/specs-go"
)

Expand Down
20 changes: 20 additions & 0 deletions libcontainer/devices/device_deprecated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package devices

import "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"

// Deprecated: use [github.com/opencontainers/runc/libcontainer/cgroups/devices/config].
const (
Wildcard = config.Wildcard
WildcardDevice = config.WildcardDevice
BlockDevice = config.BlockDevice
CharDevice = config.CharDevice
FifoDevice = config.FifoDevice
)

// Deprecated: use [github.com/opencontainers/runc/libcontainer/cgroups/devices/config].
type (
Device = config.Device
Permissions = config.Permissions
Type = config.Type
Rule = config.Rule
)
7 changes: 0 additions & 7 deletions libcontainer/devices/device_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ var (
osReadDir = os.ReadDir
)

func mkDev(d *Rule) (uint64, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Just only a question, in fact, it seems that all things in this file are not used by runc after we moved this function to another place, maybe we can mark the whole package deprecated? But maybe they are used by some other projects, so we need to keep these unneeded codes here?

Copy link
Member

Choose a reason for hiding this comment

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

Because of the above reason, I suggest to move all this file to ‘github.com/opencontainers/runc/libcontainer/cgroups/devices/config’, the other reason is that if someone wants to use these functions besides with ‘github.com/opencontainers/runc/libcontainer/cgroups/devices/config’, he needs to import these two packages, it will make us confused. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems that all things in this file are not used by runc after we moved this function to another place, maybe we can mark the whole package deprecated?

We could, but usually while doing so, you have to suggest a replacement. Alas, I can't suggest any replacement at this time. As noted in this PR description, there are forks of this code (in containerd/pkg/oci and podman/pkg/util) but they are not a direct replacement.

But maybe they are used by some other projects, so we need to keep these unneeded codes here?

Exactly. As noted in description:

  1. A few functions (HostDevices, DeviceFromPath, GetDevices).
    Those are not used by runc directly, but have some external users
    (cri-o, microsoft/hcsshim)

So,

  • we can't remove this;
  • we can't mark them deprecated (as we can't offer an alternative);

The long-term plan for this is also in the description:

(As for (2), I think something like moby/sys#181 should be done)

Now,

Because of the above reason, I suggest to move all this file to ‘github.com/opencontainers/runc/libcontainer/cgroups/devices/config’

So, these two things (1 and 2 in the description) are really not related. 1 is used by runc (in particular, by libcontainer/cgroups) and this is why this PR moves it there. 2 has some external users, and the long term plan is to move it to moby (see moby/sys#181).

The big picture here is, we want to move libcontainer/cgroups to a separate repository (see https://github.com/opencontainers/tob/blob/main/proposals/cgroups.md), and for that we need to make it as self-contained as possible. For this, (1) from the description goes to libcontainer/cgroups, and (2) stays where it is.

Let me know if I can clarify this further.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I got it now, thanks your explanation, it seems that the current solution is the best decision at this time.

if d.Major == Wildcard || d.Minor == Wildcard {
return 0, errors.New("cannot mkdev() device with wildcards")
}
return unix.Mkdev(uint32(d.Major), uint32(d.Minor)), nil
}

// DeviceFromPath takes the path to a device and its cgroup_permissions (which
// cannot be easily queried) to look up the information about a linux device
// and returns that information as a Device struct.
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/integration/template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"time"

"github.com/opencontainers/runc/libcontainer/cgroups"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/opencontainers/runc/libcontainer/specconv"
"golang.org/x/sys/unix"
)
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/integration/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"testing"

"github.com/opencontainers/runc/libcontainer"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
"github.com/opencontainers/runc/libcontainer/devices"
)

func testUpdateDevices(t *testing.T, systemd bool) {
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/rootfs_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import (
"golang.org/x/sys/unix"

"github.com/opencontainers/runc/libcontainer/cgroups"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/cgroups/fs2"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/opencontainers/runc/libcontainer/utils"
)

Expand Down
2 changes: 1 addition & 1 deletion libcontainer/specconv/spec_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
systemdDbus "github.com/coreos/go-systemd/v22/dbus"
dbus "github.com/godbus/dbus/v5"
"github.com/opencontainers/runc/libcontainer/cgroups"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/opencontainers/runc/libcontainer/internal/userns"
"github.com/opencontainers/runc/libcontainer/seccomp"
libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/specconv/spec_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import (
"testing"

dbus "github.com/godbus/dbus/v5"
devices "github.com/opencontainers/runc/libcontainer/cgroups/devices/config"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/configs/validate"
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/opencontainers/runtime-spec/specs-go"
"golang.org/x/sys/unix"
)
Expand Down