You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by GitBox <gi...@apache.org> on 2021/12/13 12:48:26 UTC

[GitHub] [libcloud] micafer opened a new pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

micafer opened a new pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638


   ## OpenStack: Move floating IP functions to use network service instead of nova
   
   ### Description
   
   See #1636
   
   ### Status
   
   - done, ready for review
   
   ### Checklist (tick everything that applies)
   
   - [x] [Code linting](http://libcloud.readthedocs.org/en/latest/development.html#code-style-guide) (required, can be done after the PR checks)
   - [ ] Documentation
   - [x] [Tests](http://libcloud.readthedocs.org/en/latest/testing.html)
   - [x] [ICLA](http://libcloud.readthedocs.org/en/latest/development.html#contributing-bigger-changes) (required for bigger changes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] codecov-commenter edited a comment on pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#issuecomment-992462288


   # [Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1638](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ebc322) into [trunk](https://codecov.io/gh/apache/libcloud/commit/90971e17bfd7b6bb97b2489986472c531cc8e140?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (90971e1) will **increase** coverage by `0.00%`.
   > The diff coverage is `90.56%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/libcloud/pull/1638/graphs/tree.svg?width=650&height=150&src=pr&token=PYoduksh69&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##            trunk    #1638   +/-   ##
   =======================================
     Coverage   83.17%   83.18%           
   =======================================
     Files         398      398           
     Lines       86709    86798   +89     
     Branches     9217     9225    +8     
   =======================================
   + Hits        72120    72202   +82     
   - Misses      11453    11455    +2     
   - Partials     3136     3141    +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [libcloud/compute/drivers/openstack.py](https://codecov.io/gh/apache/libcloud/pull/1638/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL29wZW5zdGFjay5weQ==) | `85.30% <84.84%> (+0.17%)` | :arrow_up: |
   | [libcloud/test/compute/test\_openstack.py](https://codecov.io/gh/apache/libcloud/pull/1638/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvdGVzdC9jb21wdXRlL3Rlc3Rfb3BlbnN0YWNrLnB5) | `94.64% <100.00%> (+0.11%)` | :arrow_up: |
   | [libcloud/test/dns/test\_base.py](https://codecov.io/gh/apache/libcloud/pull/1638/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvdGVzdC9kbnMvdGVzdF9iYXNlLnB5) | `97.01% <0.00%> (-2.99%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [90971e1...1ebc322](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] Kami commented on a change in pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
Kami commented on a change in pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#discussion_r794905799



##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4441,6 +4510,74 @@ def __repr__(self):
         )
 
 
+class OpenStack_2_FloatingIpAddress(OpenStack_1_1_FloatingIpAddress):
+    """
+    Floating IP info 2.0.
+    """
+
+    def __init__(self, id, ip_address, pool, node_id=None, driver=None, extra=None):
+        self.id = str(id)
+        self.ip_address = ip_address
+        self._pool = pool
+        self._node_id = node_id
+        self.driver = driver
+        self.extra = extra if extra else {}
+
+    @property
+    def pool(self):

Review comment:
       Hm, looks like this method will always try to perform the API call (``ex_get_network()``) on subsequent calls even if the code throws an exception and ``self._pool`` is not populated.
   
   It looks like it's similar story with ``node_id`` property.
   
   I wonder if there is something we could to improve that to avoid surprises - if it was regular method, I would be fine with it, but with a variable / property, this call time overhead is kinda hidden and could be substantial in case operation which is triggered when ``self._pool`` is not populated yet gets called on each property access because that internal variable wasn't populated for some reason (exception or similar).
   
   I guess one simple option would be to simplify set that variable to None on error and return that in case of an error (and use some other special marker value for "not set"). That would only really work though if the underlying operation is synchronous and there is no chance for network errors or other similar exceptions.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] dpeschman commented on a change in pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
dpeschman commented on a change in pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#discussion_r819682920



##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4441,6 +4510,64 @@ def __repr__(self):
         )
 
 
+class OpenStack_2_FloatingIpAddress(OpenStack_1_1_FloatingIpAddress):
+    """
+    Floating IP info 2.0.
+    """
+
+    def __init__(self, id, ip_address, pool, node_id=None, driver=None, extra=None):
+        self.id = str(id)
+        self.ip_address = ip_address
+        self.pool = pool
+        self.node_id = node_id
+        self.driver = driver
+        self.extra = extra if extra else {}
+
+    def get_pool(self):
+        if not self.pool:
+            try:
+                # If pool is not set, get the info from the floating_network_id
+                net = self.driver.ex_get_network(self.extra["floating_network_id"])
+            except Exception:
+                net = None
+            if net:
+                self.pool = OpenStack_2_FloatingIpPool(
+                    net.id, net.name, self.driver.network_connection
+                )
+        return self.pool
+
+    def get_node_id(self):
+        if not self.node_id:
+            # if not is not set, get it from port_details

Review comment:
       Sorry. This phrase "if not is not set" - I think you meant "if node_id is not set".




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] Kami merged pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
Kami merged pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] Kami commented on pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
Kami commented on pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#issuecomment-1024659518


   It would be good if you could also add an upgrade notes entry on this change with a short example of how the updated / new code looks like.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] codecov-commenter commented on pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#issuecomment-992462288


   # [Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1638](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e5b7681) into [trunk](https://codecov.io/gh/apache/libcloud/commit/90971e17bfd7b6bb97b2489986472c531cc8e140?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (90971e1) will **increase** coverage by `0.00%`.
   > The diff coverage is `84.61%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/libcloud/pull/1638/graphs/tree.svg?width=650&height=150&src=pr&token=PYoduksh69&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##            trunk    #1638   +/-   ##
   =======================================
     Coverage   83.17%   83.17%           
   =======================================
     Files         398      398           
     Lines       86709    86799   +90     
     Branches     9217     9226    +9     
   =======================================
   + Hits        72120    72199   +79     
   - Misses      11453    11459    +6     
   - Partials     3136     3141    +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [libcloud/compute/drivers/openstack.py](https://codecov.io/gh/apache/libcloud/pull/1638/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL29wZW5zdGFjay5weQ==) | `84.89% <75.00%> (-0.25%)` | :arrow_down: |
   | [libcloud/test/compute/test\_openstack.py](https://codecov.io/gh/apache/libcloud/pull/1638/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvdGVzdC9jb21wdXRlL3Rlc3Rfb3BlbnN0YWNrLnB5) | `94.64% <100.00%> (+0.11%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [90971e1...e5b7681](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] Kami commented on a change in pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
Kami commented on a change in pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#discussion_r794906364



##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4441,6 +4510,74 @@ def __repr__(self):
         )
 
 
+class OpenStack_2_FloatingIpAddress(OpenStack_1_1_FloatingIpAddress):
+    """
+    Floating IP info 2.0.
+    """
+
+    def __init__(self, id, ip_address, pool, node_id=None, driver=None, extra=None):
+        self.id = str(id)
+        self.ip_address = ip_address
+        self._pool = pool
+        self._node_id = node_id
+        self.driver = driver
+        self.extra = extra if extra else {}
+
+    @property
+    def pool(self):

Review comment:
       Another even simpler option may be to not use a property but a method named ``get_pool()`` and ``get_node_id()`` - this makes it more obvious that the method call likely has a side affect.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] dpeschman commented on a change in pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
dpeschman commented on a change in pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#discussion_r819043873



##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4441,6 +4510,64 @@ def __repr__(self):
         )
 
 
+class OpenStack_2_FloatingIpAddress(OpenStack_1_1_FloatingIpAddress):
+    """
+    Floating IP info 2.0.
+    """
+
+    def __init__(self, id, ip_address, pool, node_id=None, driver=None, extra=None):
+        self.id = str(id)
+        self.ip_address = ip_address
+        self.pool = pool
+        self.node_id = node_id
+        self.driver = driver
+        self.extra = extra if extra else {}
+
+    def get_pool(self):
+        if not self.pool:
+            try:
+                # If pool is not set, get the info from the floating_network_id
+                net = self.driver.ex_get_network(self.extra["floating_network_id"])
+            except Exception:
+                net = None
+            if net:
+                self.pool = OpenStack_2_FloatingIpPool(
+                    net.id, net.name, self.driver.network_connection
+                )
+        return self.pool
+
+    def get_node_id(self):
+        if not self.node_id:
+            # if not is not set, get it from port_details

Review comment:
       "if not" > "if node_id"

##########
File path: CHANGES.rst
##########
@@ -25,6 +25,20 @@ Compute
   (GITHUB-1629)
   [Miguel Caballer - @micafer]
 
+- [OpenStack] OpenStack: Move floating IP functions to use network service
+  instead of nova.
+
+  This change affects all the floating ip related functions of the
+  ``OpenStack_2_NodeDriver`` class. Two new classes have been added
+  ``OpenStack_2_FloatingIpPool`` and ``OpenStack_2_FloatingIpAddress``.
+  The main change applies to the FloatingIP class where ``node_id``
+  property cannot be directly obtained from FloatingIP information and it
+  must be get from the related Port information with the ``get_node_id``

Review comment:
       I think "must be get" should be "must be got" or "must be gotten".

##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4330,6 +4331,74 @@ def ex_del_server_group(self, server_group):
         )
         return resp.status in (httplib.NO_CONTENT, httplib.ACCEPTED)
 
+    def _to_floating_ips(self, obj):
+        ip_elements = obj["floatingips"]
+        return [self._to_floating_ip(ip) for ip in ip_elements]
+
+    def _to_floating_ip(self, obj):
+        extra = {}
+
+        # In neutron version prior to 13.0.0 port_details does not exists
+        extra["port_details"] = obj.get("port_details")
+        extra["port_id"] = obj.get("port_id")
+        extra["floating_network_id"] = obj.get("floating_network_id")
+
+        return OpenStack_2_FloatingIpAddress(
+            id=obj["id"],
+            ip_address=obj["floating_ip_address"],
+            pool=None,
+            node_id=None,
+            driver=self,
+            extra=extra,
+        )
+
+    def ex_list_floating_ips(self):
+        """
+        List floating IPs
+        :rtype: ``list`` of :class:`OpenStack_2_FloatingIpAddress`
+        """
+        return self._to_floating_ips(
+            self.network_connection.request("/v2.0/floatingips").object
+        )
+
+    def ex_get_floating_ip(self, ip):
+        """
+        Get specified floating IP from the pool
+        :param      ip: floating IP to get
+        :type       ip: ``str``
+        :rtype: :class:`OpenStack_2_FloatingIpAddress`
+        """
+        floating_ips = self._to_floating_ips(
+            self.network_connection.request(
+                "/v2.0/floatingips?floating_ip_address" "=%s" % ip
+            ).object
+        )
+        return floating_ips[0] if floating_ips else None
+
+    def ex_create_floating_ip(self, ip_pool):
+        """
+        Create new floating IP. The ip_pool attribute is optional only if your
+        infrastructure has only one IP pool available.
+        :param      ip_pool: name or id of the floating IP pool
+        :type       ip_pool: ``str``
+        :rtype: :class:`OpenStack_2_FloatingIpAddress`
+        """
+        for pool in self.ex_list_floating_ip_pools():
+            if ip_pool == pool.name or ip_pool == pool.id:

Review comment:
       If ip_pool is None, as the docstring suggests should be possible if the infrastructure has only one pool, I do not think this implementation would create the IP.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] micafer commented on a change in pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
micafer commented on a change in pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#discussion_r819327937



##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4330,6 +4331,74 @@ def ex_del_server_group(self, server_group):
         )
         return resp.status in (httplib.NO_CONTENT, httplib.ACCEPTED)
 
+    def _to_floating_ips(self, obj):
+        ip_elements = obj["floatingips"]
+        return [self._to_floating_ip(ip) for ip in ip_elements]
+
+    def _to_floating_ip(self, obj):
+        extra = {}
+
+        # In neutron version prior to 13.0.0 port_details does not exists
+        extra["port_details"] = obj.get("port_details")
+        extra["port_id"] = obj.get("port_id")
+        extra["floating_network_id"] = obj.get("floating_network_id")
+
+        return OpenStack_2_FloatingIpAddress(
+            id=obj["id"],
+            ip_address=obj["floating_ip_address"],
+            pool=None,
+            node_id=None,
+            driver=self,
+            extra=extra,
+        )
+
+    def ex_list_floating_ips(self):
+        """
+        List floating IPs
+        :rtype: ``list`` of :class:`OpenStack_2_FloatingIpAddress`
+        """
+        return self._to_floating_ips(
+            self.network_connection.request("/v2.0/floatingips").object
+        )
+
+    def ex_get_floating_ip(self, ip):
+        """
+        Get specified floating IP from the pool
+        :param      ip: floating IP to get
+        :type       ip: ``str``
+        :rtype: :class:`OpenStack_2_FloatingIpAddress`
+        """
+        floating_ips = self._to_floating_ips(
+            self.network_connection.request(
+                "/v2.0/floatingips?floating_ip_address" "=%s" % ip
+            ).object
+        )
+        return floating_ips[0] if floating_ips else None
+
+    def ex_create_floating_ip(self, ip_pool):
+        """
+        Create new floating IP. The ip_pool attribute is optional only if your
+        infrastructure has only one IP pool available.
+        :param      ip_pool: name or id of the floating IP pool
+        :type       ip_pool: ``str``
+        :rtype: :class:`OpenStack_2_FloatingIpAddress`
+        """
+        for pool in self.ex_list_floating_ip_pools():
+            if ip_pool == pool.name or ip_pool == pool.id:

Review comment:
       You are right. Fixed in new commit.

##########
File path: CHANGES.rst
##########
@@ -25,6 +25,20 @@ Compute
   (GITHUB-1629)
   [Miguel Caballer - @micafer]
 
+- [OpenStack] OpenStack: Move floating IP functions to use network service
+  instead of nova.
+
+  This change affects all the floating ip related functions of the
+  ``OpenStack_2_NodeDriver`` class. Two new classes have been added
+  ``OpenStack_2_FloatingIpPool`` and ``OpenStack_2_FloatingIpAddress``.
+  The main change applies to the FloatingIP class where ``node_id``
+  property cannot be directly obtained from FloatingIP information and it
+  must be get from the related Port information with the ``get_node_id``

Review comment:
       You are right. Fixed in new commit.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] micafer commented on a change in pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
micafer commented on a change in pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#discussion_r795607112



##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4441,6 +4510,74 @@ def __repr__(self):
         )
 
 
+class OpenStack_2_FloatingIpAddress(OpenStack_1_1_FloatingIpAddress):
+    """
+    Floating IP info 2.0.
+    """
+
+    def __init__(self, id, ip_address, pool, node_id=None, driver=None, extra=None):
+        self.id = str(id)
+        self.ip_address = ip_address
+        self._pool = pool
+        self._node_id = node_id
+        self.driver = driver
+        self.extra = extra if extra else {}
+
+    @property
+    def pool(self):

Review comment:
       Finally I use simpler option.

##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4441,6 +4510,74 @@ def __repr__(self):
         )
 
 
+class OpenStack_2_FloatingIpAddress(OpenStack_1_1_FloatingIpAddress):
+    """
+    Floating IP info 2.0.
+    """
+
+    def __init__(self, id, ip_address, pool, node_id=None, driver=None, extra=None):
+        self.id = str(id)
+        self.ip_address = ip_address
+        self._pool = pool
+        self._node_id = node_id
+        self.driver = driver
+        self.extra = extra if extra else {}
+
+    @property
+    def pool(self):

Review comment:
       Finally I used simpler option.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] micafer commented on a change in pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
micafer commented on a change in pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#discussion_r819328182



##########
File path: libcloud/compute/drivers/openstack.py
##########
@@ -4441,6 +4510,64 @@ def __repr__(self):
         )
 
 
+class OpenStack_2_FloatingIpAddress(OpenStack_1_1_FloatingIpAddress):
+    """
+    Floating IP info 2.0.
+    """
+
+    def __init__(self, id, ip_address, pool, node_id=None, driver=None, extra=None):
+        self.id = str(id)
+        self.ip_address = ip_address
+        self.pool = pool
+        self.node_id = node_id
+        self.driver = driver
+        self.extra = extra if extra else {}
+
+    def get_pool(self):
+        if not self.pool:
+            try:
+                # If pool is not set, get the info from the floating_network_id
+                net = self.driver.ex_get_network(self.extra["floating_network_id"])
+            except Exception:
+                net = None
+            if net:
+                self.pool = OpenStack_2_FloatingIpPool(
+                    net.id, net.name, self.driver.network_connection
+                )
+        return self.pool
+
+    def get_node_id(self):
+        if not self.node_id:
+            # if not is not set, get it from port_details

Review comment:
       I don't get your point here ...




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [libcloud] codecov-commenter edited a comment on pull request #1638: OpenStack: Move floating IP functions to use network service instead of nova

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1638:
URL: https://github.com/apache/libcloud/pull/1638#issuecomment-992462288


   # [Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1638](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1e9d9ea) into [trunk](https://codecov.io/gh/apache/libcloud/commit/4bacff18ecfcd4e8ef3726708dbb8dd2ddf49625?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4bacff1) will **increase** coverage by `0.01%`.
   > The diff coverage is `92.72%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/libcloud/pull/1638/graphs/tree.svg?width=650&height=150&src=pr&token=PYoduksh69&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##            trunk    #1638      +/-   ##
   ==========================================
   + Coverage   83.20%   83.21%   +0.01%     
   ==========================================
     Files         400      400              
     Lines       86953    87038      +85     
     Branches     9234     9242       +8     
   ==========================================
   + Hits        72350    72432      +82     
   + Misses      11463    11462       -1     
   - Partials     3140     3144       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [libcloud/compute/drivers/openstack.py](https://codecov.io/gh/apache/libcloud/pull/1638/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL29wZW5zdGFjay5weQ==) | `85.56% <87.09%> (+0.26%)` | :arrow_up: |
   | [libcloud/test/compute/test\_openstack.py](https://codecov.io/gh/apache/libcloud/pull/1638/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvdGVzdC9jb21wdXRlL3Rlc3Rfb3BlbnN0YWNrLnB5) | `94.63% <100.00%> (+0.10%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4bacff1...1e9d9ea](https://codecov.io/gh/apache/libcloud/pull/1638?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org