Skip to content

Commit 651b463

Browse files
TomerNewmank8s-ci-robot
TomerNewman
authored and
k8s-ci-robot
committed
move GetRelevantBuild and GetRelevantSign from Combiner to kernelMapperHelperAPI
Refactor the module.Combiner interface by removing the GetRelevantBuild and GetRelevantSign methods. These methods are now part of the module.kernelMapperHelperAPI interface. As part of the change, renaming Combiner to BuildArgOverrider.
1 parent ea0bce4 commit 651b463

File tree

15 files changed

+482
-483
lines changed

15 files changed

+482
-483
lines changed

‎cmd/manager-hub/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,15 @@ func main() {
110110
metricsAPI := metrics.New()
111111
metricsAPI.Register()
112112

113-
buildSignCombiner := module.NewCombiner()
114-
resourceManager := buildsignresource.NewResourceManager(client, buildSignCombiner, scheme)
113+
buildArgOverrider := module.NewBuildArgOverrider()
114+
resourceManager := buildsignresource.NewResourceManager(client, buildArgOverrider, scheme)
115115

116116
micAPI := mic.New(client, scheme)
117117
mbscAPI := mbsc.New(client, scheme)
118118
imagePullerAPI := pod.NewImagePuller(client, scheme)
119119
builSignAPI := buildsign.NewManager(client, resourceManager, scheme)
120120

121-
kernelAPI := module.NewKernelMapper(buildSignCombiner)
121+
kernelAPI := module.NewKernelMapper(buildArgOverrider)
122122

123123
ctrlLogger := setupLogger.WithValues("name", hub.ManagedClusterModuleReconcilerName)
124124
ctrlLogger.Info("Adding controller")

‎cmd/manager/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,10 @@ func main() {
115115
metricsAPI := metrics.New()
116116
metricsAPI.Register()
117117

118-
buildSignCombinerAPI := module.NewCombiner()
119-
resourceManager := buildsignresource.NewResourceManager(client, buildSignCombinerAPI, scheme)
118+
buildArgOverriderAPI := module.NewBuildArgOverrider()
119+
resourceManager := buildsignresource.NewResourceManager(client, buildArgOverriderAPI, scheme)
120120
nodeAPI := node.NewNode(client)
121-
kernelAPI := module.NewKernelMapper(buildSignCombinerAPI)
121+
kernelAPI := module.NewKernelMapper(buildArgOverriderAPI)
122122
micAPI := mic.New(client, scheme)
123123
mbscAPI := mbsc.New(client, scheme)
124124
imagePullerAPI := pod.NewImagePuller(client, scheme)

‎internal/buildsign/resource/common.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (rm *resourceManager) buildSpec(mld *api.ModuleLoaderData, destinationImg s
4545
{Name: "MOD_NAME", Value: mld.Name},
4646
{Name: "MOD_NAMESPACE", Value: mld.Namespace},
4747
}
48-
buildArgs := rm.combiner.ApplyBuildArgOverrides(
48+
buildArgs := rm.buildArgOverrider.ApplyBuildArgOverrides(
4949
buildConfig.BuildArgs,
5050
overrides...,
5151
)

‎internal/buildsign/resource/common_test.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,18 @@ var _ = Describe("makeBuildTemplate", func() {
3838
var (
3939
ctrl *gomock.Controller
4040
clnt *client.MockClient
41-
mc *module.MockCombiner
41+
mbao *module.MockBuildArgOverrider
4242
rm *resourceManager
4343
)
4444

4545
BeforeEach(func() {
4646
ctrl = gomock.NewController(GinkgoT())
4747
clnt = client.NewMockClient(ctrl)
48-
mc = module.NewMockCombiner(ctrl)
48+
mbao = module.NewMockBuildArgOverrider(ctrl)
4949
rm = &resourceManager{
50-
client: clnt,
51-
combiner: mc,
52-
scheme: scheme,
50+
client: clnt,
51+
buildArgOverrider: mbao,
52+
scheme: scheme,
5353
}
5454
})
5555

@@ -235,7 +235,7 @@ var _ = Describe("makeBuildTemplate", func() {
235235
expected.SetAnnotations(annotations)
236236

237237
gomock.InOrder(
238-
mc.EXPECT().ApplyBuildArgOverrides(buildArgs, defaultBuildArgs).Return(append(slices.Clone(buildArgs), defaultBuildArgs...)),
238+
mbao.EXPECT().ApplyBuildArgOverrides(buildArgs, defaultBuildArgs).Return(append(slices.Clone(buildArgs), defaultBuildArgs...)),
239239
clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn(
240240
func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error {
241241
cm.Data = dockerfileCMData
@@ -300,7 +300,7 @@ var _ = Describe("makeBuildTemplate", func() {
300300
}
301301

302302
gomock.InOrder(
303-
mc.EXPECT().ApplyBuildArgOverrides(nil, defaultBuildArgs),
303+
mbao.EXPECT().ApplyBuildArgOverrides(nil, defaultBuildArgs),
304304
clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mod.Namespace}, gomock.Any()).DoAndReturn(
305305
func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error {
306306
cm.Data = dockerfileCMData
@@ -380,7 +380,7 @@ var _ = Describe("makeBuildTemplate", func() {
380380
}
381381

382382
gomock.InOrder(
383-
mc.EXPECT().ApplyBuildArgOverrides(buildArgs, defaultBuildArgs),
383+
mbao.EXPECT().ApplyBuildArgOverrides(buildArgs, defaultBuildArgs),
384384
clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn(
385385
func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error {
386386
cm.Data = dockerfileCMData
@@ -418,7 +418,7 @@ var _ = Describe("makeBuildTemplate", func() {
418418
expectedImageName := mld.ContainerImage + ":" + mld.Namespace + "_" + mld.Name + "_kmm_unsigned"
419419

420420
gomock.InOrder(
421-
mc.EXPECT().ApplyBuildArgOverrides(buildArgs, defaultBuildArgs),
421+
mbao.EXPECT().ApplyBuildArgOverrides(buildArgs, defaultBuildArgs),
422422
clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn(
423423
func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error {
424424
cm.Data = dockerfileCMData
@@ -467,11 +467,11 @@ COPY --from=signimage /tmp/signroot/modules/simple-procfs-kmod.ko /modules/simpl
467467
)
468468

469469
var (
470-
ctrl *gomock.Controller
471-
clnt *client.MockClient
472-
mld api.ModuleLoaderData
473-
mockCombiner *module.MockCombiner
474-
rm *resourceManager
470+
ctrl *gomock.Controller
471+
clnt *client.MockClient
472+
mld api.ModuleLoaderData
473+
mockBuildArgOverrider *module.MockBuildArgOverrider
474+
rm *resourceManager
475475

476476
filesToSign = []string{
477477
"/modules/simple-kmod.ko",
@@ -482,11 +482,11 @@ COPY --from=signimage /tmp/signroot/modules/simple-procfs-kmod.ko /modules/simpl
482482
BeforeEach(func() {
483483
ctrl = gomock.NewController(GinkgoT())
484484
clnt = client.NewMockClient(ctrl)
485-
mockCombiner = module.NewMockCombiner(ctrl)
485+
mockBuildArgOverrider = module.NewMockBuildArgOverrider(ctrl)
486486
rm = &resourceManager{
487-
client: clnt,
488-
combiner: mockCombiner,
489-
scheme: scheme,
487+
client: clnt,
488+
buildArgOverrider: mockBuildArgOverrider,
489+
scheme: scheme,
490490
}
491491
mld = api.ModuleLoaderData{
492492
Name: moduleName,

‎internal/buildsign/resource/resourcemanager.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ const (
2323
)
2424

2525
type resourceManager struct {
26-
client client.Client
27-
combiner module.Combiner
28-
scheme *runtime.Scheme
26+
client client.Client
27+
buildArgOverrider module.BuildArgOverrider
28+
scheme *runtime.Scheme
2929
}
3030

31-
func NewResourceManager(client client.Client, combiner module.Combiner,
31+
func NewResourceManager(client client.Client, buildArgOverrider module.BuildArgOverrider,
3232
scheme *runtime.Scheme) buildsign.ResourceManager {
3333

3434
return &resourceManager{
35-
client: client,
36-
combiner: combiner,
37-
scheme: scheme,
35+
client: client,
36+
buildArgOverrider: buildArgOverrider,
37+
scheme: scheme,
3838
}
3939
}
4040

‎internal/buildsign/resource/resourcemanager_test.go‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,17 @@ import (
2222

2323
var _ = Describe("GetResourceByKernel", func() {
2424
var (
25-
ctrl *gomock.Controller
26-
clnt *client.MockClient
27-
rm buildsign.ResourceManager
28-
mockCombiner *module.MockCombiner
25+
ctrl *gomock.Controller
26+
clnt *client.MockClient
27+
rm buildsign.ResourceManager
28+
mockBuildArgOverrider *module.MockBuildArgOverrider
2929
)
3030

3131
BeforeEach(func() {
3232
ctrl = gomock.NewController(GinkgoT())
3333
clnt = client.NewMockClient(ctrl)
34-
mockCombiner = module.NewMockCombiner(ctrl)
35-
rm = NewResourceManager(clnt, mockCombiner, scheme)
34+
mockBuildArgOverrider = module.NewMockBuildArgOverrider(ctrl)
35+
rm = NewResourceManager(clnt, mockBuildArgOverrider, scheme)
3636
})
3737

3838
It("should return only one pod", func() {
@@ -190,16 +190,16 @@ var _ = Describe("GetResourceByKernel", func() {
190190

191191
var _ = Describe("GetModuleResources", func() {
192192
var (
193-
ctrl *gomock.Controller
194-
clnt *client.MockClient
195-
rm buildsign.ResourceManager
196-
mockCombiner *module.MockCombiner
193+
ctrl *gomock.Controller
194+
clnt *client.MockClient
195+
rm buildsign.ResourceManager
196+
mockBuildArgOverrider *module.MockBuildArgOverrider
197197
)
198198

199199
BeforeEach(func() {
200200
ctrl = gomock.NewController(GinkgoT())
201201
clnt = client.NewMockClient(ctrl)
202-
rm = NewResourceManager(clnt, mockCombiner, scheme)
202+
rm = NewResourceManager(clnt, mockBuildArgOverrider, scheme)
203203
})
204204

205205
It("return all found pods", func() {
@@ -300,16 +300,16 @@ var _ = Describe("GetModuleResources", func() {
300300

301301
var _ = Describe("DeleteResource", func() {
302302
var (
303-
ctrl *gomock.Controller
304-
clnt *client.MockClient
305-
rm buildsign.ResourceManager
306-
mockCombiner *module.MockCombiner
303+
ctrl *gomock.Controller
304+
clnt *client.MockClient
305+
rm buildsign.ResourceManager
306+
mockBuildArgOverrider *module.MockBuildArgOverrider
307307
)
308308

309309
BeforeEach(func() {
310310
ctrl = gomock.NewController(GinkgoT())
311311
clnt = client.NewMockClient(ctrl)
312-
rm = NewResourceManager(clnt, mockCombiner, scheme)
312+
rm = NewResourceManager(clnt, mockBuildArgOverrider, scheme)
313313
})
314314

315315
It("good flow", func() {
@@ -345,16 +345,16 @@ var _ = Describe("DeleteResource", func() {
345345

346346
var _ = Describe("CreateResource", func() {
347347
var (
348-
ctrl *gomock.Controller
349-
clnt *client.MockClient
350-
rm buildsign.ResourceManager
351-
mockCombiner *module.MockCombiner
348+
ctrl *gomock.Controller
349+
clnt *client.MockClient
350+
rm buildsign.ResourceManager
351+
mockBuildArgOverrider *module.MockBuildArgOverrider
352352
)
353353

354354
BeforeEach(func() {
355355
ctrl = gomock.NewController(GinkgoT())
356356
clnt = client.NewMockClient(ctrl)
357-
rm = NewResourceManager(clnt, mockCombiner, scheme)
357+
rm = NewResourceManager(clnt, mockBuildArgOverrider, scheme)
358358
})
359359

360360
It("good flow", func() {
@@ -384,16 +384,16 @@ var _ = Describe("CreateResource", func() {
384384

385385
var _ = Describe("PodStatus", func() {
386386
var (
387-
ctrl *gomock.Controller
388-
clnt *client.MockClient
389-
rm buildsign.ResourceManager
390-
mockCombiner *module.MockCombiner
387+
ctrl *gomock.Controller
388+
clnt *client.MockClient
389+
rm buildsign.ResourceManager
390+
mockBuildArgOverrider *module.MockBuildArgOverrider
391391
)
392392

393393
BeforeEach(func() {
394394
ctrl = gomock.NewController(GinkgoT())
395395
clnt = client.NewMockClient(ctrl)
396-
rm = NewResourceManager(clnt, mockCombiner, scheme)
396+
rm = NewResourceManager(clnt, mockBuildArgOverrider, scheme)
397397
})
398398

399399
DescribeTable("should return the correct status depending on the pod status",
@@ -417,16 +417,16 @@ var _ = Describe("PodStatus", func() {
417417

418418
var _ = Describe("IsPodChnaged", func() {
419419
var (
420-
ctrl *gomock.Controller
421-
clnt *client.MockClient
422-
rm buildsign.ResourceManager
423-
mockCombiner *module.MockCombiner
420+
ctrl *gomock.Controller
421+
clnt *client.MockClient
422+
rm buildsign.ResourceManager
423+
mockBuildArgOverrider *module.MockBuildArgOverrider
424424
)
425425

426426
BeforeEach(func() {
427427
ctrl = gomock.NewController(GinkgoT())
428428
clnt = client.NewMockClient(ctrl)
429-
rm = NewResourceManager(clnt, mockCombiner, scheme)
429+
rm = NewResourceManager(clnt, mockBuildArgOverrider, scheme)
430430
})
431431

432432
DescribeTable("should detect if a pod has changed",

‎internal/module/buildargoverrider.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package module
2+
3+
import (
4+
"k8s.io/apimachinery/pkg/util/sets"
5+
6+
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
7+
)
8+
9+
//go:generate mockgen -source=buildargoverrider.go -package=module -destination=mock_buildargoverrider.go
10+
11+
type BuildArgOverrider interface {
12+
ApplyBuildArgOverrides(args []kmmv1beta1.BuildArg, overrides ...kmmv1beta1.BuildArg) []kmmv1beta1.BuildArg
13+
}
14+
15+
type buildArgOverrider struct{}
16+
17+
func NewBuildArgOverrider() BuildArgOverrider {
18+
return &buildArgOverrider{}
19+
}
20+
21+
func (c *buildArgOverrider) ApplyBuildArgOverrides(args []kmmv1beta1.BuildArg, overrides ...kmmv1beta1.BuildArg) []kmmv1beta1.BuildArg {
22+
overridesMap := make(map[string]kmmv1beta1.BuildArg, len(overrides))
23+
24+
for _, o := range overrides {
25+
overridesMap[o.Name] = o
26+
}
27+
28+
unusedOverrides := sets.StringKeySet(overridesMap)
29+
30+
for i := 0; i < len(args); i++ {
31+
argName := args[i].Name
32+
33+
if o, ok := overridesMap[argName]; ok {
34+
args[i] = o
35+
unusedOverrides.Delete(argName)
36+
}
37+
}
38+
39+
for _, overrideName := range unusedOverrides.List() {
40+
args = append(args, overridesMap[overrideName])
41+
}
42+
43+
return args
44+
}

‎internal/module/buildargoverrider_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package module
2+
3+
import (
4+
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
5+
. "github.com/onsi/ginkgo/v2"
6+
. "github.com/onsi/gomega"
7+
)
8+
9+
var _ = Describe("ApplyBuildArgOverrides", func() {
10+
11+
var bao BuildArgOverrider
12+
13+
BeforeEach(func() {
14+
bao = NewBuildArgOverrider()
15+
})
16+
17+
It("apply overrides", func() {
18+
args := []kmmv1beta1.BuildArg{
19+
{
20+
Name: "name1",
21+
Value: "value1",
22+
},
23+
{
24+
Name: "name2",
25+
Value: "value2",
26+
},
27+
}
28+
overrides := []kmmv1beta1.BuildArg{
29+
{
30+
Name: "name1",
31+
Value: "valueOverride1",
32+
},
33+
{
34+
Name: "overrideName2",
35+
Value: "overrideValue2",
36+
},
37+
}
38+
39+
expected := []kmmv1beta1.BuildArg{
40+
{
41+
Name: "name1",
42+
Value: "valueOverride1",
43+
},
44+
{
45+
Name: "name2",
46+
Value: "value2",
47+
},
48+
{
49+
Name: "overrideName2",
50+
Value: "overrideValue2",
51+
},
52+
}
53+
54+
res := bao.ApplyBuildArgOverrides(args, overrides...)
55+
Expect(res).To(Equal(expected))
56+
})
57+
})

0 commit comments

Comments
 (0)