From f6a4986c15eedf60b985956dbda8218c84ecfa54 Mon Sep 17 00:00:00 2001
From: Evan Lezar <elezar@nvidia.com>
Date: Fri, 11 Aug 2023 15:23:13 +0200
Subject: [PATCH 1/2] Add support for creating oci hook to nvidia-ctk

This change extends the nvidia-ctk runtime configure command
with a --config-mode=oci-hook that creates an OCI hook json file.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
---
 CHANGELOG.md                                  |  1 +
 cmd/nvidia-ctk/runtime/configure/configure.go | 62 +++++++++++--
 .../crio => pkg/config/ocihook}/hooks.go      |  2 +-
 pkg/config/ocihook/oci-hook.go                | 89 +++++++++++++++++++
 tools/container/crio/crio.go                  | 53 +----------
 5 files changed, 152 insertions(+), 55 deletions(-)
 rename {tools/container/crio => pkg/config/ocihook}/hooks.go (99%)
 create mode 100644 pkg/config/ocihook/oci-hook.go

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 93996ced..a0024a5c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # NVIDIA Container Toolkit Changelog
 
 ## v1.14.0-rc.3
+* Added support for generating OCI hook JSON file to `nvidia-ctk runtime configure` command.
 
 ## v1.14.0-rc.2
 
diff --git a/cmd/nvidia-ctk/runtime/configure/configure.go b/cmd/nvidia-ctk/runtime/configure/configure.go
index bb6129bc..90af0cb6 100644
--- a/cmd/nvidia-ctk/runtime/configure/configure.go
+++ b/cmd/nvidia-ctk/runtime/configure/configure.go
@@ -25,6 +25,7 @@ import (
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/containerd"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/crio"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/docker"
+	"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/ocihook"
 	"github.com/urfave/cli/v2"
 )
 
@@ -34,8 +35,9 @@ const (
 	// defaultNVIDIARuntimeName is the default name to use in configs for the NVIDIA Container Runtime
 	defaultNVIDIARuntimeName = "nvidia"
 	// defaultNVIDIARuntimeExecutable is the default NVIDIA Container Runtime executable file name
-	defaultNVIDIARuntimeExecutable      = "nvidia-container-runtime"
-	defailtNVIDIARuntimeExpecutablePath = "/usr/bin/nvidia-container-runtime"
+	defaultNVIDIARuntimeExecutable          = "nvidia-container-runtime"
+	defaultNVIDIARuntimeExpecutablePath     = "/usr/bin/nvidia-container-runtime"
+	defaultNVIDIARuntimeHookExpecutablePath = "/usr/bin/nvidia-container-runtime-hook"
 
 	defaultContainerdConfigFilePath = "/etc/containerd/config.toml"
 	defaultCrioConfigFilePath       = "/etc/crio/crio.conf"
@@ -60,10 +62,13 @@ type config struct {
 	dryRun         bool
 	runtime        string
 	configFilePath string
+	mode           string
+	hookFilePath   string
 
 	nvidiaRuntime struct {
 		name         string
 		path         string
+		hookPath     string
 		setAsDefault bool
 	}
 }
@@ -77,7 +82,7 @@ func (m command) build() *cli.Command {
 		Name:  "configure",
 		Usage: "Add a runtime to the specified container engine",
 		Before: func(c *cli.Context) error {
-			return validateFlags(c, &config)
+			return m.validateFlags(c, &config)
 		},
 		Action: func(c *cli.Context) error {
 			return m.configureWrapper(c, &config)
@@ -101,6 +106,16 @@ func (m command) build() *cli.Command {
 			Usage:       "path to the config file for the target runtime",
 			Destination: &config.configFilePath,
 		},
+		&cli.StringFlag{
+			Name:        "config-mode",
+			Usage:       "the config mode for runtimes that support multiple configuration mechanisms",
+			Destination: &config.mode,
+		},
+		&cli.StringFlag{
+			Name:        "oci-hook-path",
+			Usage:       "the path to the OCI runtime hook to create if --config-mode=oci-hook is specified. If no path is specified, the generated hook is output to STDOUT.\n\tNote: The use of OCI hooks is deprecated.",
+			Destination: &config.hookFilePath,
+		},
 		&cli.StringFlag{
 			Name:        "nvidia-runtime-name",
 			Usage:       "specify the name of the NVIDIA runtime that will be added",
@@ -114,6 +129,12 @@ func (m command) build() *cli.Command {
 			Value:       defaultNVIDIARuntimeExecutable,
 			Destination: &config.nvidiaRuntime.path,
 		},
+		&cli.StringFlag{
+			Name:        "nvidia-runtime-hook-path",
+			Usage:       "specify the path to the NVIDIA Container Runtime hook executable",
+			Value:       defaultNVIDIARuntimeHookExpecutablePath,
+			Destination: &config.nvidiaRuntime.hookPath,
+		},
 		&cli.BoolFlag{
 			Name:        "nvidia-set-as-default",
 			Aliases:     []string{"set-as-default"},
@@ -125,7 +146,18 @@ func (m command) build() *cli.Command {
 	return &configure
 }
 
-func validateFlags(c *cli.Context, config *config) error {
+func (m command) validateFlags(c *cli.Context, config *config) error {
+	if config.mode == "oci-hook" {
+		if !filepath.IsAbs(config.nvidiaRuntime.hookPath) {
+			return fmt.Errorf("the NVIDIA runtime hook path %q is not an absolute path", config.nvidiaRuntime.hookPath)
+		}
+		return nil
+	}
+	if config.mode != "" && config.mode != "config-file" {
+		m.logger.Warningf("Ignoring unsupported config mode for %v: %q", config.runtime, config.mode)
+	}
+	config.mode = "config-file"
+
 	switch config.runtime {
 	case "containerd", "crio", "docker":
 		break
@@ -136,7 +168,7 @@ func validateFlags(c *cli.Context, config *config) error {
 	switch config.runtime {
 	case "containerd", "crio":
 		if config.nvidiaRuntime.path == defaultNVIDIARuntimeExecutable {
-			config.nvidiaRuntime.path = defailtNVIDIARuntimeExpecutablePath
+			config.nvidiaRuntime.path = defaultNVIDIARuntimeExpecutablePath
 		}
 		if !filepath.IsAbs(config.nvidiaRuntime.path) {
 			return fmt.Errorf("the NVIDIA runtime path %q is not an absolute path", config.nvidiaRuntime.path)
@@ -148,6 +180,17 @@ func validateFlags(c *cli.Context, config *config) error {
 
 // configureWrapper updates the specified container engine config to enable the NVIDIA runtime
 func (m command) configureWrapper(c *cli.Context, config *config) error {
+	switch config.mode {
+	case "oci-hook":
+		return m.configureOCIHook(c, config)
+	case "config-file":
+		return m.configureConfigFile(c, config)
+	}
+	return fmt.Errorf("unsupported config-mode: %v", config.mode)
+}
+
+// configureConfigFile updates the specified container engine config file to enable the NVIDIA runtime.
+func (m command) configureConfigFile(c *cli.Context, config *config) error {
 	configFilePath := config.resolveConfigFilePath()
 
 	var cfg engine.Interface
@@ -225,3 +268,12 @@ func (c *config) getOuputConfigPath() string {
 	}
 	return c.resolveConfigFilePath()
 }
+
+// configureOCIHook creates and configures the OCI hook for the NVIDIA runtime
+func (m *command) configureOCIHook(c *cli.Context, config *config) error {
+	err := ocihook.CreateHook(config.hookFilePath, config.nvidiaRuntime.hookPath)
+	if err != nil {
+		return fmt.Errorf("error creating OCI hook: %v", err)
+	}
+	return nil
+}
diff --git a/tools/container/crio/hooks.go b/pkg/config/ocihook/hooks.go
similarity index 99%
rename from tools/container/crio/hooks.go
rename to pkg/config/ocihook/hooks.go
index aba31774..017ada6e 100644
--- a/tools/container/crio/hooks.go
+++ b/pkg/config/ocihook/hooks.go
@@ -14,7 +14,7 @@
 # limitations under the License.
 **/
 
-package main
+package ocihook
 
 // podmanHook is the hook configuration structure.
 // This is taken from `Hook` at https://github.com/containers/podman/blob/3c53200e9d61fdf95fe1da825bb2a89372551350/pkg/hooks/1.0.0/hook.go#L18
diff --git a/pkg/config/ocihook/oci-hook.go b/pkg/config/ocihook/oci-hook.go
new file mode 100644
index 00000000..a64ec420
--- /dev/null
+++ b/pkg/config/ocihook/oci-hook.go
@@ -0,0 +1,89 @@
+/**
+# 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 ocihook
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// CreateHook creates an OCI hook file for the specified NVIDIA Container Runtime hook path
+func CreateHook(hookFilePath string, nvidiaContainerRuntimeHookExecutablePath string) error {
+	var output io.Writer
+	if hookFilePath == "" {
+		output = os.Stdout
+	} else {
+		if hooksDir := filepath.Dir(hookFilePath); hooksDir != "" {
+			err := os.MkdirAll(hooksDir, 0755)
+			if err != nil {
+				return fmt.Errorf("error creating hooks directory %v: %v", hooksDir, err)
+			}
+		}
+
+		hookFile, err := os.Create(hookFilePath)
+		if err != nil {
+			return fmt.Errorf("error creating hook file '%v': %v", hookFilePath, err)
+		}
+		defer hookFile.Close()
+		output = hookFile
+	}
+
+	encoder := json.NewEncoder(output)
+	encoder.SetIndent("", "  ")
+	if err := encoder.Encode(generateOciHook(nvidiaContainerRuntimeHookExecutablePath)); err != nil {
+		return fmt.Errorf("error writing hook file: %v", err)
+	}
+	return nil
+}
+
+func generateOciHook(executablePath string) podmanHook {
+	pathParts := []string{"/usr/local/sbin", "/usr/local/bin", "/usr/sbin", "/usr/bin", "/sbin", "/bin"}
+
+	dir := filepath.Dir(executablePath)
+	var found bool
+	for _, pathPart := range pathParts {
+		if pathPart == dir {
+			found = true
+			break
+		}
+	}
+	if !found {
+		pathParts = append(pathParts, dir)
+	}
+
+	envPath := "PATH=" + strings.Join(pathParts, ":")
+	always := true
+
+	hook := podmanHook{
+		Version: "1.0.0",
+		Stages:  []string{"prestart"},
+		Hook: specHook{
+			Path: executablePath,
+			Args: []string{filepath.Base(executablePath), "prestart"},
+			Env:  []string{envPath},
+		},
+		When: When{
+			Always:   &always,
+			Commands: []string{".*"},
+		},
+	}
+	return hook
+}
diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go
index 497951cd..60f5cda8 100644
--- a/tools/container/crio/crio.go
+++ b/tools/container/crio/crio.go
@@ -17,7 +17,6 @@
 package main
 
 import (
-	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -25,6 +24,7 @@ import (
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/info"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine/crio"
+	"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/ocihook"
 	"github.com/NVIDIA/nvidia-container-toolkit/tools/container"
 	log "github.com/sirupsen/logrus"
 	cli "github.com/urfave/cli/v2"
@@ -206,13 +206,8 @@ func Setup(c *cli.Context, o *options) error {
 func setupHook(o *options) error {
 	log.Infof("Installing prestart hook")
 
-	err := os.MkdirAll(o.hooksDir, 0755)
-	if err != nil {
-		return fmt.Errorf("error creating hooks directory %v: %v", o.hooksDir, err)
-	}
-
-	hookPath := getHookPath(o.hooksDir, o.hookFilename)
-	err = createHook(o.RuntimeDir, hookPath)
+	hookPath := filepath.Join(o.hooksDir, o.hookFilename)
+	err := ocihook.CreateHook(hookPath, filepath.Join(o.RuntimeDir, config.NVIDIAContainerRuntimeHookExecutable))
 	if err != nil {
 		return fmt.Errorf("error creating hook: %v", err)
 	}
@@ -262,7 +257,7 @@ func Cleanup(c *cli.Context, o *options) error {
 func cleanupHook(o *options) error {
 	log.Infof("Removing prestart hook")
 
-	hookPath := getHookPath(o.hooksDir, o.hookFilename)
+	hookPath := filepath.Join(o.hooksDir, o.hookFilename)
 	err := os.Remove(hookPath)
 	if err != nil {
 		return fmt.Errorf("error removing hook '%v': %v", hookPath, err)
@@ -295,46 +290,6 @@ func cleanupConfig(o *options) error {
 	return nil
 }
 
-func createHook(toolkitDir string, hookPath string) error {
-	hook, err := os.Create(hookPath)
-	if err != nil {
-		return fmt.Errorf("error creating hook file '%v': %v", hookPath, err)
-	}
-	defer hook.Close()
-
-	encoder := json.NewEncoder(hook)
-	err = encoder.Encode(generateOciHook(toolkitDir))
-	if err != nil {
-		return fmt.Errorf("error writing hook file '%v': %v", hookPath, err)
-	}
-	return nil
-}
-
-func getHookPath(hooksDir string, hookFilename string) string {
-	return filepath.Join(hooksDir, hookFilename)
-}
-
-func generateOciHook(toolkitDir string) podmanHook {
-	hookPath := filepath.Join(toolkitDir, config.NVIDIAContainerRuntimeHookExecutable)
-	envPath := "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:" + toolkitDir
-	always := true
-
-	hook := podmanHook{
-		Version: "1.0.0",
-		Stages:  []string{"prestart"},
-		Hook: specHook{
-			Path: hookPath,
-			Args: []string{filepath.Base(config.NVIDIAContainerRuntimeHookExecutable), "prestart"},
-			Env:  []string{envPath},
-		},
-		When: When{
-			Always:   &always,
-			Commands: []string{".*"},
-		},
-	}
-	return hook
-}
-
 // RestartCrio restarts crio depending on the value of restartModeFlag
 func RestartCrio(o *options) error {
 	return o.Restart("crio", func(string) error { return fmt.Errorf("supporting crio via signal is unsupported") })

From 65f6f46846041bd2e01dc48eba2334c6214e66bc Mon Sep 17 00:00:00 2001
From: Evan Lezar <elezar@nvidia.com>
Date: Fri, 11 Aug 2023 15:40:00 +0200
Subject: [PATCH 2/2] Remove installation of oci-nvidia-hook files in RPM
 packages

This change removes installation of the oci-nvidia-hook files.
These files conflict with CDI use in runtimes that support it.

The use of the hook should be considered deprecated on these platforms.

If a hook is required, the

nvidia-ctk runtime configure --config-mode=oci-hook

command should be used to create the hook file(s).

Signed-off-by: Evan Lezar <elezar@nvidia.com>
---
 CHANGELOG.md                                  |  1 +
 docker/Dockerfile.opensuse-leap               |  6 ------
 docker/Dockerfile.rpm-yum                     |  6 ------
 oci-nvidia-hook                               |  2 --
 oci-nvidia-hook.json                          | 15 --------------
 .../rpm/SPECS/nvidia-container-toolkit.spec   | 20 +++++--------------
 6 files changed, 6 insertions(+), 44 deletions(-)
 delete mode 100755 oci-nvidia-hook
 delete mode 100644 oci-nvidia-hook.json

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a0024a5c..106741b0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## v1.14.0-rc.3
 * Added support for generating OCI hook JSON file to `nvidia-ctk runtime configure` command.
+* Remove installation of OCI hook JSON from RPM package.
 
 ## v1.14.0-rc.2
 
diff --git a/docker/Dockerfile.opensuse-leap b/docker/Dockerfile.opensuse-leap
index 93e70a92..3de7a392 100644
--- a/docker/Dockerfile.opensuse-leap
+++ b/docker/Dockerfile.opensuse-leap
@@ -44,12 +44,6 @@ 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
-COPY oci-nvidia-hook $DIST_DIR/oci-nvidia-hook
-
-# Hook for libpod/CRI-O: https://github.com/containers/libpod/blob/v0.8.5/pkg/hooks/docs/oci-hooks.5.md
-COPY oci-nvidia-hook.json $DIST_DIR/oci-nvidia-hook.json
-
 WORKDIR $DIST_DIR/..
 COPY packaging/rpm .
 
diff --git a/docker/Dockerfile.rpm-yum b/docker/Dockerfile.rpm-yum
index 91f2d75c..4f3e248f 100644
--- a/docker/Dockerfile.rpm-yum
+++ b/docker/Dockerfile.rpm-yum
@@ -62,12 +62,6 @@ 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
-COPY oci-nvidia-hook $DIST_DIR/oci-nvidia-hook
-
-# Hook for libpod/CRI-O: https://github.com/containers/libpod/blob/v0.8.5/pkg/hooks/docs/oci-hooks.5.md
-COPY oci-nvidia-hook.json $DIST_DIR/oci-nvidia-hook.json
-
 WORKDIR $DIST_DIR/..
 COPY packaging/rpm .
 
diff --git a/oci-nvidia-hook b/oci-nvidia-hook
deleted file mode 100755
index 5e8f3b7d..00000000
--- a/oci-nvidia-hook
+++ /dev/null
@@ -1,2 +0,0 @@
-#!/bin/sh
-PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" exec /usr/bin/nvidia-container-runtime-hook "$@"
diff --git a/oci-nvidia-hook.json b/oci-nvidia-hook.json
deleted file mode 100644
index 6cbbe5cf..00000000
--- a/oci-nvidia-hook.json
+++ /dev/null
@@ -1,15 +0,0 @@
-{
-    "version": "1.0.0",
-    "hook": {
-        "path": "/usr/bin/nvidia-container-runtime-hook",
-        "args": ["nvidia-container-runtime-hook", "prestart"],
-        "env": [
-            "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
-        ]
-    },
-    "when": {
-        "always": true,
-	"commands": [".*"]
-    },
-    "stages": ["prestart"]
-}
diff --git a/packaging/rpm/SPECS/nvidia-container-toolkit.spec b/packaging/rpm/SPECS/nvidia-container-toolkit.spec
index e7f1536f..4d1db18d 100644
--- a/packaging/rpm/SPECS/nvidia-container-toolkit.spec
+++ b/packaging/rpm/SPECS/nvidia-container-toolkit.spec
@@ -12,12 +12,10 @@ License: Apache-2.0
 
 Source0: nvidia-container-runtime-hook
 Source1: nvidia-ctk
-Source2: oci-nvidia-hook
-Source3: oci-nvidia-hook.json
-Source4: LICENSE
-Source5: nvidia-container-runtime
-Source6: nvidia-container-runtime.cdi
-Source7: nvidia-container-runtime.legacy
+Source2: LICENSE
+Source3: nvidia-container-runtime
+Source4: nvidia-container-runtime.cdi
+Source5: nvidia-container-runtime.legacy
 
 Obsoletes: nvidia-container-runtime <= 3.5.0-1, nvidia-container-runtime-hook <= 1.4.0-2
 Provides: nvidia-container-runtime
@@ -36,7 +34,7 @@ Requires: libseccomp
 Provides tools and utilities to enable GPU support in containers.
 
 %prep
-cp %{SOURCE0} %{SOURCE1} %{SOURCE2} %{SOURCE3} %{SOURCE4} %{SOURCE5} %{SOURCE6} %{SOURCE7} .
+cp %{SOURCE0} %{SOURCE1} %{SOURCE2} %{SOURCE3} %{SOURCE4} %{SOURCE5} .
 
 %install
 mkdir -p %{buildroot}%{_bindir}
@@ -46,12 +44,6 @@ install -m 755 -t %{buildroot}%{_bindir} nvidia-container-runtime.cdi
 install -m 755 -t %{buildroot}%{_bindir} nvidia-container-runtime.legacy
 install -m 755 -t %{buildroot}%{_bindir} nvidia-ctk
 
-mkdir -p %{buildroot}/usr/libexec/oci/hooks.d
-install -m 755 -t %{buildroot}/usr/libexec/oci/hooks.d oci-nvidia-hook
-
-mkdir -p %{buildroot}/usr/share/containers/oci/hooks.d
-install -m 644 -t %{buildroot}/usr/share/containers/oci/hooks.d oci-nvidia-hook.json
-
 %post
 if [ $1 -gt 1 ]; then  # only on package upgrade
   mkdir -p %{_localstatedir}/lib/rpm-state/nvidia-container-toolkit
@@ -77,8 +69,6 @@ fi
 %files
 %license LICENSE
 %{_bindir}/nvidia-container-runtime-hook
-/usr/libexec/oci/hooks.d/oci-nvidia-hook
-/usr/share/containers/oci/hooks.d/oci-nvidia-hook.json
 
 %changelog
 # As of 1.10.0-1 we generate the release information automatically