Bring v2.0.2 commits (#754)

* Add a webhook for namespace deletion (#719) This commit makes KMM set the kmm.node.k8s.io/contains-modules label on all namespaces that contain at least one Module. It also adds a new webhook to the bundle and the corresponding handler in the manager. The new webhook rejects namespace deletions if the kmm.node.k8s.io/contains-modules label is present on the namespace. This avoids entering situations where the namespace is being deleted and KMM cannot create unloading Pods to honor Module deletion. * Restrict checks for image existence (#734) Before populating the NodeModulesConfig object, only check if built or signed image exist on the registry. Add a log message when the NodeModulesConfig is not populated for that reason. * Make slight changes to the CRDs (#736) Module: make moduleName an optional field. ManagedClusterModule: make spokeNamespace a required field.
kubernetes-sigs · Mar 6, 2024 · 2b97c9e · 2b97c9e
1 parent a468303
commit 2b97c9e
Show file tree

Hide file tree

Showing 20 changed files with 529 additions and 67 deletions.
diff --git a/Makefile b/Makefile
@@ -100,7 +100,7 @@ help: ## Display this help.
 .PHONY: manifests
 manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
 	$(CONTROLLER_GEN) crd paths="./api/..." output:crd:artifacts:config=config/crd/bases
-	$(CONTROLLER_GEN) webhook paths="./api/..." output:webhook:artifacts:config=config/webhook
+	$(CONTROLLER_GEN) webhook paths="./api/..." paths="./internal/webhook/..." output:webhook:artifacts:config=config/webhook
 	$(CONTROLLER_GEN) rbac:roleName=manager-role paths="./internal/controllers" output:rbac:artifacts:config=config/rbac
 	# Hub
 	$(CONTROLLER_GEN) crd paths="./api-hub/..." output:crd:artifacts:config=config/crd-hub/bases

diff --git a/api-hub/v1beta1/managedclustermodule_types.go b/api-hub/v1beta1/managedclustermodule_types.go
@@ -27,7 +27,7 @@ type ManagedClusterModuleSpec struct {
 	ModuleSpec kmmv1beta1.ModuleSpec `json:"moduleSpec,omitempty"`
 
 	// SpokeNamespace describes the Spoke namespace, in which the ModuleSpec should be applied.
-	SpokeNamespace string `json:"spokeNamespace,omitempty"`
+	SpokeNamespace string `json:"spokeNamespace"`
 
 	// Selector describes on which managed clusters the ModuleSpec should be applied.
 	Selector map[string]string `json:"selector"`

diff --git a/api/v1beta1/module_types.go b/api/v1beta1/module_types.go
@@ -136,7 +136,9 @@ type ModprobeArgs struct {
 
 type ModprobeSpec struct {
 	// ModuleName is the name of the Module to be loaded.
-	ModuleName string `json:"moduleName"`
+	// This field can only be unset if rawArgs is set.
+	// +optional
+	ModuleName string `json:"moduleName,omitempty"`
 
 	// Parameters is an optional list of kernel module parameters to be provided to modprobe.
 	// They should be in the form of key=value and will be separated by spaces in the modprobe command.

diff --git a/cmd/manager/main.go b/cmd/manager/main.go
@@ -22,6 +22,7 @@ import (
 	"os"
 	"strconv"
 
+	"github.com/kubernetes-sigs/kernel-module-management/internal/webhook"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -207,6 +208,10 @@ func main() {
 		}
 	}
 
+	if err = (&webhook.NamespaceDeletion{}).SetupWebhookWithManager(mgr); err != nil {
+		cmd.FatalError(setupLogger, err, "unable to create webhook", "webhook", "Namespace")
+	}
+
 	if err = (&v1beta12.Module{}).SetupWebhookWithManager(mgr); err != nil {
 		cmd.FatalError(setupLogger, err, "unable to create webhook", "webhook", "Module")
 	}

diff --git a/config/crd-hub/bases/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml b/config/crd-hub/bases/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml
@@ -2362,7 +2362,8 @@ spec:
                                 type: string
                               moduleName:
                                 description: ModuleName is the name of the Module
-                                  to be loaded.
+                                  to be loaded. This field can only be unset if rawArgs
+                                  is set.
                                 type: string
                               modulesLoadingOrder:
                                 description: 'ModulesLoadingOrder defines the dependency
@@ -2408,8 +2409,6 @@ spec:
                                     minItems: 1
                                     type: array
                                 type: object
-                            required:
-                            - moduleName
                             type: object
                           registryTLS:
                             description: RegistryTLS set the TLS configs for accessing
@@ -2520,6 +2519,7 @@ spec:
                 type: string
             required:
             - selector
+            - spokeNamespace
             type: object
           status:
             description: ManagedClusterModuleStatus defines the observed state of

diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_modules.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_modules.yaml
@@ -2261,7 +2261,7 @@ spec:
                             type: string
                           moduleName:
                             description: ModuleName is the name of the Module to be
-                              loaded.
+                              loaded. This field can only be unset if rawArgs is set.
                             type: string
                           modulesLoadingOrder:
                             description: 'ModulesLoadingOrder defines the dependency
@@ -2307,8 +2307,6 @@ spec:
                                 minItems: 1
                                 type: array
                             type: object
-                        required:
-                        - moduleName
                         type: object
                       registryTLS:
                         description: RegistryTLS set the TLS configs for accessing

diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml
@@ -90,7 +90,8 @@ spec:
                               type: string
                             moduleName:
                               description: ModuleName is the name of the Module to
-                                be loaded.
+                                be loaded. This field can only be unset if rawArgs
+                                is set.
                               type: string
                             modulesLoadingOrder:
                               description: 'ModulesLoadingOrder defines the dependency
@@ -136,8 +137,6 @@ spec:
                                   minItems: 1
                                   type: array
                               type: object
-                          required:
-                          - moduleName
                           type: object
                       required:
                       - containerImage
@@ -226,7 +225,8 @@ spec:
                               type: string
                             moduleName:
                               description: ModuleName is the name of the Module to
-                                be loaded.
+                                be loaded. This field can only be unset if rawArgs
+                                is set.
                               type: string
                             modulesLoadingOrder:
                               description: 'ModulesLoadingOrder defines the dependency
@@ -272,8 +272,6 @@ spec:
                                   minItems: 1
                                   type: array
                               type: object
-                          required:
-                          - moduleName
                           type: object
                       required:
                       - containerImage

diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -42,6 +42,15 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - ""
+  resources:
+  - namespaces
+  verbs:
+  - get
+  - list
+  - patch
+  - watch
 - apiGroups:
   - ""
   resources:

diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml
@@ -7,3 +7,6 @@ resources:
 
 configurations:
 - kustomizeconfig.yaml
+
+patches:
+- path: manifests_namespace_selector_patch.yaml
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -24,3 +24,22 @@ webhooks:
     resources:
     - modules
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate--v1-namespace
+  failurePolicy: Fail
+  name: namespace-deletion.kmm.sigs.k8s.io
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - DELETE
+    resources:
+    - namespaces
+  sideEffects: None
diff --git a/config/webhook/manifests_namespace_selector_patch.yaml b/config/webhook/manifests_namespace_selector_patch.yaml
@@ -0,0 +1,9 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: validating-webhook-configuration
+webhooks:
+- name: namespace-deletion.kmm.sigs.k8s.io
+  namespaceSelector:
+    matchLabels:
+      kmm.node.k8s.io/contains-modules: ''
diff --git a/docs/mkdocs/documentation/deploy_kmod.md b/docs/mkdocs/documentation/deploy_kmod.md
@@ -220,12 +220,26 @@ The following `Module` fields support shell-like variable substitution:
 
 The following variables will be substituted:
 
-| Name                          | Description                            | Example                 |
-|-------------------------------|----------------------------------------|-------------------------|
-| `KERNEL_FULL_VERSION`         | The kernel version we are building for | `6.3.5-200.fc38.x86_64` |
-| `KERNEL_VERSION` (deprecated) | The kernel version we are building for | `6.3.5-200.fc38.x86_64` |
-| `MOD_NAME`                    | The `Module`'s name                    | `my-mod`                |
-| `MOD_NAMESPACE`               | The `Module`'s namespace               | `my-namespace`          |
+| Name                  | Description                            | Example                 |
+|-----------------------|----------------------------------------|-------------------------|
+| `KERNEL_FULL_VERSION` | The kernel version we are building for | `6.3.5-200.fc38.x86_64` |
+| `MOD_NAME`            | The `Module`'s name                    | `my-mod`                |
+| `MOD_NAMESPACE`       | The `Module`'s namespace               | `my-namespace`          |
+
+### Unloading the kernel module
+
+To unload a module loaded with KMM from nodes, simply delete the corresponding `Module` resource.
+KMM will then create worker Pods where required to run `modprobe -r` and unload the kernel module from nodes.
+
+!!! warning
+    To create unloading worker Pods, KMM needs all the resources it used when loading the kernel module.
+    This includes the `ServiceAccount` that are referenced in the `Module` as well as any RBAC you may have defined to
+    allow privileged KMM worker Pods to run.
+    It also includes any pull secret referenced in `.spec.imageRepoSecret`.  
+    To avoid situations where KMM is unable to unload the kernel module from nodes, make sure those resources are not
+    deleted while the `Module` resource is still present in the cluster in any state, including `Terminating`.  
+    KMM ships with a validating admission webhook that rejects the deletion of namespaces that contain at least one
+    `Module` resource.
 
 ## Security and permissions
 

diff --git a/internal/constants/constants.go b/internal/constants/constants.go
@@ -8,6 +8,7 @@ const (
 	PodHashAnnotation    = "kmm.node.kubernetes.io/last-hash"
 	KernelLabel          = "kmm.node.kubernetes.io/kernel-version.full"
 	DaemonSetRole        = "kmm.node.kubernetes.io/role"
+	NamespaceLabelKey    = "kmm.node.k8s.io/contains-modules"
 
 	WorkerPodVersionLabelPrefix    = "beta.kmm.node.kubernetes.io/version-worker-pod"
 	DevicePluginVersionLabelPrefix = "beta.kmm.node.kubernetes.io/version-device-plugin"

diff --git a/internal/controllers/mock_module_nmc_reconciler.go b/internal/controllers/mock_module_nmc_reconciler.go