Skip to content

Commit 4e618b5

Browse files
yevgeny-shnaidmank8s-ci-robot
yevgeny-shnaidman
authored and
k8s-ci-robot
committed
Changing ImagePuller interface to more elaborate parameters
Since ImagePuller interface is going to be used not only by MIC, but also by Preflight controller, the APIs should be changed to receive specific parameters, and not use ModuleImagesConfig or ImageSpec structures, that are alient to Preflight
1 parent 24a5194 commit 4e618b5

File tree

5 files changed

+69
-65
lines changed

5 files changed

+69
-65
lines changed

Diff for: ‎internal/controllers/mic_reconciler.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (r *micReconciler) Reconcile(ctx context.Context, micObj *kmmv1beta1.Module
7373
return res, nil
7474
}
7575

76-
pods, err := r.imagePullerAPI.ListPullPods(ctx, micObj)
76+
pods, err := r.imagePullerAPI.ListPullPods(ctx, micObj.Name, micObj.Namespace)
7777
if err != nil {
7878
return res, fmt.Errorf("failed to get the image pods for mic %s: %v", micObj.Name, err)
7979
}
@@ -237,7 +237,13 @@ func (mrhi *micReconcilerHelperImpl) processImagesSpecs(ctx context.Context, mic
237237
// image State is not set: either new image or pull pod is still running
238238
if mrhi.imagePullerAPI.GetPullPodForImage(pullPods, imageSpec.Image) == nil {
239239
// no pull pod- create it, otherwise we wait for it to finish
240-
err := mrhi.imagePullerAPI.CreatePullPod(ctx, &imageSpec, micObj)
240+
err := mrhi.imagePullerAPI.CreatePullPod(ctx,
241+
micObj.Name,
242+
micObj.Namespace,
243+
imageSpec.Image,
244+
(imageSpec.Build != nil || imageSpec.Sign != nil),
245+
micObj.Spec.ImageRepoSecret,
246+
micObj)
241247
errs = append(errs, err)
242248
}
243249
case kmmv1beta1.ImageDoesNotExist:

Diff for: ‎internal/controllers/mic_reconciler_test.go

+10-4
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ var _ = Describe("MicReconciler_Reconcile", func() {
3838
})
3939

4040
ctx := context.Background()
41-
testMic := kmmv1beta1.ModuleImagesConfig{}
41+
testMic := kmmv1beta1.ModuleImagesConfig{
42+
ObjectMeta: metav1.ObjectMeta{Name: "some name", Namespace: "some namespace"},
43+
}
4244

4345
DescribeTable("check good and error flows", func(listPullPodsError,
4446
updateStatusByPodsError,
@@ -49,10 +51,10 @@ var _ = Describe("MicReconciler_Reconcile", func() {
4951
expectedErr := returnedError
5052
pullPods := []v1.Pod{}
5153
if listPullPodsError {
52-
mockImagePuller.EXPECT().ListPullPods(ctx, &testMic).Return(nil, returnedError)
54+
mockImagePuller.EXPECT().ListPullPods(ctx, "some name", "some namespace").Return(nil, returnedError)
5355
goto executeTestFunction
5456
}
55-
mockImagePuller.EXPECT().ListPullPods(ctx, &testMic).Return(pullPods, nil)
57+
mockImagePuller.EXPECT().ListPullPods(ctx, "some name", "some namespace").Return(pullPods, nil)
5658
if updateStatusByPodsError {
5759
mockMicReconHelper.EXPECT().updateStatusByPullPods(ctx, &testMic, pullPods).Return(returnedError)
5860
goto executeTestFunction
@@ -360,6 +362,10 @@ var _ = Describe("processImagesSpecs", func() {
360362
ctx := context.Background()
361363
pullPods := []v1.Pod{}
362364
testMic = kmmv1beta1.ModuleImagesConfig{
365+
ObjectMeta: metav1.ObjectMeta{
366+
Name: "some name",
367+
Namespace: "some namespace",
368+
},
363369
Spec: kmmv1beta1.ModuleImagesConfigSpec{
364370
Images: []kmmv1beta1.ModuleImageSpec{
365371
{
@@ -373,7 +379,7 @@ var _ = Describe("processImagesSpecs", func() {
373379
gomock.InOrder(
374380
micHelper.EXPECT().GetImageState(&testMic, "image 1").Return(kmmv1beta1.ImageState("")),
375381
mockImagePuller.EXPECT().GetPullPodForImage(pullPods, "image 1").Return(nil),
376-
mockImagePuller.EXPECT().CreatePullPod(ctx, &testMic.Spec.Images[0], &testMic).Return(nil),
382+
mockImagePuller.EXPECT().CreatePullPod(ctx, "some name", "some namespace", "image 1", true, nil, &testMic).Return(nil),
377383
)
378384
err := mrh.processImagesSpecs(ctx, &testMic, pullPods)
379385
Expect(err).To(BeNil())

Diff for: ‎internal/pod/imagepuller.go

+23-23
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66

7-
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
87
v1 "k8s.io/api/core/v1"
98
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
109
"k8s.io/apimachinery/pkg/runtime"
@@ -23,7 +22,7 @@ const (
2322
imagePullBackOffReason = "ImagePullBackOff"
2423
errImagePullReason = "ErrImagePull"
2524

26-
moduleImageLabelKey = "kmm.node.kubernetes.io/module-image-config"
25+
imageOwnerLabelKey = "kmm.node.kubernetes.io/image-owner"
2726
pullPodTypeLabelKey = "kmm.node.kubernetes.io/pull-pod-type"
2827

2928
pullerContainerName = "puller"
@@ -35,9 +34,10 @@ const (
3534
//go:generate mockgen -source=imagepuller.go -package=pod -destination=mock_imagepuller.go
3635

3736
type ImagePuller interface {
38-
CreatePullPod(ctx context.Context, imageSpec *kmmv1beta1.ModuleImageSpec, micObj *kmmv1beta1.ModuleImagesConfig) error
37+
CreatePullPod(ctx context.Context, name, namespace, imageToPull string, oneTimePod bool,
38+
imageRepoSecret *v1.LocalObjectReference, owner metav1.Object) error
3939
DeletePod(ctx context.Context, pod *v1.Pod) error
40-
ListPullPods(ctx context.Context, micObj *kmmv1beta1.ModuleImagesConfig) ([]v1.Pod, error)
40+
ListPullPods(ctx context.Context, name, namespace string) ([]v1.Pod, error)
4141
GetPullPodForImage(pods []v1.Pod, image string) *v1.Pod
4242
GetPullPodImage(pod v1.Pod) string
4343
GetPullPodStatus(pod *v1.Pod) PullPodStatus
@@ -55,33 +55,33 @@ func NewImagePuller(client client.Client, scheme *runtime.Scheme) ImagePuller {
5555
}
5656
}
5757

58-
func (ipi *imagePullerImpl) CreatePullPod(ctx context.Context, imageSpec *kmmv1beta1.ModuleImageSpec,
59-
micObj *kmmv1beta1.ModuleImagesConfig) error {
58+
func (ipi *imagePullerImpl) CreatePullPod(ctx context.Context, name, namespace, imageToPull string, oneTimePod bool,
59+
imageRepoSecret *v1.LocalObjectReference, owner metav1.Object) error {
6060

61-
pullPodTypeLabeValue := pullPodUntilSuccess
62-
if imageSpec.Build != nil || imageSpec.Sign != nil {
63-
pullPodTypeLabeValue = pullPodTypeOneTime
61+
pullPodTypeLabelValue := pullPodUntilSuccess
62+
if oneTimePod {
63+
pullPodTypeLabelValue = pullPodTypeOneTime
6464
}
6565

6666
imagePullSecrets := []v1.LocalObjectReference{}
67-
if micObj.Spec.ImageRepoSecret != nil {
68-
imagePullSecrets = []v1.LocalObjectReference{*micObj.Spec.ImageRepoSecret}
67+
if imageRepoSecret != nil {
68+
imagePullSecrets = []v1.LocalObjectReference{*imageRepoSecret}
6969
}
7070

7171
pullPod := v1.Pod{
7272
ObjectMeta: metav1.ObjectMeta{
73-
GenerateName: micObj.Name + "-pull-pod-",
74-
Namespace: micObj.Namespace,
73+
GenerateName: name + "-pull-pod-",
74+
Namespace: namespace,
7575
Labels: map[string]string{
76-
moduleImageLabelKey: micObj.Name,
77-
pullPodTypeLabelKey: pullPodTypeLabeValue,
76+
imageOwnerLabelKey: name,
77+
pullPodTypeLabelKey: pullPodTypeLabelValue,
7878
},
7979
},
8080
Spec: v1.PodSpec{
8181
Containers: []v1.Container{
8282
{
8383
Name: pullerContainerName,
84-
Image: imageSpec.Image,
84+
Image: imageToPull,
8585
Command: []string{"/bin/sh", "-c", "exit 0"},
8686
},
8787
},
@@ -90,9 +90,9 @@ func (ipi *imagePullerImpl) CreatePullPod(ctx context.Context, imageSpec *kmmv1b
9090
},
9191
}
9292

93-
err := ctrl.SetControllerReference(micObj, &pullPod, ipi.scheme)
93+
err := ctrl.SetControllerReference(owner, &pullPod, ipi.scheme)
9494
if err != nil {
95-
return fmt.Errorf("failed to set MIC object %s as owner on pullPod for image %s: %v", micObj.Name, imageSpec.Image, err)
95+
return fmt.Errorf("failed to set owner for pullPod for image %s: %v", imageToPull, err)
9696
}
9797

9898
return ipi.client.Create(ctx, &pullPod)
@@ -103,17 +103,17 @@ func (ipi *imagePullerImpl) DeletePod(ctx context.Context, pod *v1.Pod) error {
103103
return deletePod(ipi.client, ctx, pod)
104104
}
105105

106-
func (ipi *imagePullerImpl) ListPullPods(ctx context.Context, micObj *kmmv1beta1.ModuleImagesConfig) ([]v1.Pod, error) {
106+
func (ipi *imagePullerImpl) ListPullPods(ctx context.Context, name, namespace string) ([]v1.Pod, error) {
107107

108108
pl := v1.PodList{}
109109

110110
hl := client.HasLabels{pullPodTypeLabelKey}
111-
ml := client.MatchingLabels{moduleImageLabelKey: micObj.Name}
111+
ml := client.MatchingLabels{imageOwnerLabelKey: name}
112112

113-
ctrl.LoggerFrom(ctx).WithValues("mic name", micObj.Name).V(1).Info("Listing mic image Pods")
113+
ctrl.LoggerFrom(ctx).WithValues("module name", name).V(1).Info("Listing module image Pods")
114114

115-
if err := ipi.client.List(ctx, &pl, client.InNamespace(micObj.Namespace), hl, ml); err != nil {
116-
return nil, fmt.Errorf("could not list mic image pods for mic %s: %v", micObj.Name, err)
115+
if err := ipi.client.List(ctx, &pl, client.InNamespace(namespace), hl, ml); err != nil {
116+
return nil, fmt.Errorf("could not list module image pods for module %s: %v", name, err)
117117
}
118118

119119
return pl.Items, nil

Diff for: ‎internal/pod/imagepuller_test.go

+19-27
Original file line numberDiff line numberDiff line change
@@ -30,36 +30,32 @@ var _ = Describe("ListPullPods", func() {
3030
})
3131

3232
ctx := context.Background()
33-
testMic := kmmv1beta1.ModuleImagesConfig{
34-
ObjectMeta: metav1.ObjectMeta{
35-
Name: "some name",
36-
Namespace: "some namespace",
37-
},
38-
}
33+
testName := "some name"
34+
testNamespace := "some namespace"
3935

4036
It("list succeeded", func() {
4137
hl := ctrlclient.HasLabels{pullPodTypeLabelKey}
42-
ml := ctrlclient.MatchingLabels{moduleImageLabelKey: testMic.Name}
38+
ml := ctrlclient.MatchingLabels{imageOwnerLabelKey: testName}
4339

44-
clnt.EXPECT().List(context.Background(), gomock.Any(), ctrlclient.InNamespace(testMic.Namespace), hl, ml).DoAndReturn(
40+
clnt.EXPECT().List(context.Background(), gomock.Any(), ctrlclient.InNamespace(testNamespace), hl, ml).DoAndReturn(
4541
func(_ interface{}, podList *v1.PodList, _ ...interface{}) error {
4642
podList.Items = []v1.Pod{v1.Pod{}, v1.Pod{}}
4743
return nil
4844
},
4945
)
5046

51-
pullPods, err := ip.ListPullPods(ctx, &testMic)
47+
pullPods, err := ip.ListPullPods(ctx, testName, testNamespace)
5248
Expect(err).To(BeNil())
5349
Expect(pullPods).ToNot(BeNil())
5450
})
5551

5652
It("list failed", func() {
5753
hl := ctrlclient.HasLabels{pullPodTypeLabelKey}
58-
ml := ctrlclient.MatchingLabels{moduleImageLabelKey: testMic.Name}
54+
ml := ctrlclient.MatchingLabels{imageOwnerLabelKey: testName}
5955

60-
clnt.EXPECT().List(context.Background(), gomock.Any(), ctrlclient.InNamespace(testMic.Namespace), hl, ml).Return(fmt.Errorf("some error"))
56+
clnt.EXPECT().List(context.Background(), gomock.Any(), ctrlclient.InNamespace(testNamespace), hl, ml).Return(fmt.Errorf("some error"))
6157

62-
pullPods, err := ip.ListPullPods(ctx, &testMic)
58+
pullPods, err := ip.ListPullPods(ctx, testName, testNamespace)
6359
Expect(err).To(HaveOccurred())
6460
Expect(pullPods).To(BeNil())
6561
})
@@ -161,36 +157,32 @@ var _ = Describe("CreatePullPod", func() {
161157
})
162158

163159
ctx := context.Background()
164-
testMic := kmmv1beta1.ModuleImagesConfig{
165-
ObjectMeta: metav1.ObjectMeta{
166-
Name: "some name",
167-
Namespace: "some namespace",
168-
},
169-
}
170-
testImageSpec := kmmv1beta1.ModuleImageSpec{
171-
Image: "some image",
172-
}
160+
testName := "some name"
161+
testNamespace := "some namespace"
162+
testImage := "some image"
163+
testMic := kmmv1beta1.ModuleImagesConfig{}
164+
testRepoSecret := v1.LocalObjectReference{}
173165

174166
It("check the pod fields", func() {
175167
expectedPod := v1.Pod{
176168
ObjectMeta: metav1.ObjectMeta{
177-
GenerateName: testMic.Name + "-pull-pod-",
178-
Namespace: testMic.Namespace,
169+
GenerateName: testName + "-pull-pod-",
170+
Namespace: testNamespace,
179171
Labels: map[string]string{
180-
moduleImageLabelKey: "some name",
172+
imageOwnerLabelKey: testName,
181173
pullPodTypeLabelKey: pullPodUntilSuccess,
182174
},
183175
},
184176
Spec: v1.PodSpec{
185177
Containers: []v1.Container{
186178
{
187179
Name: pullerContainerName,
188-
Image: "some image",
180+
Image: testImage,
189181
Command: []string{"/bin/sh", "-c", "exit 0"},
190182
},
191183
},
192184
RestartPolicy: v1.RestartPolicyNever,
193-
ImagePullSecrets: []v1.LocalObjectReference{},
185+
ImagePullSecrets: []v1.LocalObjectReference{testRepoSecret},
194186
},
195187
}
196188

@@ -206,7 +198,7 @@ var _ = Describe("CreatePullPod", func() {
206198
}
207199
return nil
208200
})
209-
err := ip.CreatePullPod(ctx, &testImageSpec, &testMic)
201+
err := ip.CreatePullPod(ctx, testName, testNamespace, testImage, false, &testRepoSecret, &testMic)
210202
Expect(err).To(BeNil())
211203
})
212204
})

Diff for: ‎internal/pod/mock_imagepuller.go

+9-9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)