From 406a5ec76f132b59f1b6f982ecbc597f159b69f5 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 7 Feb 2023 21:09:53 +0100 Subject: [PATCH] Implement runtime package for creating runtime CLI Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/main.go | 80 +------------- cmd/nvidia-container-runtime/main_test.go | 16 +-- internal/runtime/api.go | 33 ++++++ .../runtime}/logger.go | 17 +-- internal/runtime/runtime.go | 101 ++++++++++++++++++ .../runtime}/runtime_factory.go | 2 +- .../runtime}/runtime_factory_test.go | 34 +++++- 7 files changed, 189 insertions(+), 94 deletions(-) create mode 100644 internal/runtime/api.go rename {cmd/nvidia-container-runtime => internal/runtime}/logger.go (93%) create mode 100644 internal/runtime/runtime.go rename {cmd/nvidia-container-runtime => internal/runtime}/runtime_factory.go (99%) rename {cmd/nvidia-container-runtime => internal/runtime}/runtime_factory_test.go (77%) diff --git a/cmd/nvidia-container-runtime/main.go b/cmd/nvidia-container-runtime/main.go index 9859d910..c11c0a6a 100644 --- a/cmd/nvidia-container-runtime/main.go +++ b/cmd/nvidia-container-runtime/main.go @@ -1,89 +1,15 @@ package main import ( - "fmt" "os" - "strings" - "github.com/NVIDIA/nvidia-container-toolkit/internal/config" - "github.com/NVIDIA/nvidia-container-toolkit/internal/info" - "github.com/opencontainers/runtime-spec/specs-go" + "github.com/NVIDIA/nvidia-container-toolkit/internal/runtime" ) -// version must be set by go build's -X main.version= option in the Makefile. -var version = "unknown" - -// gitCommit will be the hash that the binary was built from -// and will be populated by the Makefile -var gitCommit = "" - -var logger = NewLogger() - func main() { - err := run(os.Args) + r := runtime.New() + err := r.Run(os.Args) if err != nil { - logger.Errorf("%v", err) os.Exit(1) } } - -// run is an entry point that allows for idiomatic handling of errors -// when calling from the main function. -func run(argv []string) (rerr error) { - printVersion := hasVersionFlag(argv) - if printVersion { - fmt.Printf("%v version %v\n", "NVIDIA Container Runtime", info.GetVersionString(fmt.Sprintf("spec: %v", specs.Version))) - } - - cfg, err := config.GetConfig() - if err != nil { - return fmt.Errorf("error loading config: %v", err) - } - - logger, err = UpdateLogger( - cfg.NVIDIAContainerRuntimeConfig.DebugFilePath, - cfg.NVIDIAContainerRuntimeConfig.LogLevel, - argv, - ) - if err != nil { - return fmt.Errorf("failed to set up logger: %v", err) - } - defer func() { - if rerr != nil { - logger.Errorf("%v", rerr) - } - logger.Reset() - }() - - logger.Debugf("Command line arguments: %v", argv) - runtime, err := newNVIDIAContainerRuntime(logger.Logger, cfg, argv) - if err != nil { - return fmt.Errorf("failed to create NVIDIA Container Runtime: %v", err) - } - - if printVersion { - fmt.Print("\n") - } - return runtime.Exec(argv) -} - -// TODO: This should be refactored / combined with parseArgs in logger. -func hasVersionFlag(args []string) bool { - for i := 0; i < len(args); i++ { - param := args[i] - - parts := strings.SplitN(param, "=", 2) - trimmed := strings.TrimLeft(parts[0], "-") - // If this is not a flag we continue - if parts[0] == trimmed { - continue - } - - // Check the version flag - if trimmed == "version" { - return true - } - } - - return false -} diff --git a/cmd/nvidia-container-runtime/main_test.go b/cmd/nvidia-container-runtime/main_test.go index 218c6d40..05809aee 100644 --- a/cmd/nvidia-container-runtime/main_test.go +++ b/cmd/nvidia-container-runtime/main_test.go @@ -13,6 +13,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/modifier" "github.com/NVIDIA/nvidia-container-toolkit/internal/test" "github.com/opencontainers/runtime-spec/specs-go" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) @@ -41,7 +42,7 @@ func TestMain(m *testing.M) { var err error moduleRoot, err := test.GetModuleRoot() if err != nil { - logger.Fatalf("error in test setup: could not get module root: %v", err) + logrus.Fatalf("error in test setup: could not get module root: %v", err) } testBinPath := filepath.Join(moduleRoot, "test", "bin") testInputPath := filepath.Join(moduleRoot, "test", "input") @@ -53,11 +54,11 @@ func TestMain(m *testing.M) { // Confirm that the environment is configured correctly runcPath, err := exec.LookPath(runcExecutableName) if err != nil || filepath.Join(testBinPath, runcExecutableName) != runcPath { - logger.Fatalf("error in test setup: mock runc path set incorrectly in TestMain(): %v", err) + logrus.Fatalf("error in test setup: mock runc path set incorrectly in TestMain(): %v", err) } hookPath, err := exec.LookPath(nvidiaHook) if err != nil || filepath.Join(testBinPath, nvidiaHook) != hookPath { - logger.Fatalf("error in test setup: mock hook path set incorrectly in TestMain(): %v", err) + logrus.Fatalf("error in test setup: mock hook path set incorrectly in TestMain(): %v", err) } // Store the root and binary paths in the test Config @@ -77,7 +78,7 @@ func TestMain(m *testing.M) { // case 1) nvidia-container-runtime run --bundle // case 2) nvidia-container-runtime create --bundle -// - Confirm the runtime handles bad input correctly +// - Confirm the runtime handles bad input correctly func TestBadInput(t *testing.T) { err := cfg.generateNewRuntimeSpec() if err != nil { @@ -91,9 +92,10 @@ func TestBadInput(t *testing.T) { } // case 1) nvidia-container-runtime run --bundle -// - Confirm the runtime runs with no errors +// - Confirm the runtime runs with no errors +// // case 2) nvidia-container-runtime create --bundle -// - Confirm the runtime inserts the NVIDIA prestart hook correctly +// - Confirm the runtime inserts the NVIDIA prestart hook correctly func TestGoodInput(t *testing.T) { err := cfg.generateNewRuntimeSpec() if err != nil { @@ -170,7 +172,7 @@ func TestDuplicateHook(t *testing.T) { // addNVIDIAHook is a basic wrapper for an addHookModifier that is used for // testing. func addNVIDIAHook(spec *specs.Spec) error { - m := modifier.NewStableRuntimeModifier(logger.Logger) + m := modifier.NewStableRuntimeModifier(logrus.StandardLogger()) return m.Modify(spec) } diff --git a/internal/runtime/api.go b/internal/runtime/api.go new file mode 100644 index 00000000..233583b8 --- /dev/null +++ b/internal/runtime/api.go @@ -0,0 +1,33 @@ +package runtime + +type rt struct { + logger *Logger + modeOverride string +} + +// Interface is the interface for the runtime library. +type Interface interface { + Run([]string) error +} + +// Option is a function that configures the runtime. +type Option func(*rt) + +// New creates a runtime with the specified options. +func New(opts ...Option) Interface { + r := rt{} + for _, opt := range opts { + opt(&r) + } + if r.logger == nil { + r.logger = NewLogger() + } + return &r +} + +// WithModeOverride allows for overriding the mode specified in the config. +func WithModeOverride(mode string) Option { + return func(r *rt) { + r.modeOverride = mode + } +} diff --git a/cmd/nvidia-container-runtime/logger.go b/internal/runtime/logger.go similarity index 93% rename from cmd/nvidia-container-runtime/logger.go rename to internal/runtime/logger.go index e886a020..2b5c9730 100644 --- a/cmd/nvidia-container-runtime/logger.go +++ b/internal/runtime/logger.go @@ -14,7 +14,7 @@ # limitations under the License. */ -package main +package runtime import ( "fmt" @@ -42,8 +42,8 @@ func NewLogger() *Logger { } } -// UpdateLogger constructs a Logger with a preddefined formatter -func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, error) { +// Update constructs a Logger with a preddefined formatter +func (l *Logger) Update(filename string, logLevel string, argv []string) error { configFromArgs := parseArgs(argv) level, logLevelError := configFromArgs.getLevel(logLevel) @@ -55,7 +55,7 @@ func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, err if !configFromArgs.version { configLogFile, err := createLogFile(filename) if err != nil { - return logger, fmt.Errorf("error opening debug log file: %v", err) + return fmt.Errorf("error opening debug log file: %v", err) } if configLogFile != nil { logFiles = append(logFiles, configLogFile) @@ -68,9 +68,10 @@ func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, err argLogFileError = err } - l := &Logger{ + previous := l.Logger + l = &Logger{ Logger: logrus.New(), - previousLogger: logger.Logger, + previousLogger: previous, logFiles: logFiles, } @@ -115,7 +116,7 @@ func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, err l.Warnf("Failed to open log file: %v", argLogFileError) } - return l, nil + return nil } // Reset closes the log file (if any) and resets the logger output to what it @@ -126,7 +127,7 @@ func (l *Logger) Reset() error { if previous == nil { previous = logrus.New() } - logger = &Logger{Logger: previous} + l = &Logger{Logger: previous} }() var errs []error diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go new file mode 100644 index 00000000..31a1187c --- /dev/null +++ b/internal/runtime/runtime.go @@ -0,0 +1,101 @@ +/** +# Copyright (c) NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package runtime + +import ( + "fmt" + "strings" + + "github.com/NVIDIA/nvidia-container-toolkit/internal/config" + "github.com/NVIDIA/nvidia-container-toolkit/internal/info" + "github.com/opencontainers/runtime-spec/specs-go" +) + +// Run is an entry point that allows for idiomatic handling of errors +// when calling from the main function. +func (r rt) Run(argv []string) (rerr error) { + defer func() { + if rerr != nil { + r.logger.Errorf("%v", rerr) + } + }() + + printVersion := hasVersionFlag(argv) + if printVersion { + fmt.Printf("%v version %v\n", "NVIDIA Container Runtime", info.GetVersionString(fmt.Sprintf("spec: %v", specs.Version))) + } + + cfg, err := config.GetConfig() + if err != nil { + return fmt.Errorf("error loading config: %v", err) + } + if r.modeOverride != "" { + cfg.NVIDIAContainerRuntimeConfig.Mode = r.modeOverride + } + + err = r.logger.Update( + cfg.NVIDIAContainerRuntimeConfig.DebugFilePath, + cfg.NVIDIAContainerRuntimeConfig.LogLevel, + argv, + ) + if err != nil { + return fmt.Errorf("failed to set up logger: %v", err) + } + defer func() { + if rerr != nil { + r.logger.Errorf("%v", rerr) + } + r.logger.Reset() + }() + + r.logger.Infof("Using config %+v", cfg) + r.logger.Debugf("Command line arguments: %v", argv) + runtime, err := newNVIDIAContainerRuntime(r.logger.Logger, cfg, argv) + if err != nil { + return fmt.Errorf("failed to create NVIDIA Container Runtime: %v", err) + } + + if printVersion { + fmt.Print("\n") + } + return runtime.Exec(argv) +} + +func (r rt) Errorf(format string, args ...interface{}) { + r.logger.Errorf(format, args...) +} + +// TODO: This should be refactored / combined with parseArgs in logger. +func hasVersionFlag(args []string) bool { + for i := 0; i < len(args); i++ { + param := args[i] + + parts := strings.SplitN(param, "=", 2) + trimmed := strings.TrimLeft(parts[0], "-") + // If this is not a flag we continue + if parts[0] == trimmed { + continue + } + + // Check the version flag + if trimmed == "version" { + return true + } + } + + return false +} diff --git a/cmd/nvidia-container-runtime/runtime_factory.go b/internal/runtime/runtime_factory.go similarity index 99% rename from cmd/nvidia-container-runtime/runtime_factory.go rename to internal/runtime/runtime_factory.go index 21c96cd8..29219ab8 100644 --- a/cmd/nvidia-container-runtime/runtime_factory.go +++ b/internal/runtime/runtime_factory.go @@ -14,7 +14,7 @@ # limitations under the License. */ -package main +package runtime import ( "fmt" diff --git a/cmd/nvidia-container-runtime/runtime_factory_test.go b/internal/runtime/runtime_factory_test.go similarity index 77% rename from cmd/nvidia-container-runtime/runtime_factory_test.go rename to internal/runtime/runtime_factory_test.go index 88d4ddb5..7f443e82 100644 --- a/cmd/nvidia-container-runtime/runtime_factory_test.go +++ b/internal/runtime/runtime_factory_test.go @@ -14,20 +14,52 @@ # limitations under the License. */ -package main +package runtime import ( "encoding/json" "os" + "os/exec" "path/filepath" "testing" "github.com/NVIDIA/nvidia-container-toolkit/internal/config" + "github.com/NVIDIA/nvidia-container-toolkit/internal/test" "github.com/opencontainers/runtime-spec/specs-go" + "github.com/sirupsen/logrus" testlog "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" ) +const ( + runcExecutableName = "runc" +) + +func TestMain(m *testing.M) { + // TEST SETUP + // Determine the module root and the test binary path + var err error + moduleRoot, err := test.GetModuleRoot() + if err != nil { + logrus.Fatalf("error in test setup: could not get module root: %v", err) + } + testBinPath := filepath.Join(moduleRoot, "test", "bin") + + // Set the environment variables for the test + os.Setenv("PATH", test.PrependToPath(testBinPath, moduleRoot)) + + // Confirm that the environment is configured correctly + runcPath, err := exec.LookPath(runcExecutableName) + if err != nil || filepath.Join(testBinPath, runcExecutableName) != runcPath { + logrus.Fatalf("error in test setup: mock runc path set incorrectly in TestMain(): %v", err) + } + + // RUN TESTS + exitCode := m.Run() + + os.Exit(exitCode) +} + func TestFactoryMethod(t *testing.T) { logger, _ := testlog.NewNullLogger()