Skip to content

Commit c50978c

Browse files
authored
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.
1 parent a520a14 commit c50978c

15 files changed

+477
-39
lines changed

Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ help: ## Display this help.
100100
.PHONY: manifests
101101
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
102102
$(CONTROLLER_GEN) crd paths="./api/..." output:crd:artifacts:config=config/crd/bases
103-
$(CONTROLLER_GEN) webhook paths="./api/..." output:webhook:artifacts:config=config/webhook
103+
$(CONTROLLER_GEN) webhook paths="./api/..." paths="./internal/webhook/..." output:webhook:artifacts:config=config/webhook
104104
$(CONTROLLER_GEN) rbac:roleName=manager-role paths="./internal/controllers" output:rbac:artifacts:config=config/rbac
105105
# Hub
106106
$(CONTROLLER_GEN) crd paths="./api-hub/..." output:crd:artifacts:config=config/crd-hub/bases

cmd/manager/main.go

+5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"os"
2323
"strconv"
2424

25+
"github.com/kubernetes-sigs/kernel-module-management/internal/webhook"
2526
"k8s.io/apimachinery/pkg/runtime"
2627
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2728
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -207,6 +208,10 @@ func main() {
207208
}
208209
}
209210

211+
if err = (&webhook.NamespaceDeletion{}).SetupWebhookWithManager(mgr); err != nil {
212+
cmd.FatalError(setupLogger, err, "unable to create webhook", "webhook", "Namespace")
213+
}
214+
210215
if err = (&v1beta12.Module{}).SetupWebhookWithManager(mgr); err != nil {
211216
cmd.FatalError(setupLogger, err, "unable to create webhook", "webhook", "Module")
212217
}

config/rbac/role.yaml

+9
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ rules:
4242
- get
4343
- list
4444
- watch
45+
- apiGroups:
46+
- ""
47+
resources:
48+
- namespaces
49+
verbs:
50+
- get
51+
- list
52+
- patch
53+
- watch
4554
- apiGroups:
4655
- ""
4756
resources:

config/webhook/kustomization.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,6 @@ resources:
77

88
configurations:
99
- kustomizeconfig.yaml
10+
11+
patches:
12+
- path: manifests_namespace_selector_patch.yaml

config/webhook/manifests.yaml

+19
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,22 @@ webhooks:
2424
resources:
2525
- modules
2626
sideEffects: None
27+
- admissionReviewVersions:
28+
- v1
29+
clientConfig:
30+
service:
31+
name: webhook-service
32+
namespace: system
33+
path: /validate--v1-namespace
34+
failurePolicy: Fail
35+
name: namespace-deletion.kmm.sigs.k8s.io
36+
rules:
37+
- apiGroups:
38+
- ""
39+
apiVersions:
40+
- v1
41+
operations:
42+
- DELETE
43+
resources:
44+
- namespaces
45+
sideEffects: None
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
apiVersion: admissionregistration.k8s.io/v1
2+
kind: ValidatingWebhookConfiguration
3+
metadata:
4+
name: validating-webhook-configuration
5+
webhooks:
6+
- name: namespace-deletion.kmm.sigs.k8s.io
7+
namespaceSelector:
8+
matchLabels:
9+
kmm.node.k8s.io/contains-modules: ''

docs/mkdocs/documentation/deploy_kmod.md

+4-5
Original file line numberDiff line numberDiff line change
@@ -236,11 +236,10 @@ KMM will then create worker Pods where required to run `modprobe -r` and unload
236236
This includes the `ServiceAccount` that are referenced in the `Module` as well as any RBAC you may have defined to
237237
allow privileged KMM worker Pods to run.
238238
It also includes any pull secret referenced in `.spec.imageRepoSecret`.
239-
To avoid situations where KMM is unable to unload the kernel module from nodes:
240-
241-
- do not delete those resources while the `Module` resource is still present in the cluster in any state,
242-
including `Terminating`;
243-
- do not delete any namespace containing at least a `Module` resource.
239+
To avoid situations where KMM is unable to unload the kernel module from nodes, make sure those resources are not
240+
deleted while the `Module` resource is still present in the cluster in any state, including `Terminating`.
241+
KMM ships with a validating admission webhook that rejects the deletion of namespaces that contain at least one
242+
`Module` resource.
244243

245244
## Security and permissions
246245

internal/constants/constants.go

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const (
88
PodHashAnnotation = "kmm.node.kubernetes.io/last-hash"
99
KernelLabel = "kmm.node.kubernetes.io/kernel-version.full"
1010
DaemonSetRole = "kmm.node.kubernetes.io/role"
11+
NamespaceLabelKey = "kmm.node.k8s.io/contains-modules"
1112

1213
WorkerPodVersionLabelPrefix = "beta.kmm.node.kubernetes.io/version-worker-pod"
1314
DevicePluginVersionLabelPrefix = "beta.kmm.node.kubernetes.io/version-device-plugin"

internal/controllers/mock_module_nmc_reconciler.go

+52-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/controllers/module_nmc_reconciler.go

+86-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"sigs.k8s.io/controller-runtime/pkg/log"
3131
)
3232

33+
//+kubebuilder:rbac:groups="core",resources=namespaces,verbs=get;list;patch;watch
3334
//+kubebuilder:rbac:groups="core",resources=nodes,verbs=get;watch
3435
//+kubebuilder:rbac:groups=kmm.sigs.x-k8s.io,resources=nodemodulesconfigs,verbs=get;list;watch;patch;create;delete
3536

@@ -47,6 +48,7 @@ type schedulingData struct {
4748

4849
type ModuleNMCReconciler struct {
4950
filter *filter.Filter
51+
nsLabeler namespaceLabeler
5052
reconHelper moduleNMCReconcilerHelperAPI
5153
}
5254

@@ -59,6 +61,7 @@ func NewModuleNMCReconciler(client client.Client,
5961
reconHelper := newModuleNMCReconcilerHelper(client, kernelAPI, registryAPI, nmcHelper, scheme)
6062
return &ModuleNMCReconciler{
6163
filter: filter,
64+
nsLabeler: newNamespaceLabeler(client),
6265
reconHelper: reconHelper,
6366
}
6467
}
@@ -78,13 +81,21 @@ func (mnr *ModuleNMCReconciler) Reconcile(ctx context.Context, req ctrl.Request)
7881
}
7982
if mod.GetDeletionTimestamp() != nil {
8083
//Module is being deleted
84+
if err = mnr.nsLabeler.tryRemovingLabel(ctx, mod.Namespace, mod.Name); err != nil {
85+
return ctrl.Result{}, fmt.Errorf("error while trying to remove the label on namespace %s: %v", mod.Namespace, err)
86+
}
87+
8188
err = mnr.reconHelper.finalizeModule(ctx, mod)
8289
if err != nil {
8390
return ctrl.Result{}, fmt.Errorf("failed to finalize %s Module: %v", req.NamespacedName, err)
8491
}
8592
return ctrl.Result{}, nil
8693
}
8794

95+
if err = mnr.nsLabeler.setLabel(ctx, mod.Namespace); err != nil {
96+
return ctrl.Result{}, fmt.Errorf("could not set label %q on namespace %s: %v", constants.NamespaceLabelKey, mod.Namespace, err)
97+
}
98+
8899
err = mnr.reconHelper.setFinalizerAndStatus(ctx, mod)
89100
if err != nil {
90101
return ctrl.Result{}, fmt.Errorf("failed to set finalizer on %s Module: %v", req.NamespacedName, err)
@@ -145,7 +156,7 @@ func (mnr *ModuleNMCReconciler) SetupWithManager(mgr ctrl.Manager) error {
145156
Complete(mnr)
146157
}
147158

148-
//go:generate mockgen -source=module_nmc_reconciler.go -package=controllers -destination=mock_module_nmc_reconciler.go moduleNMCReconcilerHelperAPI
159+
//go:generate mockgen -source=module_nmc_reconciler.go -package=controllers -destination=mock_module_nmc_reconciler.go moduleNMCReconcilerHelperAPI,namespaceLabeler
149160

150161
type moduleNMCReconcilerHelperAPI interface {
151162
setFinalizerAndStatus(ctx context.Context, mod *kmmv1beta1.Module) error
@@ -439,6 +450,80 @@ func (mnrh *moduleNMCReconcilerHelper) moduleUpdateWorkerPodsStatus(ctx context.
439450
return mnrh.client.Status().Patch(ctx, mod, client.MergeFrom(unmodifiedMod))
440451
}
441452

453+
type namespaceLabeler interface {
454+
setLabel(ctx context.Context, name string) error
455+
tryRemovingLabel(ctx context.Context, name, moduleName string) error
456+
}
457+
458+
type namespaceLabelerImpl struct {
459+
client client.Client
460+
}
461+
462+
func newNamespaceLabeler(client client.Client) namespaceLabeler {
463+
return &namespaceLabelerImpl{client: client}
464+
}
465+
466+
func (h *namespaceLabelerImpl) setLabel(ctx context.Context, name string) error {
467+
logger := log.FromContext(ctx)
468+
469+
ns := v1.Namespace{}
470+
471+
if err := h.client.Get(ctx, types.NamespacedName{Name: name}, &ns); err != nil {
472+
return fmt.Errorf("could not get namespace %s: %v", name, err)
473+
}
474+
475+
if !meta.HasLabel(&ns, constants.NamespaceLabelKey) {
476+
nsCopy := ns.DeepCopy()
477+
478+
logger.Info("Setting namespace label")
479+
meta.SetLabel(&ns, constants.NamespaceLabelKey, "")
480+
481+
return h.client.Patch(ctx, &ns, client.MergeFrom(nsCopy))
482+
}
483+
484+
return nil
485+
}
486+
487+
func (h *namespaceLabelerImpl) tryRemovingLabel(ctx context.Context, name, moduleName string) error {
488+
logger := log.FromContext(ctx)
489+
490+
modList := kmmv1beta1.ModuleList{}
491+
492+
opt := client.InNamespace(name)
493+
494+
if err := h.client.List(ctx, &modList, opt); err != nil {
495+
return fmt.Errorf("could not list modules in namespace %s: %v", name, err)
496+
}
497+
498+
if count := len(modList.Items); count > 1 {
499+
logger.Info("Namespace still contains modules; not removing the label", "count", count)
500+
501+
if verboseLogger := logger.V(1); verboseLogger.Enabled() {
502+
modNames := make([]string, 0, count)
503+
504+
for _, m := range modList.Items {
505+
modNames = append(modNames, m.Name)
506+
}
507+
508+
verboseLogger.Info("Remaining modules", "names", modNames)
509+
}
510+
511+
return nil
512+
}
513+
514+
ns := &v1.Namespace{}
515+
516+
if err := h.client.Get(ctx, types.NamespacedName{Name: name}, ns); err != nil {
517+
return fmt.Errorf("could not get namespace %s: %v", name, err)
518+
}
519+
520+
nsCopy := ns.DeepCopy()
521+
522+
meta.RemoveLabel(ns, constants.NamespaceLabelKey)
523+
524+
return h.client.Patch(ctx, ns, client.MergeFrom(nsCopy))
525+
}
526+
442527
func prepareNodeSchedulingData(node v1.Node, mld *api.ModuleLoaderData, currentNMCs sets.Set[string]) schedulingData {
443528
versionLabel := ""
444529
present := false

0 commit comments

Comments
 (0)