Fix e2e limits #70329
Fix e2e limits #70329
Conversation
k8s-ci-robot
added
sig/scheduling
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ravisantoshgudimetla The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
needs-kind
| return err | ||
| } | ||
| _, err = c.CoreV1().Nodes().Patch(string(node.Name), types.StrategicMergePatchType, patchBytes) | ||
| _, err = c.CoreV1().Nodes().UpdateStatus(node) |
There was a problem hiding this comment.
Is that to say, Patch() doesn't work for API fields in status? Or, it has bug on updating both memory and cpu fields? (b/c I checked the history, it seems that our code was always using Patch(), but updating memory only)
There was a problem hiding this comment.
Is that to say, Patch() doesn't work for API fields in status?
Yeah, that's right. Patch wasn't working for updating cpu or memory.
In order to test it, I added taints to node and applied patch, this worked while cpu and memory were not getting updated. So, I had to use UpdateStatus.
| @@ -281,14 +279,22 @@ var _ = SIGDescribe("SchedulerPriorities [Serial]", func() { | |||
| Expect(found).To(Equal(true)) | |||
| nodeOriginalMemoryVal := nodeOriginalMemory.Value() | |||
| nodeOriginalCPUVal := nodeOriginalCPU.MilliValue() | |||
| err := updateNodeAllocatable(cs, nodeName, int64(10000), int64(12000)) | |||
| err := updateNodeAllocatable(cs, nodeName, int64(10737418240), int64(12000)) | |||
There was a problem hiding this comment.
Any reason to set this particular value? Isn't 10000 (greater than incoming limit 5000m) good enough?
There was a problem hiding this comment.
This value is for memory not cpu
There was a problem hiding this comment.
sry, I misread... Let me rephrase: so the original mem val 10000 (greater than incoming limit 3000Mi) should be put as a form 10737418240, to represent 10Gi, right?
There was a problem hiding this comment.
Yes, that is right and that code path was not getting exercised as patch had not effect.
|
/kind failing-test |
k8s-ci-robot
added
kind/failing-test
k8s-ci-robot
added
the
lgtm
What type of PR is this?
/kind flake
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
This should help in fixing #69989
Special notes for your reviewer:
Seems
clientset.CoreV1().Nodes().Patch(string(node.Name), types.StrategicMergePatchType, patchBytes)doesn't update the node status which makes the all the nodes to have same allocatable values there by causing the test failure. Changed it toupdateStatuswhich should help in resolving the bug./sig scheduling
/cc @bsalamat @Huang-Wei
Does this PR introduce a user-facing change?: