From 446e63579cba6d375b229305e113ea4ed756822b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 28 May 2024 14:56:30 +0200 Subject: [PATCH] remove uses of platforms.Platform alias Commit 3c8469a78269d9ba2ead4798d4469d95f8c3cdd5 removed uses of the api types.Platform type from public interfaces, instead using the type from the OCI image spec. For convenience, it also introduced an alias in the platforms package. While this alias allows packages that already import containerd's platforms package (now a separate module), it may also cause confusion (it's not clear that it's an alias for the OCI type), and for packages that do not depend on containerd's platforms package / module may now be resulting in an extra dependency. Let's remove the use of this alias, and instead use the OCI type directly. Signed-off-by: Sebastiaan van Stijn --- core/sandbox/controller.go | 4 ++-- core/sandbox/proxy/controller.go | 8 ++++---- internal/cri/server/container_create.go | 8 ++++---- internal/cri/server/container_create_test.go | 10 +++++----- internal/cri/server/images/service.go | 3 ++- internal/cri/server/podsandbox/controller.go | 3 ++- internal/cri/server/sandbox_service.go | 6 +++--- internal/cri/server/service.go | 4 ++-- internal/cri/server/service_test.go | 5 +++-- plugins/sandbox/controller.go | 10 +++++----- 10 files changed, 32 insertions(+), 29 deletions(-) diff --git a/core/sandbox/controller.go b/core/sandbox/controller.go index 5450c0c78..1c99dd6d9 100644 --- a/core/sandbox/controller.go +++ b/core/sandbox/controller.go @@ -23,8 +23,8 @@ import ( "github.com/containerd/containerd/api/types" "github.com/containerd/containerd/v2/core/mount" - "github.com/containerd/platforms" "github.com/containerd/typeurl/v2" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" ) type CreateOptions struct { @@ -99,7 +99,7 @@ type Controller interface { Start(ctx context.Context, sandboxID string) (ControllerInstance, error) // Platform returns target sandbox OS that will be used by Controller. // containerd will rely on this to generate proper OCI spec. - Platform(_ctx context.Context, _sandboxID string) (platforms.Platform, error) + Platform(_ctx context.Context, _sandboxID string) (imagespec.Platform, error) // Stop will stop sandbox instance Stop(ctx context.Context, sandboxID string, opts ...StopOpt) error // Wait blocks until sandbox process exits. diff --git a/core/sandbox/proxy/controller.go b/core/sandbox/proxy/controller.go index 92078671a..a6ceb72b8 100644 --- a/core/sandbox/proxy/controller.go +++ b/core/sandbox/proxy/controller.go @@ -24,7 +24,7 @@ import ( "github.com/containerd/containerd/v2/core/mount" "github.com/containerd/containerd/v2/core/sandbox" "github.com/containerd/errdefs" - "github.com/containerd/platforms" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" "google.golang.org/protobuf/types/known/anypb" ) @@ -78,14 +78,14 @@ func (s *remoteSandboxController) Start(ctx context.Context, sandboxID string) ( }, nil } -func (s *remoteSandboxController) Platform(ctx context.Context, sandboxID string) (platforms.Platform, error) { +func (s *remoteSandboxController) Platform(ctx context.Context, sandboxID string) (imagespec.Platform, error) { resp, err := s.client.Platform(ctx, &api.ControllerPlatformRequest{SandboxID: sandboxID}) if err != nil { - return platforms.Platform{}, errdefs.FromGRPC(err) + return imagespec.Platform{}, errdefs.FromGRPC(err) } platform := resp.GetPlatform() - return platforms.Platform{ + return imagespec.Platform{ Architecture: platform.GetArchitecture(), OS: platform.GetOS(), Variant: platform.GetVariant(), diff --git a/internal/cri/server/container_create.go b/internal/cri/server/container_create.go index ca170c473..7d8fc9a49 100644 --- a/internal/cri/server/container_create.go +++ b/internal/cri/server/container_create.go @@ -351,7 +351,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta // volumeMounts sets up image volumes for container. Rely on the removal of container // root directory to do cleanup. Note that image volume will be skipped, if there is criMounts // specified with the same destination. -func (c *criService) volumeMounts(platform platforms.Platform, containerRootDir string, containerConfig *runtime.ContainerConfig, config *imagespec.ImageConfig) []*runtime.Mount { +func (c *criService) volumeMounts(platform imagespec.Platform, containerRootDir string, containerConfig *runtime.ContainerConfig, config *imagespec.ImageConfig) []*runtime.Mount { var uidMappings, gidMappings []*runtime.IDMapping if platform.OS == "linux" { if usernsOpts := containerConfig.GetLinux().GetSecurityContext().GetNamespaceOptions().GetUsernsOptions(); usernsOpts != nil { @@ -399,7 +399,7 @@ func (c *criService) volumeMounts(platform platforms.Platform, containerRootDir } // runtimeSpec returns a default runtime spec used in cri-containerd. -func (c *criService) runtimeSpec(id string, platform platforms.Platform, baseSpecFile string, opts ...oci.SpecOpts) (*runtimespec.Spec, error) { +func (c *criService) runtimeSpec(id string, platform imagespec.Platform, baseSpecFile string, opts ...oci.SpecOpts) (*runtimespec.Spec, error) { // GenerateSpec needs namespace. ctx := util.NamespacedContext() container := &containers.Container{ID: id} @@ -482,7 +482,7 @@ func generateUserString(username string, uid, gid *runtime.Int64Value) (string, // runtime information (rootfs mounted), or platform specific checks with // no defined workaround (yet) to specify for other platforms. func (c *criService) platformSpecOpts( - platform platforms.Platform, + platform imagespec.Platform, config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig, ) ([]oci.SpecOpts, error) { @@ -529,7 +529,7 @@ func (c *criService) platformSpecOpts( // buildContainerSpec build container's OCI spec depending on controller's target platform OS. func (c *criService) buildContainerSpec( - platform platforms.Platform, + platform imagespec.Platform, id string, sandboxID string, sandboxPid uint32, diff --git a/internal/cri/server/container_create_test.go b/internal/cri/server/container_create_test.go index 5887f13a0..8697d5e0f 100644 --- a/internal/cri/server/container_create_test.go +++ b/internal/cri/server/container_create_test.go @@ -241,7 +241,7 @@ func TestVolumeMounts(t *testing.T) { for _, test := range []struct { desc string - platform platforms.Platform + platform imagespec.Platform criMounts []*runtime.Mount usernsEnabled bool imageVolumes map[string]struct{} @@ -293,7 +293,7 @@ func TestVolumeMounts(t *testing.T) { }, { desc: "should make relative paths absolute on Linux", - platform: platforms.Platform{OS: "linux"}, + platform: imagespec.Platform{OS: "linux"}, imageVolumes: map[string]struct{}{ "./test-volume-1": {}, "C:/test-volume-2": {}, @@ -309,7 +309,7 @@ func TestVolumeMounts(t *testing.T) { }, { desc: "should include mappings for image volumes on Linux", - platform: platforms.Platform{OS: "linux"}, + platform: imagespec.Platform{OS: "linux"}, usernsEnabled: true, imageVolumes: map[string]struct{}{ "/test-volume-1/": {}, @@ -323,7 +323,7 @@ func TestVolumeMounts(t *testing.T) { }, { desc: "should NOT include mappings for image volumes on Linux if !userns", - platform: platforms.Platform{OS: "linux"}, + platform: imagespec.Platform{OS: "linux"}, usernsEnabled: false, imageVolumes: map[string]struct{}{ "/test-volume-1/": {}, @@ -336,7 +336,7 @@ func TestVolumeMounts(t *testing.T) { }, { desc: "should convert rel imageVolume paths to abs paths and add userns mappings", - platform: platforms.Platform{OS: "linux"}, + platform: imagespec.Platform{OS: "linux"}, usernsEnabled: true, imageVolumes: map[string]struct{}{ "test-volume-1/": {}, diff --git a/internal/cri/server/images/service.go b/internal/cri/server/images/service.go index 94a4113d8..1e2463370 100644 --- a/internal/cri/server/images/service.go +++ b/internal/cri/server/images/service.go @@ -32,6 +32,7 @@ import ( "github.com/containerd/platforms" docker "github.com/distribution/reference" imagedigest "github.com/opencontainers/go-digest" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" ) @@ -44,7 +45,7 @@ type imageClient interface { type ImagePlatform struct { Snapshotter string - Platform platforms.Platform + Platform imagespec.Platform } type CRIImageService struct { diff --git a/internal/cri/server/podsandbox/controller.go b/internal/cri/server/podsandbox/controller.go index 76d764d5e..7fcba26c8 100644 --- a/internal/cri/server/podsandbox/controller.go +++ b/internal/cri/server/podsandbox/controller.go @@ -24,6 +24,7 @@ import ( "github.com/containerd/log" "github.com/containerd/plugin" "github.com/containerd/plugin/registry" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" eventtypes "github.com/containerd/containerd/api/events" @@ -134,7 +135,7 @@ type Controller struct { var _ sandbox.Controller = (*Controller)(nil) -func (c *Controller) Platform(_ctx context.Context, _sandboxID string) (platforms.Platform, error) { +func (c *Controller) Platform(_ctx context.Context, _sandboxID string) (imagespec.Platform, error) { return platforms.DefaultSpec(), nil } diff --git a/internal/cri/server/sandbox_service.go b/internal/cri/server/sandbox_service.go index 1a568ae93..8cec187ca 100644 --- a/internal/cri/server/sandbox_service.go +++ b/internal/cri/server/sandbox_service.go @@ -21,7 +21,7 @@ import ( "fmt" "time" - "github.com/containerd/platforms" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/sandbox" @@ -94,10 +94,10 @@ func (c *criSandboxService) SandboxStatus(ctx context.Context, sandboxer string, return ctrl.Status(ctx, sandboxID, verbose) } -func (c *criSandboxService) SandboxPlatform(ctx context.Context, sandboxer string, sandboxID string) (platforms.Platform, error) { +func (c *criSandboxService) SandboxPlatform(ctx context.Context, sandboxer string, sandboxID string) (imagespec.Platform, error) { ctrl, err := c.SandboxController(sandboxer) if err != nil { - return platforms.Platform{}, err + return imagespec.Platform{}, err } return ctrl.Platform(ctx, sandboxID) } diff --git a/internal/cri/server/service.go b/internal/cri/server/service.go index 4b02cad0d..fd2b21acc 100644 --- a/internal/cri/server/service.go +++ b/internal/cri/server/service.go @@ -28,8 +28,8 @@ import ( "github.com/containerd/go-cni" "github.com/containerd/log" - "github.com/containerd/platforms" "github.com/containerd/typeurl/v2" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-spec/specs-go/features" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" "k8s.io/kubelet/pkg/cri/streaming" @@ -79,7 +79,7 @@ type sandboxService interface { StopSandbox(ctx context.Context, sandboxer, sandboxID string, opts ...sandbox.StopOpt) error ShutdownSandbox(ctx context.Context, sandboxer string, sandboxID string) error SandboxStatus(ctx context.Context, sandboxer string, sandboxID string, verbose bool) (sandbox.ControllerStatus, error) - SandboxPlatform(ctx context.Context, sandboxer string, sandboxID string) (platforms.Platform, error) + SandboxPlatform(ctx context.Context, sandboxer string, sandboxID string) (imagespec.Platform, error) SandboxController(sandboxer string) (sandbox.Controller, error) } diff --git a/internal/cri/server/service_test.go b/internal/cri/server/service_test.go index 4946dace5..8db1bcbbc 100644 --- a/internal/cri/server/service_test.go +++ b/internal/cri/server/service_test.go @@ -22,6 +22,7 @@ import ( "github.com/containerd/errdefs" "github.com/containerd/go-cni" "github.com/containerd/platforms" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/containerd/containerd/api/types" containerd "github.com/containerd/containerd/v2/client" @@ -62,7 +63,7 @@ func (f *fakeSandboxService) SandboxStatus(ctx context.Context, sandboxer string return sandbox.ControllerStatus{}, errdefs.ErrNotImplemented } -func (f *fakeSandboxService) SandboxPlatform(ctx context.Context, sandboxer string, sandboxID string) (platforms.Platform, error) { +func (f *fakeSandboxService) SandboxPlatform(ctx context.Context, sandboxer string, sandboxID string) (imagespec.Platform, error) { return platforms.DefaultSpec(), nil } @@ -80,7 +81,7 @@ func (f fakeSandboxController) Start(ctx context.Context, sandboxID string) (san return sandbox.ControllerInstance{}, errdefs.ErrNotImplemented } -func (f fakeSandboxController) Platform(_ctx context.Context, _sandboxID string) (platforms.Platform, error) { +func (f fakeSandboxController) Platform(_ctx context.Context, _sandboxID string) (imagespec.Platform, error) { return platforms.DefaultSpec(), nil } diff --git a/plugins/sandbox/controller.go b/plugins/sandbox/controller.go index e72f5639d..ed6436d79 100644 --- a/plugins/sandbox/controller.go +++ b/plugins/sandbox/controller.go @@ -24,9 +24,9 @@ import ( "github.com/containerd/errdefs" "github.com/containerd/log" - "github.com/containerd/platforms" "github.com/containerd/plugin" "github.com/containerd/plugin/registry" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" "google.golang.org/protobuf/types/known/anypb" runtimeAPI "github.com/containerd/containerd/api/runtime/sandbox/v1" @@ -198,18 +198,18 @@ func (c *controllerLocal) Start(ctx context.Context, sandboxID string) (sandbox. }, nil } -func (c *controllerLocal) Platform(ctx context.Context, sandboxID string) (platforms.Platform, error) { +func (c *controllerLocal) Platform(ctx context.Context, sandboxID string) (imagespec.Platform, error) { svc, err := c.getSandbox(ctx, sandboxID) if err != nil { - return platforms.Platform{}, err + return imagespec.Platform{}, err } response, err := svc.Platform(ctx, &runtimeAPI.PlatformRequest{SandboxID: sandboxID}) if err != nil { - return platforms.Platform{}, fmt.Errorf("failed to get sandbox platform: %w", errdefs.FromGRPC(err)) + return imagespec.Platform{}, fmt.Errorf("failed to get sandbox platform: %w", errdefs.FromGRPC(err)) } - var platform platforms.Platform + var platform imagespec.Platform if p := response.GetPlatform(); p != nil { platform.OS = p.OS platform.Architecture = p.Architecture