Skip to content

Commit 41910e7

Browse files
mresvanismresvanis
committed
Add module loader and device plugin SAs when not defined
This change adds ServiceAccounts for the module loader and the device plugin workloads of a Module, when the ServiceAccounts are not defined in the Module.Spec.ModuleLoader.ServiceAccountName and Module.Spec.DevicePlugin.ServiceAccountName respectively. This way we avoid using the K8S default ServiceAccount, which may have different permissions that the module loader and/or the device plugin workloads require. Specifically, this change: - adds the required ServiceAccount permissions to the module reconciler - adds the following Module reconciliation steps when the Module.Spec.ModuleLoader.ServiceAccountName is empty: - adds a module loader ServiceAccount - adds the module loaderServiceAccount to the module loader DaemonSet - adds the following Module reconciliation steps when the Module.Spec.DevicePlugin.ServiceAccountName is empty: - adds a device plugin ServiceAccount - adds the device plugin ServiceAccount to the device plugin DaemonSet Signed-off-by: Michail Resvanis <[email protected]>
1 parent 56b8bb3 commit 41910e7

File tree

10 files changed

+589
-20
lines changed

10 files changed

+589
-20
lines changed

‎config/rbac/role.yaml

+11
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,17 @@ rules:
5151
- get
5252
- list
5353
- watch
54+
- apiGroups:
55+
- ""
56+
resources:
57+
- serviceaccounts
58+
verbs:
59+
- create
60+
- delete
61+
- get
62+
- list
63+
- patch
64+
- watch
5465
- apiGroups:
5566
- kmm.sigs.k8s.io
5667
resources:

‎controllers/module_reconciler.go

+17
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
2929
"github.com/kubernetes-sigs/kernel-module-management/internal/metrics"
3030
"github.com/kubernetes-sigs/kernel-module-management/internal/module"
31+
"github.com/kubernetes-sigs/kernel-module-management/internal/rbac"
3132
"github.com/kubernetes-sigs/kernel-module-management/internal/registry"
3233
"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
3334
appsv1 "k8s.io/api/apps/v1"
@@ -52,6 +53,7 @@ type ModuleReconciler struct {
5253
client.Client
5354

5455
buildAPI build.Manager
56+
rbacAPI rbac.RBACCreator
5557
daemonAPI daemonset.DaemonSetCreator
5658
kernelAPI module.KernelMapper
5759
metricsAPI metrics.Metrics
@@ -63,6 +65,7 @@ type ModuleReconciler struct {
6365
func NewModuleReconciler(
6466
client client.Client,
6567
buildAPI build.Manager,
68+
rbacAPI rbac.RBACCreator,
6669
daemonAPI daemonset.DaemonSetCreator,
6770
kernelAPI module.KernelMapper,
6871
metricsAPI metrics.Metrics,
@@ -72,6 +75,7 @@ func NewModuleReconciler(
7275
return &ModuleReconciler{
7376
Client: client,
7477
buildAPI: buildAPI,
78+
rbacAPI: rbacAPI,
7579
daemonAPI: daemonAPI,
7680
kernelAPI: kernelAPI,
7781
metricsAPI: metricsAPI,
@@ -87,6 +91,7 @@ func NewModuleReconciler(
8791
//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;patch;watch
8892
//+kubebuilder:rbac:groups="core",resources=nodes,verbs=get;list;watch
8993
//+kubebuilder:rbac:groups="core",resources=secrets,verbs=get;list;watch
94+
//+kubebuilder:rbac:groups="core",resources=serviceaccounts,verbs=create;delete;get;list;patch;watch
9095
//+kubebuilder:rbac:groups="batch",resources=jobs,verbs=create;list;watch;delete
9196

9297
// Reconcile lists all nodes and looks for kernels that match its mappings.
@@ -109,6 +114,17 @@ func (r *ModuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
109114

110115
r.setKMMOMetrics(ctx)
111116

117+
if mod.Spec.ModuleLoader.ServiceAccountName == "" {
118+
if err := r.rbacAPI.CreateModuleLoaderRBAC(ctx, *mod); err != nil {
119+
return res, fmt.Errorf("could not create module-loader's RBAC: %w", err)
120+
}
121+
}
122+
if mod.Spec.DevicePlugin != nil && mod.Spec.DevicePlugin.ServiceAccountName == "" {
123+
if err := r.rbacAPI.CreateDevicePluginRBAC(ctx, *mod); err != nil {
124+
return res, fmt.Errorf("could not create device-plugin's RBAC: %w", err)
125+
}
126+
}
127+
112128
targetedNodes, err := r.getNodesListBySelector(ctx, mod)
113129
if err != nil {
114130
return res, fmt.Errorf("could get targeted nodes for module %s: %w", mod.Name, err)
@@ -359,6 +375,7 @@ func (r *ModuleReconciler) SetupWithManager(mgr ctrl.Manager, kernelLabel string
359375
return ctrl.NewControllerManagedBy(mgr).
360376
For(&kmmv1beta1.Module{}).
361377
Owns(&appsv1.DaemonSet{}).
378+
Owns(&v1.ServiceAccount{}).
362379
Owns(&batchv1.Job{}).
363380
Watches(
364381
&source.Kind{Type: &v1.Node{}},

‎controllers/module_reconciler_test.go

+111-17
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/kubernetes-sigs/kernel-module-management/internal/daemonset"
1212
"github.com/kubernetes-sigs/kernel-module-management/internal/metrics"
1313
"github.com/kubernetes-sigs/kernel-module-management/internal/module"
14+
"github.com/kubernetes-sigs/kernel-module-management/internal/rbac"
1415
"github.com/kubernetes-sigs/kernel-module-management/internal/registry"
1516
"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
1617
. "github.com/onsi/ginkgo/v2"
@@ -34,6 +35,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
3435
ctrl *gomock.Controller
3536
clnt *client.MockClient
3637
mockBM *build.MockManager
38+
mockRC *rbac.MockRBACCreator
3739
mockDC *daemonset.MockDaemonSetCreator
3840
mockKM *module.MockKernelMapper
3941
mockMetrics *metrics.MockMetrics
@@ -45,6 +47,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
4547
ctrl = gomock.NewController(GinkgoT())
4648
clnt = client.NewMockClient(ctrl)
4749
mockBM = build.NewMockManager(ctrl)
50+
mockRC = rbac.NewMockRBACCreator(ctrl)
4851
mockDC = daemonset.NewMockDaemonSetCreator(ctrl)
4952
mockKM = module.NewMockKernelMapper(ctrl)
5053
mockMetrics = metrics.NewMockMetrics(ctrl)
@@ -71,22 +74,95 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
7174
apierrors.NewNotFound(schema.GroupResource{}, moduleName),
7275
)
7376

74-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
77+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
7578
Expect(
7679
mr.Reconcile(ctx, req),
7780
).To(
7881
Equal(reconcile.Result{}),
7982
)
8083
})
8184

85+
It("should add the module loader and device plugin ServiceAccounts if they are not set", func() {
86+
mod := kmmv1beta1.Module{
87+
ObjectMeta: metav1.ObjectMeta{
88+
Name: moduleName,
89+
Namespace: namespace,
90+
},
91+
Spec: kmmv1beta1.ModuleSpec{
92+
Selector: map[string]string{"key": "value"},
93+
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
94+
ServiceAccountName: "",
95+
},
96+
DevicePlugin: &kmmv1beta1.DevicePluginSpec{
97+
ServiceAccountName: "",
98+
},
99+
},
100+
}
101+
ds := appsv1.DaemonSet{
102+
ObjectMeta: metav1.ObjectMeta{
103+
Name: moduleName + "-device-plugin",
104+
Namespace: namespace,
105+
},
106+
}
107+
108+
gomock.InOrder(
109+
clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn(
110+
func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error {
111+
m.ObjectMeta = mod.ObjectMeta
112+
m.Spec = mod.Spec
113+
return nil
114+
},
115+
),
116+
clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
117+
func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error {
118+
list.Items = []kmmv1beta1.Module{mod}
119+
return nil
120+
},
121+
),
122+
mockMetrics.EXPECT().SetExistingKMMOModules(1),
123+
mockRC.EXPECT().CreateModuleLoaderRBAC(ctx, gomock.Any()).Return(nil),
124+
mockRC.EXPECT().CreateDevicePluginRBAC(ctx, gomock.Any()).Return(nil),
125+
clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
126+
func(_ interface{}, list *v1.NodeList, _ ...interface{}) error {
127+
list.Items = []v1.Node{}
128+
return nil
129+
},
130+
),
131+
clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")),
132+
clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")),
133+
mockDC.EXPECT().SetDevicePluginAsDesired(context.Background(), &ds, gomock.AssignableToTypeOf(&mod)),
134+
clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil),
135+
mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, "", metrics.DevicePluginStage, false),
136+
)
137+
138+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
139+
140+
dsByKernelVersion := make(map[string]*appsv1.DaemonSet)
141+
142+
gomock.InOrder(
143+
mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil),
144+
mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()),
145+
mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil),
146+
)
147+
148+
res, err := mr.Reconcile(context.Background(), req)
149+
Expect(err).NotTo(HaveOccurred())
150+
Expect(res).To(Equal(reconcile.Result{}))
151+
})
152+
82153
It("should do nothing when no nodes match the selector", func() {
154+
const serviceAccountName = "module-loader-service-account"
155+
83156
mod := kmmv1beta1.Module{
84157
ObjectMeta: metav1.ObjectMeta{
85158
Name: moduleName,
86159
Namespace: namespace,
87160
},
88161
Spec: kmmv1beta1.ModuleSpec{
89162
Selector: map[string]string{"key": "value"},
163+
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
164+
ServiceAccountName: serviceAccountName,
165+
},
90166
},
91167
}
92168

@@ -113,7 +189,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
113189
),
114190
)
115191

116-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
192+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
117193

118194
dsByKernelVersion := make(map[string]*appsv1.DaemonSet)
119195

@@ -129,7 +205,10 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
129205
})
130206

131207
It("should remove obsolete DaemonSets when no nodes match the selector", func() {
132-
const kernelVersion = "1.2.3"
208+
const (
209+
kernelVersion = "1.2.3"
210+
serviceAccountName = "module-loader-service-account"
211+
)
133212

134213
mod := kmmv1beta1.Module{
135214
ObjectMeta: metav1.ObjectMeta{
@@ -138,6 +217,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
138217
},
139218
Spec: kmmv1beta1.ModuleSpec{
140219
Selector: map[string]string{"key": "value"},
220+
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
221+
ServiceAccountName: serviceAccountName,
222+
},
141223
},
142224
}
143225

@@ -171,7 +253,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
171253
),
172254
)
173255

174-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
256+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
175257

176258
dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds}
177259

@@ -188,8 +270,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
188270

189271
It("should create a DaemonSet when a node matches the selector", func() {
190272
const (
191-
imageName = "test-image"
192-
kernelVersion = "1.2.3"
273+
imageName = "test-image"
274+
kernelVersion = "1.2.3"
275+
serviceAccountName = "module-loader-service-account"
193276
)
194277

195278
mappings := []kmmv1beta1.KernelMapping{
@@ -208,6 +291,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
208291
},
209292
Spec: kmmv1beta1.ModuleSpec{
210293
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
294+
ServiceAccountName: serviceAccountName,
211295
Container: kmmv1beta1.ModuleLoaderContainerSpec{
212296
KernelMappings: mappings,
213297
},
@@ -232,7 +316,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
232316

233317
dsByKernelVersion := make(map[string]*appsv1.DaemonSet)
234318

235-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
319+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
236320

237321
ds := appsv1.DaemonSet{
238322
ObjectMeta: metav1.ObjectMeta{
@@ -280,8 +364,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
280364

281365
It("should patch the DaemonSet when it already exists", func() {
282366
const (
283-
imageName = "test-image"
284-
kernelVersion = "1.2.3"
367+
imageName = "test-image"
368+
kernelVersion = "1.2.3"
369+
serviceAccountName = "module-loader-service-account"
285370
)
286371

287372
osConfig := module.NodeOSConfig{}
@@ -302,6 +387,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
302387
},
303388
Spec: kmmv1beta1.ModuleSpec{
304389
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
390+
ServiceAccountName: serviceAccountName,
305391
Container: kmmv1beta1.ModuleLoaderContainerSpec{
306392
KernelMappings: mappings,
307393
},
@@ -361,7 +447,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
361447
clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()),
362448
)
363449

364-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
450+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
365451

366452
dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds}
367453

@@ -387,6 +473,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
387473
const (
388474
imageName = "test-image"
389475
kernelVersion = "1.2.3"
476+
477+
moduleLoaderServiceAccountName = "module-loader-service-account"
478+
devicePluginServiceAccountName = "device-plugin-service-account"
390479
)
391480

392481
mappings := []kmmv1beta1.KernelMapping{
@@ -402,8 +491,11 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
402491
Namespace: namespace,
403492
},
404493
Spec: kmmv1beta1.ModuleSpec{
405-
DevicePlugin: &kmmv1beta1.DevicePluginSpec{},
494+
DevicePlugin: &kmmv1beta1.DevicePluginSpec{
495+
ServiceAccountName: devicePluginServiceAccountName,
496+
},
406497
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
498+
ServiceAccountName: moduleLoaderServiceAccountName,
407499
Container: kmmv1beta1.ModuleLoaderContainerSpec{
408500
KernelMappings: mappings,
409501
},
@@ -412,7 +504,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
412504
},
413505
}
414506

415-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
507+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
416508

417509
ds := appsv1.DaemonSet{
418510
ObjectMeta: metav1.ObjectMeta{
@@ -462,6 +554,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
462554
ctrl *gomock.Controller
463555
clnt *client.MockClient
464556
mockBM *build.MockManager
557+
mockRC *rbac.MockRBACCreator
465558
mockDC *daemonset.MockDaemonSetCreator
466559
mockKM *module.MockKernelMapper
467560
mockMetrics *metrics.MockMetrics
@@ -473,6 +566,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
473566
ctrl = gomock.NewController(GinkgoT())
474567
clnt = client.NewMockClient(ctrl)
475568
mockBM = build.NewMockManager(ctrl)
569+
mockRC = rbac.NewMockRBACCreator(ctrl)
476570
mockDC = daemonset.NewMockDaemonSetCreator(ctrl)
477571
mockKM = module.NewMockKernelMapper(ctrl)
478572
mockMetrics = metrics.NewMockMetrics(ctrl)
@@ -494,7 +588,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
494588

495589
mod := &kmmv1beta1.Module{}
496590

497-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
591+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
498592

499593
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
500594
Expect(err).NotTo(HaveOccurred())
@@ -524,7 +618,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
524618
},
525619
)
526620

527-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
621+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
528622
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
529623
Expect(err).NotTo(HaveOccurred())
530624
Expect(res).To(BeFalse())
@@ -550,7 +644,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
550644
mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false),
551645
)
552646

553-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
647+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
554648
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
555649
Expect(err).NotTo(HaveOccurred())
556650
Expect(res).To(BeTrue())
@@ -576,7 +670,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
576670
mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false),
577671
)
578672

579-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
673+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
580674
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
581675
Expect(err).NotTo(HaveOccurred())
582676
Expect(res).To(BeTrue())
@@ -602,7 +696,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
602696
mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, true),
603697
)
604698

605-
mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
699+
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
606700
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
607701
Expect(err).NotTo(HaveOccurred())
608702
Expect(res).To(BeFalse())

0 commit comments

Comments
 (0)