Skip to content

Commit

Permalink
Ignore ldconfig option unless explicitly requested
Browse files Browse the repository at this point in the history
Signed-off-by: Evan Lezar <[email protected]>
  • Loading branch information
elezar committed Sep 11, 2024
1 parent 61cbdaf commit 24ede29
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 22 deletions.
2 changes: 1 addition & 1 deletion internal/config/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (c *ContainerCLIConfig) NormalizeLDConfigPath() string {
// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
// This is only done for host LDConfigs and is required to handle systems where
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
func NormalizeLDConfigPath(path string) string {
func NormalizeLDConfigPath(path string) (rpath string) {
if !strings.HasPrefix(path, "@") {
return path
}
Expand Down
12 changes: 11 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,17 @@ func GetDefault() (*Config, error) {
return &d, nil
}

func getLdConfigPath() string {
func (c *Config) checkOverrides() *Config {
if !c.Features.AllowLDConfigOverride.isEnabled("") {
c.NVIDIAContainerCLIConfig.Ldconfig = getLdConfigPath()
}
return c
}

// getLdConfigPath allows us to override this function for testing.
var getLdConfigPath = getLdConfigPathStub

func getLdConfigPathStub() string {
return NormalizeLDConfigPath("@/sbin/ldconfig")
}

Expand Down
83 changes: 68 additions & 15 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "",
LoadKmods: true,
Ldconfig: "WAS_CHECKED",
Ldconfig: "/test/ld/config/path",
},
NVIDIAContainerRuntimeConfig: RuntimeConfig{
DebugFilePath: "/dev/null",
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "/bar/baz",
LoadKmods: false,
Ldconfig: "/foo/bar/ldconfig",
Ldconfig: "/test/ld/config/path",
User: "foo:bar",
},
NVIDIAContainerRuntimeConfig: RuntimeConfig{
Expand Down Expand Up @@ -146,6 +146,53 @@ func TestGetConfig(t *testing.T) {
},
},
},
{
description: "feature allows ldconfig to be overridden",
contents: []string{
"[nvidia-container-cli]",
"ldconfig = \"/foo/bar/ldconfig\"",
"[features]",
"allow-ldconfig-override = true",
},
expectedConfig: &Config{
AcceptEnvvarUnprivileged: true,
SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video",
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Ldconfig: "/foo/bar/ldconfig",
LoadKmods: true,
},
NVIDIAContainerRuntimeConfig: RuntimeConfig{
DebugFilePath: "/dev/null",
LogLevel: "info",
Runtimes: []string{"docker-runc", "runc", "crun"},
Mode: "auto",
Modes: modesConfig{
CSV: csvModeConfig{
MountSpecPath: "/etc/nvidia-container-runtime/host-files-for-container.d",
},
CDI: cdiModeConfig{
DefaultKind: "nvidia.com/gpu",
AnnotationPrefixes: []string{
"cdi.k8s.io/",
},
SpecDirs: []string{
"/etc/cdi",
"/var/run/cdi",
},
},
},
},
NVIDIAContainerRuntimeHookConfig: RuntimeHookConfig{
Path: "nvidia-container-runtime-hook",
},
NVIDIACTKConfig: CTKConfig{
Path: "nvidia-ctk",
},
Features: features{
AllowLDConfigOverride: ptr(feature(true)),
},
},
},
{
description: "config options set in section",
contents: []string{
Expand Down Expand Up @@ -179,7 +226,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "/bar/baz",
LoadKmods: false,
Ldconfig: "/foo/bar/ldconfig",
Ldconfig: "/test/ld/config/path",
User: "foo:bar",
},
NVIDIAContainerRuntimeConfig: RuntimeConfig{
Expand Down Expand Up @@ -222,7 +269,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "",
LoadKmods: true,
Ldconfig: "WAS_CHECKED",
Ldconfig: "/test/ld/config/path",
User: "root:video",
},
NVIDIAContainerRuntimeConfig: RuntimeConfig{
Expand Down Expand Up @@ -262,7 +309,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "",
LoadKmods: true,
Ldconfig: "WAS_CHECKED",
Ldconfig: "/test/ld/config/path",
User: "foo:bar",
},
NVIDIAContainerRuntimeConfig: RuntimeConfig{
Expand Down Expand Up @@ -293,6 +340,7 @@ func TestGetConfig(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
defer setGetLdConfigPathForTest()
defer setGetDistIDLikeForTest(tc.distIdsLike)()
reader := strings.NewReader(strings.Join(tc.contents, "\n"))

Expand All @@ -305,16 +353,6 @@ func TestGetConfig(t *testing.T) {
cfg, err := tomlCfg.Config()
require.NoError(t, err)

// We first handle the ldconfig path since this is currently system-dependent.
if tc.inspectLdconfig {
ldconfig := cfg.NVIDIAContainerCLIConfig.Ldconfig
require.True(t, strings.HasPrefix(ldconfig, "@/sbin/ldconfig"))
remaining := strings.TrimPrefix(ldconfig, "@/sbin/ldconfig")
require.True(t, remaining == ".real" || remaining == "")

cfg.NVIDIAContainerCLIConfig.Ldconfig = "WAS_CHECKED"
}

require.EqualValues(t, tc.expectedConfig, cfg)
})
}
Expand All @@ -335,3 +373,18 @@ func setGetDistIDLikeForTest(ids []string) func() {
getDistIDLike = original
}
}

// prt returns a reference to whatever type is passed into it
func ptr[T any](x T) *T {
return &x
}

func setGetLdConfigPathForTest() func() {
previous := getLdConfigPath
getLdConfigPath = func() string {
return "/test/ld/config/path"
}
return func() {
getLdConfigPath = previous
}
}
14 changes: 10 additions & 4 deletions internal/config/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ package config
type featureName string

const (
FeatureGDS = featureName("gds")
FeatureMOFED = featureName("mofed")
FeatureNVSWITCH = featureName("nvswitch")
FeatureGDRCopy = featureName("gdrcopy")
FeatureGDS = featureName("gds")
FeatureMOFED = featureName("mofed")
FeatureNVSWITCH = featureName("nvswitch")
FeatureGDRCopy = featureName("gdrcopy")
FeatureAllowLdconfigOverride = featureName("allow-ldconfig-override")
)

// features specifies a set of named features.
Expand All @@ -31,6 +32,9 @@ type features struct {
MOFED *feature `toml:"mofed,omitempty"`
NVSWITCH *feature `toml:"nvswitch,omitempty"`
GDRCopy *feature `toml:"gdrcopy,omitempty"`
// AllowLDConfigOverride forces the nvidia-container-cli.ldconfig config
// option to be used instead of the default of @/sbin/ldconfig.
AllowLDConfigOverride *feature `toml:"allow-ldconfig-override,omitempty"`
}

type feature bool
Expand All @@ -56,6 +60,8 @@ func (fs features) IsEnabled(n featureName, in ...getenver) bool {
return fs.NVSWITCH.isEnabled(envvar, in...)
case FeatureGDRCopy:
return fs.GDRCopy.isEnabled(envvar, in...)
case FeatureAllowLdconfigOverride:
return fs.AllowLDConfigOverride.isEnabled("", nil)
default:
return false
}
Expand Down
2 changes: 1 addition & 1 deletion internal/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (t *Toml) Config() (*Config, error) {
if err := t.Unmarshal(cfg); err != nil {
return nil, fmt.Errorf("failed to unmarshal config: %v", err)
}
return cfg, nil
return cfg.checkOverrides(), nil
}

// Unmarshal wraps the toml.Tree Unmarshal function.
Expand Down
31 changes: 31 additions & 0 deletions internal/config/toml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ func TestTomlContents(t *testing.T) {
}

func TestConfigFromToml(t *testing.T) {
defer setGetLdConfigPathForTest()

testCases := []struct {
description string
contents map[string]interface{}
Expand Down Expand Up @@ -226,6 +228,35 @@ func TestConfigFromToml(t *testing.T) {
return c
}(),
},
{
description: "ldconfig value is ignored",
contents: map[string]interface{}{
"nvidia-container-cli": map[string]interface{}{
"ldconfig": "/some/ldconfig/path",
},
},
expectedConfig: func() *Config {
c, _ := GetDefault()
return c
}(),
},
{
description: "feature allows ldconfig override",
contents: map[string]interface{}{
"nvidia-container-cli": map[string]interface{}{
"ldconfig": "/some/ldconfig/path",
},
"features": map[string]interface{}{
"allow-ldconfig-override": true,
},
},
expectedConfig: func() *Config {
c, _ := GetDefault()
c.NVIDIAContainerCLIConfig.Ldconfig = "/some/ldconfig/path"
c.Features.AllowLDConfigOverride = ptr(feature(true))
return c
}(),
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 24ede29

Please sign in to comment.