Repository: kubernetes/kubernetes (language: go)

Issue:
[Failing Test][[sig-node] Pod InPlace Resize Container (scheduler-focused) [Feature:InPlacePodVerticalScaling]
### Which jobs are failing?

master-blocking
- gce-cos-master-alpha-features

### Which tests are failing?

Kubernetes e2e suite.[It] [sig-node] [Serial] Pod InPlace Resize Container (scheduler-focused) [Feature:InPlacePodVerticalScaling] pod-resize-scheduler-tests[Changes](https://github.com/kubernetes/kubernetes/compare/de7e1e7b2...0daa75b97?)

### Since when has it been failing?

[15/10/2024, 21:22:42](https://prow.k8s.io/view/gs/kubernetes-ci-logs/logs/e2e-ci-kubernetes-e2e-cos-gce-alpha-features/1846134514531700736)
[15/10/2024, 13:22:40](https://prow.k8s.io/view/gs/kubernetes-ci-logs/logs/e2e-ci-kubernetes-e2e-cos-gce-alpha-features/1846013717926383616)
View more at: [Triage](https://storage.googleapis.com/k8s-triage/index.html?test=Pod%20InPlace%20Resize%20Container), [Prow](https://prow.k8s.io/view/gs/kubernetes-ci-logs/logs/ci-kubernetes-e2e-gci-gce-alpha-features/1847415318503231488)

### Testgrid link

https://testgrid.k8s.io/sig-release-master-blocking#gce-cos-master-alpha-features

### Reason for failure (if possible)

```
{ failed [FAILED] Timed out after 300.000s.
Expected Pod to be in <v1.PodPhase>: "Running"
Got instead:
    <*v1.Pod | 0xc002e92908>: 
        metadata:
          creationTimestamp: "2024-10-19T00:10:50Z"
          labels:
            time: "192498591"
          managedFields:
          - apiVersion: v1
            fieldsType: FieldsV1
            fieldsV1:
              f:metadata:
                f:labels:
                  .: {}
                  f:time: {}
              f:spec:
                f:affinity:
                  .: {}
                  f:nodeAffinity:
                    .: {}
                    f:requiredDuringSchedulingIgnoredDuringExecution: {}
                    ```

### Anything else we need to know?

N/A

### Relevant SIG(s)

/sig node
@kubernetes/release-team-release-signal


Discussion / hints:
This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
/cc
Is this the cause of the change? /cc @AnishShah 
https://github.com/kubernetes/kubernetes/pull/128143
Hmm. Weird, the test passed during presubmit. Which containerd version is used for this test?

In the testgrid, I noticed one green run in between too
@googs1025 @sanposhiho tests started to fail after https://github.com/kubernetes/kubernetes/pull/127797, the board was still green when #128143 merged.

https://github.com/kubernetes/kubernetes/compare/de7e1e7b2...0daa75b97

![image](https://github.com/user-attachments/assets/2df0f62c-f233-4a32-b403-44121f1edd29)

GCE Alpha feature tab is using:
```
Oct 18 15:22:17.825027 localhost containerd[389]: time="2024-10-18T15:22:17.824940245Z" level=info msg="starting containerd" revision=926c9586fe4a6236699318391cd44976a98e31f1 version=1.7.22
```
> Hmm. Weird, the test passed during presubmit. Which containerd version is used for this test?
> 
> In the testgrid, I noticed one green run in between too

It passed once in 10-19 as well, which means the test is flaking in a high rate. So presubmit CI may pass.

I triggered some presubmit CI in https://github.com/kubernetes/kubernetes/pull/128215 (a revert of https://github.com/kubernetes/kubernetes/pull/127797) to confirm if it is a flake that is caused by https://github.com/kubernetes/kubernetes/pull/128143 or not.
- the first presubmit CI passed https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/128215/pull-kubernetes-e2e-gce-cos-alpha-features/1848204025686659072, which means most likely, the flake may be caused by https://github.com/kubernetes/kubernetes/pull/127797. let me trigger another run.

In #128143, I ran this failing test multiple times during presubmit and it passed
> In #128143, I ran this failing test multiple times during presubmit and it passed

/sig scheduling
/cc @alculquicondor 
https://github.com/kubernetes/kubernetes/pull/127797 may be the root cause.

/assign

Sorry for the trouble! I believe https://github.com/kubernetes/kubernetes/pull/128223 should fix it.
By the way, why don't we run this alpha e2e test job in all PRs? 
Like this time, PRs which the author thinks are not related to the alpha features could break them actually. 
> By the way, why don't we run this alpha e2e test job in all PRs? Like this time, PRs which the author thinks are not related to the alpha features could break them actually.

because is unstable and flaky, and impact developer workflow ... we want to have a very high bar in presubmits for required jobs, people developing alphas or experiment has to rely on optional and periodic jobs to not impact the rest of the project
 we need to encourage the right behaviors on developers, owners of features must also maintain their own testing and jobs, if we make everyone responsible of the testing of all features, then no one is responsible and we get a flaky CI that will hide legit bugs... as part of graduation on features we have e2e and conformance test, so that is when those tests will be automatically run on presubmits, before that each feature owner should be accountable and responsible of their features included testing and reviewing the code and PRs that may intersect with their feature

> because is unstable and flaky

Then, that's the point, right? I do believe, regardless of whether the tests are for the alpha feature or not, we shouldn't have flaky tests. I don't think "this is the alpha feature" is an appropriate excuse that we can accept.

Fix patch:
diff --git a/pkg/scheduler/backend/queue/scheduling_queue.go b/pkg/scheduler/backend/queue/scheduling_queue.go
index a16cccd6f1b22..66e7334f61ab9 100644
--- a/pkg/scheduler/backend/queue/scheduling_queue.go
+++ b/pkg/scheduler/backend/queue/scheduling_queue.go
@@ -989,10 +989,10 @@ func (p *PriorityQueue) AssignedPodAdded(logger klog.Logger, pod *v1.Pod) {
 // may make pending pods with matching affinity terms schedulable.
 func (p *PriorityQueue) AssignedPodUpdated(logger klog.Logger, oldPod, newPod *v1.Pod, event framework.ClusterEvent) {
 	p.lock.Lock()
-	if event.Resource == framework.Pod && event.ActionType&framework.UpdatePodScaleDown != 0 {
+	if (framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.UpdatePodScaleDown}.Match(event)) {
 		// In this case, we don't want to pre-filter Pods by getUnschedulablePodsWithCrossTopologyTerm
 		// because Pod related events may make Pods that were rejected by NodeResourceFit schedulable.
-		p.moveAllToActiveOrBackoffQueue(logger, framework.EventAssignedPodUpdate, oldPod, newPod, nil)
+		p.moveAllToActiveOrBackoffQueue(logger, event, oldPod, newPod, nil)
 	} else {
 		// Pre-filter Pods to move by getUnschedulablePodsWithCrossTopologyTerm
 		// because Pod related events only make Pods rejected by cross topology term schedulable.


Test patch:
diff --git a/pkg/scheduler/backend/queue/scheduling_queue_test.go b/pkg/scheduler/backend/queue/scheduling_queue_test.go
index 83d9e707da74a..aff395abf0049 100644
--- a/pkg/scheduler/backend/queue/scheduling_queue_test.go
+++ b/pkg/scheduler/backend/queue/scheduling_queue_test.go
@@ -2053,6 +2053,121 @@ func TestPriorityQueue_AssignedPodAdded_(t *testing.T) {
 	}
 }
 
+func TestPriorityQueue_AssignedPodUpdated(t *testing.T) {
+	metrics.Register()
+	tests := []struct {
+		name               string
+		unschedPod         *v1.Pod
+		unschedPlugin      string
+		updatedAssignedPod *v1.Pod
+		event              framework.ClusterEvent
+		wantToRequeue      bool
+	}{
+		{
+			name:               "Pod rejected by pod affinity is requeued with matching Pod's update",
+			unschedPod:         st.MakePod().Name("afp").Namespace("ns1").UID("afp").Annotation("annot2", "val2").PodAffinityExists("service", "region", st.PodAffinityWithRequiredReq).Obj(),
+			unschedPlugin:      names.InterPodAffinity,
+			event:              framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.UpdatePodLabel},
+			updatedAssignedPod: st.MakePod().Name("lbp").Namespace("ns1").Label("service", "securityscan").Node("node1").Obj(),
+			wantToRequeue:      true,
+		},
+		{
+			name:               "Pod rejected by pod affinity isn't requeued with unrelated Pod's update",
+			unschedPod:         st.MakePod().Name("afp").Namespace("ns1").UID("afp").Annotation("annot2", "val2").PodAffinityExists("service", "region", st.PodAffinityWithRequiredReq).Obj(),
+			unschedPlugin:      names.InterPodAffinity,
+			event:              framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.UpdatePodLabel},
+			updatedAssignedPod: st.MakePod().Name("lbp").Namespace("unrelated").Label("unrelated", "unrelated").Node("node1").Obj(),
+			wantToRequeue:      false,
+		},
+		{
+			name:               "Pod rejected by pod topology spread is requeued with Pod's update in the same namespace",
+			unschedPod:         st.MakePod().Name("tsp").Namespace("ns1").UID("tsp").SpreadConstraint(1, "node", v1.DoNotSchedule, nil, nil, nil, nil, nil).Obj(),
+			unschedPlugin:      names.PodTopologySpread,
+			event:              framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.UpdatePodLabel},
+			updatedAssignedPod: st.MakePod().Name("lbp").Namespace("ns1").Label("service", "securityscan").Node("node1").Obj(),
+			wantToRequeue:      true,
+		},
+		{
+			name:               "Pod rejected by pod topology spread isn't requeued with unrelated Pod's update",
+			unschedPod:         st.MakePod().Name("afp").Namespace("ns1").UID("afp").Annotation("annot2", "val2").PodAffinityExists("service", "region", st.PodAffinityWithRequiredReq).Obj(),
+			unschedPlugin:      names.PodTopologySpread,
+			event:              framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.UpdatePodLabel},
+			updatedAssignedPod: st.MakePod().Name("lbp").Namespace("unrelated").Label("unrelated", "unrelated").Node("node1").Obj(),
+			wantToRequeue:      false,
+		},
+		{
+			name:               "Pod rejected by resource fit is requeued with assigned Pod's scale down",
+			unschedPod:         st.MakePod().Name("rp").Namespace("ns1").UID("afp").Annotation("annot2", "val2").Req(map[v1.ResourceName]string{v1.ResourceCPU: "1"}).Obj(),
+			unschedPlugin:      names.NodeResourcesFit,
+			event:              framework.ClusterEvent{Resource: framework.EventResource("AssignedPod"), ActionType: framework.UpdatePodScaleDown},
+			updatedAssignedPod: st.MakePod().Name("lbp").Namespace("ns2").Node("node1").Obj(),
+			wantToRequeue:      true,
+		},
+		{
+			name:               "Pod rejected by other plugins isn't requeued with any Pod's update",
+			unschedPod:         st.MakePod().Name("afp").Namespace("ns1").UID("afp").Annotation("annot2", "val2").Obj(),
+			unschedPlugin:      "fakePlugin",
+			event:              framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.UpdatePodLabel},
+			updatedAssignedPod: st.MakePod().Name("lbp").Namespace("unrelated").Label("unrelated", "unrelated").Node("node1").Obj(),
+			wantToRequeue:      false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			logger, ctx := ktesting.NewTestContext(t)
+			ctx, cancel := context.WithCancel(ctx)
+			defer cancel()
+
+			c := testingclock.NewFakeClock(time.Now())
+			m := makeEmptyQueueingHintMapPerProfile()
+			m[""] = map[framework.ClusterEvent][]*QueueingHintFunction{
+				{Resource: framework.Pod, ActionType: framework.UpdatePodLabel}: {
+					{
+						PluginName:     "fakePlugin",
+						QueueingHintFn: queueHintReturnQueue,
+					},
+					{
+						PluginName:     names.InterPodAffinity,
+						QueueingHintFn: queueHintReturnQueue,
+					},
+					{
+						PluginName:     names.PodTopologySpread,
+						QueueingHintFn: queueHintReturnQueue,
+					},
+				},
+				{Resource: framework.Pod, ActionType: framework.UpdatePodScaleDown}: {
+					{
+						PluginName:     names.NodeResourcesFit,
+						QueueingHintFn: queueHintReturnQueue,
+					},
+				},
+			}
+			q := NewTestQueue(ctx, newDefaultQueueSort(), WithClock(c), WithQueueingHintMapPerProfile(m))
+
+			// To simulate the pod is failed in scheduling in the real world, Pop() the pod from activeQ before AddUnschedulableIfNotPresent()s below.
+			q.Add(logger, tt.unschedPod)
+			if p, err := q.Pop(logger); err != nil || p.Pod != tt.unschedPod {
+				t.Errorf("Expected: %v after Pop, but got: %v", tt.unschedPod.Name, p.Pod.Name)
+			}
+
+			err := q.AddUnschedulableIfNotPresent(logger, q.newQueuedPodInfo(tt.unschedPod, tt.unschedPlugin), q.SchedulingCycle())
+			if err != nil {
+				t.Fatalf("unexpected error from AddUnschedulableIfNotPresent: %v", err)
+			}
+
+			// Move clock to make the unschedulable pods complete backoff.
+			c.Step(DefaultPodInitialBackoffDuration + time.Second)
+
+			q.AssignedPodUpdated(logger, nil, tt.updatedAssignedPod, tt.event)
+
+			if q.activeQ.has(newQueuedPodInfoForLookup(tt.unschedPod)) != tt.wantToRequeue {
+				t.Fatalf("unexpected Pod move: Pod should be requeued: %v. Pod is actually requeued: %v", tt.wantToRequeue, !tt.wantToRequeue)
+			}
+		})
+	}
+}
+
 func TestPriorityQueue_NominatedPodsForNode(t *testing.T) {
 	objs := []runtime.Object{medPriorityPodInfo.Pod, unschedulablePodInfo.Pod, highPriorityPodInfo.Pod}
 	logger, ctx := ktesting.NewTestContext(t)
diff --git a/pkg/scheduler/framework/types_test.go b/pkg/scheduler/framework/types_test.go
index 1b1d81bd56105..ed354387daee8 100644
--- a/pkg/scheduler/framework/types_test.go
+++ b/pkg/scheduler/framework/types_test.go
@@ -1709,6 +1709,12 @@ func TestCloudEvent_Match(t *testing.T) {
 			comingEvent: ClusterEvent{Resource: Pod, ActionType: UpdateNodeLabel},
 			wantResult:  true,
 		},
+		{
+			name:        "event with resource = 'Pod' matching with coming events carries unschedulablePod",
+			event:       ClusterEvent{Resource: Pod, ActionType: UpdateNodeLabel | UpdateNodeTaint},
+			comingEvent: ClusterEvent{Resource: unschedulablePod, ActionType: UpdateNodeLabel},
+			wantResult:  true,
+		},
 		{
 			name:        "event with resource = '*' matching with coming events carries same actionType",
 			event:       ClusterEvent{Resource: WildCard, ActionType: UpdateNodeLabel},