Skip to content

Add ServiceAccounts for the ModuleLoader and DevicePlugin workloads #90

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ rules:
- get
- list
- watch
- apiGroups:
- ""
resources:
- serviceaccounts
verbs:
- create
- delete
- get
- list
- patch
- watch
- apiGroups:
- kmm.sigs.k8s.io
resources:
Expand Down
17 changes: 17 additions & 0 deletions controllers/module_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
"github.com/kubernetes-sigs/kernel-module-management/internal/metrics"
"github.com/kubernetes-sigs/kernel-module-management/internal/module"
"github.com/kubernetes-sigs/kernel-module-management/internal/rbac"
"github.com/kubernetes-sigs/kernel-module-management/internal/registry"
"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
appsv1 "k8s.io/api/apps/v1"
Expand All @@ -52,6 +53,7 @@ type ModuleReconciler struct {
client.Client

buildAPI build.Manager
rbacAPI rbac.RBACCreator
daemonAPI daemonset.DaemonSetCreator
kernelAPI module.KernelMapper
metricsAPI metrics.Metrics
Expand All @@ -63,6 +65,7 @@ type ModuleReconciler struct {
func NewModuleReconciler(
client client.Client,
buildAPI build.Manager,
rbacAPI rbac.RBACCreator,
daemonAPI daemonset.DaemonSetCreator,
kernelAPI module.KernelMapper,
metricsAPI metrics.Metrics,
Expand All @@ -72,6 +75,7 @@ func NewModuleReconciler(
return &ModuleReconciler{
Client: client,
buildAPI: buildAPI,
rbacAPI: rbacAPI,
daemonAPI: daemonAPI,
kernelAPI: kernelAPI,
metricsAPI: metricsAPI,
Expand All @@ -87,6 +91,7 @@ func NewModuleReconciler(
//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;patch;watch
//+kubebuilder:rbac:groups="core",resources=nodes,verbs=get;list;watch
//+kubebuilder:rbac:groups="core",resources=secrets,verbs=get;list;watch
//+kubebuilder:rbac:groups="core",resources=serviceaccounts,verbs=create;delete;get;list;patch;watch
//+kubebuilder:rbac:groups="batch",resources=jobs,verbs=create;list;watch;delete

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

r.setKMMOMetrics(ctx)

if mod.Spec.ModuleLoader.ServiceAccountName == "" {
if err := r.rbacAPI.CreateModuleLoaderServiceAccount(ctx, *mod); err != nil {
return res, fmt.Errorf("could not create module-loader's ServiceAccount: %w", err)
}
}
if mod.Spec.DevicePlugin != nil && mod.Spec.DevicePlugin.ServiceAccountName == "" {
if err := r.rbacAPI.CreateDevicePluginServiceAccount(ctx, *mod); err != nil {
return res, fmt.Errorf("could not create device-plugin's ServiceAccount: %w", err)
}
}

targetedNodes, err := r.getNodesListBySelector(ctx, mod)
if err != nil {
return res, fmt.Errorf("could get targeted nodes for module %s: %w", mod.Name, err)
Expand Down Expand Up @@ -359,6 +375,7 @@ func (r *ModuleReconciler) SetupWithManager(mgr ctrl.Manager, kernelLabel string
return ctrl.NewControllerManagedBy(mgr).
For(&kmmv1beta1.Module{}).
Owns(&appsv1.DaemonSet{}).
Owns(&v1.ServiceAccount{}).
Owns(&batchv1.Job{}).
Watches(
&source.Kind{Type: &v1.Node{}},
Expand Down
128 changes: 111 additions & 17 deletions controllers/module_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/kubernetes-sigs/kernel-module-management/internal/daemonset"
"github.com/kubernetes-sigs/kernel-module-management/internal/metrics"
"github.com/kubernetes-sigs/kernel-module-management/internal/module"
"github.com/kubernetes-sigs/kernel-module-management/internal/rbac"
"github.com/kubernetes-sigs/kernel-module-management/internal/registry"
"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
. "github.com/onsi/ginkgo/v2"
Expand All @@ -34,6 +35,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
ctrl *gomock.Controller
clnt *client.MockClient
mockBM *build.MockManager
mockRC *rbac.MockRBACCreator
mockDC *daemonset.MockDaemonSetCreator
mockKM *module.MockKernelMapper
mockMetrics *metrics.MockMetrics
Expand All @@ -45,6 +47,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
ctrl = gomock.NewController(GinkgoT())
clnt = client.NewMockClient(ctrl)
mockBM = build.NewMockManager(ctrl)
mockRC = rbac.NewMockRBACCreator(ctrl)
mockDC = daemonset.NewMockDaemonSetCreator(ctrl)
mockKM = module.NewMockKernelMapper(ctrl)
mockMetrics = metrics.NewMockMetrics(ctrl)
Expand All @@ -71,22 +74,95 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
apierrors.NewNotFound(schema.GroupResource{}, moduleName),
)

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
Expect(
mr.Reconcile(ctx, req),
).To(
Equal(reconcile.Result{}),
)
})

It("should add the module loader and device plugin ServiceAccounts if they are not set", func() {
mod := kmmv1beta1.Module{
ObjectMeta: metav1.ObjectMeta{
Name: moduleName,
Namespace: namespace,
},
Spec: kmmv1beta1.ModuleSpec{
Selector: map[string]string{"key": "value"},
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
ServiceAccountName: "",
},
DevicePlugin: &kmmv1beta1.DevicePluginSpec{
ServiceAccountName: "",
},
},
}
ds := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: moduleName + "-device-plugin",
Namespace: namespace,
},
}

gomock.InOrder(
clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn(
func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error {
m.ObjectMeta = mod.ObjectMeta
m.Spec = mod.Spec
return nil
},
),
clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error {
list.Items = []kmmv1beta1.Module{mod}
return nil
},
),
mockMetrics.EXPECT().SetExistingKMMOModules(1),
mockRC.EXPECT().CreateModuleLoaderServiceAccount(ctx, gomock.Any()).Return(nil),
mockRC.EXPECT().CreateDevicePluginServiceAccount(ctx, gomock.Any()).Return(nil),
clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
func(_ interface{}, list *v1.NodeList, _ ...interface{}) error {
list.Items = []v1.Node{}
return nil
},
),
clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")),
clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")),
mockDC.EXPECT().SetDevicePluginAsDesired(context.Background(), &ds, gomock.AssignableToTypeOf(&mod)),
clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil),
mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, "", metrics.DevicePluginStage, false),
)

mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)

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

gomock.InOrder(
mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil),
mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()),
mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil),
)

res, err := mr.Reconcile(context.Background(), req)
Expect(err).NotTo(HaveOccurred())
Expect(res).To(Equal(reconcile.Result{}))
})

It("should do nothing when no nodes match the selector", func() {
const serviceAccountName = "module-loader-service-account"

mod := kmmv1beta1.Module{
ObjectMeta: metav1.ObjectMeta{
Name: moduleName,
Namespace: namespace,
},
Spec: kmmv1beta1.ModuleSpec{
Selector: map[string]string{"key": "value"},
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
ServiceAccountName: serviceAccountName,
},
},
}

Expand All @@ -113,7 +189,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
),
)

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)

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

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

It("should remove obsolete DaemonSets when no nodes match the selector", func() {
const kernelVersion = "1.2.3"
const (
kernelVersion = "1.2.3"
serviceAccountName = "module-loader-service-account"
)

mod := kmmv1beta1.Module{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -138,6 +217,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
},
Spec: kmmv1beta1.ModuleSpec{
Selector: map[string]string{"key": "value"},
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
ServiceAccountName: serviceAccountName,
},
},
}

Expand Down Expand Up @@ -171,7 +253,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
),
)

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)

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

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

It("should create a DaemonSet when a node matches the selector", func() {
const (
imageName = "test-image"
kernelVersion = "1.2.3"
imageName = "test-image"
kernelVersion = "1.2.3"
serviceAccountName = "module-loader-service-account"
)

mappings := []kmmv1beta1.KernelMapping{
Expand All @@ -208,6 +291,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
},
Spec: kmmv1beta1.ModuleSpec{
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
ServiceAccountName: serviceAccountName,
Container: kmmv1beta1.ModuleLoaderContainerSpec{
KernelMappings: mappings,
},
Expand All @@ -232,7 +316,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {

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

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)

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

It("should patch the DaemonSet when it already exists", func() {
const (
imageName = "test-image"
kernelVersion = "1.2.3"
imageName = "test-image"
kernelVersion = "1.2.3"
serviceAccountName = "module-loader-service-account"
)

osConfig := module.NodeOSConfig{}
Expand All @@ -302,6 +387,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
},
Spec: kmmv1beta1.ModuleSpec{
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
ServiceAccountName: serviceAccountName,
Container: kmmv1beta1.ModuleLoaderContainerSpec{
KernelMappings: mappings,
},
Expand Down Expand Up @@ -361,7 +447,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()),
)

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)

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

Expand All @@ -387,6 +473,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
const (
imageName = "test-image"
kernelVersion = "1.2.3"

moduleLoaderServiceAccountName = "module-loader-service-account"
devicePluginServiceAccountName = "device-plugin-service-account"
)

mappings := []kmmv1beta1.KernelMapping{
Expand All @@ -402,8 +491,11 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
Namespace: namespace,
},
Spec: kmmv1beta1.ModuleSpec{
DevicePlugin: &kmmv1beta1.DevicePluginSpec{},
DevicePlugin: &kmmv1beta1.DevicePluginSpec{
ServiceAccountName: devicePluginServiceAccountName,
},
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
ServiceAccountName: moduleLoaderServiceAccountName,
Container: kmmv1beta1.ModuleLoaderContainerSpec{
KernelMappings: mappings,
},
Expand All @@ -412,7 +504,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
},
}

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)

ds := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -462,6 +554,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
ctrl *gomock.Controller
clnt *client.MockClient
mockBM *build.MockManager
mockRC *rbac.MockRBACCreator
mockDC *daemonset.MockDaemonSetCreator
mockKM *module.MockKernelMapper
mockMetrics *metrics.MockMetrics
Expand All @@ -473,6 +566,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
ctrl = gomock.NewController(GinkgoT())
clnt = client.NewMockClient(ctrl)
mockBM = build.NewMockManager(ctrl)
mockRC = rbac.NewMockRBACCreator(ctrl)
mockDC = daemonset.NewMockDaemonSetCreator(ctrl)
mockKM = module.NewMockKernelMapper(ctrl)
mockMetrics = metrics.NewMockMetrics(ctrl)
Expand All @@ -494,7 +588,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {

mod := &kmmv1beta1.Module{}

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)

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

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

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

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

mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
mr := NewModuleReconciler(clnt, mockBM, mockRC, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU)
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
Expect(err).NotTo(HaveOccurred())
Expect(res).To(BeFalse())
Expand Down
Loading