From b8ebbe99191671cde3ac8f4f0a6f335fc09caab9 Mon Sep 17 00:00:00 2001 From: Adam Harwell Date: Fri, 6 Oct 2017 12:53:14 -0700 Subject: [PATCH] Add cached_zone to the amphora record This will enable a number of possible features that need to select amphorae based on their availability zone. This would allow for quick-lookups on large lists and could be stale, but it would be expected that future code that uses this would check with nova for an update if it needs fully accurate data. Having it be explicitly "cached" should take care of concerns about users (operators, in this case) being confused about correctness. Using simply the word "zone" should address concerns about commonality between compute providers. Change-Id: I8e26f99bca3496a454ba7bae2570f517e07d5fc2 Story: 2001221 Task: 5732 --- api-ref/source/parameters.yaml | 8 +++++ api-ref/source/v2/amphora.inc | 14 ++++++++ .../v2/examples/amphora-list-response.json | 8 +++-- .../v2/examples/amphora-show-response.json | 5 +-- octavia/api/v2/types/amphora.py | 1 + octavia/common/data_models.py | 3 +- octavia/compute/drivers/nova_driver.py | 10 +++++- .../controller/worker/tasks/database_tasks.py | 6 ++-- .../bf171d0d91c3_amphora_add_cached_zone.py | 33 +++++++++++++++++++ octavia/db/models.py | 1 + .../tests/functional/api/v2/test_amphora.py | 1 + octavia/tests/functional/db/test_models.py | 4 ++- .../unit/compute/drivers/test_nova_driver.py | 8 +++++ ...o-the-amphora-record-7c3231c2b5b96574.yaml | 10 ++++++ 14 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 octavia/db/migration/alembic_migrations/versions/bf171d0d91c3_amphora_add_cached_zone.py create mode 100644 releasenotes/notes/Add-cached_zone-to-the-amphora-record-7c3231c2b5b96574.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 98e1141ae9..0af41a7998 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -185,6 +185,14 @@ bytes_out: in: body required: true type: integer +cached-zone: + description: | + The availability zone of a compute instance, cached at create time. This + is not guaranteed to be current. May be an empty-string if the compute + service does not use zones. + in: body + required: true + type: string cert-busy: description: | Whether the certificate is in the process of being replaced. diff --git a/api-ref/source/v2/amphora.inc b/api-ref/source/v2/amphora.inc index 142b16ef87..d7e64e1228 100644 --- a/api-ref/source/v2/amphora.inc +++ b/api-ref/source/v2/amphora.inc @@ -16,6 +16,12 @@ by using query string parameters. For information, see :ref:`filtering`. The list might be empty. +.. NOTE:: + + The field `cached_zone` should be used for quick filtering and reference + only, as it may out of date. If an up-to-date zone is vital, we recommend + retrieving details directly from the compute service. + .. rest_status_code:: success ../http-status.yaml - 200 @@ -60,6 +66,7 @@ Response Parameters - vrrp_interface: vrrp-interface - vrrp_id: vrrp-id - vrrp_priority: vrrp-priority + - cached_zone: cached-zone Response Example ---------------- @@ -79,6 +86,12 @@ If you are not an administrative user, the service returns the HTTP This operation does not require a request body. +.. NOTE:: + + The field `cached_zone` should be used for quick filtering and reference + only, as it may out of date. If an up-to-date zone is vital, we recommend + retrieving details directly from the compute service. + .. rest_status_code:: success ../http-status.yaml - 200 @@ -124,6 +137,7 @@ Response Parameters - vrrp_interface: vrrp-interface - vrrp_id: vrrp-id - vrrp_priority: vrrp-priority + - cached_zone: cached-zone Response Example ---------------- diff --git a/api-ref/source/v2/examples/amphora-list-response.json b/api-ref/source/v2/examples/amphora-list-response.json index 56a0168ec2..9c8c66efef 100644 --- a/api-ref/source/v2/examples/amphora-list-response.json +++ b/api-ref/source/v2/examples/amphora-list-response.json @@ -15,7 +15,8 @@ "status": "ALLOCATED", "vrrp_interface": "eth1", "vrrp_id": 1, - "vrrp_priority": 100 + "vrrp_priority": 100, + "cached_zone": "zone1" }, { "id": "89c186a3-cb16-497b-b099-c4bd40316642", @@ -32,7 +33,8 @@ "status": "ALLOCATED", "vrrp_interface": "eth1", "vrrp_id": 1, - "vrrp_priority": 200 + "vrrp_priority": 200, + "cached_zone": "zone2" } ] -} \ No newline at end of file +} diff --git a/api-ref/source/v2/examples/amphora-show-response.json b/api-ref/source/v2/examples/amphora-show-response.json index 186292e648..86f7c8949b 100644 --- a/api-ref/source/v2/examples/amphora-show-response.json +++ b/api-ref/source/v2/examples/amphora-show-response.json @@ -14,6 +14,7 @@ "status": "ALLOCATED", "vrrp_interface": "eth1", "vrrp_id": 1, - "vrrp_priority": 100 + "vrrp_priority": 100, + "cached_zone": "zone1" } -} \ No newline at end of file +} diff --git a/octavia/api/v2/types/amphora.py b/octavia/api/v2/types/amphora.py index 3c71d3f185..e63ac1809c 100644 --- a/octavia/api/v2/types/amphora.py +++ b/octavia/api/v2/types/amphora.py @@ -39,6 +39,7 @@ class AmphoraResponse(BaseAmphoraType): vrrp_interface = wtypes.wsattr(wtypes.StringType()) vrrp_id = wtypes.wsattr(wtypes.IntegerType()) vrrp_priority = wtypes.wsattr(wtypes.IntegerType()) + cached_zone = wtypes.wsattr(wtypes.StringType()) @classmethod def from_data_model(cls, data_model, children=False): diff --git a/octavia/common/data_models.py b/octavia/common/data_models.py index 202778ef79..29f54e2a80 100644 --- a/octavia/common/data_models.py +++ b/octavia/common/data_models.py @@ -488,7 +488,7 @@ class Amphora(BaseDataModel): ha_ip=None, vrrp_port_id=None, ha_port_id=None, load_balancer=None, role=None, cert_expiration=None, cert_busy=False, vrrp_interface=None, vrrp_id=None, - vrrp_priority=None): + vrrp_priority=None, cached_zone=None): self.id = id self.load_balancer_id = load_balancer_id self.compute_id = compute_id @@ -505,6 +505,7 @@ class Amphora(BaseDataModel): self.load_balancer = load_balancer self.cert_expiration = cert_expiration self.cert_busy = cert_busy + self.cached_zone = cached_zone def delete(self): for amphora in self.load_balancer.amphorae: diff --git a/octavia/compute/drivers/nova_driver.py b/octavia/compute/drivers/nova_driver.py index e0d0f56292..4b0c65c28e 100644 --- a/octavia/compute/drivers/nova_driver.py +++ b/octavia/compute/drivers/nova_driver.py @@ -214,6 +214,7 @@ class VirtualMachineManager(compute_base.ComputeBase): # fields lb_network_ip = None + availability_zone = None fault = None try: @@ -229,6 +230,12 @@ class VirtualMachineManager(compute_base.ComputeBase): if is_boot_network or no_boot_networks: lb_network_ip = interface.fixed_ips[0]['ip_address'] break + try: + availability_zone = getattr( + nova_response, 'OS-EXT-AZ:availability_zone') + except AttributeError: + LOG.info('No availability zone listed for server %s', + nova_response.id) fault = getattr(nova_response, 'fault', None) except Exception: LOG.debug('Extracting virtual interfaces through nova ' @@ -237,7 +244,8 @@ class VirtualMachineManager(compute_base.ComputeBase): response = models.Amphora( compute_id=nova_response.id, status=nova_response.status, - lb_network_ip=lb_network_ip + lb_network_ip=lb_network_ip, + cached_zone=availability_zone ) return response, fault diff --git a/octavia/controller/worker/tasks/database_tasks.py b/octavia/controller/worker/tasks/database_tasks.py index 9549070ced..e7396b7a16 100644 --- a/octavia/controller/worker/tasks/database_tasks.py +++ b/octavia/controller/worker/tasks/database_tasks.py @@ -872,8 +872,10 @@ class UpdateAmphoraInfo(BaseDatabaseTask): :param compute_obj: Compute on which an amphora resides :returns: Updated amphora object """ - self.amphora_repo.update(db_apis.get_session(), amphora_id, - lb_network_ip=compute_obj.lb_network_ip) + self.amphora_repo.update( + db_apis.get_session(), amphora_id, + lb_network_ip=compute_obj.lb_network_ip, + cached_zone=compute_obj.cached_zone) return self.amphora_repo.get(db_apis.get_session(), id=amphora_id) diff --git a/octavia/db/migration/alembic_migrations/versions/bf171d0d91c3_amphora_add_cached_zone.py b/octavia/db/migration/alembic_migrations/versions/bf171d0d91c3_amphora_add_cached_zone.py new file mode 100644 index 0000000000..40c8bf0225 --- /dev/null +++ b/octavia/db/migration/alembic_migrations/versions/bf171d0d91c3_amphora_add_cached_zone.py @@ -0,0 +1,33 @@ +# Copyright 2017 GoDaddy +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""add cached_zone to amphora + +Revision ID: bf171d0d91c3 +Revises: 4aeb9e23ad43 +Create Date: 2017-10-06 12:07:34.290451 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'bf171d0d91c3' +down_revision = '4aeb9e23ad43' + + +def upgrade(): + op.add_column(u'amphora', sa.Column(u'cached_zone', sa.String(255), + nullable=True)) diff --git a/octavia/db/models.py b/octavia/db/models.py index b86072e25a..bc0c5ecc5c 100644 --- a/octavia/db/models.py +++ b/octavia/db/models.py @@ -511,6 +511,7 @@ class Amphora(base_models.BASE, base_models.IdMixin): vrrp_interface = sa.Column(sa.String(16), nullable=True) vrrp_id = sa.Column(sa.Integer(), nullable=True) vrrp_priority = sa.Column(sa.Integer(), nullable=True) + cached_zone = sa.Column(sa.String(255), nullable=True) class AmphoraHealth(base_models.BASE): diff --git a/octavia/tests/functional/api/v2/test_amphora.py b/octavia/tests/functional/api/v2/test_amphora.py index e0fd3a26ab..7c78aeba1d 100644 --- a/octavia/tests/functional/api/v2/test_amphora.py +++ b/octavia/tests/functional/api/v2/test_amphora.py @@ -52,6 +52,7 @@ class TestAmphora(base.BaseAPITest): 'vrrp_interface': 'eth1', 'vrrp_id': 1, 'vrrp_priority': 100, + 'cached_zone': None } self.amp = self.amphora_repo.create(self.session, **self.amp_args) self.amp_id = self.amp.id diff --git a/octavia/tests/functional/db/test_models.py b/octavia/tests/functional/db/test_models.py index be279ef39a..2d94641a88 100644 --- a/octavia/tests/functional/db/test_models.py +++ b/octavia/tests/functional/db/test_models.py @@ -29,6 +29,7 @@ class ModelTestMixin(object): FAKE_IP = '10.0.0.1' FAKE_UUID_1 = uuidutils.generate_uuid() FAKE_UUID_2 = uuidutils.generate_uuid() + FAKE_AZ = 'zone1' def _insert(self, session, model_cls, model_kwargs): with session.begin(): @@ -138,7 +139,8 @@ class ModelTestMixin(object): 'ha_port_id': self.FAKE_UUID_2, 'lb_network_ip': self.FAKE_IP, 'cert_expiration': datetime.datetime.utcnow(), - 'cert_busy': False} + 'cert_busy': False, + 'cached_zone': self.FAKE_AZ} kwargs.update(overrides) return self._insert(session, models.Amphora, kwargs) diff --git a/octavia/tests/unit/compute/drivers/test_nova_driver.py b/octavia/tests/unit/compute/drivers/test_nova_driver.py index 72859e0efe..7435cbc6b7 100644 --- a/octavia/tests/unit/compute/drivers/test_nova_driver.py +++ b/octavia/tests/unit/compute/drivers/test_nova_driver.py @@ -107,6 +107,7 @@ class TestNovaClient(base.TestCase): self.nova_response.id = self.amphora.compute_id self.nova_response.status = 'ACTIVE' self.nova_response.fault = 'FAKE_FAULT' + setattr(self.nova_response, 'OS-EXT-AZ:availability_zone', None) self.interface_list = mock.MagicMock() self.interface_list.net_id = '1' @@ -286,6 +287,13 @@ class TestNovaClient(base.TestCase): self.assertEqual(self.nova_response.fault, fault) self.nova_response.interface_list.called_with() + def test_translate_amphora_no_availability_zone(self): + delattr(self.nova_response, 'OS-EXT-AZ:availability_zone') + amphora, fault = self.manager._translate_amphora(self.nova_response) + self.assertEqual(self.amphora, amphora) + self.assertEqual(self.nova_response.fault, fault) + self.nova_response.interface_list.called_with() + def test_bad_translate_amphora(self): self.nova_response.interface_list.side_effect = Exception self.manager._nova_client.networks.get.side_effect = Exception diff --git a/releasenotes/notes/Add-cached_zone-to-the-amphora-record-7c3231c2b5b96574.yaml b/releasenotes/notes/Add-cached_zone-to-the-amphora-record-7c3231c2b5b96574.yaml new file mode 100644 index 0000000000..053c75cecc --- /dev/null +++ b/releasenotes/notes/Add-cached_zone-to-the-amphora-record-7c3231c2b5b96574.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + The compute zone (if applicable) is now cached in the database and returned + in the Amphora API as `cached_zone`. Please note that this is only set at + the original time of provisioning, and could be stale for various reasons + (for example, if live-migrations have taken place due to maintenances). We + recommend it be used for reference only, unless you are absolutey certain + it is current in your environment. The source of truth is still the system + you use for compute.