From 19138a21102ecb3634f332b7343b82a1a12359c4 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 12 May 2022 12:05:55 +0200 Subject: [PATCH 1/5] Skip setting of log file for --version flag Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/logger.go | 71 ++++++++++++++++---------- cmd/nvidia-container-runtime/main.go | 2 + 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/cmd/nvidia-container-runtime/logger.go b/cmd/nvidia-container-runtime/logger.go index f39f8421..56ebad6c 100644 --- a/cmd/nvidia-container-runtime/logger.go +++ b/cmd/nvidia-container-runtime/logger.go @@ -49,18 +49,23 @@ func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, err level, logLevelError := configFromArgs.getLevel(logLevel) var logFiles []*os.File + var argLogFileError error - configLogFile, err := createLogFile(filename) - if err != nil { - return logger, fmt.Errorf("error opening debug log file: %v", err) - } - if configLogFile != nil { - logFiles = append(logFiles, configLogFile) - } + // We don't create log files if the version argument is supplied + if !configFromArgs.version { + configLogFile, err := createLogFile(filename) + if err != nil { + return logger, fmt.Errorf("error opening debug log file: %v", err) + } + if configLogFile != nil { + logFiles = append(logFiles, configLogFile) + } - argLogFile, argLogFileError := createLogFile(configFromArgs.file) - if argLogFile != nil { - logFiles = append(logFiles, argLogFile) + argLogFile, err := createLogFile(configFromArgs.file) + if argLogFile != nil { + logFiles = append(logFiles, argLogFile) + } + argLogFileError = err } l := &Logger{ @@ -69,18 +74,6 @@ func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, err logFiles: logFiles, } - if len(logFiles) == 0 { - l.SetOutput(io.Discard) - } else if len(logFiles) == 1 { - l.SetOutput(logFiles[0]) - } else { - var writers []io.Writer - for _, f := range logFiles { - writers = append(writers, f) - } - l.SetOutput(io.MultiWriter(writers...)) - } - l.SetLevel(level) if level == logrus.DebugLevel { logrus.SetReportCaller(true) @@ -102,6 +95,18 @@ func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, err l.SetFormatter(new(logrus.JSONFormatter)) } + if len(logFiles) == 0 { + l.SetOutput(io.Discard) + } else if len(logFiles) == 1 { + l.SetOutput(logFiles[0]) + } else if len(logFiles) > 1 { + var writers []io.Writer + for _, f := range logFiles { + writers = append(writers, f) + } + l.SetOutput(io.MultiWriter(writers...)) + } + if logLevelError != nil { l.Warn(logLevelError) } @@ -153,9 +158,10 @@ func createLogFile(filename string) (*os.File, error) { } type loggerConfig struct { - file string - format string - debug bool + file string + format string + debug bool + version bool } func (c loggerConfig) getLevel(logLevel string) (logrus.Level, error) { @@ -182,15 +188,24 @@ func parseArgs(args []string) loggerConfig { found := make(map[string]bool) for i := 0; i < len(args); i++ { - if len(found) == 3 { + if len(found) == 4 { break } param := args[i] parts := strings.SplitN(param, "=", 2) - trimmed := strings.TrimPrefix(parts[0], "--") - if !strings.HasPrefix(parts[0], "--") { + 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" { + c.version = true + found["version"] = true + // For the version flag we don't process any other flags continue } diff --git a/cmd/nvidia-container-runtime/main.go b/cmd/nvidia-container-runtime/main.go index 0f3f59cb..ba5b3514 100644 --- a/cmd/nvidia-container-runtime/main.go +++ b/cmd/nvidia-container-runtime/main.go @@ -7,6 +7,8 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/config" ) +var Version string + var logger = NewLogger() func main() { From 60dacb76b6b49a51876be0ab6fbea22a40d49492 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 12 May 2022 15:42:42 +0200 Subject: [PATCH 2/5] Call logger.Reset() to ensure errors are captured Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/logger.go | 4 ++-- cmd/nvidia-container-runtime/main.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/nvidia-container-runtime/logger.go b/cmd/nvidia-container-runtime/logger.go index 56ebad6c..e886a020 100644 --- a/cmd/nvidia-container-runtime/logger.go +++ b/cmd/nvidia-container-runtime/logger.go @@ -118,9 +118,9 @@ func UpdateLogger(filename string, logLevel string, argv []string) (*Logger, err return l, nil } -// CloseFile closes the log file (if any) and resets the logger output to what it +// Reset closes the log file (if any) and resets the logger output to what it // was before UpdateLogger was called. -func (l *Logger) CloseFile() error { +func (l *Logger) Reset() error { defer func() { previous := l.previousLogger if previous == nil { diff --git a/cmd/nvidia-container-runtime/main.go b/cmd/nvidia-container-runtime/main.go index ba5b3514..4b8651fb 100644 --- a/cmd/nvidia-container-runtime/main.go +++ b/cmd/nvidia-container-runtime/main.go @@ -35,6 +35,7 @@ func run(argv []string) (rerr error) { if err != nil { return fmt.Errorf("failed to set up logger: %v", err) } + defer logger.Reset() logger.Debugf("Command line arguments: %v", argv) runtime, err := newNVIDIAContainerRuntime(logger.Logger, cfg, argv) From c77e86137ed148af94a9d41782c75856420af2b1 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 12 May 2022 14:02:06 +0200 Subject: [PATCH 3/5] Add version output to CLIs This change adds version output to the nvidia-continer-runtime, nvidia-container-toolkit, and nvidia-ctk CLIs. The same version is used in all cases and includes a version string and a git revision if set. The construction of the version string mirrors what is done in runc. Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/main.go | 39 ++++++++++++++++++++++++- cmd/nvidia-container-toolkit/main.go | 10 +++++-- cmd/nvidia-ctk/main.go | 6 ++-- internal/info/version.go | 43 ++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 internal/info/version.go diff --git a/cmd/nvidia-container-runtime/main.go b/cmd/nvidia-container-runtime/main.go index 4b8651fb..ea935aca 100644 --- a/cmd/nvidia-container-runtime/main.go +++ b/cmd/nvidia-container-runtime/main.go @@ -3,11 +3,19 @@ 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" ) -var Version string +// 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() @@ -22,6 +30,11 @@ func main() { // 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) @@ -43,5 +56,29 @@ func run(argv []string) (rerr error) { 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-toolkit/main.go b/cmd/nvidia-container-toolkit/main.go index 3ecf1ba0..6b29da8b 100644 --- a/cmd/nvidia-container-toolkit/main.go +++ b/cmd/nvidia-container-toolkit/main.go @@ -18,8 +18,9 @@ import ( ) var ( - debugflag = flag.Bool("debug", false, "enable debug output") - configflag = flag.String("config", "", "configuration file") + debugflag = flag.Bool("debug", false, "enable debug output") + versionflag = flag.Bool("version", false, "enable version output") + configflag = flag.String("config", "", "configuration file") ) func exit() { @@ -159,6 +160,11 @@ func main() { flag.Usage = usage flag.Parse() + if *versionflag { + fmt.Printf("%v version %v\n", "NVIDIA Container Runtime Hook", info.GetVersionString()) + return + } + args := flag.Args() if len(args) == 0 { flag.Usage() diff --git a/cmd/nvidia-ctk/main.go b/cmd/nvidia-ctk/main.go index ca9de11b..c635e3cc 100644 --- a/cmd/nvidia-ctk/main.go +++ b/cmd/nvidia-ctk/main.go @@ -20,12 +20,11 @@ import ( "os" "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-ctk/hook" + "github.com/NVIDIA/nvidia-container-toolkit/internal/info" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" ) -var version string - var logger = log.New() // config defines the options that can be set for the CLI through config files, @@ -41,10 +40,11 @@ func main() { // Create the top-level CLI c := cli.NewApp() + c.Name = "NVIDIA Container Toolkit CLI" c.UseShortOptionHandling = true c.EnableBashCompletion = true c.Usage = "Tools to configure the NVIDIA Container Toolkit" - c.Version = version + c.Version = info.GetVersionString() // Setup the flags for this command c.Flags = []cli.Flag{ diff --git a/internal/info/version.go b/internal/info/version.go new file mode 100644 index 00000000..86d8cf08 --- /dev/null +++ b/internal/info/version.go @@ -0,0 +1,43 @@ +/** +# Copyright (c) 2022, 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 info + +import "strings" + +// 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 = "" + +// GetVersionParts returns the different version components +func GetVersionParts() []string { + v := []string{version} + + if gitCommit != "" { + v = append(v, "commit: "+gitCommit) + } + + return v +} + +// GetVersionString returns the string representation of the version +func GetVersionString(more ...string) string { + v := append(GetVersionParts(), more...) + return strings.Join(v, "\n") +} From 4a19bf16a880d669823167bac04bbae24487daef Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 12 May 2022 14:02:26 +0200 Subject: [PATCH 4/5] Set the version and gitCommit in the Makefile This change ensures that the variables used to construct the version strings for CMDs are set in the makefile. Signed-off-by: Evan Lezar --- Makefile | 10 +++++++--- versions.mk | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index bc1a4e5f..2b995457 100644 --- a/Makefile +++ b/Makefile @@ -38,8 +38,6 @@ EXAMPLE_TARGETS := $(patsubst %,example-%, $(EXAMPLES)) CMDS := $(patsubst ./cmd/%/,%,$(sort $(dir $(wildcard ./cmd/*/)))) CMD_TARGETS := $(patsubst %,cmd-%, $(CMDS)) -$(info CMD_TARGETS=$(CMD_TARGETS)) - CHECK_TARGETS := assert-fmt vet lint ineffassign misspell MAKE_TARGETS := binaries build check fmt lint-internal test examples cmds coverage generate $(CHECK_TARGETS) @@ -48,6 +46,12 @@ TARGETS := $(MAKE_TARGETS) $(EXAMPLE_TARGETS) $(CMD_TARGETS) DOCKER_TARGETS := $(patsubst %,docker-%, $(TARGETS)) .PHONY: $(TARGETS) $(DOCKER_TARGETS) +ifeq ($(VERSION),) +CLI_VERSION = $(LIB_VERSION)$(if $(LIB_TAG),-$(LIB_TAG)) +else +CLI_VERSION = $(VERSION) +endif + GOOS ?= linux binaries: cmds @@ -56,7 +60,7 @@ cmd-%: COMMAND_BUILD_OPTIONS = -o $(PREFIX)/$(*) endif cmds: $(CMD_TARGETS) $(CMD_TARGETS): cmd-%: - GOOS=$(GOOS) go build -ldflags "-s -w" $(COMMAND_BUILD_OPTIONS) $(MODULE)/cmd/$(*) + GOOS=$(GOOS) go build -ldflags "-s -w -X github.com/NVIDIA/nvidia-container-toolkit/internal/info.gitCommit=$(GIT_COMMIT) -X github.com/NVIDIA/nvidia-container-toolkit/internal/info.version=$(CLI_VERSION)" $(COMMAND_BUILD_OPTIONS) $(MODULE)/cmd/$(*) build: GOOS=$(GOOS) go build ./... diff --git a/versions.mk b/versions.mk index e7e3311a..e1300e38 100644 --- a/versions.mk +++ b/versions.mk @@ -26,3 +26,5 @@ LIBNVIDIA_CONTAINER0_VERSION := 0.10.0+jetpack CUDA_VERSION := 11.6.0 GOLANG_VERSION := 1.17.8 + +GIT_COMMIT ?= $(shell git describe --dirty --long --always 2> /dev/null || echo "") From 25710468dcb926a2c0b526154fd0ad5a9b41a7b3 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 12 May 2022 14:19:29 +0200 Subject: [PATCH 5/5] Ensure that git commit is set in docker build Signed-off-by: Evan Lezar --- docker/Dockerfile.amazonlinux | 2 ++ docker/Dockerfile.centos | 2 ++ docker/Dockerfile.debian | 2 ++ docker/Dockerfile.opensuse-leap | 2 ++ docker/Dockerfile.ubuntu | 2 ++ docker/docker.mk | 1 + 6 files changed, 11 insertions(+) diff --git a/docker/Dockerfile.amazonlinux b/docker/Dockerfile.amazonlinux index e7a49fcc..77296029 100644 --- a/docker/Dockerfile.amazonlinux +++ b/docker/Dockerfile.amazonlinux @@ -41,6 +41,8 @@ RUN mkdir -p $DIST_DIR /dist WORKDIR $GOPATH/src/nvidia-container-toolkit COPY . . +ARG GIT_COMMIT +ENV GIT_COMMIT ${GIT_COMMIT} RUN make PREFIX=${DIST_DIR} cmds ARG CONFIG_TOML_SUFFIX diff --git a/docker/Dockerfile.centos b/docker/Dockerfile.centos index 38edfeef..abbec683 100644 --- a/docker/Dockerfile.centos +++ b/docker/Dockerfile.centos @@ -41,6 +41,8 @@ RUN mkdir -p $DIST_DIR /dist WORKDIR $GOPATH/src/nvidia-container-toolkit COPY . . +ARG GIT_COMMIT +ENV GIT_COMMIT ${GIT_COMMIT} RUN make PREFIX=${DIST_DIR} cmds ARG CONFIG_TOML_SUFFIX diff --git a/docker/Dockerfile.debian b/docker/Dockerfile.debian index 169984fe..3dcd9627 100644 --- a/docker/Dockerfile.debian +++ b/docker/Dockerfile.debian @@ -48,6 +48,8 @@ RUN mkdir -p $DIST_DIR /dist WORKDIR $GOPATH/src/nvidia-container-toolkit COPY . . +ARG GIT_COMMIT +ENV GIT_COMMIT ${GIT_COMMIT} RUN make PREFIX=${DIST_DIR} cmds ARG CONFIG_TOML_SUFFIX diff --git a/docker/Dockerfile.opensuse-leap b/docker/Dockerfile.opensuse-leap index f57b6e76..fd02f4d5 100644 --- a/docker/Dockerfile.opensuse-leap +++ b/docker/Dockerfile.opensuse-leap @@ -39,6 +39,8 @@ RUN mkdir -p $DIST_DIR /dist WORKDIR $GOPATH/src/nvidia-container-toolkit COPY . . +ARG GIT_COMMIT +ENV GIT_COMMIT ${GIT_COMMIT} RUN make PREFIX=${DIST_DIR} cmds # Hook for Project Atomic's fork of Docker: https://github.com/projectatomic/docker/tree/docker-1.13.1-rhel#add-dockerhooks-exec-custom-hooks-for-prestartpoststop-containerspatch diff --git a/docker/Dockerfile.ubuntu b/docker/Dockerfile.ubuntu index b24835e0..7a5e39b2 100644 --- a/docker/Dockerfile.ubuntu +++ b/docker/Dockerfile.ubuntu @@ -46,6 +46,8 @@ RUN mkdir -p $DIST_DIR /dist WORKDIR $GOPATH/src/nvidia-container-toolkit COPY . . +ARG GIT_COMMIT +ENV GIT_COMMIT ${GIT_COMMIT} RUN make PREFIX=${DIST_DIR} cmds ARG CONFIG_TOML_SUFFIX diff --git a/docker/docker.mk b/docker/docker.mk index f97d3d8a..614b97bc 100644 --- a/docker/docker.mk +++ b/docker/docker.mk @@ -131,6 +131,7 @@ docker-build-%: --build-arg PKG_VERS="$(LIB_VERSION)" \ --build-arg PKG_REV="$(PKG_REV)" \ --build-arg CONFIG_TOML_SUFFIX="$(CONFIG_TOML_SUFFIX)" \ + --build-arg GIT_COMMIT="$(GIT_COMMIT)" \ --tag $(BUILDIMAGE) \ --file $(DOCKERFILE) . $(DOCKER) run \