Fix stale cached container state after successful Cancel.

Avoids needless retries between a successful Cancel and the next sync.

No issue #

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/lib/dispatchcloud/container/queue.go b/lib/dispatchcloud/container/queue.go
index af17aaf3927ce9f3b8b94a03ca289201c11640d2..50e73189efbc854433f8713e0a7762efafc0fe70 100644 (file)
--- a/lib/dispatchcloud/container/queue.go
+++ b/lib/dispatchcloud/container/queue.go
@@ -314,15 +314,14 @@ func (cq *Queue) setRuntimeError(uuid, errorString string) error {
 
 // Cancel cancels the given container.
 func (cq *Queue) Cancel(uuid string) error {
-       err := cq.client.RequestAndDecode(nil, "PUT", "arvados/v1/containers/"+uuid, nil, map[string]map[string]interface{}{
+       var resp arvados.Container
+       err := cq.client.RequestAndDecode(&resp, "PUT", "arvados/v1/containers/"+uuid, nil, map[string]map[string]interface{}{
                "container": {"state": arvados.ContainerStateCancelled},
        })
        if err != nil {
                return err
        }
-       cq.mtx.Lock()
-       defer cq.mtx.Unlock()
-       cq.notify()
+       cq.updateWithResp(uuid, resp)
        return nil
 }
 
@@ -332,7 +331,13 @@ func (cq *Queue) apiUpdate(uuid, action string) error {
        if err != nil {
                return err
        }
+       cq.updateWithResp(uuid, resp)
+       return nil
+}
 
+// Update the local queue with the response received from a
+// state-changing API request (lock/unlock/cancel).
+func (cq *Queue) updateWithResp(uuid string, resp arvados.Container) {
        cq.mtx.Lock()
        defer cq.mtx.Unlock()
        if cq.dontupdate != nil {
@@ -345,7 +350,6 @@ func (cq *Queue) apiUpdate(uuid, action string) error {
                cq.current[uuid] = ent
        }
        cq.notify()
-       return nil
 }
 
 func (cq *Queue) poll() (map[string]*arvados.Container, error) {

diff --git a/lib/dispatchcloud/container/queue_test.go b/lib/dispatchcloud/container/queue_test.go
index 91d65359e884a91955f47523a7d11836a52767df..3c63fe51e6e89a116a40ea5c72917a5d4528ab41 100644 (file)
--- a/lib/dispatchcloud/container/queue_test.go
+++ b/lib/dispatchcloud/container/queue_test.go
@@ -74,6 +74,7 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
                        defer wg.Done()
                        err := cq.Unlock(uuid)
                        c.Check(err, check.NotNil)
+
                        err = cq.Lock(uuid)
                        c.Check(err, check.IsNil)
                        ctr, ok := cq.Get(uuid)
@@ -81,6 +82,7 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
                        c.Check(ctr.State, check.Equals, arvados.ContainerStateLocked)
                        err = cq.Lock(uuid)
                        c.Check(err, check.NotNil)
+
                        err = cq.Unlock(uuid)
                        c.Check(err, check.IsNil)
                        ctr, ok = cq.Get(uuid)
@@ -88,6 +90,14 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
                        c.Check(ctr.State, check.Equals, arvados.ContainerStateQueued)
                        err = cq.Unlock(uuid)
                        c.Check(err, check.NotNil)
+
+                       err = cq.Cancel(uuid)
+                       c.Check(err, check.IsNil)
+                       ctr, ok = cq.Get(uuid)
+                       c.Check(ok, check.Equals, true)
+                       c.Check(ctr.State, check.Equals, arvados.ContainerStateCancelled)
+                       err = cq.Lock(uuid)
+                       c.Check(err, check.NotNil)
                }()
        }
        wg.Wait()