15942: Fix race in test case.

When each call to Update from the test case was starting its own
lock-update-cancel goroutine, some of these would still be
running (and attempting to cancel the container) after the test case
had succeeded and called /database/reset -- and thus cause a different
test case to fail.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

diff --git a/lib/dispatchcloud/container/queue.go b/lib/dispatchcloud/container/queue.go
index 21b41ec4d86d6cdc6c909b63106511dfe548f3c4..a4a270dd1013eee601cd214eb7e336f6cd8a29e2 100644 (file)
--- a/lib/dispatchcloud/container/queue.go
+++ b/lib/dispatchcloud/container/queue.go
@@ -240,19 +240,15 @@ func (cq *Queue) addEnt(uuid string, ctr arvados.Container) {
                go func() {
                        if ctr.State == arvados.ContainerStateQueued {
                                // Can't set runtime error without
-                               // locking first. If Lock() is
-                               // successful, it will call addEnt()
-                               // again itself, and we'll fall
-                               // through to the
-                               // setRuntimeError/Cancel code below.
+                               // locking first.
                                err := cq.Lock(ctr.UUID)
                                if err != nil {
                                        logger.WithError(err).Warn("lock failed")
+                                       return
                                        // ...and try again on the
                                        // next Update, if the problem
                                        // still exists.
                                }
-                               return
                        }
                        var err error
                        defer func() {

diff --git a/lib/dispatchcloud/container/queue_test.go b/lib/dispatchcloud/container/queue_test.go
index bb8226118cb4dc91a36d5c5382fa5b3e8e5a1487..31f321488e686929080f2b1b6a9bdba7c7bfeff3 100644 (file)
--- a/lib/dispatchcloud/container/queue_test.go
+++ b/lib/dispatchcloud/container/queue_test.go
@@ -134,10 +134,11 @@ func (suite *IntegrationSuite) TestCancelIfNoInstanceType(c *check.C) {
        c.Check(err, check.IsNil)
        c.Check(ctr.State, check.Equals, arvados.ContainerStateQueued)
 
+       go cq.Update()
+
        // Wait for the cancel operation to take effect. Container
        // will have state=Cancelled or just disappear from the queue.
        suite.waitfor(c, time.Second, func() bool {
-               cq.Update()
                err := client.RequestAndDecode(&ctr, "GET", "arvados/v1/containers/"+arvadostest.QueuedContainerUUID, nil, nil)
                return err == nil && ctr.State == arvados.ContainerStateCancelled
        })