Skip to content

Commit b0c8b9b

Browse files
committed
Use patch instead of update for GCPControllerManager
1 parent 84d41b3 commit b0c8b9b

File tree

6 files changed

+127
-11
lines changed

6 files changed

+127
-11
lines changed

cmd/gcp-controller-manager/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ go_library(
103103
"//vendor/k8s.io/kubernetes/pkg/controller",
104104
"//vendor/k8s.io/kubernetes/pkg/controller/certificates",
105105
"//vendor/k8s.io/kubernetes/pkg/features",
106+
"//vendor/k8s.io/kubernetes/pkg/util/pod",
106107
"//vendor/k8s.io/kubernetes/pkg/util/taints",
107108
],
108109
)
@@ -139,6 +140,7 @@ go_test(
139140
"//vendor/k8s.io/apimachinery/pkg/runtime",
140141
"//vendor/k8s.io/apimachinery/pkg/runtime/schema",
141142
"//vendor/k8s.io/apimachinery/pkg/types",
143+
"//vendor/k8s.io/apimachinery/pkg/util/strategicpatch",
142144
"//vendor/k8s.io/client-go/kubernetes/fake",
143145
"//vendor/k8s.io/client-go/listers/core/v1:core",
144146
"//vendor/k8s.io/client-go/testing",

cmd/gcp-controller-manager/node_csr_approver.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import (
5656
certutil "k8s.io/kubernetes/pkg/apis/certificates/v1"
5757
"k8s.io/kubernetes/pkg/controller/certificates"
5858
"k8s.io/kubernetes/pkg/features"
59+
utilpod "k8s.io/kubernetes/pkg/util/pod"
5960
)
6061

6162
const (
@@ -1038,17 +1039,15 @@ func markFailedAndDeletePodWithCondition(ctx *controllerContext, pod *v1.Pod) er
10381039
// is orphaned, in which case the pod would remain in the Running phase
10391040
// forever as there is no kubelet running to change the phase.
10401041
if pod.Status.Phase != v1.PodSucceeded && pod.Status.Phase != v1.PodFailed {
1041-
newPod := pod.DeepCopy()
1042-
newPod.Status.Phase = v1.PodFailed
1043-
apipod.UpdatePodCondition(&newPod.Status, &v1.PodCondition{
1042+
newStatus := pod.Status.DeepCopy()
1043+
newStatus.Phase = v1.PodFailed
1044+
apipod.UpdatePodCondition(newStatus, &v1.PodCondition{
10441045
Type: v1.DisruptionTarget,
10451046
Status: v1.ConditionTrue,
10461047
Reason: "DeletionByGCPControllerManager",
10471048
Message: fmt.Sprintf("%s: node no longer exists", fieldManager),
10481049
})
1049-
if _, err := ctx.client.CoreV1().Pods(pod.Namespace).UpdateStatus(context.TODO(), newPod, metav1.UpdateOptions{
1050-
FieldManager: fieldManager,
1051-
}); err != nil {
1050+
if _, _, _, err := utilpod.PatchPodStatus(context.TODO(), ctx.client, pod.Namespace, pod.Name, pod.UID, pod.Status, *newStatus); err != nil {
10521051
return err
10531052
}
10541053
}

cmd/gcp-controller-manager/node_csr_approver_test.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import (
5050
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5151
"k8s.io/apimachinery/pkg/runtime"
5252
"k8s.io/apimachinery/pkg/runtime/schema"
53+
"k8s.io/apimachinery/pkg/util/strategicpatch"
5354
"k8s.io/client-go/kubernetes/fake"
5455
testclient "k8s.io/client-go/testing"
5556
"k8s.io/cloud-provider-gcp/pkg/nodeidentity"
@@ -1875,12 +1876,12 @@ func TestDeleteAllPodsBoundToNode(t *testing.T) {
18751876
t.Fatalf("Unexpected number of actions, got %v, want 3 (list pods, patch pod, delete pod)", len(actions))
18761877
}
18771878

1878-
var updateAction testclient.UpdateAction
1879+
var patchAction testclient.PatchAction
18791880
var deleteAction testclient.DeleteAction
18801881

18811882
for _, action := range actions {
1882-
if action.GetVerb() == "update" {
1883-
updateAction = action.(testclient.UpdateAction)
1883+
if action.GetVerb() == "patch" {
1884+
patchAction = action.(testclient.PatchAction)
18841885
}
18851886

18861887
if action.GetVerb() == "delete" {
@@ -1889,8 +1890,23 @@ func TestDeleteAllPodsBoundToNode(t *testing.T) {
18891890
}
18901891

18911892
if tc.expectedPatchedPod != nil {
1892-
patchedPod := updateAction.GetObject().(*v1.Pod)
1893-
if diff := cmp.Diff(tc.expectedPatchedPod, patchedPod, cmpopts.IgnoreFields(v1.Pod{}, "TypeMeta"), cmpopts.IgnoreFields(v1.PodCondition{}, "LastTransitionTime")); diff != "" {
1893+
patchedPodBytes := patchAction.GetPatch()
1894+
1895+
originalPod, err := json.Marshal(tc.pod)
1896+
if err != nil {
1897+
t.Fatalf("Failed to marshal original pod %#v: %v", originalPod, err)
1898+
}
1899+
updated, err := strategicpatch.StrategicMergePatch(originalPod, patchedPodBytes, v1.Pod{})
1900+
if err != nil {
1901+
t.Fatalf("Failed to apply strategic merge patch %q on pod %#v: %v", patchedPodBytes, originalPod, err)
1902+
}
1903+
1904+
updatedPod := &v1.Pod{}
1905+
if err := json.Unmarshal(updated, updatedPod); err != nil {
1906+
t.Fatalf("Failed to unmarshal updated pod %q: %v", updated, err)
1907+
}
1908+
1909+
if diff := cmp.Diff(tc.expectedPatchedPod, updatedPod, cmpopts.IgnoreFields(v1.Pod{}, "TypeMeta"), cmpopts.IgnoreFields(v1.PodCondition{}, "LastTransitionTime")); diff != "" {
18941910
t.Fatalf("Unexpected diff on pod (-want,+got):\n%s", diff)
18951911
}
18961912
}

vendor/k8s.io/kubernetes/pkg/util/pod/BUILD

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

vendor/k8s.io/kubernetes/pkg/util/pod/pod.go

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

vendor/modules.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1443,6 +1443,7 @@ k8s.io/kubernetes/pkg/features
14431443
k8s.io/kubernetes/pkg/fieldpath
14441444
k8s.io/kubernetes/pkg/util/hash
14451445
k8s.io/kubernetes/pkg/util/parsers
1446+
k8s.io/kubernetes/pkg/util/pod
14461447
k8s.io/kubernetes/pkg/util/taints
14471448
# k8s.io/metrics v0.28.0 => k8s.io/metrics v0.28.0
14481449
## explicit; go 1.20

0 commit comments

Comments
 (0)