Skip to content

Commit 0634552

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #5259 from RishabhSaini/crcMissing
OCPBUGS-56558: Recheck `generatedByControllerVersion` annotation prior to deleting a degraded MC
2 parents 7a56cf0 + c2c1b70 commit 0634552

File tree

5 files changed

+32
-2
lines changed

5 files changed

+32
-2
lines changed

‎pkg/controller/container-runtime-config/container_runtime_config_controller.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -779,8 +779,13 @@ func (ctrl *Controller) cleanUpDuplicatedMC() error {
779779
if !strings.Contains(mc.Name, generatedCtrCfg) {
780780
continue
781781
}
782+
// Recheck to ensure nothing changed in between
783+
mcToBeDeleted, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), mc.Name, metav1.GetOptions{})
784+
if err != nil {
785+
return err
786+
}
782787
// delete the containerruntime mc if its degraded
783-
if mc.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] != version.Hash {
788+
if mcToBeDeleted.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] != version.Hash {
784789
if err := ctrl.client.MachineconfigurationV1().MachineConfigs().Delete(context.TODO(), mc.Name, metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) {
785790
return fmt.Errorf("error deleting degraded containerruntime machine config %s: %w", mc.Name, err)
786791
}

‎pkg/controller/container-runtime-config/container_runtime_config_controller_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,7 @@ func TestContainerRuntimeConfigCreate(t *testing.T) {
658658
f.expectUpdateContainerRuntimeConfigRoot(ctrcfg1)
659659
f.expectCreateMachineConfigAction(mcs1)
660660
f.expectPatchContainerRuntimeConfig(ctrcfg1, ctrcfgPatchBytes)
661+
f.expectGetMachineConfigAction(mcs2)
661662
f.expectUpdateContainerRuntimeConfig(ctrcfg1)
662663

663664
f.run(getKey(ctrcfg1, t))
@@ -698,6 +699,7 @@ func TestContainerRuntimeConfigUpdate(t *testing.T) {
698699
f.expectUpdateContainerRuntimeConfigRoot(ctrcfg1)
699700
f.expectCreateMachineConfigAction(mcs)
700701
f.expectPatchContainerRuntimeConfig(ctrcfg1, ctrcfgPatchBytes)
702+
f.expectGetMachineConfigAction(mcs)
701703
f.expectUpdateContainerRuntimeConfig(ctrcfg1)
702704

703705
c := f.newController()
@@ -740,6 +742,7 @@ func TestContainerRuntimeConfigUpdate(t *testing.T) {
740742
f.expectUpdateContainerRuntimeConfig(ctrcfgUpdate)
741743
f.expectUpdateMachineConfigAction(mcsUpdate)
742744
f.expectPatchContainerRuntimeConfig(ctrcfgUpdate, ctrcfgPatchBytes)
745+
f.expectGetMachineConfigAction(mcsUpdate)
743746
f.expectUpdateContainerRuntimeConfig(ctrcfgUpdate)
744747

745748
f.validateActions()
@@ -1651,6 +1654,7 @@ func TestCtrruntimeConfigMultiCreate(t *testing.T) {
16511654
f.expectUpdateContainerRuntimeConfigRoot(ccr)
16521655
f.expectCreateMachineConfigAction(mcs)
16531656
f.expectPatchContainerRuntimeConfig(ccr, []byte(expectedPatch))
1657+
f.expectGetMachineConfigAction(mcs)
16541658
f.expectUpdateContainerRuntimeConfig(ccr)
16551659
f.run(poolName)
16561660
}
@@ -1760,6 +1764,7 @@ func TestAddAnnotationExistingContainerRuntimeConfig(t *testing.T) {
17601764
f.expectUpdateContainerRuntimeConfigRoot(ctrc)
17611765
f.expectUpdateMachineConfigAction(ctrcfgMC)
17621766
f.expectPatchContainerRuntimeConfig(ctrc, []byte("{}"))
1767+
f.expectGetMachineConfigAction(ctrcfgMC)
17631768
f.expectUpdateContainerRuntimeConfig(ctrc)
17641769

17651770
c := f.newController()

‎pkg/controller/kubelet-config/kubelet_config_controller.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,8 +751,13 @@ func (ctrl *Controller) cleanUpDuplicatedMC(prefix string) error {
751751
if !strings.HasPrefix(mc.Name, prefix) {
752752
continue
753753
}
754+
// Recheck to ensure nothing changed in between
755+
mcToBeDeleted, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), mc.Name, metav1.GetOptions{})
756+
if err != nil {
757+
return err
758+
}
754759
// delete the mc if its degraded
755-
if mc.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] != version.Hash {
760+
if mcToBeDeleted.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] != version.Hash {
756761
if err := ctrl.client.MachineconfigurationV1().MachineConfigs().Delete(context.TODO(), mc.Name, metav1.DeleteOptions{}); err != nil && !macherrors.IsNotFound(err) {
757762
return fmt.Errorf("error deleting degraded kubelet machine config %s: %w", mc.Name, err)
758763
}

‎pkg/controller/kubelet-config/kubelet_config_controller_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ func TestKubeletConfigCreate(t *testing.T) {
464464
f.expectUpdateKubeletConfigRoot(kc1)
465465
f.expectCreateMachineConfigAction(mcs)
466466
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
467+
f.expectGetMachineConfigAction(mcs)
467468
f.expectUpdateKubeletConfig(kc1)
468469

469470
f.run(getKey(kc1, t))
@@ -510,6 +511,7 @@ func TestKubeletConfigMultiCreate(t *testing.T) {
510511
f.expectUpdateKubeletConfigRoot(kc)
511512
f.expectCreateMachineConfigAction(mcs)
512513
f.expectPatchKubeletConfig(kc, []byte(expectedPatch))
514+
f.expectGetMachineConfigAction(mcs)
513515
f.expectUpdateKubeletConfig(kc)
514516
f.run(poolName)
515517
}
@@ -558,6 +560,7 @@ func TestKubeletConfigAutoSizingReserved(t *testing.T) {
558560
f.expectUpdateKubeletConfigRoot(kc1)
559561
f.expectCreateMachineConfigAction(mcs)
560562
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
563+
f.expectGetMachineConfigAction(mcs)
561564
f.expectUpdateKubeletConfig(kc1)
562565

563566
f.run(getKey(kc1, t))
@@ -601,6 +604,7 @@ func TestKubeletConfiglogFile(t *testing.T) {
601604
f.expectUpdateKubeletConfigRoot(kc1)
602605
f.expectCreateMachineConfigAction(mcs)
603606
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
607+
f.expectGetMachineConfigAction(mcs)
604608
f.expectUpdateKubeletConfig(kc1)
605609

606610
f.run(getKey(kc1, t))
@@ -638,6 +642,7 @@ func TestKubeletConfigUpdates(t *testing.T) {
638642
f.expectUpdateKubeletConfigRoot(kc1)
639643
f.expectCreateMachineConfigAction(mcs)
640644
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
645+
f.expectGetMachineConfigAction(mcs)
641646
f.expectUpdateKubeletConfig(kc1)
642647

643648
c := f.newController(fgHandler)
@@ -696,6 +701,7 @@ func TestKubeletConfigUpdates(t *testing.T) {
696701
f.expectGetMachineConfigAction(mcs)
697702
f.expectUpdateMachineConfigAction(mcUpdate)
698703
f.expectPatchKubeletConfig(kcUpdate, kcfgPatchBytes)
704+
f.expectGetMachineConfigAction(mcs)
699705
f.expectUpdateKubeletConfig(kcUpdate)
700706

701707
f.validateActions()
@@ -742,6 +748,7 @@ func TestMachineConfigUpdateUponFeatureGateUpdate(t *testing.T) {
742748
f.expectUpdateKubeletConfigRoot(kc1)
743749
f.expectCreateMachineConfigAction(mcs)
744750
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
751+
f.expectGetMachineConfigAction(mcs)
745752
f.expectUpdateKubeletConfig(kc1)
746753

747754
c := f.newController(fgHandler)
@@ -787,6 +794,7 @@ func TestMachineConfigUpdateUponFeatureGateUpdate(t *testing.T) {
787794
f.expectGetMachineConfigAction(mcs)
788795
f.expectUpdateMachineConfigAction(mcUpdate)
789796
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
797+
f.expectGetMachineConfigAction(mcs)
790798
f.expectUpdateKubeletConfig(kc1)
791799

792800
f.validateActions()
@@ -834,6 +842,7 @@ func TestMachineConfigSkipUpdate(t *testing.T) {
834842
f.expectUpdateKubeletConfigRoot(kc1)
835843
f.expectCreateMachineConfigAction(mcs)
836844
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
845+
f.expectGetMachineConfigAction(mcs)
837846
f.expectUpdateKubeletConfig(kc1)
838847

839848
c := f.newController(fgHandler)
@@ -981,6 +990,7 @@ func TestKubeletFeatureExists(t *testing.T) {
981990
f.expectUpdateKubeletConfigRoot(kc1)
982991
f.expectCreateMachineConfigAction(mcs)
983992
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
993+
f.expectGetMachineConfigAction(mcs)
984994
f.expectUpdateKubeletConfig(kc1)
985995

986996
f.run(getKey(kc1, t))
@@ -1200,6 +1210,7 @@ func TestAddAnnotationExistingKubeletConfig(t *testing.T) {
12001210
f.expectUpdateKubeletConfigRoot(kc)
12011211
f.expectUpdateMachineConfigAction(kcMC)
12021212
f.expectPatchKubeletConfig(kc, []byte("{}"))
1213+
f.expectGetMachineConfigAction(kcMC)
12031214
f.expectUpdateKubeletConfig(kc)
12041215

12051216
c := f.newController(fgHandler)

‎pkg/controller/kubelet-config/kubelet_config_features_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ func TestFeaturesDefault(t *testing.T) {
9797
f.expectGetMachineConfigAction(mcs2Deprecated)
9898
f.expectGetMachineConfigAction(mcs2)
9999
f.expectCreateMachineConfigAction(mcs2)
100+
f.expectGetMachineConfigAction(mcs2Deprecated)
101+
f.expectGetMachineConfigAction(mcs2)
100102

101103
f.runFeature(getKeyFromFeatureGate(features, t), fgHandler)
102104
})
@@ -153,6 +155,8 @@ func TestFeaturesCustomNoUpgrade(t *testing.T) {
153155
f.expectGetMachineConfigAction(mcs2Deprecated)
154156
f.expectGetMachineConfigAction(mcs2)
155157
f.expectCreateMachineConfigAction(mcs2)
158+
f.expectGetMachineConfigAction(mcs2Deprecated)
159+
f.expectGetMachineConfigAction(mcs2)
156160
f.runFeature(getKeyFromFeatureGate(features, t), fgHandler)
157161
})
158162
}

0 commit comments

Comments
 (0)