agent: bounce session after failed status update

[stevvooe]

Previously, failed calls on an agent session to UpdateTaskStatus weren't
causing the agent to fall back into the session creation retry loop.
Without the retry loop, the agent ends up pounding the dispatcher with
status updates that will never drain.

This PR closes the session on a fatal error, ensuring that the agent
will fallback into a retry loop.

Consider backporting this change to 1.12.2.

Signed-off-by: Stephen J Day [email protected]

cc @aaronlehmann

Closes #1533

[codecov-io]

Current coverage is 53.72% (diff: 0.00%)

Merging #1539 into master will decrease coverage by 0.03%

@@             master      #1539   diff @@
==========================================
  Files            82         82          
  Lines         13432      13430     -2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           7221       7215     -6   
- Misses         5224       5227     +3   
- Partials        987        988     +1   

Powered by Codecov. Last update 3f3d300...353423b

[stevvooe]

Confirmed fix with reproduction that always returns errors from UpdateTaskStatus:

diff --git a/manager/dispatcher/dispatcher.go b/manager/dispatcher/dispatcher.go
index 9ae3e0b..432af44 100644
--- a/manager/dispatcher/dispatcher.go
+++ b/manager/dispatcher/dispatcher.go
@@ -436,6 +436,8 @@ func (d *Dispatcher) register(ctx context.Context, nodeID string, description *a
 // UpdateTaskStatus updates status of task. Node should send such updates
 // on every status change of its tasks.
 func (d *Dispatcher) UpdateTaskStatus(ctx context.Context, r *api.UpdateTaskStatusRequest) (*api.UpdateTaskStatusResponse, error) {
+       return nil, grpc.Errorf(codes.InvalidArgument, "testing agent problem")
+
        nodeInfo, err := ca.RemoteNode(ctx)
        if err != nil {
                return nil, err

Before this patch, we get the following in the logs:

DEBU[0002] (*Agent).UpdateTaskStatus                     module=node/agent task.id=9qdufjres0iky2n2gx25yn8ut
ERRO[0002] sending task status update failed             error=rpc error: code = 3 desc = testing agent problem module=node/agent
ERRO[0002] failed reporting status to agent              error=rpc error: code = 3 desc = testing agent problem module=node/agent

After applying this PR we get the following:

DEBU[0002] (*Agent).UpdateTaskStatus                     module=node/agent task.id=9dzxad2v62bq2e305guckajck
DEBU[0002] agent: rebuild session                        module=node/agent
DEBU[0002] (*session).start                              module=node/agent

While it will spam the logs with this simple recreation because we keep reestablishing the session, note that we are clearly in the backoff loop ("agent: rebuild session").

[aaronlehmann]

Do we need the select with the addition of this sync.Once?

[stevvooe]

Fixed.

[aaronlehmann]

LGTM

[LK4D4]

LGTM