Skip to content

Commit

Permalink
Reduce logging for the NVIDIA Container runtime
Browse files Browse the repository at this point in the history
This change moves most of the logging for the NVIDIA Container runtime
from Info to Debug or Trace -- especially in the case where no
modifications are requried.

This removes execessive logging.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
  • Loading branch information
elezar committed Jul 1, 2024
1 parent b4e6c5d commit be516e9
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Ensure that `nvidia-ctk cdi transform` creates specs with world-readable permissions.
* Remove provenance information from published images.
* Fix bug in processing of `--log=` argument of `nvidia-container-runtime`.
* Reduce verbosity of logging in the NVIDIA Container Runtime especially for non-`create` commands.

## v1.15.0

Expand Down
1 change: 1 addition & 0 deletions internal/logger/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ type Interface interface {
Infof(string, ...interface{})
Warning(...interface{})
Warningf(string, ...interface{})
Tracef(string, ...interface{})
}
3 changes: 3 additions & 0 deletions internal/logger/lib.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ func (l *NullLogger) Warning(...interface{}) {}

// Warningf is a no-op for the null logger
func (l *NullLogger) Warningf(string, ...interface{}) {}

// Tracef is a no-op for the null logger
func (l *NullLogger) Tracef(string, ...interface{}) {}
5 changes: 3 additions & 2 deletions internal/oci/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@

package oci

//go:generate moq -stub -out runtime_mock.go . Runtime

// Runtime is an interface for a runtime shim. The Exec method accepts a list
// of command line arguments, and returns an error / nil.
//
//go:generate moq -rm -stub -out runtime_mock.go . Runtime
type Runtime interface {
Exec([]string) error
String() string
}
7 changes: 2 additions & 5 deletions internal/oci/runtime_low_level.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ func NewLowLevelRuntime(logger logger.Interface, candidates []string) (Runtime,
if err != nil {
return nil, fmt.Errorf("error locating runtime: %v", err)
}

logger.Infof("Using low-level runtime %v", runtimePath)
return NewRuntimeForPath(logger, runtimePath)
}

Expand All @@ -45,13 +43,12 @@ func findRuntime(logger logger.Interface, candidates []string) (string, error) {

locator := lookup.NewExecutableLocator(logger, "/")
for _, candidate := range candidates {
logger.Debugf("Looking for runtime binary '%v'", candidate)
logger.Tracef("Looking for runtime binary '%v'", candidate)
targets, err := locator.Locate(candidate)
if err == nil && len(targets) > 0 {
logger.Debugf("Found runtime binary '%v'", targets)
logger.Tracef("Found runtime binary '%v'", targets)
return targets[0], nil
}
logger.Debugf("Runtime binary '%v' not found: %v (targets=%v)", candidate, err, targets)
}

return "", fmt.Errorf("no runtime binary found from candidate list: %v", candidates)
Expand Down
42 changes: 41 additions & 1 deletion internal/oci/runtime_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 10 additions & 6 deletions internal/oci/runtime_modifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ Runtime = (*modifyingRuntimeWrapper)(nil)
// before invoking the wrapped runtime. If the modifier is nil, the input runtime is returned.
func NewModifyingRuntimeWrapper(logger logger.Interface, runtime Runtime, spec Spec, modifier SpecModifier) Runtime {
if modifier == nil {
logger.Infof("Using low-level runtime with no modification")
logger.Tracef("Using low-level runtime with no modification")
return runtime
}

Expand All @@ -52,16 +52,15 @@ func NewModifyingRuntimeWrapper(logger logger.Interface, runtime Runtime, spec S
// into the wrapped runtime.
func (r *modifyingRuntimeWrapper) Exec(args []string) error {
if HasCreateSubcommand(args) {
r.logger.Debugf("Create command detected; applying OCI specification modifications")
err := r.modify()
if err != nil {
return fmt.Errorf("could not apply required modification to OCI specification: %v", err)
return fmt.Errorf("could not apply required modification to OCI specification: %w", err)
}
r.logger.Infof("Applied required modification to OCI specification")
} else {
r.logger.Infof("No modification of OCI specification required")
r.logger.Debugf("Applied required modification to OCI specification")
}

r.logger.Infof("Forwarding command to runtime")
r.logger.Debugf("Forwarding command to runtime %v", r.runtime.String())
return r.runtime.Exec(args)
}

Expand All @@ -83,3 +82,8 @@ func (r *modifyingRuntimeWrapper) modify() error {
}
return nil
}

// String returns a string representation of the runtime.
func (r *modifyingRuntimeWrapper) String() string {
return fmt.Sprintf("modify on-create and forward to %s", r.runtime.String())
}
5 changes: 5 additions & 0 deletions internal/oci/runtime_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,8 @@ func (s pathRuntime) Exec(args []string) error {

return s.execRuntime.Exec(runtimeArgs)
}

// String returns the path to the specified runtime as the string representation.
func (s pathRuntime) String() string {
return s.path
}
4 changes: 4 additions & 0 deletions internal/oci/runtime_syscall_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,7 @@ func (r syscallExec) Exec(args []string) error {
// err is nil or not.
return fmt.Errorf("unexpected return from exec '%v'", args[0])
}

func (r syscallExec) String() string {
return "exec"
}
19 changes: 7 additions & 12 deletions internal/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package runtime

import (
"encoding/json"
"errors"
"fmt"
"strings"
Expand All @@ -26,6 +25,7 @@ import (

"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
"github.com/NVIDIA/nvidia-container-toolkit/internal/info"
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup/root"
)

Expand Down Expand Up @@ -66,23 +66,18 @@ func (r rt) Run(argv []string) (rerr error) {
if r.modeOverride != "" {
cfg.NVIDIAContainerRuntimeConfig.Mode = r.modeOverride
}
cfg.NVIDIACTKConfig.Path = config.ResolveNVIDIACTKPath(r.logger, cfg.NVIDIACTKConfig.Path)
cfg.NVIDIAContainerRuntimeHookConfig.Path = config.ResolveNVIDIAContainerRuntimeHookPath(r.logger, cfg.NVIDIAContainerRuntimeHookConfig.Path)

// Print the config to the output.
configJSON, err := json.MarshalIndent(cfg, "", " ")
if err == nil {
r.logger.Infof("Running with config:\n%v", string(configJSON))
} else {
r.logger.Infof("Running with config:\n%+v", cfg)
}
cfg.NVIDIACTKConfig.Path = config.ResolveNVIDIACTKPath(&logger.NullLogger{}, cfg.NVIDIACTKConfig.Path)
cfg.NVIDIAContainerRuntimeHookConfig.Path = config.ResolveNVIDIAContainerRuntimeHookPath(&logger.NullLogger{}, cfg.NVIDIAContainerRuntimeHookConfig.Path)

// Log the config at Trace to allow for debugging if required.
r.logger.Tracef("Running with config: %+v", cfg)

driver := root.New(
root.WithLogger(r.logger),
root.WithDriverRoot(cfg.NVIDIAContainerCLIConfig.Root),
)

r.logger.Debugf("Command line arguments: %v", argv)
r.logger.Tracef("Command line arguments: %v", argv)
runtime, err := newNVIDIAContainerRuntime(r.logger, cfg, argv, driver)
if err != nil {
return fmt.Errorf("failed to create NVIDIA Container Runtime: %v", err)
Expand Down
5 changes: 3 additions & 2 deletions internal/runtime/runtime_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ func newNVIDIAContainerRuntime(logger logger.Interface, cfg *config.Config, argv
return nil, fmt.Errorf("error constructing low-level runtime: %v", err)
}

logger.Tracef("Using low-level runtime %v", lowLevelRuntime.String())
if !oci.HasCreateSubcommand(argv) {
logger.Debugf("Skipping modifier for non-create subcommand")
logger.Tracef("Skipping modifier for non-create subcommand")
return lowLevelRuntime, nil
}

Expand All @@ -50,7 +51,7 @@ func newNVIDIAContainerRuntime(logger logger.Interface, cfg *config.Config, argv
return nil, fmt.Errorf("failed to construct OCI spec modifier: %v", err)
}

// Create the wrapping runtime with the specified modifier
// Create the wrapping runtime with the specified modifier.
r := oci.NewModifyingRuntimeWrapper(
logger,
lowLevelRuntime,
Expand Down

0 comments on commit be516e9

Please sign in to comment.