-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(controller): Fix active pods count in node pending status with pod deleted. #5836
Conversation
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #5836 +/- ##
==========================================
+ Coverage 47.20% 47.36% +0.16%
==========================================
Files 246 247 +1
Lines 15520 15595 +75
==========================================
+ Hits 7326 7387 +61
Misses 7280 7280
- Partials 914 928 +14
Continue to review full report at Codecov.
|
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Signed-off-by: Saravanan Balasubramanian <[email protected]>
go.mod
Outdated
@@ -24,6 +24,7 @@ require ( | |||
github.com/evanphx/json-patch v4.9.0+incompatible | |||
github.com/fatih/structs v1.1.0 // indirect | |||
github.com/gavv/httpexpect/v2 v2.2.0 | |||
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the sig yaml library
p.yaml
Outdated
@@ -0,0 +1,86 @@ | |||
apiVersion: v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please delete this file (you can delete wft.yaml while you're at it)
podObjs = append(podObjs, pods.Items...) | ||
for _, pod := range podObjs { | ||
err = woc.controller.podInformer.GetIndexer().Add(&pod) | ||
fmt.Println(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should panic?
@@ -1395,6 +1395,8 @@ func TestWorkflowParallelismLimit(t *testing.T) { | |||
assert.Equal(t, 2, len(pods.Items)) | |||
// operate again and make sure we don't schedule any more pods | |||
makePodsPhase(ctx, woc, apiv1.PodRunning) | |||
syncPodsInformer(ctx, woc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion - could makePodsPhase
call syncPodsInformer
?
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Checklist:
closes #5811