Skip to content
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.

Sign up for GitHub

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

Jump to bottom

check for pending, not running, state in next build start tests #16924

Merged
merged 1 commit into from
Oct 20, 2017
Merged

check for pending, not running, state in next build start tests #16924

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions test/extended/builds/run_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import (
exutil "github.com/openshift/origin/test/extended/util"
)

// this test is very latency sensitive so run it by itself (serially).
var _ = g.Describe("[Feature:Builds][Slow][Serial] using build configuration runPolicy", func() {
var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy", func() {
defer g.GinkgoRecover()
var (
// Use invalid source here as we don't care about the result
Expand Down Expand Up @@ -152,7 +151,7 @@ var _ = g.Describe("[Feature:Builds][Slow][Serial] using build configuration run
o.Expect(build.Status.CompletionTimestamp).ToNot(o.BeNil(), "completed builds should have a valid completion time")
sawCompletion = true
}
if build.Status.Phase == buildapi.BuildPhaseRunning {
if build.Status.Phase == buildapi.BuildPhaseRunning || build.Status.Phase == buildapi.BuildPhasePending {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to consider builds in new state? or can we safely assume they will always be transitioned in a timely enough fashion to pending

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point of the test is to create a bunch of builds in the new state, then ensure that one at a time they enter pending(or running), with the next one entering pending(or running) when the previous one completes.

considering new would break the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point of the change here is that it might take a while for a pod to enter running, which screws up the confirmatoin this test does to say "did this pod enter running sooner after the last build finished".

So instead of needing the pod to enter running, it's sufficient to see that the build entered pending (Which means we created the pod for the build, which we won't do until the previous build completes). This will be less vulnerable to problem w/ scheduling and container creation which might prevent a pod from entering a running state, despite the build controller doing the right thing.

latency := lastCompletion.Sub(time.Now())
o.Expect(latency).To(o.BeNumerically("<", 20*time.Second), "next build should have started less than 20s after last completed build")

Expand Down Expand Up @@ -256,6 +255,17 @@ var _ = g.Describe("[Feature:Builds][Slow][Serial] using build configuration run
select {
case event := <-buildWatch.ResultChan():
build := event.Object.(*buildapi.Build)
if build.Status.Phase == buildapi.BuildPhasePending {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same new phase question here

if build.Name == "sample-serial-build-fail-2" {
duration := time.Now().Sub(failTime)
o.Expect(duration).To(o.BeNumerically("<", 20*time.Second), "next build should have started less than 20s after failed build")
}
if build.Name == "sample-serial-build-fail-3" {
duration := time.Now().Sub(failTime2)
o.Expect(duration).To(o.BeNumerically("<", 20*time.Second), "next build should have started less than 20s after failed build")
}
}

if build.Status.Phase == buildapi.BuildPhaseFailed {
if build.Name == "sample-serial-build-fail-1" {
// this may not be set on the first build modified to failed event because
Expand Down Expand Up @@ -301,7 +311,6 @@ var _ = g.Describe("[Feature:Builds][Slow][Serial] using build configuration run
o.Expect(timestamps1).To(o.BeTrue(), "failed builds should have start and completion timestamps set")
o.Expect(timestamps2).To(o.BeTrue(), "failed builds should have start and completion timestamps set")
o.Expect(timestamps3).To(o.BeTrue(), "failed builds should have start and completion timestamps set")

})
})

Expand Down Expand Up @@ -362,7 +371,7 @@ var _ = g.Describe("[Feature:Builds][Slow][Serial] using build configuration run
}
}
// Only first and third build should actually run (serially).
if build.Status.Phase == buildapi.BuildPhaseRunning {
if build.Status.Phase == buildapi.BuildPhaseRunning || build.Status.Phase == buildapi.BuildPhasePending {
// Ignore events from complete builds (if there are any) if we already
// verified the build.
if _, exists := buildVerified[build.Name]; exists {
Expand Down