Use named argument for cloud:ssh

[nickpeirson]

Grabbing the server argument based on position doesn't work when multistage is enabled as the stage precedes the task, e.g.:

cap staging cloud:ssh 1

This means that the code picking up the argument based on position sees 'cloud:ssh' as the instance id, instead of '1'.

This patch changes to a named argument, e.g.:

cap cloud:ssh -s cloud_instance=1

This will work regardless of the number of arguments and using the argument cloud_instance should minimise the risk of collisions with other capistrano plugins.

[ncantor]

If you run cap staging cloud:status, what do you get back?

[nickpeirson]

$ cap staging cloud:status
    triggering load callbacks
  * executing `staging'
    triggering start callbacks for `cloud:status'
  * executing `multistage:ensure'
  * executing `cloud:status'
00:  Admin                            AWS         i-12345670   m1.small     xx.xx.xx.xx      us-east-1b   (admin)  ()
01:  Admin                            AWS         i-12345671   m1.small     xx.xx.xx.xx      us-east-1a   (admin)  ()
02:  API-Master                       AWS         i-12345672   m1.small     xx.xx.xx.xx      us-east-1b   (api)  ()
03:  API-Master                       AWS         i-12345673   m1.small     xx.xx.xx.xx      us-east-1a   (api)  ()

My stage files look like:

@capify_cloud = CapifyCloud.new(fetch(:cloud_config, 'config/cloud.staging.yml'))

set :branch, "develop"

cloud_roles :admin
cloud_roles :api

Not sure that's the best way of driving config for stages with capify-cloud, but it works well enough. Would be nice to be able to just override certain params per stage as most of my cloud.*.yml files have just one line changed

[ncantor]

I've been giving it a fair bit of thought, and have come to the conclusion that there are bits of your pull request I like, and some I don't.
I would prefer not to lose the original functionality. If you want to enhance it, so that it looks for capistrano options first, and if it finds nothing, then uses the first variable, that would work a lot better for me.
Also, I prefer if/else to only be used as guard clauses. Something akin to:
raise "Cannot find instance" if instance.nil? would fit better than the construct you've put in.

Let me know what you think. If you're happy to update and submit the new pull request, I'd be happy to include it.

--Noah

[nickpeirson]

Sorry for the delay in updating this pull request, but finally got back to it.

I've put the original behaviour back in as a fallback and switched it over to guard clauses. The new behaviour and the exception are tested, however the reason for the PR is that the old behaviour doesn't work for me, so I've been unable to test it, however it only differs by one line and I've just put it back in from prior to my first commit.

Cheers
Nick