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

Add support for security groups in OpenStack driver

    Details

      Description

      Add parameter ex_security_groups in create_node method.
      Add ex_list_security_groups and ex_get_node_security_groups methods.
      Add ex_create_security_group and ex_delete_security_group methods
      Add ex_create_security_group_rule and ex_delete_security_group_rule methods

        Activity

        Hide
        kami Tomaz Muraus added a comment -

        Are you working on a patch with this functionality?

        Show
        kami Tomaz Muraus added a comment - Are you working on a patch with this functionality?
        Hide
        schaubl L. Schaub added a comment -

        Yes, I just finished the patch

        Show
        schaubl L. Schaub added a comment - Yes, I just finished the patch
        Hide
        schaubl L. Schaub added a comment - - edited

        I just attach the patch

        Show
        schaubl L. Schaub added a comment - - edited I just attach the patch
        Hide
        schaubl L. Schaub added a comment -

        Hello, is that someone is in charge of my patch ?

        Show
        schaubl L. Schaub added a comment - Hello, is that someone is in charge of my patch ?
        Hide
        kami Tomaz Muraus added a comment -

        Hm, I don't remember seeing a notification email when you uploaded a patch.

        Thanks for bumping this issue, I will review the patch later today.

        Show
        kami Tomaz Muraus added a comment - Hm, I don't remember seeing a notification email when you uploaded a patch. Thanks for bumping this issue, I will review the patch later today.
        Hide
        kami Tomaz Muraus added a comment -

        First thanks for the patch again. I just reviewed it and noticed some issues:

        • there are no tests - one of the requirements for the patch being merged is presence of good test cases
        • variable = type() variable = some_value - this syntax is just weird, there is no need to do this in Python (I assume you wanted to declare a variable type or something)
        • if type(security_group) == OpenStackSecurityGroup - I think we should only support one way of passing in a group - either by passing in a string or instance of OpenStackSecurityGroup
        Show
        kami Tomaz Muraus added a comment - First thanks for the patch again. I just reviewed it and noticed some issues: there are no tests - one of the requirements for the patch being merged is presence of good test cases variable = type() variable = some_value - this syntax is just weird, there is no need to do this in Python (I assume you wanted to declare a variable type or something) if type(security_group) == OpenStackSecurityGroup - I think we should only support one way of passing in a group - either by passing in a string or instance of OpenStackSecurityGroup
        Hide
        schaubl L. Schaub added a comment -

        Ok, I'll try to fix them in a week because I do not have time this week.

        Show
        schaubl L. Schaub added a comment - Ok, I'll try to fix them in a week because I do not have time this week.
        Hide
        schaubl L. Schaub added a comment -

        I just attach the new patch with unit tests and some corrections.

        Show
        schaubl L. Schaub added a comment - I just attach the new patch with unit tests and some corrections.
        Hide
        kami Tomaz Muraus added a comment -

        Thanks, patch looks better now but it's still missing docstrings which explain every argument in OpenStackSecurityGroup and OpenStackSecurityGroupRule class constructor.

        Show
        kami Tomaz Muraus added a comment - Thanks, patch looks better now but it's still missing docstrings which explain every argument in OpenStackSecurityGroup and OpenStackSecurityGroupRule class constructor.
        Hide
        kami Tomaz Muraus added a comment -

        Ignore my last comment. Since I'm already looking at the patch I will just fix this issue myself.

        Show
        kami Tomaz Muraus added a comment - Ignore my last comment. Since I'm already looking at the patch I will just fix this issue myself.
        Hide
        kami Tomaz Muraus added a comment -

        Made some changes and fixed to your patch and merged it into turnk - http://svn.apache.org/viewvc?view=revision&revision=r1426060.

        Thanks!

        Show
        kami Tomaz Muraus added a comment - Made some changes and fixed to your patch and merged it into turnk - http://svn.apache.org/viewvc?view=revision&revision=r1426060 . Thanks!

          People

          • Assignee:
            kami Tomaz Muraus
            Reporter:
            schaubl L. Schaub
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 168h
              168h
              Remaining:
              Remaining Estimate - 168h
              168h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development