Skip to content

Commit 92005ee

Browse files
chr15pyevgeny-shnaidman
authored andcommitted
centralise the name of the image to build in the controller (rh-ecosystem-edge#88)
The name of the image to build is assumed to be km.ContainerImage in several places in the code, this commit centralises that assumption into a single point in Reconcile(). In the future this will allow us to determine the image the stage produces once E.g if we add signing (or other) intermediate stages to the image build process.
1 parent c84e53e commit 92005ee

File tree

6 files changed

+27
-26
lines changed

6 files changed

+27
-26
lines changed

controllers/module_reconciler.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func (r *ModuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
147147
}
148148

149149
for kernelVersion, m := range mappings {
150-
requeue, err := r.handleBuild(ctx, mod, m, kernelVersion)
150+
requeue, err := r.handleBuild(ctx, mod, m, kernelVersion, m.ContainerImage)
151151
if err != nil {
152152
return res, fmt.Errorf("failed to handle build for kernel version %s: %w", kernelVersion, err)
153153
}
@@ -260,17 +260,18 @@ func (r *ModuleReconciler) getNodesListBySelector(ctx context.Context, mod *kmmv
260260
func (r *ModuleReconciler) handleBuild(ctx context.Context,
261261
mod *kmmv1beta1.Module,
262262
km *kmmv1beta1.KernelMapping,
263-
kernelVersion string) (bool, error) {
263+
kernelVersion string,
264+
containerImage string) (bool, error) {
264265
if mod.Spec.ModuleLoader.Container.Build == nil && km.Build == nil {
265266
return false, nil
266267
}
267268

268-
logger := log.FromContext(ctx, "kernel version", kernelVersion, "image", km.ContainerImage)
269+
logger := log.FromContext(ctx, "kernel version", kernelVersion, "image", containerImage)
269270
logger.Info("Trying to pull the output image")
270271

271-
exists, err := r.checkImageExists(ctx, mod, km)
272+
exists, err := r.checkImageExists(ctx, mod, km, containerImage)
272273
if err != nil {
273-
return false, fmt.Errorf("failed to check image existence for kernel %s: %w", kernelVersion, err)
274+
return false, fmt.Errorf("failed to check existence of image %s for kernel %s: %w", containerImage, kernelVersion, err)
274275
}
275276
if exists {
276277
logger.Info("Image already exists; skipping build")
@@ -279,7 +280,7 @@ func (r *ModuleReconciler) handleBuild(ctx context.Context,
279280

280281
logger.Info("Image not pull-able; building in-cluster")
281282

282-
buildRes, err := r.buildAPI.Sync(log.IntoContext(ctx, logger), *mod, *km, kernelVersion, true)
283+
buildRes, err := r.buildAPI.Sync(log.IntoContext(ctx, logger), *mod, *km, kernelVersion, containerImage, true)
283284
if err != nil {
284285
return false, fmt.Errorf("could not synchronize the build: %w", err)
285286
}
@@ -294,10 +295,10 @@ func (r *ModuleReconciler) handleBuild(ctx context.Context,
294295
return buildRes.Requeue, nil
295296
}
296297

297-
func (r *ModuleReconciler) checkImageExists(ctx context.Context, mod *kmmv1beta1.Module, km *kmmv1beta1.KernelMapping) (bool, error) {
298+
func (r *ModuleReconciler) checkImageExists(ctx context.Context, mod *kmmv1beta1.Module, km *kmmv1beta1.KernelMapping, imageName string) (bool, error) {
298299
registryAuthGetter := r.authFactory.NewRegistryAuthGetterFrom(mod)
299300
pullOptions := module.GetRelevantPullOptions(mod, km)
300-
imageAvailable, err := r.registry.ImageExists(ctx, km.ContainerImage, pullOptions, registryAuthGetter)
301+
imageAvailable, err := r.registry.ImageExists(ctx, imageName, pullOptions, registryAuthGetter)
301302
if err != nil {
302303
return false, fmt.Errorf("could not check if the image is available: %v", err)
303304
}

controllers/module_reconciler_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
745745
mockSU,
746746
)
747747

748-
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
748+
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion, km.ContainerImage)
749749
Expect(err).NotTo(HaveOccurred())
750750
Expect(res).To(BeFalse())
751751
})
@@ -796,7 +796,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
796796
mockSU,
797797
)
798798

799-
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
799+
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion, km.ContainerImage)
800800
Expect(err).NotTo(HaveOccurred())
801801
Expect(res).To(BeFalse())
802802
})
@@ -840,7 +840,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
840840
mockSU,
841841
)
842842

843-
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
843+
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion, km.ContainerImage)
844844
Expect(err).NotTo(HaveOccurred())
845845
Expect(res).To(BeFalse())
846846
})
@@ -865,7 +865,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
865865
NewRegistryAuthGetterFrom(mod).
866866
Return(authGetter),
867867
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).Return(false, nil),
868-
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), true).Return(buildRes, nil),
868+
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), km.ContainerImage, true).Return(buildRes, nil),
869869
mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false),
870870
)
871871

@@ -882,7 +882,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
882882
mockSU,
883883
)
884884

885-
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
885+
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion, km.ContainerImage)
886886
Expect(err).NotTo(HaveOccurred())
887887
Expect(res).To(BeTrue())
888888
})
@@ -907,7 +907,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
907907
NewRegistryAuthGetterFrom(mod).
908908
Return(authGetter),
909909
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).Return(false, nil),
910-
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), true).Return(buildRes, nil),
910+
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), km.ContainerImage, true).Return(buildRes, nil),
911911
mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false),
912912
)
913913

@@ -924,7 +924,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
924924
mockSU,
925925
)
926926

927-
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
927+
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion, km.ContainerImage)
928928
Expect(err).NotTo(HaveOccurred())
929929
Expect(res).To(BeTrue())
930930
})
@@ -949,7 +949,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
949949
NewRegistryAuthGetterFrom(mod).
950950
Return(authGetter),
951951
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).Return(false, nil),
952-
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), true).Return(buildRes, nil),
952+
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), km.ContainerImage, true).Return(buildRes, nil),
953953
mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, true),
954954
)
955955

@@ -966,7 +966,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
966966
mockSU,
967967
)
968968

969-
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion)
969+
res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion, km.ContainerImage)
970970
Expect(err).NotTo(HaveOccurred())
971971
Expect(res).To(BeFalse())
972972
})

internal/build/buildconfig/manager.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ func NewManager(client client.Client, maker Maker, ocpBuildsHelper OpenShiftBuil
3434
}
3535
}
3636

37-
func (bcm *buildConfigManager) Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1beta1.KernelMapping, targetKernel string, pushImage bool) (build.Result, error) {
37+
func (bcm *buildConfigManager) Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1beta1.KernelMapping, targetKernel, targetImage string, pushImage bool) (build.Result, error) {
3838
logger := log.FromContext(ctx)
3939

40-
buildConfigTemplate, err := bcm.maker.MakeBuildConfigTemplate(mod, m, targetKernel, m.ContainerImage, pushImage)
40+
buildConfigTemplate, err := bcm.maker.MakeBuildConfigTemplate(mod, m, targetKernel, targetImage, pushImage)
4141
if err != nil {
4242
return build.Result{}, fmt.Errorf("could not make BuildConfig template: %v", err)
4343
}

internal/build/buildconfig/manager_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ var _ = Describe("Manager_Sync", func() {
8181
mockKubeClient.EXPECT().Create(ctx, &buildConfig),
8282
)
8383

84-
res, err := m.Sync(ctx, mod, mapping, targetKernel, true)
84+
res, err := m.Sync(ctx, mod, mapping, targetKernel, mapping.ContainerImage, true)
8585
Expect(err).NotTo(HaveOccurred())
8686
Expect(res.Status).To(BeEquivalentTo(build.StatusCreated))
8787
Expect(res.Requeue).To(BeTrue())
@@ -131,7 +131,7 @@ var _ = Describe("Manager_Sync", func() {
131131
mockOpenShiftBuildsHelper.EXPECT().GetLatestBuild(ctx, namespace, buildConfigName).Return(&b, nil),
132132
)
133133

134-
res, err := m.Sync(ctx, mod, mapping, targetKernel, true)
134+
res, err := m.Sync(ctx, mod, mapping, targetKernel, mapping.ContainerImage, true)
135135

136136
if expectError {
137137
Expect(err).To(HaveOccurred())

internal/build/manager.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ type Result struct {
2222
//go:generate mockgen -source=manager.go -package=build -destination=mock_manager.go
2323

2424
type Manager interface {
25-
Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1beta1.KernelMapping, targetKernel string, pushImage bool) (Result, error)
25+
Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1beta1.KernelMapping, targetKernel string, targetImage string, pushImage bool) (Result, error)
2626
}

internal/build/mock_manager.go

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)