added copykey job and tasks #168
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lib/jobs/copy-publickey.js
Outdated
| {cmd: 'mkdir -p .ssh'}, | ||
| {cmd: 'echo '+node.sshSettings.publicKey+' >> .ssh/authorized_keys'} | ||
| ]; | ||
| return Promise.map(commands, function(commandData) { |
Contributor
There was a problem hiding this comment.
The order of the commands matters, but the order that Promise.map calls the mapper function is not guaranteed. Promise.each or Promise.mapSeries guarantees order.
Contributor
There was a problem hiding this comment.
@zyoung51 does it queue them off in order, but the resolution order isn't guaranteed in the result object? Or is order of execution also not guaranteed?
Contributor
There was a problem hiding this comment.
Here it is applicable as you want to wait for the command utility to resolve before issuing the next command utility call. Map will call the mapping function for each input item in whatever order they resolve and, unless you fiddle with concurrency, will not wait for the returned promise to resolve before calling the next iteration of the mapping function.
Mapseries and each will sequentially wait for the input promise to resolve and also wait for the mapping function's returned promise to resolve before iterating on the next item in the array.
Contributor
|
So the diff will become smaller after #150 is merged? |
Contributor
Author
|
Yes the diff will shrink |
Contributor
|
AlliumApotheosis
force-pushed
the
copy-key
branch
from
d22ff9e to
3a8cb61
Compare
Contributor
|
👍 |
Contributor
Author
|
@RackHD/corecommitters |
AlliumApotheosis
force-pushed
the
copy-key
branch
from
3a8cb61 to
8ceb8d9
Compare
lib/jobs/copy-publickey.js
Outdated
| self._done(); | ||
| }) | ||
| .catch(function(err) { | ||
| logger.debug("Error copying public key", {error: err}); |
Contributor
There was a problem hiding this comment.
This should probably be logger.error ?
Contributor
|
| ) { | ||
| var logger = Logger.initialize(copyKeyJobFactory); | ||
| var commandUtil; | ||
| function CopyKeyJob(options, context, taskId) { |
Member
There was a problem hiding this comment.
a little jsdoc noting that context.target should be a nodeID would be potentially useful here
Member
|
👍 with the logger.debug => logger.error change |
AlliumApotheosis
force-pushed
the
copy-key
branch
from
8ceb8d9 to
3515b3a
Compare
Contributor
|
Contributor
|
test this please |
Contributor
|
Merged
Contributor
|
test this please |
Contributor
|
Contributor
|
test this please |
Contributor
Contributor
|
Contributor
Author
|
closing because the changes are being included in #171 |
kellylu2sym
pushed a commit
to kellylu2sym/on-tasks
that referenced
this pull request
Aug 8, 2017
changed decommission graph to use sgdisk
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
more support for in-band management: added a job/tasks for copying an ssh public key to a node. Also refactored the sshExec method from ssh-job into command-util.
should probably wait on #150 to merge
@benbp