Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use assignments instead of tasks #1508

Merged
merged 11 commits into from
Sep 9, 2016
Merged

Conversation

cyli
Copy link
Contributor

@cyli cyli commented Sep 7, 2016

Cherry-picked the assignments-only related code from #1377, as per suggestion from @stevvooe.

diogomonica and others added 8 commits September 7, 2016 13:28
Add a Secret top-level object type. Add a SecretReference that allows a
service to reference the secrets it needs.

Add dispatcher Assignments method which will replace Tasks going
forward. This provides a stream with incremental task and secret
updates. Additional object types can be supported in the assignment set
in the future. The first message returned from the Assignments stream is
the complete set of tasks and secrets for the node, and this is used to
synchronize the node's view with the manager's. Additional messages
returned by the stream are incremental updates that add, update, or
remove one or more tasks or secrets. If the agent gets out of sync with
the manager, it can reinitiate the Assignments stream to sync up.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Signed-off-by: cyli <cyli@twistedmatrix.com>
Signed-off-by: Diogo Monica <diogo.monica@gmail.com>
Signed-off-by: Diogo Monica <diogo.monica@gmail.com>
Signed-off-by: cyli <cyli@twistedmatrix.com>
(removed the proto parts - cyli)

Signed-off-by: Diogo Monica <diogo.monica@gmail.com>
Signed-off-by: cyli <cyli@twistedmatrix.com>
…ments

Signed-off-by: Diogo Monica <diogo.monica@gmail.com>
Signed-off-by: cyli <cyli@twistedmatrix.com>
@codecov-io
Copy link

codecov-io commented Sep 7, 2016

Current coverage is 54.80% (diff: 47.34%)

Merging #1508 into master will decrease coverage by 0.46%

@@             master      #1508   diff @@
==========================================
  Files            82         82          
  Lines         12917      13073   +156   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           7140       7165    +25   
- Misses         4782       4908   +126   
- Partials        995       1000     +5   

Sunburst

Powered by Codecov. Last update 90be98a...443a457

Signed-off-by: cyli <cyli@twistedmatrix.com>
}
tasksFallback = true
assignmentWatch = nil
log.WithError(err).Errorf("falling back to Tasks")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe Warning or Info?

@@ -221,4 +221,4 @@ message Cluster {
// and agents to unambiguously identify the older key to be deleted when
// a new key is allocated on key rotation.
uint64 encryption_key_lamport_clock = 6;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the line ending on the last line got removed from this file somehow.

@stevvooe
Copy link
Contributor

stevvooe commented Sep 7, 2016

@cyli @diogomonica Just realized what you did here! Thanks! It is much easier to review.

@cyli cyli mentioned this pull request Sep 7, 2016
Signed-off-by: Diogo Monica <diogo.monica@gmail.com>
Signed-off-by: Diogo Monica <diogo.monica@gmail.com>
return err
}
log := log.G(ctx).WithFields(logrus.Fields{"method": "(*session).watch"})
log.Debugf("")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what mean for the debugf log

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We wanted to log the call to (*session).watch (see https://github.com/docker/swarmkit/pull/1508/files#diff-15ffe95a2da45f70696ffa3c01949601L208), just to indicate that it was starting, but we also wanted those fields to be attached to the logger so it can be used to log errors later. This call to Debugf just logs the fields of the logger without any additional info.

@aaronlehmann
Copy link
Collaborator

Passed Docker integration tests except a known flaky test

LGTM

@stevvooe
Copy link
Contributor

stevvooe commented Sep 9, 2016

LGTM

@stevvooe stevvooe merged commit 27ae8c7 into moby:master Sep 9, 2016
@aaronlehmann
Copy link
Collaborator

Also tested the fallback when running with an older manager. Seems to work correctly.

@cyli cyli deleted the tasks-to-assignments branch September 10, 2016 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants