From 08cf87eb21bb7bcbfbf6446c4c50390f2b57ea41 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 28 Sep 2021 16:44:41 +0200 Subject: [PATCH] Use oci package from experimental branch for nvidia-container-runtime Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/nvcr.go | 2 +- cmd/nvidia-container-runtime/nvcr_test.go | 23 ++++---- .../runtime_factory.go | 44 +++------------ .../runtime_factory_test.go | 54 ------------------- 4 files changed, 20 insertions(+), 103 deletions(-) diff --git a/cmd/nvidia-container-runtime/nvcr.go b/cmd/nvidia-container-runtime/nvcr.go index 1eb23a26..15822be4 100644 --- a/cmd/nvidia-container-runtime/nvcr.go +++ b/cmd/nvidia-container-runtime/nvcr.go @@ -22,7 +22,7 @@ import ( "os/exec" "strings" - "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" + "github.com/NVIDIA/nvidia-container-toolkit/pkg/oci" "github.com/opencontainers/runtime-spec/specs-go" log "github.com/sirupsen/logrus" ) diff --git a/cmd/nvidia-container-runtime/nvcr_test.go b/cmd/nvidia-container-runtime/nvcr_test.go index 65cfa524..00963ae9 100644 --- a/cmd/nvidia-container-runtime/nvcr_test.go +++ b/cmd/nvidia-container-runtime/nvcr_test.go @@ -22,7 +22,7 @@ import ( "strings" "testing" - "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" + "github.com/NVIDIA/nvidia-container-toolkit/pkg/oci" "github.com/opencontainers/runtime-spec/specs-go" testlog "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" @@ -185,9 +185,14 @@ func TestNvidiaContainerRuntime(t *testing.T) { tc.shim.logger = logger hook.Reset() - spec := &specs.Spec{} - ociMock := oci.NewMockSpec(spec, tc.writeError, tc.modifyError) - + ociMock := &oci.SpecMock{ + ModifyFunc: func(specModifier oci.SpecModifier) error { + return tc.modifyError + }, + FlushFunc: func() error { + return tc.writeError + }, + } require.Equal(t, tc.shouldModify, tc.shim.modificationRequired(tc.args), "%d: %v", i, tc) tc.shim.ociSpec = ociMock @@ -201,18 +206,16 @@ func TestNvidiaContainerRuntime(t *testing.T) { } if tc.shouldModify { - require.Equal(t, 1, ociMock.MockModify.Callcount, "%d: %v", i, tc) - require.Equal(t, 1, nvidiaHookCount(spec.Hooks), "%d: %v", i, tc) + require.Equal(t, 1, len(ociMock.ModifyCalls()), "%d: %v", i, tc) } else { - require.Equal(t, 0, ociMock.MockModify.Callcount, "%d: %v", i, tc) - require.Nil(t, spec.Hooks, "%d: %v", i, tc) + require.Equal(t, 0, len(ociMock.ModifyCalls()), "%d: %v", i, tc) } writeExpected := tc.shouldModify && tc.modifyError == nil if writeExpected { - require.Equal(t, 1, ociMock.MockFlush.Callcount, "%d: %v", i, tc) + require.Equal(t, 1, len(ociMock.FlushCalls()), "%d: %v", i, tc) } else { - require.Equal(t, 0, ociMock.MockFlush.Callcount, "%d: %v", i, tc) + require.Equal(t, 0, len(ociMock.FlushCalls()), "%d: %v", i, tc) } } } diff --git a/cmd/nvidia-container-runtime/runtime_factory.go b/cmd/nvidia-container-runtime/runtime_factory.go index 79db2415..8d879182 100644 --- a/cmd/nvidia-container-runtime/runtime_factory.go +++ b/cmd/nvidia-container-runtime/runtime_factory.go @@ -19,11 +19,10 @@ package main import ( "fmt" "os" - "os/exec" "path/filepath" "strings" - "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" + "github.com/NVIDIA/nvidia-container-toolkit/pkg/oci" ) const ( @@ -70,17 +69,11 @@ func newOCISpec(argv []string) (oci.Spec, error) { // newRuncRuntime locates the runc binary and wraps it in a SyscallExecRuntime func newRuncRuntime() (oci.Runtime, error) { - runtimePath, err := findRunc() - if err != nil { - return nil, fmt.Errorf("error locating runtime: %v", err) - } - - runc, err := oci.NewSyscallExecRuntimeWithLogger(logger.Logger, runtimePath) - if err != nil { - return nil, fmt.Errorf("error constructing runtime: %v", err) - } - - return runc, nil + return oci.NewLowLevelRuntimeWithLogger( + logger.Logger, + dockerRuncExecutableName, + runcExecutableName, + ) } // getBundlePath checks the specified slice of strings (argv) for a 'bundle' flag as allowed by runc. @@ -120,31 +113,6 @@ func getBundlePath(argv []string) (string, error) { return bundlePath, nil } -// findRunc locates runc in the path, returning the full path to the -// binary or an error. -func findRunc() (string, error) { - runtimeCandidates := []string{ - dockerRuncExecutableName, - runcExecutableName, - } - - return findRuntime(runtimeCandidates) -} - -func findRuntime(runtimeCandidates []string) (string, error) { - for _, candidate := range runtimeCandidates { - logger.Infof("Looking for runtime binary '%v'", candidate) - runcPath, err := exec.LookPath(candidate) - if err == nil { - logger.Infof("Found runtime binary '%v'", runcPath) - return runcPath, nil - } - logger.Warnf("Runtime binary '%v' not found: %v", candidate, err) - } - - return "", fmt.Errorf("no runtime binary found from candidate list: %v", runtimeCandidates) -} - func isBundleFlag(arg string) bool { if !strings.HasPrefix(arg, "-") { return false diff --git a/cmd/nvidia-container-runtime/runtime_factory_test.go b/cmd/nvidia-container-runtime/runtime_factory_test.go index b5a6d461..257e6aa8 100644 --- a/cmd/nvidia-container-runtime/runtime_factory_test.go +++ b/cmd/nvidia-container-runtime/runtime_factory_test.go @@ -17,10 +17,8 @@ package main import ( - "path/filepath" "testing" - testlog "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" ) @@ -138,55 +136,3 @@ func TestGetBundlePath(t *testing.T) { require.Equalf(t, tc.expected.bundle, bundle, "%d: %v", i, tc) } } - -func TestFindRunc(t *testing.T) { - testLogger, _ := testlog.NewNullLogger() - logger.Logger = testLogger - - runcPath, err := findRunc() - require.NoError(t, err) - require.Equal(t, filepath.Join(cfg.binPath, runcExecutableName), runcPath) -} - -func TestFindRuntime(t *testing.T) { - testLogger, _ := testlog.NewNullLogger() - logger.Logger = testLogger - - testCases := []struct { - candidates []string - expectedPath string - }{ - { - candidates: []string{}, - }, - { - candidates: []string{"not-runc"}, - }, - { - candidates: []string{"not-runc", "also-not-runc"}, - }, - { - candidates: []string{runcExecutableName}, - expectedPath: filepath.Join(cfg.binPath, runcExecutableName), - }, - { - candidates: []string{runcExecutableName, "not-runc"}, - expectedPath: filepath.Join(cfg.binPath, runcExecutableName), - }, - { - candidates: []string{"not-runc", runcExecutableName}, - expectedPath: filepath.Join(cfg.binPath, runcExecutableName), - }, - } - - for i, tc := range testCases { - runcPath, err := findRuntime(tc.candidates) - if tc.expectedPath == "" { - require.Error(t, err, "%d: %v", i, tc) - } else { - require.NoError(t, err, "%d: %v", i, tc) - } - require.Equal(t, tc.expectedPath, runcPath, "%d: %v", i, tc) - } - -}