Skip to content

Commit

Permalink
Make Module a namespaced resource (#45)
Browse files Browse the repository at this point in the history
Create all resources in the module's namespace.
Adapt all tests.
  • Loading branch information
qbarrand authored Apr 14, 2022
1 parent f957c2e commit 1567f4f
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 84 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha1/module_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ type ModuleStatus struct {
}

//+kubebuilder:object:root=true
//+kubebuilder:resource:scope=Cluster
//+kubebuilder:resource:scope=Namespaced
//+kubebuilder:subresource:status

// Module is the Schema for the modules API
Expand Down
2 changes: 1 addition & 1 deletion config/crd/bases/ooto.sigs.k8s.io_modules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ spec:
listKind: ModuleList
plural: modules
singular: module
scope: Cluster
scope: Namespaced
versions:
- name: v1alpha1
schema:
Expand Down
15 changes: 5 additions & 10 deletions controllers/build/job/maker.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,12 @@ type Maker interface {
}

type maker struct {
mh build.ModuleHelper
namespace string
scheme *runtime.Scheme
mh build.ModuleHelper
scheme *runtime.Scheme
}

func NewMaker(mh build.ModuleHelper, namespace string, scheme *runtime.Scheme) Maker {
return &maker{
mh: mh,
namespace: namespace,
scheme: scheme,
}
func NewMaker(mh build.ModuleHelper, scheme *runtime.Scheme) Maker {
return &maker{mh: mh, scheme: scheme}
}

func (m *maker) MakeJob(mod ootov1alpha1.Module, km ootov1alpha1.KernelMapping, targetKernel string) (*batchv1.Job, error) {
Expand Down Expand Up @@ -80,7 +75,7 @@ func (m *maker) MakeJob(mod ootov1alpha1.Module, km ootov1alpha1.KernelMapping,
job := &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{
GenerateName: mod.Name + "-build-",
Namespace: m.namespace,
Namespace: mod.Namespace,
Labels: Labels(mod, targetKernel),
},
Spec: batchv1.JobSpec{
Expand Down
7 changes: 5 additions & 2 deletions controllers/build/job/maker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ var _ = Describe("Maker", func() {
)

mod := ootov1alpha1.Module{
ObjectMeta: metav1.ObjectMeta{Name: moduleName},
ObjectMeta: metav1.ObjectMeta{
Name: moduleName,
Namespace: namespace,
},
}

buildArgs := []ootov1alpha1.BuildArg{
Expand All @@ -52,7 +55,7 @@ var _ = Describe("Maker", func() {
BeforeEach(func() {
ctrl := gomock.NewController(GinkgoT())
mh = build.NewMockModuleHelper(ctrl)
m = job.NewMaker(mh, namespace, scheme)
m = job.NewMaker(mh, scheme)
})

It("should set fields correctly", func() {
Expand Down
18 changes: 8 additions & 10 deletions controllers/build/job/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,16 @@ import (
var errNoMatchingBuild = errors.New("no matching build")

type jobManager struct {
client client.Client
getter build.Getter
maker Maker
namespace string
client client.Client
getter build.Getter
maker Maker
}

func NewBuildManager(client client.Client, getter build.Getter, maker Maker, namespace string) build.Manager {
func NewBuildManager(client client.Client, getter build.Getter, maker Maker) build.Manager {
return &jobManager{
client: client,
getter: getter,
maker: maker,
namespace: namespace,
client: client,
getter: getter,
maker: maker,
}
}

Expand All @@ -43,7 +41,7 @@ func (jbm *jobManager) getJob(ctx context.Context, mod ootov1alpha1.Module, targ

opts := []client.ListOption{
client.MatchingLabels(Labels(mod, targetKernel)),
client.InNamespace(jbm.namespace),
client.InNamespace(mod.Namespace),
}

if err := jbm.client.List(ctx, &jobList, opts...); err != nil {
Expand Down
10 changes: 5 additions & 5 deletions controllers/build/job/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ var _ = Describe("JobManager", func() {

getter.EXPECT().ImageExists(ctx, imageName, po).Return(false, errors.New("random error"))

mgr := job.NewBuildManager(nil, getter, maker, namespace)
mgr := job.NewBuildManager(nil, getter, maker)

_, err := mgr.Sync(ctx, ootov1alpha1.Module{}, km, "")
Expect(err).To(HaveOccurred())
Expand All @@ -77,7 +77,7 @@ var _ = Describe("JobManager", func() {

getter.EXPECT().ImageExists(ctx, imageName, po).Return(true, nil)

mgr := job.NewBuildManager(nil, getter, maker, namespace)
mgr := job.NewBuildManager(nil, getter, maker)

Expect(
mgr.Sync(ctx, ootov1alpha1.Module{}, km, ""),
Expand Down Expand Up @@ -112,7 +112,7 @@ var _ = Describe("JobManager", func() {

getter.EXPECT().ImageExists(ctx, imageName, po).Return(false, nil)

mgr := job.NewBuildManager(client, getter, maker, namespace)
mgr := job.NewBuildManager(client, getter, maker)

res, err := mgr.Sync(ctx, mod, km, kernelVersion)

Expand All @@ -138,7 +138,7 @@ var _ = Describe("JobManager", func() {
Return(nil, errors.New("random error")),
)

mgr := job.NewBuildManager(fake.NewClientBuilder().Build(), getter, maker, namespace)
mgr := job.NewBuildManager(fake.NewClientBuilder().Build(), getter, maker)

Expect(
mgr.Sync(ctx, mod, km, kernelVersion),
Expand Down Expand Up @@ -170,7 +170,7 @@ var _ = Describe("JobManager", func() {

client := fake.NewClientBuilder().Build()

mgr := job.NewBuildManager(client, getter, maker, namespace)
mgr := job.NewBuildManager(client, getter, maker)

Expect(
mgr.Sync(ctx, mod, km, kernelVersion),
Expand Down
6 changes: 2 additions & 4 deletions controllers/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,13 @@ type DaemonSetCreator interface {
type daemonSetGenerator struct {
client client.Client
kernelLabel string
namespace string
scheme *runtime.Scheme
}

func NewDaemonSetCreator(client client.Client, kernelLabel, namespace string, scheme *runtime.Scheme) *daemonSetGenerator {
func NewDaemonSetCreator(client client.Client, kernelLabel string, scheme *runtime.Scheme) *daemonSetGenerator {
return &daemonSetGenerator{
client: client,
kernelLabel: kernelLabel,
namespace: namespace,
scheme: scheme,
}
}
Expand All @@ -61,7 +59,7 @@ func (dc *daemonSetGenerator) ModuleDaemonSetsByKernelVersion(ctx context.Contex

opts := []client.ListOption{
client.MatchingLabels(map[string]string{constants.ModuleNameLabel: mod.Name}),
client.InNamespace(dc.namespace),
client.InNamespace(mod.Namespace),
}

if err := dc.client.List(ctx, &dsList, opts...); err != nil {
Expand Down
46 changes: 27 additions & 19 deletions controllers/daemonset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@ import (
)

const (
dsNamespace = "ds-namespace"
kernelVersion = "1.2.3"
moduleName = "module-name"
namespace = "namespace"
)

var _ = Describe("daemonSetGenerator", func() {
const kernelLabel = "kernel-label"

Describe("SetAsDesired", func() {
dg := controllers.NewDaemonSetCreator(nil, kernelLabel, "", scheme)
dg := controllers.NewDaemonSetCreator(nil, kernelLabel, scheme)

It("should return an error if the DaemonSet is nil", func() {
Expect(
Expand Down Expand Up @@ -107,7 +107,10 @@ var _ = Describe("daemonSetGenerator", func() {
APIVersion: ootov1alpha1.GroupVersion.String(),
Kind: "Module",
},
ObjectMeta: metav1.ObjectMeta{Name: moduleName},
ObjectMeta: metav1.ObjectMeta{
Name: moduleName,
Namespace: namespace,
},
Spec: ootov1alpha1.ModuleSpec{
DriverContainer: v1.Container{
VolumeMounts: []v1.VolumeMount{dcVolMount},
Expand All @@ -123,7 +126,7 @@ var _ = Describe("daemonSetGenerator", func() {
ds := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: dsName,
Namespace: dsNamespace,
Namespace: namespace,
},
}

Expand All @@ -141,7 +144,7 @@ var _ = Describe("daemonSetGenerator", func() {
expected := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: dsName,
Namespace: dsNamespace,
Namespace: namespace,
Labels: podLabels,
OwnerReferences: []metav1.OwnerReference{
{
Expand Down Expand Up @@ -246,16 +249,16 @@ var _ = Describe("daemonSetGenerator", func() {
)

dsLegit := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: legitName, Namespace: dsNamespace},
ObjectMeta: metav1.ObjectMeta{Name: legitName, Namespace: namespace},
}

dsNotLegit := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Name: notLegitName, Namespace: dsNamespace},
ObjectMeta: metav1.ObjectMeta{Name: notLegitName, Namespace: namespace},
}

client := fake.NewClientBuilder().WithObjects(&dsLegit, &dsNotLegit).Build()

dc := controllers.NewDaemonSetCreator(client, "", dsNamespace, scheme)
dc := controllers.NewDaemonSetCreator(client, "", scheme)

existingDS := map[string]*appsv1.DaemonSet{
legitKernelVersion: &dsLegit,
Expand Down Expand Up @@ -286,7 +289,6 @@ var _ = Describe("daemonSetGenerator", func() {
dc := controllers.NewDaemonSetCreator(
fake.NewClientBuilder().Build(),
"",
dsNamespace,
scheme)

existingDS := map[string]*appsv1.DaemonSet{
Expand All @@ -303,11 +305,13 @@ var _ = Describe("daemonSetGenerator", func() {
dc := controllers.NewDaemonSetCreator(
fake.NewClientBuilder().WithScheme(scheme).Build(),
kernelLabel,
dsNamespace,
scheme)

mod := ootov1alpha1.Module{
ObjectMeta: metav1.ObjectMeta{Name: moduleName},
ObjectMeta: metav1.ObjectMeta{
Name: moduleName,
Namespace: namespace,
},
}

m, err := dc.ModuleDaemonSetsByKernelVersion(context.TODO(), mod)
Expand All @@ -324,27 +328,29 @@ var _ = Describe("daemonSetGenerator", func() {
ds1 := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: "ds1",
Namespace: dsNamespace,
Namespace: namespace,
Labels: dsLabels,
},
}

ds2 := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: "ds2",
Namespace: dsNamespace,
Namespace: namespace,
Labels: dsLabels,
},
}

dc := controllers.NewDaemonSetCreator(
fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ds1, &ds2).Build(),
kernelLabel,
dsNamespace,
scheme)

mod := ootov1alpha1.Module{
ObjectMeta: metav1.ObjectMeta{Name: moduleName},
ObjectMeta: metav1.ObjectMeta{
Name: moduleName,
Namespace: namespace,
},
}

_, err := dc.ModuleDaemonSetsByKernelVersion(context.TODO(), mod)
Expand All @@ -357,7 +363,7 @@ var _ = Describe("daemonSetGenerator", func() {
ds1 := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: "ds1",
Namespace: dsNamespace,
Namespace: namespace,
Labels: map[string]string{
"oot.node.kubernetes.io/module.name": moduleName,
kernelLabel: kernelVersion,
Expand All @@ -368,7 +374,7 @@ var _ = Describe("daemonSetGenerator", func() {
ds2 := appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: "ds2",
Namespace: dsNamespace,
Namespace: namespace,
Labels: map[string]string{
"oot.node.kubernetes.io/module.name": moduleName,
kernelLabel: otherKernelVersion,
Expand All @@ -379,11 +385,13 @@ var _ = Describe("daemonSetGenerator", func() {
dc := controllers.NewDaemonSetCreator(
fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ds1, &ds2).Build(),
kernelLabel,
dsNamespace,
scheme)

mod := ootov1alpha1.Module{
ObjectMeta: metav1.ObjectMeta{Name: moduleName},
ObjectMeta: metav1.ObjectMeta{
Name: moduleName,
Namespace: namespace,
},
}

m, err := dc.ModuleDaemonSetsByKernelVersion(context.TODO(), mod)
Expand Down
23 changes: 10 additions & 13 deletions controllers/module_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,25 @@ import (
type ModuleReconciler struct {
client.Client

namespace string
bm build.Manager
dc DaemonSetCreator
km module.KernelMapper
su module.ConditionsUpdater
bm build.Manager
dc DaemonSetCreator
km module.KernelMapper
su module.ConditionsUpdater
}

func NewModuleReconciler(
client client.Client,
namespace string,
bm build.Manager,
dg DaemonSetCreator,
km module.KernelMapper,
su module.ConditionsUpdater,
) *ModuleReconciler {
return &ModuleReconciler{
Client: client,
bm: bm,
dc: dg,
km: km,
namespace: namespace,
su: su,
Client: client,
bm: bm,
dc: dg,
km: km,
su: su,
}
}

Expand Down Expand Up @@ -159,7 +156,7 @@ func (r *ModuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
}

ds := &appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{Namespace: r.namespace},
ObjectMeta: metav1.ObjectMeta{Namespace: req.Namespace},
}

if existingDS := dsByKernelVersion[kernelVersion]; existingDS != nil {
Expand Down
Loading

0 comments on commit 1567f4f

Please sign in to comment.