Cleanup post rebase to remove redundant methods and install D-Bus

Signed-off-by: Kate Goldenring <[email protected]>

Author: kate-goldenring

api/v1alpha1/shim_types.go

@@ -26,9 +26,9 @@ type ShimSpec struct {
 	FetchStrategy   FetchStrategy     `json:"fetchStrategy"`
 	RuntimeClass    RuntimeClassSpec  `json:"runtimeClass"`
 	RolloutStrategy RolloutStrategy   `json:"rolloutStrategy"`
-	// RuntimeOptions is a map of containerd runtime options for the shim plugin.
+	// ContainerdRuntimeOptions is a map of containerd runtime options for the shim plugin.
 	// See an example of configuring cgroup driver via runtime options: https://github.com/containerd/containerd/blob/main/docs/cri/config.md#cgroup-driver
-	RuntimeOptions map[string]string `json:"runtimeOptions"`
+	ContainerdRuntimeOptions map[string]string `json:"containerdRuntimeOptions"`
 }
 
 type FetchStrategy struct {

api/v1alpha1/zz_generated.deepcopy.go

@@ -116,6 +116,14 @@ func RunInstall(config Config, rootFs, hostFs afero.Fs, restarter containerd.Res
 		return nil
 	}
 
+	// Ensure D-Bus is installed and running if using systemd
+	if _, err := containerd.ListSystemdUnits(); err == nil {
+		err = containerd.InstallDbus()
+		if err != nil {
+			return fmt.Errorf("failed to install D-Bus: %w", err)
+		}
+	}
+
 	slog.Info("restarting containerd")
 	err = containerdConfig.RestartRuntime()
 	if err != nil {

cmd/node-installer/install.go

@@ -49,6 +49,13 @@ spec:
           spec:
             description: ShimSpec defines the desired state of Shim
             properties:
+              containerdRuntimeOptions:
+                additionalProperties:
+                  type: string
+                description: |-
+                  ContainerdRuntimeOptions is a map of containerd runtime options for the shim plugin.
+                  See an example of configuring cgroup driver via runtime options: https://github.com/containerd/containerd/blob/main/docs/cri/config.md#cgroup-driver
+                type: object
               fetchStrategy:
                 properties:
                   anonHttp:
@@ -95,18 +102,11 @@ spec:
                 - handler
                 - name
                 type: object
-              runtimeOptions:
-                additionalProperties:
-                  type: string
-                description: |-
-                  RuntimeOptions is a map of containerd runtime options for the shim plugin.
-                  See an example of configuring cgroup driver via runtime options: https://github.com/containerd/containerd/blob/main/docs/cri/config.md#cgroup-driver
-                type: object
             required:
+            - containerdRuntimeOptions
             - fetchStrategy
             - rolloutStrategy
             - runtimeClass
-            - runtimeOptions
             type: object
           status:
             description: ShimStatus defines the observed state of Shim

config/crd/bases/runtime.spinkube.dev_shims.yaml

@@ -17,7 +17,7 @@ spec:
     anonHttp:
       location: "https://github.com/spinframework/containerd-shim-spin/releases/download/v0.19.0/containerd-shim-spin-v2-linux-aarch64.tar.gz"
 
-  runtimeOptions:
+  containerdRuntimeOptions:
     SystemdCgroup: "true"
 
   runtimeClass:

config/samples/test_shim_spin.yaml

@@ -3,20 +3,8 @@ package containerd
 import (
 	"fmt"
 	"log/slog"
-	"os"
-	"os/exec"
 )
 
-// UsesSystemd checks if the system is using systemd.
-func UsesSystemd() bool {
-	cmd := nsenterCmd("systemctl", "list-units", "|", "grep", "-q", "containerd.service")
-	if err := cmd.Run(); err != nil {
-		slog.Info("Error with systemctl: %w\n", "error", err)
-		return false
-	}
-	return true
-}
-
 // InstallDbus checks if D-Bus service is installed and active. If not, installs D-Bus
 // and starts the service.
 // NOTE: this limits support to systems using systemctl to manage systemd.
@@ -31,18 +19,24 @@ func InstallDbus() error {
 	type pkgManager struct {
 		name    string
 		check   []string
+		update  []string
 		install []string
 	}
 
 	managers := []pkgManager{
-		{"apt-get", []string{"which", "apt-get"}, []string{"apt-get", "update", "--yes", "&&", "apt-get", "install", "--yes", "dbus"}},
-		{"dnf", []string{"which", "dnf"}, []string{"dnf", "install", "--yes", "dbus"}},
-		{"apk", []string{"which", "apk"}, []string{"apk", "add", "dbus"}},
-		{"yum", []string{"which", "yum"}, []string{"yum", "install", "--yes", "dbus"}},
+		{"apt-get", []string{"which", "apt-get"}, []string{"apt-get", "update", "--yes"}, []string{"apt-get", "install", "--yes", "dbus"}},
+		{"dnf", []string{"which", "dnf"}, []string{}, []string{"dnf", "install", "--yes", "dbus"}},
+		{"apk", []string{"which", "apk"}, []string{}, []string{"apk", "add", "dbus"}},
+		{"yum", []string{"which", "yum"}, []string{}, []string{"yum", "install", "--yes", "dbus"}},
 	}
 	installed := false
 	for _, mgr := range managers {
 		if err := nsenterCmd(mgr.check...).Run(); err == nil {
+			if len(mgr.update) != 0 {
+				if err := nsenterCmd(mgr.update...).Run(); err != nil {
+					return fmt.Errorf("failed to update package manager %s: %w", mgr.name, err)
+				}
+			}
 			if err := nsenterCmd(mgr.install...).Run(); err != nil {
 				return fmt.Errorf("failed to install D-Bus with %s: %w", mgr.name, err)
 			}
@@ -63,8 +57,3 @@ func InstallDbus() error {
 
 	return nil
 }
-
-func nsenterCmd(cmd ...string) *exec.Cmd {
-	return exec.Command("nsenter",
-		append([]string{fmt.Sprintf("-m/%s/proc/1/ns/mnt", os.Getenv("HOST_ROOT")), "--"}, cmd...)...) // #nosec G204
-}

internal/containerd/install_dbus.go

@@ -40,7 +40,7 @@ func NewDefaultRestarter() Restarter {
 
 func (c defaultRestarter) Restart() error {
 	// If listing systemd units succeeds, prefer systemctl restart; otherwise kill pid
-	if _, err := listSystemdUnits(); err == nil {
+	if _, err := ListSystemdUnits(); err == nil {
 		out, err := nsenterCmd("systemctl", "restart", "containerd").CombinedOutput()
 		slog.Debug(string(out))
 		if err != nil {
@@ -67,7 +67,7 @@ type K0sRestarter struct{}
 func (c K0sRestarter) Restart() error {
 	// First, collect systemd units to determine which mode k0s is running in, eg
 	// k0sworker or k0scontroller
-	units, err := listSystemdUnits()
+	units, err := ListSystemdUnits()
 	if err != nil {
 		return fmt.Errorf("unable to list systemd units: %w", err)
 	}
@@ -88,7 +88,7 @@ func (c K3sRestarter) Restart() error {
 	// This restarter will be used both for stock K3s distros, which use systemd as well as K3d, which does not.
 
 	// If listing systemd units succeeds, prefer systemctl restart; otherwise kill pid
-	if _, err := listSystemdUnits(); err == nil {
+	if _, err := ListSystemdUnits(); err == nil {
 		out, err := nsenterCmd("systemctl", "restart", "k3s").CombinedOutput()
 		slog.Debug(string(out))
 		if err != nil {
@@ -130,7 +130,7 @@ type RKE2Restarter struct{}
 func (c RKE2Restarter) Restart() error {
 	// First, collect systemd units to determine which mode rke2 is running in, eg
 	// rke2-agent or rke2-server
-	units, err := listSystemdUnits()
+	units, err := ListSystemdUnits()
 	if err != nil {
 		return fmt.Errorf("unable to list systemd units: %w", err)
 	}
@@ -145,7 +145,7 @@ func (c RKE2Restarter) Restart() error {
 	return nil
 }
 
-func listSystemdUnits() ([]byte, error) {
+func ListSystemdUnits() ([]byte, error) {
 	return nsenterCmd("systemctl", "list-units", "--type", "service").CombinedOutput()
 }
 

internal/containerd/restart_unix.go

@@ -380,6 +380,8 @@ func (sr *ShimReconciler) setOperationConfiguration(shim *rcmv1.Shim, opConfig *
 }
 
 // createJobManifest creates a Job manifest for a Shim.
+//
+//nolint:funlen // function is longer due to scaffolding an entire K8s Job manifest
 func (sr *ShimReconciler) createJobManifest(shim *rcmv1.Shim, node *corev1.Node, operation string) (*batchv1.Job, error) {
 	opConfig := opConfig{
 		operation:  operation,
@@ -463,8 +465,8 @@ func (sr *ShimReconciler) createJobManifest(shim *rcmv1.Shim, node *corev1.Node,
 		},
 	}
 
-	if shim.Spec.RuntimeOptions != nil {
-		optionsJSON, err := json.Marshal(shim.Spec.RuntimeOptions)
+	if shim.Spec.ContainerdRuntimeOptions != nil {
+		optionsJSON, err := json.Marshal(shim.Spec.ContainerdRuntimeOptions)
 		if err != nil {
 			log.Error().Msgf("Unable to marshal runtime options: %s", err)
 		} else {