pr feedback: additional const and move linux specific code

Signed-off-by: Maksim An <maksiman@microsoft.com>

internal/constants/constants.go

@@ -11,5 +11,9 @@ const (
  // WCOWRootPrefixInUVM is the path inside UVM where WCOW container's root file system will be mounted
  WCOWRootPrefixInUVM = `C:\c`
 
+ // SandboxMountPrefix is mount prefix used in container spec to mark a sandbox-mount
  SandboxMountPrefix = "sandbox://"
+
+ // HugePagesMountPrefix is mount prefix used in container spec to mark a huge-pages mount
+ HugePagesMountPrefix = "hugepages://"
 )

internal/guest/runtime/hcsv2/nvidia_utils.go

@@ -6,6 +6,7 @@ package hcsv2
 import (
  "context"
  "fmt"
+ "os"
  "os/exec"
  "strings"
 
@@ -77,9 +78,24 @@ func addNvidiaDevicePreHook(ctx context.Context, spec *oci.Spec) error {
 // updateEnvWithNvidiaVariables creates an env with the nvidia gpu vhd in PATH and insecure mode set
 func updateEnvWithNvidiaVariables() []string {
  nvidiaBin := fmt.Sprintf("%s/bin", constants.LCOWNvidiaMountPath)
- env := hooks.UpdatePathEnv(nvidiaBin)
+ env := updatePathEnv(nvidiaBin)
  // NVC_INSECURE_MODE allows us to run nvidia-container-cli without seccomp
  // we don't currently use seccomp in the uvm, so avoid using it here for now as well
  env = append(env, "NVC_INSECURE_MODE=1")
  return env
 }
+
+// updatePathEnv adds specified `dirs` to PATH variable and returns the result environment variables.
+func updatePathEnv(dirs ...string) []string {
+ pathPrefix := "PATH="
+ additionalDirs := strings.Join(dirs, ":")
+ env := os.Environ()
+ for i, v := range env {
+ if strings.HasPrefix(v, pathPrefix) {
+ newPath := fmt.Sprintf("%s:%s", v, additionalDirs)
+ env[i] = newPath
+ return env
+ }
+ }
+ return append(env, fmt.Sprintf("PATH=%s", additionalDirs))
+}

internal/guest/runtime/hcsv2/workload_container.go

@@ -60,11 +60,10 @@ func updateSandboxMounts(sbid string, spec *oci.Spec) error {
 }
 
 func updateHugePageMounts(sbid string, spec *oci.Spec) error {
- mountPrefix := "hugepages://"
  for i, m := range spec.Mounts {
- if strings.HasPrefix(m.Source, mountPrefix) {
+ if strings.HasPrefix(m.Source, constants.HugePagesMountPrefix) {
  mountsDir := getSandboxHugePageMountsDir(sbid)
- subPath := strings.TrimPrefix(m.Source, mountPrefix)
+ subPath := strings.TrimPrefix(m.Source, constants.HugePagesMountPrefix)
  pageSize := strings.Split(subPath, string(os.PathSeparator))[0]
  hugePageMountSource := filepath.Join(mountsDir, subPath)
 

internal/hcsoci/resources_lcow.go

@@ -116,11 +116,15 @@ func allocateLinuxResources(ctx context.Context, coi *createOptionsInternal, r *
  // Mounts that map to a path in UVM are specified with 'sandbox://' prefix.
  // example: sandbox:///a/dirInUvm destination:/b/dirInContainer
  uvmPathForFile = mount.Source
- } else if strings.HasPrefix(mount.Source, "hugepages://") {
+ } else if strings.HasPrefix(mount.Source, constants.HugePagesMountPrefix) {
  // currently we only support 2M hugepage size
- hugePageSubDirs := strings.Split(strings.TrimPrefix(mount.Source, "hugepages://"), "/")
+ hugePageSubDirs := strings.Split(strings.TrimPrefix(mount.Source, constants.HugePagesMountPrefix), "/")
  if len(hugePageSubDirs) < 2 {
- return errors.Errorf(`%s mount path is invalid, expected format: hugepages://<hugepage-size>/<hugepage-src-location>`, mount.Source)
+ return errors.Errorf(
+ `%s mount path is invalid, expected format: %s<hugepage-size>/<hugepage-src-location>`,
+ mount.Source,
+ constants.HugePagesMountPrefix,
+ )
  }
 
  // hugepages:// should be followed by pagesize

internal/hooks/spec.go

@@ -2,8 +2,6 @@ package hooks
 
 import (
  "fmt"
- "os"
- "strings"
 
  oci "github.com/opencontainers/runtime-spec/specs-go"
 )
@@ -53,18 +51,3 @@ func AddOCIHook(spec *oci.Spec, hn HookName, hk oci.Hook) error {
  }
  return nil
 }
-
-// UpdatePathEnv adds specified `dirs` to PATH variable and returns the result environment variables.
-func UpdatePathEnv(dirs ...string) []string {
- pathPrefix := "PATH="
- additionalDirs := strings.Join(dirs, ":")
- env := os.Environ()
- for i, v := range env {
- if strings.HasPrefix(v, pathPrefix) {
- newPath := fmt.Sprintf("%s:%s", v, additionalDirs)
- env[i] = newPath
- return env
- }
- }
- return append(env, fmt.Sprintf("PATH=%s", additionalDirs))
-}

test/cri-containerd/container_test.go

@@ -6,6 +6,7 @@ package cri_containerd
 import (
  "bufio"
  "context"
+ "fmt"
  "io"
  "os"
  "path/filepath"
@@ -14,9 +15,11 @@ import (
  "testing"
  "time"
 
- "github.com/Microsoft/hcsshim/pkg/annotations"
  "github.com/sirupsen/logrus"
  runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
+
+ "github.com/Microsoft/hcsshim/internal/constants"
+ "github.com/Microsoft/hcsshim/pkg/annotations"
 )
 
 func runLogRotationContainer(t *testing.T, sandboxRequest *runtime.RunPodSandboxRequest, request *runtime.CreateContainerRequest, log string, logArchive string) {
@@ -725,7 +728,7 @@ func Test_CreateContainer_HugePageMount_LCOW(t *testing.T) {
  },
  Mounts: []*runtime.Mount{
  {
- HostPath: "hugepages://2M/hugepage2M",
+ HostPath: fmt.Sprintf("%s2M/hugepage2M", constants.HugePagesMountPrefix),
  ContainerPath: "/mnt/hugepage2M",
  Readonly: false,
  Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,