Bug 1748150: Modify scheduler preemption tests (fixup #23645) by damemi · Pull Request #23719 · openshift/origin

damemi · 2019-09-03T14:04:08Z

This fixes a bug introduced in #23645 by initializing the pod array with a length of 0 and a capacity of 4, due to the fact that some nodes may be unschedulable and thus we can't assume that len(pods) will always have 4 pods

openshift-ci-robot · 2019-09-03T14:04:13Z

@damemi: This pull request references Bugzilla bug 1748150, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1748150: Modify scheduler preemption tests (fixup #23645)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

soltysh

/lgtm
/approve

openshift-bot · 2019-09-03T14:26:22Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-03T15:05:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-03T15:18:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

damemi · 2019-09-03T16:01:41Z

/retest

mfojtik · 2019-09-03T18:39:29Z

/lgtm

openshift-bot · 2019-09-03T19:25:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-03T20:04:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-03T20:17:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-03T20:43:09Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-03T22:01:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-03T22:53:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-04T00:50:06Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-04T03:00:05Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-04T04:57:05Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-04T07:20:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-04T09:17:10Z

/retest

Please review the full test history for this PR and help us cut down flakes.

soltysh · 2019-09-04T09:32:27Z

/hold
it looks like there's a another problem, in line 336 of this file, see:

	/usr/local/go/src/runtime/panic.go:522 +0x1b5
github.com/openshift/origin/vendor/k8s.io/kubernetes/test/e2e/scheduling.glob..func4.5()
	/go/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/vendor/k8s.io/kubernetes/test/e2e/scheduling/preemption.go:336 +0x15f2

soltysh · 2019-09-04T09:44:51Z

@damemi stupid me, when you create a slice like this make([]*v1.Pod, 0, numPods) you're actually creating an empty slice with given capacity, this means you need to use append to add pods into it.

soltysh

/lgtm cancel

…#23645) This fixes a bug introduced in openshift#23645 by initializing the pod array with a length of 0 and a capacity of 4

damemi · 2019-09-04T19:20:14Z

@soltysh good catch, could you /lgtm this again?

soltysh

/lgtm
/hold cancel

openshift-ci-robot · 2019-09-04T19:49:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damemi, mfojtik, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mfojtik,soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

damemi · 2019-09-04T22:09:04Z

/retest

damemi · 2019-09-05T01:07:38Z

/retest

openshift-ci-robot · 2019-09-05T02:23:23Z

@damemi: All pull requests linked via external trackers have merged. Bugzilla bug 1748150 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1748150: Modify scheduler preemption tests (fixup #23645)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

This test was marked as flaky but has had a number of fixes applied to it (pr openshift#23645 and openshift#23719) tht have adjusted the node utilization calculations and structure of the test to hopefully make it more stable. In order to address the comments in the BZ for this test's flakiness (https://bugzilla.redhat.com/show_bug.cgi?id=1731263#c9) we should remove the [flaky] tag to re-enable the tests.

…-enable it This test was marked as flaky but has had a number of fixes applied to it (pr openshift#23645 and openshift#23719) tht have adjusted the node utilization calculations and structure of the test to hopefully make it more stable. In order to address the comments in the BZ for this test's flakiness (https://bugzilla.redhat.com/show_bug.cgi?id=1731263#c9) we should remove the [flaky] tag to re-enable the tests.

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 3, 2019

openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 3, 2019

openshift-ci-robot requested review from soltysh and timlnx September 3, 2019 14:04

soltysh approved these changes Sep 3, 2019

View reviewed changes

openshift-ci-robot assigned soltysh Sep 3, 2019

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 3, 2019

damemi force-pushed the fixup-23645 branch from c5d9998 to aa5068f Compare September 3, 2019 17:53

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2019

openshift-ci-robot assigned mfojtik Sep 3, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2019

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 4, 2019

soltysh suggested changes Sep 4, 2019

View reviewed changes

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2019

UPSTREAM: <carry>: Modify scheduler preemption tests (fixup openshift…

5690efe

…#23645) This fixes a bug introduced in openshift#23645 by initializing the pod array with a length of 0 and a capacity of 4

damemi force-pushed the fixup-23645 branch from aa5068f to 5690efe Compare September 4, 2019 13:05

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 4, 2019

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Sep 4, 2019

soltysh approved these changes Sep 4, 2019

View reviewed changes

openshift-merge-robot merged commit ab0a4e4 into openshift:master Sep 5, 2019

damemi mentioned this pull request Sep 10, 2019

Bug 1731263: Reenable basic pod preemption e2es #23758

Closed

Conversation

damemi commented Sep 3, 2019

Uh oh!

openshift-ci-robot commented Sep 3, 2019

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

damemi commented Sep 3, 2019

Uh oh!

mfojtik commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 3, 2019

Uh oh!

openshift-bot commented Sep 4, 2019

Uh oh!

openshift-bot commented Sep 4, 2019

Uh oh!

openshift-bot commented Sep 4, 2019

Uh oh!

openshift-bot commented Sep 4, 2019

Uh oh!

openshift-bot commented Sep 4, 2019

Uh oh!

soltysh commented Sep 4, 2019

Uh oh!

soltysh commented Sep 4, 2019

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

damemi commented Sep 4, 2019

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Sep 4, 2019

Uh oh!

damemi commented Sep 4, 2019

Uh oh!

damemi commented Sep 5, 2019

Uh oh!

openshift-ci-robot commented Sep 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants