Skip to content

Commit f8a14a8

Browse files
sriram-30k8s-ci-robot
sriram-30
authored and
k8s-ci-robot
committed
BootID support for KMM
BootID KMM support: KMM is checking if a node has rebooted by inspecting the Ready timestamp on the node and check if it newer than the last Ready timestamp recorded. Kubernetes has a grace period in which if a node stop reporting heartbeats it is then marked by the k8s API server as not ready. In some cases, the reboot is so fast that the node become Ready again before the k8s API has even noticed it went down, and in those cases we need to make sure KMM catches it and reload the kmod to the node. This is being done by comparing the node's status.nodeInfo.bootID, which is unique per reboot, with the last recorded value.
1 parent b252575 commit f8a14a8

File tree

8 files changed

+69
-70
lines changed

8 files changed

+69
-70
lines changed

Diff for: ‎api/v1beta1/nodemodulesconfig_types.go

+2
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ type NodeModuleStatus struct {
6868
Config ModuleConfig `json:"config,omitempty"`
6969
//+optional
7070
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
71+
//+optional
72+
BootId string `json:"bootId,omitempty"`
7173
}
7274

7375
// NodeModuleConfigStatus is the most recently observed status of the KMM modules on node.

Diff for: ‎config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,8 @@ spec:
241241
state status
242242
items:
243243
properties:
244+
bootId:
245+
type: string
244246
config:
245247
properties:
246248
containerImage:

Diff for: ‎internal/controllers/mock_nmc_reconciler.go

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

Diff for: ‎internal/controllers/nmc_reconciler.go

+11-10
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,12 @@ func (r *NMCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r
9090
return reconcile.Result{}, fmt.Errorf("could not get NodeModuleState %s: %v", req.NamespacedName, err)
9191
}
9292

93-
if err := r.helper.SyncStatus(ctx, &nmcObj); err != nil {
93+
node := v1.Node{}
94+
if err := r.client.Get(ctx, types.NamespacedName{Name: nmcObj.Name}, &node); err != nil {
95+
return ctrl.Result{}, fmt.Errorf("could not get node %s: %v", nmcObj.Name, err)
96+
}
97+
98+
if err := r.helper.SyncStatus(ctx, &nmcObj, &node); err != nil {
9499
return reconcile.Result{}, fmt.Errorf("could not reconcile status for NodeModulesConfig %s: %v", nmcObj.Name, err)
95100
}
96101

@@ -103,11 +108,6 @@ func (r *NMCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r
103108
statusMap[status.Namespace+"/"+status.Name] = &nmcObj.Status.Modules[i]
104109
}
105110

106-
node := v1.Node{}
107-
if err := r.client.Get(ctx, types.NamespacedName{Name: nmcObj.Name}, &node); err != nil {
108-
return ctrl.Result{}, fmt.Errorf("could not get node %s: %v", nmcObj.Name, err)
109-
}
110-
111111
errs := make([]error, 0, len(nmcObj.Spec.Modules)+len(nmcObj.Status.Modules))
112112
var readyLabelsToRemove []string
113113
for _, mod := range nmcObj.Spec.Modules {
@@ -220,7 +220,7 @@ type nmcReconcilerHelper interface {
220220
ProcessModuleSpec(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, spec *kmmv1beta1.NodeModuleSpec, status *kmmv1beta1.NodeModuleStatus, node *v1.Node) error
221221
ProcessUnconfiguredModuleStatus(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, status *kmmv1beta1.NodeModuleStatus, node *v1.Node) error
222222
RemovePodFinalizers(ctx context.Context, nodeName string) error
223-
SyncStatus(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig) error
223+
SyncStatus(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, node *v1.Node) error
224224
UpdateNodeLabels(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, node *v1.Node) ([]types.NamespacedName, []types.NamespacedName, error)
225225
RecordEvents(node *v1.Node, loadedModules, unloadedModules []types.NamespacedName)
226226
}
@@ -374,7 +374,7 @@ func (h *nmcReconcilerHelperImpl) ProcessModuleSpec(
374374
return h.podManager.CreateLoaderPod(ctx, nmcObj, spec)
375375
}
376376

377-
if h.nodeAPI.NodeBecomeReadyAfter(node, status.LastTransitionTime) {
377+
if h.nodeAPI.IsNodeRebooted(node, status.BootId) {
378378
logger.Info("node has been rebooted and become ready after kernel module was loaded; creating loader Pod")
379379
return h.podManager.CreateLoaderPod(ctx, nmcObj, spec)
380380
}
@@ -425,7 +425,7 @@ func (h *nmcReconcilerHelperImpl) ProcessUnconfiguredModuleStatus(
425425
/* node was rebooted, spec not set so no kernel module is loaded, no need to unload.
426426
it also fixes the scenario when node's kernel was upgraded, so unload pod will fail anyway
427427
*/
428-
if h.nodeAPI.NodeBecomeReadyAfter(node, status.LastTransitionTime) {
428+
if h.nodeAPI.IsNodeRebooted(node, status.BootId) {
429429
logger.Info("node was rebooted and spec is missing: delete the status to allow Module CR unload, if needed")
430430
patchFrom := client.MergeFrom(nmcObj.DeepCopy())
431431
nmc.RemoveModuleStatus(&nmcObj.Status.Modules, status.Namespace, status.Name)
@@ -493,7 +493,7 @@ func (h *nmcReconcilerHelperImpl) RemovePodFinalizers(ctx context.Context, nodeN
493493
return errors.Join(errs...)
494494
}
495495

496-
func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1beta1.NodeModulesConfig) error {
496+
func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1beta1.NodeModulesConfig, node *v1.Node) error {
497497
logger := ctrl.LoggerFrom(ctx)
498498

499499
logger.Info("Syncing status")
@@ -579,6 +579,7 @@ func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1b
579579
FinishedAt
580580

581581
status.LastTransitionTime = podLTT
582+
status.BootId = node.Status.NodeInfo.BootID
582583

583584
nmc.SetModuleStatus(&nmcObj.Status.Modules, *status)
584585

Diff for: ‎internal/controllers/nmc_reconciler_test.go

+24-24
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,16 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
8888
nmc := &kmmv1beta1.NodeModulesConfig{
8989
ObjectMeta: metav1.ObjectMeta{Name: nmcName},
9090
}
91-
91+
node := v1.Node{}
9292
gomock.InOrder(
9393
kubeClient.
9494
EXPECT().
9595
Get(ctx, nmcNsn, &kmmv1beta1.NodeModulesConfig{}).
9696
Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) {
9797
*kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc
9898
}),
99-
wh.EXPECT().SyncStatus(ctx, nmc).Return(errors.New("random error")),
99+
kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &node).Return(nil),
100+
wh.EXPECT().SyncStatus(ctx, nmc, &node).Return(errors.New("random error")),
100101
)
101102

102103
_, err := r.Reconcile(ctx, req)
@@ -115,7 +116,6 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
115116
Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) {
116117
*kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc
117118
}),
118-
wh.EXPECT().SyncStatus(ctx, nmc),
119119
kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &node).Return(fmt.Errorf("some error")),
120120
)
121121

@@ -158,13 +158,13 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
158158
Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) {
159159
*kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc
160160
}),
161-
wh.EXPECT().SyncStatus(ctx, nmc),
162161
kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &v1.Node{}).DoAndReturn(
163162
func(_ context.Context, _ types.NamespacedName, fetchedNode *v1.Node, _ ...ctrlclient.Options) error {
164163
*fetchedNode = node
165164
return nil
166165
},
167166
),
167+
wh.EXPECT().SyncStatus(ctx, nmc, &node),
168168
nm.EXPECT().IsNodeSchedulable(&node, nil).Return(false),
169169
nm.EXPECT().UpdateLabels(ctx, &node, nil, []string{kmodName}).DoAndReturn(
170170
func(_ context.Context, obj ctrlclient.Object, _, _ []string) error {
@@ -214,13 +214,13 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
214214
Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) {
215215
*kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc
216216
}),
217-
wh.EXPECT().SyncStatus(ctx, nmc),
218217
kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &v1.Node{}).DoAndReturn(
219218
func(_ context.Context, _ types.NamespacedName, fetchedNode *v1.Node, _ ...ctrlclient.Options) error {
220219
*fetchedNode = node
221220
return nil
222221
},
223222
),
223+
wh.EXPECT().SyncStatus(ctx, nmc, &node),
224224
nm.EXPECT().IsNodeSchedulable(&node, nil).Return(false),
225225
nm.EXPECT().UpdateLabels(ctx, &node, nil, []string{kmodName}).DoAndReturn(
226226
func(_ context.Context, obj ctrlclient.Object, _, _ []string) error {
@@ -295,8 +295,8 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
295295
Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) {
296296
*kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc
297297
}),
298-
wh.EXPECT().SyncStatus(ctx, nmc),
299298
kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &node).Return(nil),
299+
wh.EXPECT().SyncStatus(ctx, nmc, &node),
300300
nm.EXPECT().IsNodeSchedulable(&node, nil).Return(true),
301301
wh.EXPECT().ProcessModuleSpec(contextWithValueMatch, nmc, &spec0, &status0, &node),
302302
nm.EXPECT().IsNodeSchedulable(&node, nil).Return(true),
@@ -377,8 +377,8 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
377377
Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) {
378378
*kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc
379379
}),
380-
wh.EXPECT().SyncStatus(ctx, nmc).Return(nil),
381380
kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &node).Return(nil),
381+
wh.EXPECT().SyncStatus(ctx, nmc, &node).Return(nil),
382382
nm.EXPECT().IsNodeSchedulable(&node, nil).Return(true),
383383
wh.EXPECT().ProcessModuleSpec(contextWithValueMatch, nmc, &spec0, &status0, &node).Return(errors.New(errorMeassge)),
384384
wh.EXPECT().ProcessUnconfiguredModuleStatus(contextWithValueMatch, nmc, &status2, &node).Return(errors.New(errorMeassge)),
@@ -751,7 +751,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessModuleSpec", func() {
751751

752752
gomock.InOrder(
753753
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace),
754-
nm.EXPECT().NodeBecomeReadyAfter(node, status.LastTransitionTime).Return(returnValue),
754+
nm.EXPECT().IsNodeRebooted(node, status.BootId).Return(returnValue),
755755
)
756756

757757
if shouldCreate {
@@ -890,7 +890,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
890890

891891
It("should do nothing , if the node has been rebooted/ready lately", func() {
892892
gomock.InOrder(
893-
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(true),
893+
nm.EXPECT().IsNodeRebooted(&node, status.BootId).Return(true),
894894
client.EXPECT().Status().Return(sw),
895895
sw.EXPECT().Patch(ctx, nmc, gomock.Any()),
896896
)
@@ -904,7 +904,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
904904

905905
It("should create an unloader Pod if no worker Pod exists", func() {
906906
gomock.InOrder(
907-
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
907+
nm.EXPECT().IsNodeRebooted(&node, status.BootId).Return(false),
908908
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace),
909909
mockWorkerPodManager.EXPECT().CreateUnloaderPod(ctx, nmc, status),
910910
)
@@ -925,7 +925,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
925925
}
926926

927927
gomock.InOrder(
928-
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
928+
nm.EXPECT().IsNodeRebooted(&node, status.BootId).Return(false),
929929
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&pod, nil),
930930
mockWorkerPodManager.EXPECT().IsLoaderPod(&pod).Return(true),
931931
mockWorkerPodManager.EXPECT().DeletePod(ctx, &pod),
@@ -947,7 +947,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
947947
}
948948

949949
gomock.InOrder(
950-
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
950+
nm.EXPECT().IsNodeRebooted(&node, status.BootId).Return(false),
951951
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&pod, nil),
952952
mockWorkerPodManager.EXPECT().IsLoaderPod(&pod).Return(false),
953953
)
@@ -976,7 +976,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
976976
}
977977

978978
gomock.InOrder(
979-
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
979+
nm.EXPECT().IsNodeRebooted(&node, status.BootId).Return(false),
980980
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&pod, nil),
981981
mockWorkerPodManager.EXPECT().IsLoaderPod(&pod).Return(false),
982982
mockWorkerPodManager.EXPECT().UnloaderPodTemplate(ctx, nmc, status).Return(nil, errors.New("random error")),
@@ -1008,7 +1008,7 @@ var _ = Describe("nmcReconcilerHelperImpl_ProcessUnconfiguredModuleStatus", func
10081008
podTemplate := p.DeepCopy()
10091009

10101010
gomock.InOrder(
1011-
nm.EXPECT().NodeBecomeReadyAfter(&node, status.LastTransitionTime).Return(false),
1011+
nm.EXPECT().IsNodeRebooted(&node, status.BootId).Return(false),
10121012
mockWorkerPodManager.EXPECT().GetWorkerPod(ctx, podName, namespace).Return(&p, nil),
10131013
mockWorkerPodManager.EXPECT().IsLoaderPod(&p).Return(false),
10141014
mockWorkerPodManager.EXPECT().UnloaderPodTemplate(ctx, nmc, status).Return(podTemplate, nil),
@@ -1055,9 +1055,9 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
10551055
nmc := &kmmv1beta1.NodeModulesConfig{
10561056
ObjectMeta: metav1.ObjectMeta{Name: nmcName},
10571057
}
1058-
1058+
node := v1.Node{}
10591059
Expect(
1060-
wh.SyncStatus(ctx, nmc),
1060+
wh.SyncStatus(ctx, nmc, &node),
10611061
).NotTo(
10621062
HaveOccurred(),
10631063
)
@@ -1110,9 +1110,9 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
11101110
mockWorkerPodManager.EXPECT().DeletePod(ctx, &podWithStatus),
11111111
mockWorkerPodManager.EXPECT().DeletePod(ctx, &podWithoutStatus),
11121112
)
1113-
1113+
node := v1.Node{}
11141114
Expect(
1115-
wh.SyncStatus(ctx, nmc),
1115+
wh.SyncStatus(ctx, nmc, &node),
11161116
).NotTo(
11171117
HaveOccurred(),
11181118
)
@@ -1157,9 +1157,9 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
11571157
sw.EXPECT().Patch(ctx, nmc, gomock.Any()),
11581158
mockWorkerPodManager.EXPECT().DeletePod(ctx, &pod),
11591159
)
1160-
1160+
node := v1.Node{}
11611161
Expect(
1162-
wh.SyncStatus(ctx, nmc),
1162+
wh.SyncStatus(ctx, nmc, &node),
11631163
).NotTo(
11641164
HaveOccurred(),
11651165
)
@@ -1241,9 +1241,9 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
12411241
sw.EXPECT().Patch(ctx, nmc, gomock.Any()),
12421242
mockWorkerPodManager.EXPECT().DeletePod(ctx, &p),
12431243
)
1244-
1244+
node := v1.Node{}
12451245
Expect(
1246-
wh.SyncStatus(ctx, nmc),
1246+
wh.SyncStatus(ctx, nmc, &node),
12471247
).NotTo(
12481248
HaveOccurred(),
12491249
)
@@ -1301,9 +1301,9 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() {
13011301
kubeClient.EXPECT().Status().Return(sw),
13021302
sw.EXPECT().Patch(ctx, nmc, gomock.Any()).Return(errors.New("some error")),
13031303
)
1304-
1304+
node := v1.Node{}
13051305
Expect(
1306-
wh.SyncStatus(ctx, nmc),
1306+
wh.SyncStatus(ctx, nmc, &node),
13071307
).To(
13081308
HaveOccurred(),
13091309
)

Diff for: ‎internal/node/mock_node.go

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

Diff for: ‎internal/node/node.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"github.com/kubernetes-sigs/kernel-module-management/internal/meta"
77
v1 "k8s.io/api/core/v1"
8-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
98
"sigs.k8s.io/controller-runtime/pkg/client"
109
"sigs.k8s.io/controller-runtime/pkg/log"
1110
)
@@ -17,7 +16,7 @@ type Node interface {
1716
GetNodesListBySelector(ctx context.Context, selector map[string]string, tolerations []v1.Toleration) ([]v1.Node, error)
1817
GetNumTargetedNodes(ctx context.Context, selector map[string]string, tolerations []v1.Toleration) (int, error)
1918
UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved []string) error
20-
NodeBecomeReadyAfter(node *v1.Node, checkTime metav1.Time) bool
19+
IsNodeRebooted(node *v1.Node, statusBootId string) bool
2120
}
2221

2322
type node struct {
@@ -85,11 +84,11 @@ func (n *node) UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeR
8584
return nil
8685
}
8786

88-
func (n *node) NodeBecomeReadyAfter(node *v1.Node, timestamp metav1.Time) bool {
87+
func (n *node) IsNodeRebooted(node *v1.Node, statusBootId string) bool {
8988
conds := node.Status.Conditions
9089
for i := 0; i < len(conds); i++ {
9190
c := conds[i]
92-
if c.Type == v1.NodeReady && c.Status == v1.ConditionTrue && timestamp.Before(&c.LastTransitionTime) {
91+
if c.Type == v1.NodeReady && c.Status == v1.ConditionTrue && (statusBootId != node.Status.NodeInfo.BootID) {
9392
return true
9493
}
9594
}

0 commit comments

Comments
 (0)