Uploaded image for project: 'Libcloud'
  1. Libcloud
  2. LIBCLOUD-309

cycle through ssh usernames for script deployment

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Compute
    • Labels:
      None

      Description

      Following up the discussion in the mailing list (http://mail-archives.apache.org/mod_mbox/libcloud-users/201303.mbox/browser) it would be nice to have this feature as long as error handling is transparent and looping is only done if the user requires it.

        Activity

        Hide
        cpsaltis Chris Psaltis added a comment -

        Attaching a patch solving this issue.

        Keep in mind that _ssh_client_connect() doesn't fail if the ssh username is 'root', but an EC2 image has something like 'ubuntu' or 'ec2-user'.

        Show
        cpsaltis Chris Psaltis added a comment - Attaching a patch solving this issue. Keep in mind that _ssh_client_connect() doesn't fail if the ssh username is 'root', but an EC2 image has something like 'ubuntu' or 'ec2-user'.
        Hide
        kami Tomaz Muraus added a comment -

        Thanks.

        The patch looks mostly good, here are some potential issues:

        • Mutating an argument which is passed by reference to a method it's usually a bad idea, unless it's explicit and documented. In this `ssh_alternate_usernames` which means is passed by reference and where you pop an element from the list you actually modify the list used passed to the method.

        You have two options to avoid that:

        1. Create a copy of the list and mutate it
        2. Iterate over a list

        I personally think copying adds unnecessary work here so I would go with option 2.

        • You catch Exception which is probably not what we want. IIRC, we can catch a more specific exception thrown by paramiko which indicats an authentication error.
        Show
        kami Tomaz Muraus added a comment - Thanks. The patch looks mostly good, here are some potential issues: Mutating an argument which is passed by reference to a method it's usually a bad idea, unless it's explicit and documented. In this `ssh_alternate_usernames` which means is passed by reference and where you pop an element from the list you actually modify the list used passed to the method. You have two options to avoid that: 1. Create a copy of the list and mutate it 2. Iterate over a list I personally think copying adds unnecessary work here so I would go with option 2. You catch Exception which is probably not what we want. IIRC, we can catch a more specific exception thrown by paramiko which indicats an authentication error.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1460106 from Tomaz Muraus
        [ https://svn.apache.org/r1460106 ]

        Allow user to specify alternative ssh usernames (ssh_alternate_usernames kwarg)
        which are used with deploy_node if the default one doesn't work.

        Contributed by Chris Psaltis, part of LIBCLOUD-309.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1460106 from Tomaz Muraus [ https://svn.apache.org/r1460106 ] Allow user to specify alternative ssh usernames (ssh_alternate_usernames kwarg) which are used with deploy_node if the default one doesn't work. Contributed by Chris Psaltis, part of LIBCLOUD-309 .
        Hide
        kami Tomaz Muraus added a comment -

        I've made changes mentioned above + refactored some of the repeated code and moved it to the "_connect_and_run_deployment_script" method.

        Besides making it easier to maintain, refactoring also makes the whole code easier to read because it removes extra levels of nesting.

        The whole deploy code is still bit of a mess and needs more love, but overall it's still an improvement.

        Thanks!

        Show
        kami Tomaz Muraus added a comment - I've made changes mentioned above + refactored some of the repeated code and moved it to the "_connect_and_run_deployment_script" method. Besides making it easier to maintain, refactoring also makes the whole code easier to read because it removes extra levels of nesting. The whole deploy code is still bit of a mess and needs more love, but overall it's still an improvement. Thanks!
        Hide
        cpsaltis Chris Psaltis added a comment -

        Been meaning to do it today, but you caught up with me. Thanks!

        As a note for the future in _connect_and_run_deployment_script(), _ssh_client_connect() will throw a paramiko exception when trying to connect with an invalid username. When using 'root' it will not throw the usual exception, even if it not the correct one, but will move on and fail at _run_deployment_script().

        Show
        cpsaltis Chris Psaltis added a comment - Been meaning to do it today, but you caught up with me. Thanks! As a note for the future in _connect_and_run_deployment_script(), _ssh_client_connect() will throw a paramiko exception when trying to connect with an invalid username. When using 'root' it will not throw the usual exception, even if it not the correct one, but will move on and fail at _run_deployment_script().

          People

          • Assignee:
            kami Tomaz Muraus
            Reporter:
            cpsaltis Chris Psaltis
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development