From c6e0fb485c31129733dc00026cac73f2455c0465 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Tue, 7 May 2024 14:24:14 +0200 Subject: [PATCH] Refactor sandbox creation function arguments - Add a dedicated list of setters for the various pod sandbox fields and use them directly on sandbox creation - Add a new method to convert a runtime spec to a sandbox and use it on restore. - Enable the revive linter and set it to a minimum amount of arguments. Signed-off-by: Sascha Grunert --- .golangci.yml | 17 +- internal/factory/container/container_test.go | 8 +- internal/hostport/hostport.go | 8 +- internal/lib/container_server.go | 69 +----- internal/lib/sandbox/history_test.go | 10 +- internal/lib/sandbox/sandbox.go | 212 ++++++++++++++++-- internal/lib/sandbox/sandbox_test.go | 25 ++- internal/lib/sandbox/sandbox_test_inject.go | 16 -- internal/lib/sandbox/suite_test.go | 10 +- internal/lib/suite_test.go | 7 +- .../high_performance_hooks_test.go | 9 +- server/sandbox_run_linux.go | 37 ++- server/suite_test.go | 8 +- 13 files changed, 279 insertions(+), 157 deletions(-) delete mode 100644 internal/lib/sandbox/sandbox_test_inject.go diff --git a/.golangci.yml b/.golangci.yml index c8d62ec1b8f..ecc8bcf2014 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -62,6 +62,7 @@ linters: - perfsprint - prealloc - reassign + - revive - rowserrcheck - sloglint - spancheck @@ -114,7 +115,6 @@ linters: # - predeclared # - promlinter # - protogetter - # - revive # - tagliatelle # - testifylint # - testpackage @@ -127,6 +127,21 @@ linters-settings: errcheck: check-type-assertions: true check-blank: true + revive: + rules: + - name: argument-limit + disabled: false + arguments: [20] + - name: unused-parameter + disabled: true + - name: unexported-return + disabled: true + - name: increment-decrement + disabled: true + - name: unused-parameter + disabled: true + - name: dot-imports + disabled: true gocritic: enabled-checks: - appendCombine diff --git a/internal/factory/container/container_test.go b/internal/factory/container/container_test.go index 8abbbfa8d7d..78c3be4ce5c 100644 --- a/internal/factory/container/container_test.go +++ b/internal/factory/container/container_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/cri-o/cri-o/internal/config/capabilities" - "github.com/cri-o/cri-o/internal/hostport" "github.com/cri-o/cri-o/internal/lib" "github.com/cri-o/cri-o/internal/lib/sandbox" oci "github.com/cri-o/cri-o/internal/oci" @@ -97,11 +96,8 @@ var _ = t.Describe("Container", func() { mountPoint := "test" configStopSignal := "test" - sb, err := sandbox.New("sandboxID", "", "", "", "test", - make(map[string]string), make(map[string]string), "", "", - &types.PodSandboxMetadata{}, "", "", false, "", "", "", - []*hostport.PortMapping{}, false, currentTime, "", nil, nil) - Expect(err).ToNot(HaveOccurred()) + sb := sandbox.New("sandboxID", "") + sb.SetLogDir("test") image, err := sut.UserRequestedImage() Expect(err).ToNot(HaveOccurred()) diff --git a/internal/hostport/hostport.go b/internal/hostport/hostport.go index f78101247d0..71251953eef 100644 --- a/internal/hostport/hostport.go +++ b/internal/hostport/hostport.go @@ -42,10 +42,10 @@ const ( // PortMapping represents a network port in a container type PortMapping struct { - HostPort int32 - ContainerPort int32 - Protocol v1.Protocol - HostIP string + HostPort int32 `json:"HostPort,omitempty"` + ContainerPort int32 `json:"ContainerPort,omitempty"` + Protocol v1.Protocol `json:"Protocol,omitempty"` + HostIP string `json:"HostIP,omitempty"` } // PodPortMapping represents a pod's network state and associated container port mappings diff --git a/internal/lib/container_server.go b/internal/lib/container_server.go index 6bef0a92015..70873e67726 100644 --- a/internal/lib/container_server.go +++ b/internal/lib/container_server.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "os" - "path/filepath" "sync" "time" @@ -13,7 +12,6 @@ import ( cstorage "github.com/containers/storage" "github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/truncindex" - "github.com/cri-o/cri-o/internal/hostport" "github.com/cri-o/cri-o/internal/lib/sandbox" statsserver "github.com/cri-o/cri-o/internal/lib/stats" "github.com/cri-o/cri-o/internal/log" @@ -175,12 +173,8 @@ func (c *ContainerServer) LoadSandbox(ctx context.Context, id string) (sb *sandb if err := json.Unmarshal(config, &m); err != nil { return nil, fmt.Errorf("error unmarshalling sandbox spec: %w", err) } - labels := make(map[string]string) - if err := json.Unmarshal([]byte(m.Annotations[annotations.Labels]), &labels); err != nil { - return nil, fmt.Errorf("error unmarshalling %s annotation: %w", annotations.Labels, err) - } - name := m.Annotations[annotations.Name] - name, err = c.ReservePodName(id, name) + + name, err := c.ReservePodName(id, m.Annotations[annotations.Name]) if err != nil { return nil, err } @@ -189,59 +183,11 @@ func (c *ContainerServer) LoadSandbox(ctx context.Context, id string) (sb *sandb c.ReleasePodName(name) } }() - var metadata types.PodSandboxMetadata - if err := json.Unmarshal([]byte(m.Annotations[annotations.Metadata]), &metadata); err != nil { - return nil, fmt.Errorf("error unmarshalling %s annotation: %w", annotations.Metadata, err) - } - - processLabel := m.Process.SelinuxLabel - mountLabel := m.Linux.MountLabel - spp := m.Annotations[annotations.SeccompProfilePath] - - kubeAnnotations := make(map[string]string) - if err := json.Unmarshal([]byte(m.Annotations[annotations.Annotations]), &kubeAnnotations); err != nil { - return nil, fmt.Errorf("error unmarshalling %s annotation: %w", annotations.Annotations, err) - } - - portMappings := []*hostport.PortMapping{} - if err := json.Unmarshal([]byte(m.Annotations[annotations.PortMappings]), &portMappings); err != nil { - return nil, fmt.Errorf("error unmarshalling %s annotation: %w", annotations.PortMappings, err) - } - - privileged := isTrue(m.Annotations[annotations.PrivilegedRuntime]) - hostNetwork := isTrue(m.Annotations[annotations.HostNetwork]) - nsOpts := types.NamespaceOption{} - if err := json.Unmarshal([]byte(m.Annotations[annotations.NamespaceOptions]), &nsOpts); err != nil { - return nil, fmt.Errorf("error unmarshalling %s annotation: %w", annotations.NamespaceOptions, err) - } - - created, err := time.Parse(time.RFC3339Nano, m.Annotations[annotations.Created]) - if err != nil { - return nil, fmt.Errorf("parsing created timestamp annotation: %w", err) - } - - podLinuxOverhead := types.LinuxContainerResources{} - if v, found := m.Annotations[annotations.PodLinuxOverhead]; found { - if err := json.Unmarshal([]byte(v), &podLinuxOverhead); err != nil { - return nil, fmt.Errorf("error unmarshalling %s annotation: %w", annotations.PodLinuxOverhead, err) - } - } - - podLinuxResources := types.LinuxContainerResources{} - if v, found := m.Annotations[annotations.PodLinuxResources]; found { - if err := json.Unmarshal([]byte(v), &podLinuxResources); err != nil { - return nil, fmt.Errorf("error unmarshalling %s annotation: %w", annotations.PodLinuxResources, err) - } - } - - sb, err = sandbox.New(id, m.Annotations[annotations.Namespace], name, m.Annotations[annotations.KubeName], filepath.Dir(m.Annotations[annotations.LogPath]), labels, kubeAnnotations, processLabel, mountLabel, &metadata, m.Annotations[annotations.ShmPath], m.Annotations[annotations.CgroupParent], privileged, m.Annotations[annotations.RuntimeHandler], m.Annotations[annotations.ResolvPath], m.Annotations[annotations.HostName], portMappings, hostNetwork, created, m.Annotations[annotations.UsernsModeAnnotation], &podLinuxOverhead, &podLinuxResources) + sb, err = sandbox.FromSpec(id, &m) if err != nil { - return nil, err + return nil, fmt.Errorf("sandbox from spec: %w", err) } - sb.AddHostnamePath(m.Annotations[annotations.HostnamePath]) - sb.SetSeccompProfilePath(spp) - sb.SetNamespaceOptions(&nsOpts) defer func() { if retErr != nil { @@ -293,13 +239,14 @@ func (c *ContainerServer) LoadSandbox(ctx context.Context, id string) (sb *sandb wasSpoofed = true } + created := time.Unix(0, sb.CreatedAt()) if !wasSpoofed { - scontainer, err = oci.NewContainer(m.Annotations[annotations.ContainerID], cname, sandboxPath, m.Annotations[annotations.LogPath], labels, m.Annotations, kubeAnnotations, m.Annotations[annotations.Image], nil, nil, "", nil, id, false, false, false, sb.RuntimeHandler(), sandboxDir, created, m.Annotations["org.opencontainers.image.stopSignal"]) + scontainer, err = oci.NewContainer(m.Annotations[annotations.ContainerID], cname, sandboxPath, m.Annotations[annotations.LogPath], sb.Labels(), m.Annotations, sb.Annotations(), m.Annotations[annotations.Image], nil, nil, "", nil, id, false, false, false, sb.RuntimeHandler(), sandboxDir, created, m.Annotations["org.opencontainers.image.stopSignal"]) if err != nil { return sb, err } } else { - scontainer = oci.NewSpoofedContainer(cID, cname, labels, id, created, sandboxPath) + scontainer = oci.NewSpoofedContainer(cID, cname, sb.Labels(), id, created, sandboxPath) } scontainer.SetSpec(&m) scontainer.SetMountPoint(m.Annotations[annotations.MountPoint]) @@ -353,7 +300,7 @@ func (c *ContainerServer) LoadSandbox(ctx context.Context, id string) (sb *sandb } sb.SetCreated() - if err := label.ReserveLabel(processLabel); err != nil { + if err := label.ReserveLabel(m.Process.SelinuxLabel); err != nil { return sb, err } diff --git a/internal/lib/sandbox/history_test.go b/internal/lib/sandbox/history_test.go index 40bb941cfcf..2e7aa75753d 100644 --- a/internal/lib/sandbox/history_test.go +++ b/internal/lib/sandbox/history_test.go @@ -1,13 +1,9 @@ package sandbox_test import ( - "time" - - "github.com/cri-o/cri-o/internal/hostport" "github.com/cri-o/cri-o/internal/lib/sandbox" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - types "k8s.io/cri-api/pkg/apis/runtime/v1" ) // The actual test suite @@ -17,11 +13,7 @@ var _ = t.Describe("History", func() { // Prepare the sut BeforeEach(func() { beforeEach() - otherTestSandbox, err := sandbox.New("sandboxID", "", "", "", "", - make(map[string]string), make(map[string]string), "", "", - &types.PodSandboxMetadata{}, "", "", false, "", "", "", - []*hostport.PortMapping{}, false, time.Now(), "", nil, nil) - Expect(err).ToNot(HaveOccurred()) + otherTestSandbox := sandbox.New("sandboxID", "") Expect(testSandbox).NotTo(BeNil()) sut = &sandbox.History{testSandbox, otherTestSandbox} }) diff --git a/internal/lib/sandbox/sandbox.go b/internal/lib/sandbox/sandbox.go index 44b4538f06d..9a86b55a23f 100644 --- a/internal/lib/sandbox/sandbox.go +++ b/internal/lib/sandbox/sandbox.go @@ -2,6 +2,7 @@ package sandbox import ( "context" + "encoding/json" "errors" "fmt" "os" @@ -13,6 +14,8 @@ import ( "github.com/cri-o/cri-o/internal/hostport" "github.com/cri-o/cri-o/internal/log" "github.com/cri-o/cri-o/internal/oci" + "github.com/cri-o/cri-o/pkg/annotations" + rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/fields" types "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -76,38 +79,110 @@ var ErrIDEmpty = errors.New("PodSandboxId should not be empty") // New creates and populates a new pod sandbox // New sandboxes have no containers, no infra container, and no network namespaces associated with them // An infra container must be attached before the sandbox is added to the state -func New(id, namespace, name, kubeName, logDir string, labels, annotations map[string]string, processLabel, mountLabel string, metadata *types.PodSandboxMetadata, shmPath, cgroupParent string, privileged bool, runtimeHandler, resolvPath, hostname string, portMappings []*hostport.PortMapping, hostNetwork bool, createdAt time.Time, usernsMode string, overhead, resources *types.LinuxContainerResources) (*Sandbox, error) { - sb := new(Sandbox) +func New(id, name string) *Sandbox { + return &Sandbox{ + criSandbox: &types.PodSandbox{ + Id: id, + CreatedAt: time.Now().UnixNano(), + Metadata: &types.PodSandboxMetadata{}, + }, + name: name, + containers: oci.NewMemoryStore(), + } +} + +// FromSpec creates a sandbox from the provided runtime spec. +func FromSpec(id string, spec *rspec.Spec) (*Sandbox, error) { + sb := &Sandbox{} + + created, err := time.Parse(time.RFC3339Nano, spec.Annotations[annotations.Created]) + if err != nil { + return nil, fmt.Errorf("parsing created timestamp annotation: %w", err) + } + + labels := make(map[string]string) + if err := json.Unmarshal([]byte(spec.Annotations[annotations.Labels]), &labels); err != nil { + return nil, fmt.Errorf("unmarshal %s annotation: %w", annotations.Labels, err) + } + + kubeAnnotations := make(map[string]string) + if err := json.Unmarshal([]byte(spec.Annotations[annotations.Annotations]), &kubeAnnotations); err != nil { + return nil, fmt.Errorf("unmarshal %s annotation: %w", annotations.Annotations, err) + } + + metadata := types.PodSandboxMetadata{} + if err := json.Unmarshal([]byte(spec.Annotations[annotations.Metadata]), &metadata); err != nil { + return nil, fmt.Errorf("unmarshal %s annotation: %w", annotations.Metadata, err) + } sb.criSandbox = &types.PodSandbox{ Id: id, - CreatedAt: createdAt.UnixNano(), + CreatedAt: created.UnixNano(), Labels: labels, - Annotations: annotations, - Metadata: metadata, + Annotations: kubeAnnotations, + Metadata: &metadata, } - sb.namespace = namespace - sb.name = name - sb.kubeName = kubeName - sb.logDir = logDir + + sb.name = spec.Annotations[annotations.Name] + sb.namespace = spec.Annotations[annotations.Namespace] + sb.kubeName = spec.Annotations[annotations.KubeName] + sb.logDir = filepath.Dir(spec.Annotations[annotations.LogPath]) sb.containers = oci.NewMemoryStore() - sb.processLabel = processLabel - sb.mountLabel = mountLabel - sb.shmPath = shmPath - sb.cgroupParent = cgroupParent - sb.privileged = privileged - sb.runtimeHandler = runtimeHandler - sb.resolvPath = resolvPath - sb.hostname = hostname + sb.shmPath = spec.Annotations[annotations.ShmPath] + sb.cgroupParent = spec.Annotations[annotations.CgroupParent] + sb.privileged = isTrue(spec.Annotations[annotations.PrivilegedRuntime]) + sb.runtimeHandler = spec.Annotations[annotations.RuntimeHandler] + sb.resolvPath = spec.Annotations[annotations.ResolvPath] + sb.hostname = spec.Annotations[annotations.HostName] + sb.hostNetwork = isTrue(spec.Annotations[annotations.HostNetwork]) + sb.usernsMode = spec.Annotations[annotations.UsernsModeAnnotation] + sb.seccompProfilePath = spec.Annotations[annotations.SeccompProfilePath] + sb.hostnamePath = spec.Annotations[annotations.HostnamePath] + + portMappings := []*hostport.PortMapping{} + if err := json.Unmarshal([]byte(spec.Annotations[annotations.PortMappings]), &portMappings); err != nil { + return nil, fmt.Errorf("unmarshal %s annotation: %w", annotations.PortMappings, err) + } sb.portMappings = portMappings - sb.hostNetwork = hostNetwork - sb.usernsMode = usernsMode - sb.podLinuxOverhead = overhead - sb.podLinuxResources = resources + + podLinuxOverhead := types.LinuxContainerResources{} + if v, found := spec.Annotations[annotations.PodLinuxOverhead]; found { + if err := json.Unmarshal([]byte(v), &podLinuxOverhead); err != nil { + return nil, fmt.Errorf("unmarshal %s annotation: %w", annotations.PodLinuxOverhead, err) + } + } + sb.podLinuxOverhead = &podLinuxOverhead + + podLinuxResources := types.LinuxContainerResources{} + if v, found := spec.Annotations[annotations.PodLinuxResources]; found { + if err := json.Unmarshal([]byte(v), &podLinuxResources); err != nil { + return nil, fmt.Errorf("unmarshal %s annotation: %w", annotations.PodLinuxResources, err) + } + } + sb.podLinuxResources = &podLinuxResources + + nsOpts := types.NamespaceOption{} + if err := json.Unmarshal([]byte(spec.Annotations[annotations.NamespaceOptions]), &nsOpts); err != nil { + return nil, fmt.Errorf("unmarshal %s annotation: %w", annotations.NamespaceOptions, err) + } + sb.nsOpts = &nsOpts + + if spec != nil { + if spec.Process != nil { + sb.processLabel = spec.Process.SelinuxLabel + } + if spec.Linux != nil { + sb.mountLabel = spec.Linux.MountLabel + } + } return sb, nil } +func isTrue(annotaton string) bool { + return annotaton == "true" +} + func (s *Sandbox) CRISandbox() *types.PodSandbox { // If a protobuf message gets mutated mid-request, then the proto library panics. // We would like to avoid deep copies when possible to avoid excessive garbage @@ -320,6 +395,101 @@ func (s *Sandbox) RemoveContainer(ctx context.Context, c *oci.Container) { s.containers.Delete(c.Name()) } +// SetMetadata can be used to set the metadata of the sandbox. +func (s *Sandbox) SetMetadata(metadata *types.PodSandboxMetadata) { + s.criSandbox.Metadata = metadata +} + +// SetLabels can be used to set the labels of the sandbox. +func (s *Sandbox) SetLabels(labels map[string]string) { + s.criSandbox.Labels = labels +} + +// SetAnnotations can be used to set the annotations of the sandbox. +func (s *Sandbox) SetAnnotations(anns map[string]string) { + s.criSandbox.Annotations = anns +} + +// SetLogDir can be used to set the log dir of the sandbox. +func (s *Sandbox) SetLogDir(logDir string) { + s.logDir = logDir +} + +// SetNamespace can be used to set the namespace of the sandbox. +func (s *Sandbox) SetNamespace(namespace string) { + s.namespace = namespace +} + +// SetKubeName can be used to set the kube name of the sandbox. +func (s *Sandbox) SetKubeName(kubeName string) { + s.kubeName = kubeName +} + +// SetProcessLabel can be used to set the process label of the sandbox. +func (s *Sandbox) SetProcessLabel(processLabel string) { + s.processLabel = processLabel +} + +// SetMountLabel can be used to set the mount label of the sandbox. +func (s *Sandbox) SetMountLabel(mountLabel string) { + s.mountLabel = mountLabel +} + +// SetShmPath can be used to set the shm path of the sandbox. +func (s *Sandbox) SetShmPath(shmPath string) { + s.shmPath = shmPath +} + +// SetCgroupParent can be used to set the cgroup parent of the sandbox. +func (s *Sandbox) SetCgroupParent(cgroupParent string) { + s.cgroupParent = cgroupParent +} + +// SetPrivileged can be used to set the privileged state of the sandbox. +func (s *Sandbox) SetPrivileged(privileged bool) { + s.privileged = privileged +} + +// SetRuntimeHandler can be used to set the runtime handler of the sandbox. +func (s *Sandbox) SetRuntimeHandler(runtimeHandler string) { + s.runtimeHandler = runtimeHandler +} + +// SetResolvPath can be used to set the resolv path of the sandbox. +func (s *Sandbox) SetResolvPath(resolvPath string) { + s.resolvPath = resolvPath +} + +// SetHostname can be used to set the hostname of the sandbox. +func (s *Sandbox) SetHostname(hostname string) { + s.hostname = hostname +} + +// SetPortMappings can be used to set the port mappings of the sandbox. +func (s *Sandbox) SetPortMappings(portMappings []*hostport.PortMapping) { + s.portMappings = portMappings +} + +// SetHostNetwork can be used to set the host network state of the sandbox. +func (s *Sandbox) SetHostNetwork(hostNetwork bool) { + s.hostNetwork = hostNetwork +} + +// SetUsernsMode can be used to set the usernamespace mode of the sandbox. +func (s *Sandbox) SetUsernsMode(usernsMode string) { + s.usernsMode = usernsMode +} + +// SetPodLinuxResources can be used to set the pod linux resources of the sandbox. +func (s *Sandbox) SetPodLinuxResources(resources *types.LinuxContainerResources) { + s.podLinuxResources = resources +} + +// SetPodLinuxOverhead can be used to set the pod linux overhead of the sandbox. +func (s *Sandbox) SetPodLinuxOverhead(resources *types.LinuxContainerResources) { + s.podLinuxOverhead = resources +} + // SetInfraContainer sets the infrastructure container of a sandbox // Attempts to set the infrastructure container after one is already present will throw an error func (s *Sandbox) SetInfraContainer(infraCtr *oci.Container) error { diff --git a/internal/lib/sandbox/sandbox_test.go b/internal/lib/sandbox/sandbox_test.go index ebd7e70c2cc..c1da47907cb 100644 --- a/internal/lib/sandbox/sandbox_test.go +++ b/internal/lib/sandbox/sandbox_test.go @@ -40,16 +40,27 @@ var _ = t.Describe("Sandbox", func() { hostname := "hostname" portMappings := []*hostport.PortMapping{{}, {}} hostNetwork := false - createdAt := time.Now() // When - sandbox, err := sandbox.New(id, namespace, name, kubeName, logDir, - labels, annotations, processLabel, mountLabel, &metadata, - shmPath, cgroupParent, privileged, runtimeHandler, - resolvPath, hostname, portMappings, hostNetwork, createdAt, "", nil, nil) + sandbox := sandbox.New(id, name) + sandbox.SetNamespace(namespace) + sandbox.SetKubeName(kubeName) + sandbox.SetLogDir(logDir) + sandbox.SetLabels(labels) + sandbox.SetAnnotations(annotations) + sandbox.SetProcessLabel(processLabel) + sandbox.SetMountLabel(mountLabel) + sandbox.SetMetadata(&metadata) + sandbox.SetShmPath(shmPath) + sandbox.SetCgroupParent(cgroupParent) + sandbox.SetPrivileged(privileged) + sandbox.SetRuntimeHandler(runtimeHandler) + sandbox.SetResolvPath(resolvPath) + sandbox.SetHostname(hostname) + sandbox.SetPortMappings(portMappings) + sandbox.SetHostNetwork(hostNetwork) // Then - Expect(err).ToNot(HaveOccurred()) Expect(sandbox).NotTo(BeNil()) Expect(sandbox.ID()).To(Equal(id)) Expect(sandbox.Namespace()).To(Equal(namespace)) @@ -71,7 +82,7 @@ var _ = t.Describe("Sandbox", func() { Expect(sandbox.HostNetwork()).To(Equal(hostNetwork)) Expect(sandbox.StopMutex()).NotTo(BeNil()) Expect(sandbox.Containers()).NotTo(BeNil()) - Expect(sandbox.CreatedAt()).To(Equal(createdAt.UnixNano())) + Expect(sandbox.CreatedAt()).NotTo(BeZero()) }) }) diff --git a/internal/lib/sandbox/sandbox_test_inject.go b/internal/lib/sandbox/sandbox_test_inject.go deleted file mode 100644 index 81b96bbba9e..00000000000 --- a/internal/lib/sandbox/sandbox_test_inject.go +++ /dev/null @@ -1,16 +0,0 @@ -//go:build test -// +build test - -// All *_inject.go files are meant to be used by tests only. Purpose of this -// files is to provide a way to inject mocked data into the current setup. - -package sandbox - -import ( - "github.com/cri-o/cri-o/internal/hostport" -) - -// SetPortMappings sets the PortMappings for the Sandbox -func (s *Sandbox) SetPortMappings(portMappings []*hostport.PortMapping) { - s.portMappings = portMappings -} diff --git a/internal/lib/sandbox/suite_test.go b/internal/lib/sandbox/suite_test.go index 0c5ca88ed6c..d0dcf98924b 100644 --- a/internal/lib/sandbox/suite_test.go +++ b/internal/lib/sandbox/suite_test.go @@ -2,15 +2,12 @@ package sandbox_test import ( "testing" - "time" - "github.com/cri-o/cri-o/internal/hostport" "github.com/cri-o/cri-o/internal/lib/sandbox" . "github.com/cri-o/cri-o/test/framework" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/sirupsen/logrus" - types "k8s.io/cri-api/pkg/apis/runtime/v1" ) // TestSandbox runs the created specs @@ -37,11 +34,6 @@ var _ = AfterSuite(func() { func beforeEach() { // Setup test vars - var err error - testSandbox, err = sandbox.New("sandboxID", "", "", "", "", - make(map[string]string), make(map[string]string), "", "", - &types.PodSandboxMetadata{}, "", "", false, "", "", "", - []*hostport.PortMapping{}, false, time.Now(), "", nil, nil) - Expect(err).ToNot(HaveOccurred()) + testSandbox = sandbox.New("sandboxID", "") Expect(testSandbox).NotTo(BeNil()) } diff --git a/internal/lib/suite_test.go b/internal/lib/suite_test.go index a038cde3eff..36a8a93e6d0 100644 --- a/internal/lib/suite_test.go +++ b/internal/lib/suite_test.go @@ -6,7 +6,6 @@ import ( "testing" "time" - "github.com/cri-o/cri-o/internal/hostport" "github.com/cri-o/cri-o/internal/lib" "github.com/cri-o/cri-o/internal/lib/sandbox" "github.com/cri-o/cri-o/internal/oci" @@ -152,11 +151,7 @@ func beforeEach() { Expect(sut).NotTo(BeNil()) // Setup test vars - mySandbox, err = sandbox.New(sandboxID, "", "", "", "", - make(map[string]string), make(map[string]string), "", "", - &types.PodSandboxMetadata{}, "", "", false, "", "", "", - []*hostport.PortMapping{}, false, time.Now(), "", nil, nil) - Expect(err).ToNot(HaveOccurred()) + mySandbox = sandbox.New(sandboxID, "") myContainer, err = oci.NewContainer(containerID, "", "", "", make(map[string]string), make(map[string]string), diff --git a/internal/runtimehandlerhooks/high_performance_hooks_test.go b/internal/runtimehandlerhooks/high_performance_hooks_test.go index d8f5d2bf35e..0468a280323 100644 --- a/internal/runtimehandlerhooks/high_performance_hooks_test.go +++ b/internal/runtimehandlerhooks/high_performance_hooks_test.go @@ -716,14 +716,13 @@ var _ = Describe("high_performance_hooks", func() { false, "", "", time.Now(), "") Expect(err).ToNot(HaveOccurred()) - sb, err := sandbox.New("", "", "", "", "", nil, + sb := sandbox.New("", "") + sb.SetAnnotations( map[string]string{ crioannotations.CPUSharedAnnotation + "/" + c.CRIContainer().GetMetadata().GetName(): annotationEnable, }, - "", "", nil, "", "", false, - "", "", "", nil, false, - time.Now(), "", nil, nil) - Expect(err).ToNot(HaveOccurred()) + ) + It("should inject env variable only to pod with cpu-shared.crio.io annotation", func() { h := HighPerformanceHooks{sharedCPUs: "3,4"} err := h.PreCreate(context.TODO(), g, sb, c) diff --git a/server/sandbox_run_linux.go b/server/sandbox_run_linux.go index 581b12dde5d..4ffa4428939 100644 --- a/server/sandbox_run_linux.go +++ b/server/sandbox_run_linux.go @@ -349,9 +349,16 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ return nil, fmt.Errorf("setting sandbox config: %w", err) } + sb := libsandbox.New(sbox.ID(), sbox.Name()) + kubeName := sbox.Config().Metadata.Name + sb.SetKubeName(kubeName) + kubePodUID := sbox.Config().Metadata.Uid + namespace := sbox.Config().Metadata.Namespace + sb.SetNamespace(namespace) + attempt := sbox.Config().Metadata.Attempt // These fields are populated by the Kubelet, but not crictl. Populate if needed. @@ -402,7 +409,9 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ if securityContext.NamespaceOptions == nil { securityContext.NamespaceOptions = &types.NamespaceOption{} } + hostNetwork := securityContext.NamespaceOptions.Network == types.NamespaceMode_NODE + sb.SetHostNetwork(hostNetwork) if err := s.config.CNIPluginReadyOrError(); err != nil && !hostNetwork { // if the cni plugin isn't ready yet, we should wait until it is @@ -421,17 +430,20 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ if err != nil { return nil, err } + sb.SetRuntimeHandler(runtimeHandler) if err := s.FilterDisallowedAnnotations(sbox.Config().Annotations, sbox.Config().Annotations, runtimeHandler); err != nil { return nil, err } kubeAnnotations := sbox.Config().Annotations + sb.SetAnnotations(kubeAnnotations) usernsMode := kubeAnnotations[annotations.UsernsModeAnnotation] if usernsMode != "" { log.Warnf(ctx, "Annotation 'io.kubernetes.cri-o.userns-mode' is deprecated, and will be replaced with native Kubernetes support for user namespaces in the future") } + sb.SetUsernsMode(usernsMode) idMappingsOptions, err := s.configureSandboxIDMappings(usernsMode, sbox.Config().Linux.SecurityContext) if err != nil { @@ -454,6 +466,7 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ } privileged := s.privilegedSandbox(req) + sb.SetPrivileged(privileged) s.resourceStore.SetStageForResource(ctx, sbox.Name(), "sandbox storage creation") pauseImage, err := s.config.ParsePauseImage() @@ -498,6 +511,7 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ if err := os.MkdirAll(logDir, 0o700); err != nil { return nil, err } + sb.SetLogDir(logDir) var sandboxIDMappings *idtools.IDMappings if idMappingsOptions != nil { @@ -511,6 +525,8 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ // add metadata metadata := sbox.Config().Metadata + sb.SetMetadata(metadata) + metadataJSON, err := json.Marshal(metadata) if err != nil { return nil, err @@ -527,6 +543,8 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ if labels != nil { labels[kubeletTypes.KubernetesContainerNameLabel] = oci.InfraContainerName } + sb.SetLabels(labels) + labelsJSON, err := json.Marshal(labels) if err != nil { return nil, err @@ -552,7 +570,9 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ } g := sbox.Spec() g.SetProcessSelinuxLabel(processLabel) + sb.SetProcessLabel(processLabel) g.SetLinuxMountLabel(mountLabel) + sb.SetMountLabel(mountLabel) // Remove the default /dev/shm mount to ensure we overwrite it g.RemoveMount(libsandbox.DevShmPath) @@ -588,6 +608,7 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ return nil }) } + sb.SetShmPath(shmPath) // Link logs if requested if emptyDirVolName, ok := kubeAnnotations[annotations.LinkLogsAnnotation]; ok { @@ -635,6 +656,8 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ return nil, err } g.SetHostname(hostname) + sb.SetHostname(hostname) + sb.SetResolvPath(sb.ResolvPath()) g.AddAnnotation(annotations.Metadata, string(metadataJSON)) g.AddAnnotation(annotations.Labels, string(labelsJSON)) @@ -663,10 +686,12 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ g.AddAnnotation("org.opencontainers.image.stopSignal", podContainer.Config.Config.StopSignal) } - created := time.Now() + created := time.Unix(0, sb.CreatedAt()) g.AddAnnotation(annotations.Created, created.Format(time.RFC3339Nano)) portMappings := convertPortMappings(sbox.Config().PortMappings) + sb.SetPortMappings(portMappings) + portMappingsJSON, err := json.Marshal(portMappings) if err != nil { return nil, err @@ -683,6 +708,7 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ if cgroupPath != "" { g.SetLinuxCgroupsPath(cgroupPath) } + sb.SetCgroupParent(cgroupParent) g.AddAnnotation(annotations.CgroupParent, cgroupParent) if sandboxIDMappings != nil { @@ -698,6 +724,8 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ } overhead := sbox.Config().GetLinux().GetOverhead() + sb.SetPodLinuxOverhead(overhead) + overheadJSON, err := json.Marshal(overhead) if err != nil { return nil, err @@ -705,17 +733,14 @@ func (s *Server) runPodSandbox(ctx context.Context, req *types.RunPodSandboxRequ g.AddAnnotation(annotations.PodLinuxOverhead, string(overheadJSON)) resources := sbox.Config().GetLinux().GetResources() + sb.SetPodLinuxResources(resources) + resourcesJSON, err := json.Marshal(resources) if err != nil { return nil, err } g.AddAnnotation(annotations.PodLinuxResources, string(resourcesJSON)) - sb, err := libsandbox.New(sbox.ID(), namespace, sbox.Name(), kubeName, logDir, labels, kubeAnnotations, processLabel, mountLabel, metadata, shmPath, cgroupParent, privileged, runtimeHandler, sbox.ResolvPath(), hostname, portMappings, hostNetwork, created, usernsMode, overhead, resources) - if err != nil { - return nil, err - } - sb.SetDNSConfig(sbox.Config().DnsConfig) if err := s.addSandbox(ctx, sb); err != nil { diff --git a/server/suite_test.go b/server/suite_test.go index faf3884a5df..dd59f8ae9a8 100644 --- a/server/suite_test.go +++ b/server/suite_test.go @@ -10,7 +10,6 @@ import ( "time" cstorage "github.com/containers/storage" - "github.com/cri-o/cri-o/internal/hostport" "github.com/cri-o/cri-o/internal/lib/sandbox" "github.com/cri-o/cri-o/internal/oci" "github.com/cri-o/cri-o/pkg/config" @@ -153,11 +152,7 @@ var beforeEach = func() { serverConfig.PluginDirs = []string{emptyDir} serverConfig.HooksDir = []string{emptyDir} // Initialize test container and sandbox - testSandbox, err = sandbox.New(sandboxID, "", "", "", ".", - make(map[string]string), make(map[string]string), "", "", - &types.PodSandboxMetadata{}, "", "", false, "", "", "", - []*hostport.PortMapping{}, false, time.Now(), "", nil, nil) - Expect(err).ToNot(HaveOccurred()) + testSandbox = sandbox.New(sandboxID, "") testContainer, err = oci.NewContainer(containerID, "", "", "", make(map[string]string), make(map[string]string), @@ -213,6 +208,7 @@ func addContainerAndSandbox() { sut.AddContainer(ctx, testContainer) Expect(sut.CtrIDIndex().Add(testContainer.ID())).To(Succeed()) Expect(sut.PodIDIndex().Add(testSandbox.ID())).To(Succeed()) + testSandbox.SetLogDir("test") testContainer.SetCreated() testSandbox.SetCreated() }