Dynamically update node inventory

[nishanttotla]

This PR makes the node inventory dynamic i.e. changes to node info are passed to the dispatcher. A ticker checks the node info periodically (currently every 20 seconds) and closes the session if changes are found, so that a new one can be created with an updated node description, and the same session id.

I'm trying to test this change thoroughly since it can likely cause race conditions in the main agent loop. The update period will likely have to be tuned to make sure that it doesn't interfere with the agent's backoff mechanism.

The underlying assumption is that node updates aren't super frequent.

Follow up to #1174 after it had to be reverted due to unstable behavior. #1230 was meant to fix some of that.

Fix #279

Signed-off-by: Nishant Totla nishanttotla@gmail.com

[codecov-io]

Current coverage is 55.38% (diff: 41.50%)

Merging #1374 into master will increase coverage by 0.15%

@@             master      #1374   diff @@
==========================================
  Files            82         82          
  Lines         12856      12871    +15   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           7101       7129    +28   
+ Misses         4774       4772     -2   
+ Partials        981        970    -11   

Powered by Codecov. Last update c374ccb...26bdb37

[nishanttotla]

It looks like docker info is unstable again, despite a previous fix. Investigating that, since it'll need fixing first. Making this PR WIP as a result.

[nishanttotla]

I confirmed that there are no issues on the Docker side as far as this PR is concerned, but it depends on #1426.

I'm currently running some more tests, and would appreciate if someone could also volunteer to test it out on their machine.

The one thing that might need tuning is

nodeUpdatePeriod = 20 * time.Second

[nishanttotla]

cc @aaronlehmann - I moved the node update code to a separate function, which can be reused both during node registration and update. The relevant changes are in dispatcher.go.

Please let me know if this is the correct way to do it. This PR works with that change.

[aaronlehmann]

What is this return value for?

[nishanttotla]

Good catch, it's not needed. I copied some code from register() that was returning two values and didn't change it.

[aaronlehmann]

LGTM

[nishanttotla]

In my testing (mostly with changing daemon labels), the behavior is fairly stable, barring the occasional error:

ERRO[0060] agent: updated node description unavailable   agent=&{client:0xc8202ecd20} error=Error response from daemon: Bad response from Docker engine module=node/agent

This is not surprising, since updating restarts the engine, and there's a brief period it's unavailable. It's not a problem though, since we just ignore when that happens and try again later (which is to say nothing bad happens when the engine is unavailable). I'm thinking that we could change the above error message to a warning perhaps if required, or else just keep it the way it is.

[stevvooe]

LGTM

[nishanttotla]

Ping @docker/core-swarmkit-maintainers it would be great if someone could test this on their machine too. If there are no further concerns, we can merge it.

[ghost]

@stevvooe You should merge this.