build controller: fix start of serial builds on cancellation/deletion #17498

csrwng · 2017-11-28T19:14:40Z

Current code will only retry a buildconfig if there's a running build and there's no pending builds waiting to be started. The conditions should be independent (ored). If there's no pending builds it may be that the cache hasn't caught up.

This fix separates the conditions and logs different messages for each.

When an active build is deleted, we should also poke the buildconfig to start the next build.

Fixes #17353

csrwng · 2017-11-28T19:15:05Z

@openshift/sig-developer-experience

bparees · 2017-11-28T19:25:12Z

pkg/build/controller/build/build_controller.go

 		glog.V(4).Infof("Build config %s/%s has running builds, will retry", bcNamespace, bcName)
 		return fmt.Errorf("build config %s/%s has running builds and cannot run more builds", bcNamespace, bcName)
 	}
+	if len(nextBuilds) == 0 {
+		glog.V(4).Infof("Build config %s/%s has no builds to run next, will retry", bcNamespace, bcName)


will retry? doesn't this mean we could potentially end up spinning forever on a buildconfig that has no builds?

We give up after 15 retries

bparees · 2017-11-28T19:28:04Z

/lgtm
/hold
(for build test pass)

bparees · 2017-11-28T19:29:58Z

test/extended/builds/run_policy.go

+
+				var deleteTime, deleteTime2 time.Time
+				for {
+					event := <-buildWatch.ResultChan()


wrap this in a select timeout like the other TCs do, so we don't want forever if we don't get the next event.

bparees · 2017-11-28T19:30:14Z

/lgtm cancel

csrwng · 2017-11-28T19:46:11Z

updated

bparees · 2017-11-28T19:55:24Z

/lgtm
(continue to wait for passing build tests)

csrwng · 2017-11-28T21:19:04Z

fixed extended test (missing resource type in oc delete call)

bparees · 2017-11-28T21:42:21Z

/lgtm

csrwng · 2017-11-28T22:01:37Z

last push was by mistake, nothing changed

bparees · 2017-11-28T22:09:37Z

/lgtm

csrwng · 2017-11-29T16:56:29Z

/retest

openshift-bot · 2017-12-01T12:23:25Z

/retest

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

csrwng · 2017-12-01T14:25:28Z

Flake #17330
/retest

csrwng · 2017-12-01T20:37:40Z

/retest

openshift-bot · 2017-12-02T04:31:34Z

/retest

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

openshift-bot · 2017-12-03T06:44:37Z

/retest

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

openshift-bot · 2017-12-04T04:54:57Z

/retest

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

openshift-bot · 2017-12-04T21:02:58Z

/retest

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

openshift-bot · 2017-12-05T11:09:16Z

/retest

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

openshift-bot · 2017-12-06T07:19:16Z

/retest

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

openshift-bot · 2017-12-06T19:25:12Z

/retest

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

openshift-bot · 2017-12-07T07:31:13Z

/retest

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

openshift-bot · 2017-12-07T17:35:57Z

/retest

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

openshift-bot · 2017-12-07T23:38:54Z

/retest

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

openshift-bot · 2017-12-08T07:42:56Z

/retest

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

openshift-bot · 2017-12-08T13:45:55Z

/retest

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

openshift-bot · 2017-12-08T21:49:58Z

/retest

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

openshift-bot · 2017-12-09T13:57:56Z

/retest

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

openshift-bot · 2017-12-10T04:04:58Z

/retest

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

openshift-bot · 2017-12-10T18:11:54Z

/retest

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

bparees · 2017-12-11T05:08:12Z

/retest

csrwng · 2017-12-11T16:18:04Z

/retest

openshift-bot · 2017-12-12T04:28:58Z

/retest

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

openshift-bot · 2017-12-12T18:39:17Z

/retest

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

openshift-bot · 2017-12-13T08:42:59Z

/retest

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

openshift-ci-robot · 2017-12-13T10:00:23Z

@csrwng: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/extended_conformance_install_update	`95b3459`	link	`/test extended_conformance_install_update`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

bparees · 2017-12-13T15:03:01Z

/retest

openshift-bot · 2017-12-13T20:48:55Z

/retest

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

openshift-merge-robot · 2017-12-14T06:27:15Z

/test all [submit-queue is verifying that this PR is safe to merge]

openshift-merge-robot · 2017-12-14T07:44:01Z

Automatic merge from submit-queue.

openshift-merge-robot assigned bparees and smarterclayton Nov 28, 2017

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 28, 2017

bparees requested changes Nov 28, 2017

View reviewed changes

openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Nov 28, 2017

openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 28, 2017

bparees requested changes Nov 28, 2017

View reviewed changes

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2017

openshift-merge-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 28, 2017

csrwng force-pushed the fix_build_controller branch from 4189afd to 18623da Compare November 28, 2017 19:45

openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 28, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2017

bparees approved these changes Nov 28, 2017

View reviewed changes

csrwng force-pushed the fix_build_controller branch from 18623da to bf128b3 Compare November 28, 2017 21:18

openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2017

csrwng force-pushed the fix_build_controller branch from bf128b3 to ccfa1b6 Compare November 28, 2017 21:55

openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2017

openshift-merge-robot merged commit d3cdfec into openshift:master Dec 14, 2017

build controller: fix start of serial builds on cancellation/deletion #17498

build controller: fix start of serial builds on cancellation/deletion #17498

Conversation

csrwng commented Nov 28, 2017

csrwng commented Nov 28, 2017

bparees Nov 28, 2017

Choose a reason for hiding this comment

csrwng Nov 28, 2017

Choose a reason for hiding this comment

bparees Nov 28, 2017

Choose a reason for hiding this comment

bparees commented Nov 28, 2017

bparees Nov 28, 2017

Choose a reason for hiding this comment

bparees commented Nov 28, 2017

csrwng commented Nov 28, 2017

bparees commented Nov 28, 2017

csrwng commented Nov 28, 2017

bparees commented Nov 28, 2017

csrwng commented Nov 28, 2017

bparees commented Nov 28, 2017

csrwng commented Nov 29, 2017

openshift-bot commented Dec 1, 2017

csrwng commented Dec 1, 2017

csrwng commented Dec 1, 2017

openshift-bot commented Dec 2, 2017

openshift-bot commented Dec 3, 2017

openshift-bot commented Dec 4, 2017

openshift-bot commented Dec 4, 2017

openshift-bot commented Dec 5, 2017

openshift-bot commented Dec 6, 2017

openshift-bot commented Dec 6, 2017

openshift-bot commented Dec 7, 2017

openshift-bot commented Dec 7, 2017

openshift-bot commented Dec 7, 2017

openshift-bot commented Dec 8, 2017

openshift-bot commented Dec 8, 2017

openshift-bot commented Dec 8, 2017

openshift-bot commented Dec 9, 2017

openshift-bot commented Dec 10, 2017

openshift-bot commented Dec 10, 2017

bparees commented Dec 11, 2017

csrwng commented Dec 11, 2017

openshift-bot commented Dec 12, 2017

openshift-bot commented Dec 12, 2017

openshift-bot commented Dec 13, 2017

openshift-ci-robot commented Dec 13, 2017 • edited Loading

bparees commented Dec 13, 2017

openshift-bot commented Dec 13, 2017

openshift-merge-robot commented Dec 14, 2017

openshift-merge-robot commented Dec 14, 2017

openshift-ci-robot commented Dec 13, 2017 •

edited

Loading