Skip to content

Commit efe36a0

Browse files
ybettank8s-ci-robot
ybettan
authored and
k8s-ci-robot
committed
Un-exporting some of WorkerPodManager interface env variables.
When this interface was transitioned to the `pod` package, it was copied as is (as much as possible) and some variables had to be exported to be used as before. Once the transition done, it was simpler to review some of those new exported env variables and convert them to usefull methods inside the `WorkerPodManager` interface. Signed-off-by: Yoni Bettan <[email protected]>
1 parent 2551d17 commit efe36a0

File tree

5 files changed

+155
-76
lines changed

5 files changed

+155
-76
lines changed

‎internal/controllers/nmc_reconciler.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ func (h *nmcReconcilerHelperImpl) ProcessModuleSpec(
342342
return nil
343343
}
344344

345-
if p.Labels[pod.ActionLabelKey] != pod.WorkerActionLoad {
345+
if !h.podManager.IsLoaderPod(p) {
346346
logger.Info("Worker Pod is not loading the kmod; doing nothing")
347347
return nil
348348
}
@@ -357,7 +357,7 @@ func (h *nmcReconcilerHelperImpl) ProcessModuleSpec(
357357
return fmt.Errorf("could not create the Pod template for %s: %v", podName, err)
358358
}
359359

360-
if podTemplate.Annotations[pod.HashAnnotationKey] != p.Annotations[pod.HashAnnotationKey] {
360+
if h.podManager.HashAnnotationDiffer(podTemplate, p) {
361361
logger.Info("Hash differs, deleting pod")
362362
return h.podManager.DeletePod(ctx, p)
363363
}
@@ -402,7 +402,7 @@ func (h *nmcReconcilerHelperImpl) ProcessUnconfiguredModuleStatus(
402402
return h.podManager.CreateUnloaderPod(ctx, nmcObj, status)
403403
}
404404

405-
if p.Labels[pod.ActionLabelKey] == pod.WorkerActionLoad {
405+
if h.podManager.IsLoaderPod(p) {
406406
logger.Info("Worker Pod is loading the kmod; deleting it")
407407
return h.podManager.DeletePod(ctx, p)
408408
}
@@ -417,7 +417,7 @@ func (h *nmcReconcilerHelperImpl) ProcessUnconfiguredModuleStatus(
417417
return fmt.Errorf("could not create the Pod template for %s: %v", podName, err)
418418
}
419419

420-
if podTemplate.Annotations[pod.HashAnnotationKey] != p.Annotations[pod.HashAnnotationKey] {
420+
if h.podManager.HashAnnotationDiffer(podTemplate, p) {
421421
logger.Info("Hash differs, deleting pod")
422422
return h.podManager.DeletePod(ctx, p)
423423
}
@@ -502,7 +502,7 @@ func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1b
502502
case v1.PodFailed:
503503
podsToDelete = append(podsToDelete, p)
504504
case v1.PodSucceeded:
505-
if p.Labels[pod.ActionLabelKey] == pod.WorkerActionUnload {
505+
if h.podManager.IsUnloaderPod(&p) {
506506
podsToDelete = append(podsToDelete, p)
507507
nmc.RemoveModuleStatus(&nmcObj.Status.Modules, modNamespace, modName)
508508
break
@@ -517,7 +517,8 @@ func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1b
517517
}
518518
}
519519

520-
if err = yaml.UnmarshalStrict([]byte(p.Annotations[pod.ConfigAnnotationKey]), &status.Config); err != nil {
520+
configAnnotation := h.podManager.GetConfigAnnotation(&p)
521+
if err = yaml.UnmarshalStrict([]byte(configAnnotation), &status.Config); err != nil {
521522
errs = append(
522523
errs,
523524
fmt.Errorf("%s: could not unmarshal the ModuleConfig from YAML: %v", podNSN, err),

‎internal/controllers/nmc_reconciler_test.go‎

Lines changed: 28 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"reflect"
88
"time"
99

10-
"github.com/kubernetes-sigs/kernel-module-management/internal/meta"
1110
"github.com/kubernetes-sigs/kernel-module-management/internal/node"
1211
"github.com/kubernetes-sigs/kernel-module-management/internal/pod"
1312
"k8s.io/apimachinery/pkg/util/sets"
@@ -682,10 +681,11 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessModuleSpec", func() {
682681
)
683682

684683
It("should do nothing if the pod is not loading a kmod", func() {
685-
mockWorkerPodManager.
686-
EXPECT().
687-
GetWorkerPod(ctx, podName, namespace).
688-
Return(&v1.Pod{}, nil)
684+
685+
gomock.InOrder(
686+
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&v1.Pod{}, nil),
687+
mockWorkerPodManager.EXPECT().IsLoaderPod(gomock.Any()).Return(true),
688+
)
689689

690690
Expect(
691691
wh.ProcessModuleSpec(ctx, nmc, spec, status, nil),
@@ -695,16 +695,11 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessModuleSpec", func() {
695695
})
696696

697697
It("should do nothing if the worker container has not restarted", func() {
698-
pod := v1.Pod{
699-
ObjectMeta: metav1.ObjectMeta{
700-
Labels: map[string]string{pod.ActionLabelKey: pod.WorkerActionLoad},
701-
},
702-
}
703698

704-
mockWorkerPodManager.
705-
EXPECT().
706-
GetWorkerPod(ctx, podName, namespace).
707-
Return(&pod, nil)
699+
gomock.InOrder(
700+
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&v1.Pod{}, nil),
701+
mockWorkerPodManager.EXPECT().IsLoaderPod(gomock.Any()).Return(true),
702+
)
708703

709704
Expect(
710705
wh.ProcessModuleSpec(ctx, nmc, spec, status, nil),
@@ -715,9 +710,6 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessModuleSpec", func() {
715710

716711
It("should return an error if there was an error making the Pod template", func() {
717712
pod := v1.Pod{
718-
ObjectMeta: metav1.ObjectMeta{
719-
Labels: map[string]string{pod.ActionLabelKey: pod.WorkerActionLoad},
720-
},
721713
Status: v1.PodStatus{
722714
ContainerStatuses: []v1.ContainerStatus{
723715
{
@@ -729,14 +721,9 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessModuleSpec", func() {
729721
}
730722

731723
gomock.InOrder(
732-
mockWorkerPodManager.
733-
EXPECT().
734-
GetWorkerPod(ctx, podName, namespace).
735-
Return(&pod, nil),
736-
mockWorkerPodManager.
737-
EXPECT().
738-
LoaderPodTemplate(ctx, nmc, spec).
739-
Return(nil, errors.New("random error")),
724+
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&pod, nil),
725+
mockWorkerPodManager.EXPECT().IsLoaderPod(&pod).Return(true),
726+
mockWorkerPodManager.EXPECT().LoaderPodTemplate(ctx, nmc, spec).Return(nil, errors.New("random error")),
740727
)
741728

742729
Expect(
@@ -748,10 +735,6 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessModuleSpec", func() {
748735

749736
It("should delete the existing pod if its hash annotation is outdated", func() {
750737
p := v1.Pod{
751-
ObjectMeta: metav1.ObjectMeta{
752-
Labels: map[string]string{pod.ActionLabelKey: pod.WorkerActionLoad},
753-
Annotations: map[string]string{pod.HashAnnotationKey: "123"},
754-
},
755738
Status: v1.PodStatus{
756739
ContainerStatuses: []v1.ContainerStatus{
757740
{
@@ -763,20 +746,13 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessModuleSpec", func() {
763746
}
764747

765748
podTemplate := p.DeepCopy()
766-
podTemplate.Annotations[pod.HashAnnotationKey] = "456"
767749

768750
gomock.InOrder(
769-
mockWorkerPodManager.
770-
EXPECT().
771-
GetWorkerPod(ctx, podName, namespace).
772-
Return(&p, nil),
773-
mockWorkerPodManager.
774-
EXPECT().
775-
LoaderPodTemplate(ctx, nmc, spec).
776-
Return(podTemplate, nil),
777-
mockWorkerPodManager.
778-
EXPECT().
779-
DeletePod(ctx, &p),
751+
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&p, nil),
752+
mockWorkerPodManager.EXPECT().IsLoaderPod(&p).Return(true),
753+
mockWorkerPodManager.EXPECT().LoaderPodTemplate(ctx, nmc, spec).Return(podTemplate, nil),
754+
mockWorkerPodManager.EXPECT().HashAnnotationDiffer(gomock.Any(), gomock.Any()).Return(true),
755+
mockWorkerPodManager.EXPECT().DeletePod(ctx, &p),
780756
)
781757

782758
Expect(
@@ -857,13 +833,13 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
857833
ObjectMeta: metav1.ObjectMeta{
858834
Name: podName,
859835
Namespace: namespace,
860-
Labels: map[string]string{pod.ActionLabelKey: pod.WorkerActionLoad},
861836
},
862837
}
863838

864839
gomock.InOrder(
865840
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
866841
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&pod, nil),
842+
mockWorkerPodManager.EXPECT().IsLoaderPod(&pod).Return(true),
867843
mockWorkerPodManager.EXPECT().DeletePod(ctx, &pod),
868844
)
869845

@@ -885,6 +861,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
885861
gomock.InOrder(
886862
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
887863
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&pod, nil),
864+
mockWorkerPodManager.EXPECT().IsLoaderPod(&pod).Return(false),
888865
)
889866

890867
Expect(
@@ -913,6 +890,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
913890
gomock.InOrder(
914891
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
915892
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&pod, nil),
893+
mockWorkerPodManager.EXPECT().IsLoaderPod(&pod).Return(false),
916894
mockWorkerPodManager.EXPECT().UnloaderPodTemplate(ctx, nmc, status).Return(nil, errors.New("random error")),
917895
)
918896

@@ -926,9 +904,8 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
926904
It("should delete the existing pod if its hash annotation is outdated", func() {
927905
p := v1.Pod{
928906
ObjectMeta: metav1.ObjectMeta{
929-
Name: podName,
930-
Namespace: namespace,
931-
Annotations: map[string]string{pod.HashAnnotationKey: "123"},
907+
Name: podName,
908+
Namespace: namespace,
932909
},
933910
Status: v1.PodStatus{
934911
ContainerStatuses: []v1.ContainerStatus{
@@ -941,12 +918,13 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
941918
}
942919

943920
podTemplate := p.DeepCopy()
944-
podTemplate.Annotations[pod.HashAnnotationKey] = "456"
945921

946922
gomock.InOrder(
947923
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
948924
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&p, nil),
925+
mockWorkerPodManager.EXPECT().IsLoaderPod(&p).Return(false),
949926
mockWorkerPodManager.EXPECT().UnloaderPodTemplate(ctx, nmc, status).Return(podTemplate, nil),
927+
mockWorkerPodManager.EXPECT().HashAnnotationDiffer(gomock.Any(), gomock.Any()).Return(true),
950928
mockWorkerPodManager.EXPECT().DeletePod(ctx, &p),
951929
)
952930

@@ -1078,7 +1056,6 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
10781056
ObjectMeta: metav1.ObjectMeta{
10791057
Namespace: modNamespace,
10801058
Labels: map[string]string{
1081-
pod.ActionLabelKey: pod.WorkerActionUnload,
10821059
constants.ModuleNameLabel: modName,
10831060
},
10841061
},
@@ -1087,6 +1064,7 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
10871064

10881065
gomock.InOrder(
10891066
mockWorkerPodManager.EXPECT().ListWorkerPodsOnNode(ctx, nmcName).Return([]v1.Pod{pod}, nil),
1067+
mockWorkerPodManager.EXPECT().IsUnloaderPod(&pod).Return(true),
10901068
kubeClient.EXPECT().Status().Return(sw),
10911069
sw.EXPECT().Patch(ctx, nmc, gomock.Any()),
10921070
mockWorkerPodManager.EXPECT().DeletePod(ctx, &pod),
@@ -1143,7 +1121,6 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
11431121
ObjectMeta: metav1.ObjectMeta{
11441122
Namespace: modNamespace,
11451123
Labels: map[string]string{
1146-
pod.ActionLabelKey: pod.WorkerActionLoad,
11471124
constants.ModuleNameLabel: modName,
11481125
},
11491126
},
@@ -1167,10 +1144,11 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
11671144

11681145
b, err := yaml.Marshal(cfg)
11691146
Expect(err).NotTo(HaveOccurred())
1170-
meta.SetAnnotation(&p, pod.ConfigAnnotationKey, string(b))
11711147

11721148
gomock.InOrder(
11731149
mockWorkerPodManager.EXPECT().ListWorkerPodsOnNode(ctx, nmcName).Return([]v1.Pod{p}, nil),
1150+
mockWorkerPodManager.EXPECT().IsUnloaderPod(&p).Return(false),
1151+
mockWorkerPodManager.EXPECT().GetConfigAnnotation(&p).Return(string(b)),
11741152
kubeClient.EXPECT().Status().Return(sw),
11751153
sw.EXPECT().Patch(ctx, nmc, gomock.Any()),
11761154
mockWorkerPodManager.EXPECT().DeletePod(ctx, &p),
@@ -1223,7 +1201,6 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
12231201
ObjectMeta: metav1.ObjectMeta{
12241202
Namespace: modNamespace,
12251203
Labels: map[string]string{
1226-
pod.ActionLabelKey: pod.WorkerActionUnload,
12271204
constants.ModuleNameLabel: modName,
12281205
},
12291206
},
@@ -1232,6 +1209,7 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
12321209

12331210
gomock.InOrder(
12341211
mockWorkerPodManager.EXPECT().ListWorkerPodsOnNode(ctx, nmcName).Return([]v1.Pod{pod}, nil),
1212+
mockWorkerPodManager.EXPECT().IsUnloaderPod(&pod).Return(true),
12351213
kubeClient.EXPECT().Status().Return(sw),
12361214
sw.EXPECT().Patch(ctx, nmc, gomock.Any()).Return(errors.New("some error")),
12371215
)

‎internal/pod/mock_workerpodmanager.go‎

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

0 commit comments

Comments
 (0)