Skip to content

Commit e5261ca

Browse files
mresvanismresvanis
authored
Add module loader and device plugin SAs when not defined (kubernetes-sigs#90) (#56)
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]> Signed-off-by: Michail Resvanis <[email protected]>
1 parent 9c5cde5 commit e5261ca

File tree

10 files changed

+594
-9
lines changed

10 files changed

+594
-9
lines changed

‎config/rbac/role.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,12 @@ rules:
6666
resources:
6767
- serviceaccounts
6868
verbs:
69+
- create
70+
- delete
6971
- get
72+
- list
73+
- patch
74+
- watch
7075
- apiGroups:
7176
- kmm.sigs.k8s.io
7277
resources:

‎controllers/module_reconciler.go

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

5354
authFactory auth.RegistryAuthGetterFactory
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,
@@ -74,6 +77,7 @@ func NewModuleReconciler(
7477
Client: client,
7578
authFactory: authFactory,
7679
buildAPI: buildAPI,
80+
rbacAPI: rbacAPI,
7781
daemonAPI: daemonAPI,
7882
kernelAPI: kernelAPI,
7983
metricsAPI: metricsAPI,
@@ -89,7 +93,7 @@ func NewModuleReconciler(
8993
//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;patch;watch
9094
//+kubebuilder:rbac:groups="core",resources=nodes,verbs=get;list;watch
9195
//+kubebuilder:rbac:groups="core",resources=secrets,verbs=get;list;watch
92-
//+kubebuilder:rbac:groups="core",resources=serviceaccounts,verbs=get
96+
//+kubebuilder:rbac:groups="core",resources=serviceaccounts,verbs=create;delete;get;list;patch;watch
9397
//+kubebuilder:rbac:groups="build.openshift.io",resources=buildconfigs,verbs=create;delete;get;list;patch;watch
9498
//+kubebuilder:rbac:groups="build.openshift.io",resources=builds,verbs=get;list;watch
9599

@@ -113,6 +117,17 @@ func (r *ModuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
113117

114118
r.setKMMOMetrics(ctx)
115119

120+
if mod.Spec.ModuleLoader.ServiceAccountName == "" {
121+
if err := r.rbacAPI.CreateModuleLoaderServiceAccount(ctx, *mod); err != nil {
122+
return res, fmt.Errorf("could not create module-loader's ServiceAccount: %w", err)
123+
}
124+
}
125+
if mod.Spec.DevicePlugin != nil && mod.Spec.DevicePlugin.ServiceAccountName == "" {
126+
if err := r.rbacAPI.CreateDevicePluginServiceAccount(ctx, *mod); err != nil {
127+
return res, fmt.Errorf("could not create device-plugin's ServiceAccount: %w", err)
128+
}
129+
}
130+
116131
targetedNodes, err := r.getNodesListBySelector(ctx, mod)
117132
if err != nil {
118133
return res, fmt.Errorf("could get targeted nodes for module %s: %w", mod.Name, err)
@@ -368,6 +383,7 @@ func (r *ModuleReconciler) SetupWithManager(mgr ctrl.Manager, kernelLabel string
368383
For(&kmmv1beta1.Module{}).
369384
Owns(&appsv1.DaemonSet{}).
370385
Owns(&buildv1.BuildConfig{}).
386+
Owns(&v1.ServiceAccount{}).
371387
Watches(
372388
&source.Kind{Type: &v1.Node{}},
373389
handler.EnqueueRequestsFromMapFunc(r.filter.FindModulesForNode),

‎controllers/module_reconciler_test.go

+122-6
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/rh-ecosystem-edge/kernel-module-management/internal/daemonset"
1414
"github.com/rh-ecosystem-edge/kernel-module-management/internal/metrics"
1515
"github.com/rh-ecosystem-edge/kernel-module-management/internal/module"
16+
"github.com/rh-ecosystem-edge/kernel-module-management/internal/rbac"
1617
"github.com/rh-ecosystem-edge/kernel-module-management/internal/registry"
1718
"github.com/rh-ecosystem-edge/kernel-module-management/internal/statusupdater"
1819
appsv1 "k8s.io/api/apps/v1"
@@ -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)
@@ -74,6 +77,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
7477
mr := NewModuleReconciler(
7578
clnt,
7679
mockBM,
80+
mockRC,
7781
mockDC,
7882
mockKM,
7983
mockMetrics,
@@ -89,14 +93,97 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
8993
)
9094
})
9195

96+
It("should add the module loader and device plugin ServiceAccounts if they are not set", func() {
97+
mod := kmmv1beta1.Module{
98+
ObjectMeta: metav1.ObjectMeta{
99+
Name: moduleName,
100+
Namespace: namespace,
101+
},
102+
Spec: kmmv1beta1.ModuleSpec{
103+
Selector: map[string]string{"key": "value"},
104+
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
105+
ServiceAccountName: "",
106+
},
107+
DevicePlugin: &kmmv1beta1.DevicePluginSpec{
108+
ServiceAccountName: "",
109+
},
110+
},
111+
}
112+
ds := appsv1.DaemonSet{
113+
ObjectMeta: metav1.ObjectMeta{
114+
Name: moduleName + "-device-plugin",
115+
Namespace: namespace,
116+
},
117+
}
118+
119+
gomock.InOrder(
120+
clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn(
121+
func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error {
122+
m.ObjectMeta = mod.ObjectMeta
123+
m.Spec = mod.Spec
124+
return nil
125+
},
126+
),
127+
clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
128+
func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error {
129+
list.Items = []kmmv1beta1.Module{mod}
130+
return nil
131+
},
132+
),
133+
mockMetrics.EXPECT().SetExistingKMMOModules(1),
134+
mockRC.EXPECT().CreateModuleLoaderServiceAccount(ctx, gomock.Any()).Return(nil),
135+
mockRC.EXPECT().CreateDevicePluginServiceAccount(ctx, gomock.Any()).Return(nil),
136+
clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
137+
func(_ interface{}, list *v1.NodeList, _ ...interface{}) error {
138+
list.Items = []v1.Node{}
139+
return nil
140+
},
141+
),
142+
clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")),
143+
clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")),
144+
mockDC.EXPECT().SetDevicePluginAsDesired(context.Background(), &ds, gomock.AssignableToTypeOf(&mod)),
145+
clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil),
146+
mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, "", metrics.DevicePluginStage, false),
147+
)
148+
149+
mr := NewModuleReconciler(
150+
clnt,
151+
mockBM,
152+
mockRC,
153+
mockDC,
154+
mockKM,
155+
mockMetrics,
156+
nil,
157+
mockRegistry,
158+
nil,
159+
mockSU)
160+
161+
dsByKernelVersion := make(map[string]*appsv1.DaemonSet)
162+
163+
gomock.InOrder(
164+
mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil),
165+
mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()),
166+
mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil),
167+
)
168+
169+
res, err := mr.Reconcile(context.Background(), req)
170+
Expect(err).NotTo(HaveOccurred())
171+
Expect(res).To(Equal(reconcile.Result{}))
172+
})
173+
92174
It("should do nothing when no nodes match the selector", func() {
175+
const serviceAccountName = "module-loader-service-account"
176+
93177
mod := kmmv1beta1.Module{
94178
ObjectMeta: metav1.ObjectMeta{
95179
Name: moduleName,
96180
Namespace: namespace,
97181
},
98182
Spec: kmmv1beta1.ModuleSpec{
99183
Selector: map[string]string{"key": "value"},
184+
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
185+
ServiceAccountName: serviceAccountName,
186+
},
100187
},
101188
}
102189

@@ -126,6 +213,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
126213
mr := NewModuleReconciler(
127214
clnt,
128215
mockBM,
216+
mockRC,
129217
mockDC,
130218
mockKM,
131219
mockMetrics,
@@ -149,7 +237,10 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
149237
})
150238

151239
It("should remove obsolete DaemonSets when no nodes match the selector", func() {
152-
const kernelVersion = "1.2.3"
240+
const (
241+
kernelVersion = "1.2.3"
242+
serviceAccountName = "module-loader-service-account"
243+
)
153244

154245
mod := kmmv1beta1.Module{
155246
ObjectMeta: metav1.ObjectMeta{
@@ -158,6 +249,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
158249
},
159250
Spec: kmmv1beta1.ModuleSpec{
160251
Selector: map[string]string{"key": "value"},
252+
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
253+
ServiceAccountName: serviceAccountName,
254+
},
161255
},
162256
}
163257

@@ -194,6 +288,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
194288
mr := NewModuleReconciler(
195289
clnt,
196290
mockBM,
291+
mockRC,
197292
mockDC,
198293
mockKM,
199294
mockMetrics,
@@ -218,8 +313,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
218313

219314
It("should create a DaemonSet when a node matches the selector", func() {
220315
const (
221-
imageName = "test-image"
222-
kernelVersion = "1.2.3"
316+
imageName = "test-image"
317+
kernelVersion = "1.2.3"
318+
serviceAccountName = "module-loader-service-account"
223319
)
224320

225321
mappings := []kmmv1beta1.KernelMapping{
@@ -238,6 +334,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
238334
},
239335
Spec: kmmv1beta1.ModuleSpec{
240336
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
337+
ServiceAccountName: serviceAccountName,
241338
Container: kmmv1beta1.ModuleLoaderContainerSpec{
242339
KernelMappings: mappings,
243340
},
@@ -265,6 +362,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
265362
mr := NewModuleReconciler(
266363
clnt,
267364
mockBM,
365+
mockRC,
268366
mockDC,
269367
mockKM,
270368
mockMetrics,
@@ -320,8 +418,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
320418

321419
It("should patch the DaemonSet when it already exists", func() {
322420
const (
323-
imageName = "test-image"
324-
kernelVersion = "1.2.3"
421+
imageName = "test-image"
422+
kernelVersion = "1.2.3"
423+
serviceAccountName = "module-loader-service-account"
325424
)
326425

327426
osConfig := module.NodeOSConfig{}
@@ -342,6 +441,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
342441
},
343442
Spec: kmmv1beta1.ModuleSpec{
344443
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
444+
ServiceAccountName: serviceAccountName,
345445
Container: kmmv1beta1.ModuleLoaderContainerSpec{
346446
KernelMappings: mappings,
347447
},
@@ -404,6 +504,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
404504
mr := NewModuleReconciler(
405505
clnt,
406506
mockBM,
507+
mockRC,
407508
mockDC,
408509
mockKM,
409510
mockMetrics,
@@ -437,6 +538,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
437538
const (
438539
imageName = "test-image"
439540
kernelVersion = "1.2.3"
541+
542+
moduleLoaderServiceAccountName = "module-loader-service-account"
543+
devicePluginServiceAccountName = "device-plugin-service-account"
440544
)
441545

442546
mappings := []kmmv1beta1.KernelMapping{
@@ -452,8 +556,11 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
452556
Namespace: namespace,
453557
},
454558
Spec: kmmv1beta1.ModuleSpec{
455-
DevicePlugin: &kmmv1beta1.DevicePluginSpec{},
559+
DevicePlugin: &kmmv1beta1.DevicePluginSpec{
560+
ServiceAccountName: devicePluginServiceAccountName,
561+
},
456562
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
563+
ServiceAccountName: moduleLoaderServiceAccountName,
457564
Container: kmmv1beta1.ModuleLoaderContainerSpec{
458565
KernelMappings: mappings,
459566
},
@@ -465,6 +572,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
465572
mr := NewModuleReconciler(
466573
clnt,
467574
mockBM,
575+
mockRC,
468576
mockDC,
469577
mockKM,
470578
mockMetrics,
@@ -524,6 +632,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
524632
clnt *client.MockClient
525633
mockAuthFactory *auth.MockRegistryAuthGetterFactory
526634
mockBM *build.MockManager
635+
mockRC *rbac.MockRBACCreator
527636
mockDC *daemonset.MockDaemonSetCreator
528637
mockKM *module.MockKernelMapper
529638
mockMetrics *metrics.MockMetrics
@@ -536,6 +645,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
536645
clnt = client.NewMockClient(ctrl)
537646
mockAuthFactory = auth.NewMockRegistryAuthGetterFactory(ctrl)
538647
mockBM = build.NewMockManager(ctrl)
648+
mockRC = rbac.NewMockRBACCreator(ctrl)
539649
mockDC = daemonset.NewMockDaemonSetCreator(ctrl)
540650
mockKM = module.NewMockKernelMapper(ctrl)
541651
mockMetrics = metrics.NewMockMetrics(ctrl)
@@ -560,6 +670,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
560670
mr := NewModuleReconciler(
561671
clnt,
562672
mockBM,
673+
mockRC,
563674
mockDC,
564675
mockKM,
565676
mockMetrics,
@@ -610,6 +721,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
610721
mr := NewModuleReconciler(
611722
clnt,
612723
mockBM,
724+
mockRC,
613725
mockDC,
614726
mockKM,
615727
mockMetrics,
@@ -653,6 +765,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
653765
mr := NewModuleReconciler(
654766
clnt,
655767
mockBM,
768+
mockRC,
656769
mockDC,
657770
mockKM,
658771
mockMetrics,
@@ -694,6 +807,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
694807
mr := NewModuleReconciler(
695808
clnt,
696809
mockBM,
810+
mockRC,
697811
mockDC,
698812
mockKM,
699813
mockMetrics,
@@ -735,6 +849,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
735849
mr := NewModuleReconciler(
736850
clnt,
737851
mockBM,
852+
mockRC,
738853
mockDC,
739854
mockKM,
740855
mockMetrics,
@@ -776,6 +891,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
776891
mr := NewModuleReconciler(
777892
clnt,
778893
mockBM,
894+
mockRC,
779895
mockDC,
780896
mockKM,
781897
mockMetrics,

0 commit comments

Comments
 (0)