Skip to content

Commit 5580b08

Browse files
authored
Merge pull request #619 from mimowo/dont-use-ssa-for-podgc
Don't use SSA in gcp-controller manager PodGC
2 parents fd5f64d + b0c8b9b commit 5580b08

File tree

6 files changed

+116
-22
lines changed

6 files changed

+116
-22
lines changed

cmd/gcp-controller-manager/BUILD

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ go_library(
7777
"//vendor/k8s.io/apiserver/pkg/server/options",
7878
"//vendor/k8s.io/apiserver/pkg/util/feature",
7979
"//vendor/k8s.io/apiserver/pkg/util/webhook",
80-
"//vendor/k8s.io/client-go/applyconfigurations/core/v1:core",
8180
"//vendor/k8s.io/client-go/informers",
8281
"//vendor/k8s.io/client-go/informers/core/v1:core",
8382
"//vendor/k8s.io/client-go/kubernetes",
@@ -98,11 +97,13 @@ go_library(
9897
"//vendor/k8s.io/controller-manager/pkg/clientbuilder",
9998
"//vendor/k8s.io/klog/v2:klog",
10099
"//vendor/k8s.io/kubernetes/pkg/api/legacyscheme",
100+
"//vendor/k8s.io/kubernetes/pkg/api/v1/pod",
101101
"//vendor/k8s.io/kubernetes/pkg/apis/certificates/install",
102102
"//vendor/k8s.io/kubernetes/pkg/apis/certificates/v1:certificates",
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
)

cmd/gcp-controller-manager/node_csr_approver.go

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,15 @@ import (
4848
utilerrors "k8s.io/apimachinery/pkg/util/errors"
4949
utilfeature "k8s.io/apiserver/pkg/util/feature"
5050
"k8s.io/apiserver/pkg/util/webhook"
51-
corev1apply "k8s.io/client-go/applyconfigurations/core/v1"
5251
"k8s.io/cloud-provider-gcp/pkg/csrmetrics"
5352
"k8s.io/cloud-provider-gcp/pkg/nodeidentity"
5453
"k8s.io/cloud-provider-gcp/pkg/tpmattest"
5554
"k8s.io/klog/v2"
55+
apipod "k8s.io/kubernetes/pkg/api/v1/pod"
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 (
@@ -1040,22 +1041,15 @@ func markFailedAndDeletePodWithCondition(ctx *controllerContext, pod *v1.Pod) er
10401041
// is orphaned, in which case the pod would remain in the Running phase
10411042
// forever as there is no kubelet running to change the phase.
10421043
if pod.Status.Phase != v1.PodSucceeded && pod.Status.Phase != v1.PodFailed {
1043-
podApply := corev1apply.Pod(pod.Name, pod.Namespace).WithStatus(corev1apply.PodStatus())
1044-
// we don't need to extract the pod apply configuration and can send
1045-
// only phase and the DisruptionTarget condition as GCPControllerManager would not
1046-
// own other fields. If the DisruptionTarget condition is owned by
1047-
// GCPControllerManager it means that it is in the Failed phase, so sending the
1048-
// condition will not be re-attempted.
1049-
podApply.Status.WithPhase(v1.PodFailed)
1050-
podApply.Status.WithConditions(
1051-
corev1apply.PodCondition().
1052-
WithType(v1.DisruptionTarget).
1053-
WithStatus(v1.ConditionTrue).
1054-
WithReason("DeletionByGCPControllerManager").
1055-
WithMessage(fmt.Sprintf("%s: node no longer exists", fieldManager)).
1056-
WithLastTransitionTime(metav1.Now()))
1057-
1058-
if _, err := ctx.client.CoreV1().Pods(pod.Namespace).ApplyStatus(context.TODO(), podApply, metav1.ApplyOptions{FieldManager: fieldManager, Force: true}); err != nil {
1044+
newStatus := pod.Status.DeepCopy()
1045+
newStatus.Phase = v1.PodFailed
1046+
apipod.UpdatePodCondition(newStatus, &v1.PodCondition{
1047+
Type: v1.DisruptionTarget,
1048+
Status: v1.ConditionTrue,
1049+
Reason: "DeletionByGCPControllerManager",
1050+
Message: fmt.Sprintf("%s: node no longer exists", fieldManager),
1051+
})
1052+
if _, _, _, err := utilpod.PatchPodStatus(context.TODO(), ctx.client, pod.Namespace, pod.Name, pod.UID, pod.Status, *newStatus); err != nil {
10591053
return err
10601054
}
10611055
}

cmd/gcp-controller-manager/node_csr_approver_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,16 +1806,16 @@ func TestDeleteAllPodsBoundToNode(t *testing.T) {
18061806
Status: v1.PodStatus{
18071807
Phase: v1.PodFailed,
18081808
Conditions: []v1.PodCondition{
1809+
{
1810+
Type: v1.PodReady,
1811+
Status: v1.ConditionTrue,
1812+
},
18091813
{
18101814
Type: v1.DisruptionTarget,
18111815
Status: v1.ConditionTrue,
18121816
Reason: "DeletionByGCPControllerManager",
18131817
Message: "GCPControllerManager: node no longer exists",
18141818
},
1815-
{
1816-
Type: v1.PodReady,
1817-
Status: v1.ConditionTrue,
1818-
},
18191819
},
18201820
},
18211821
},

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)