Skip to content

Commit

Permalink
typing: Don't abuse Proxy._update
Browse files Browse the repository at this point in the history
The failover and configure operations of load balancers can and should
be implemented as discrete methods, rather than using a custom resource
that has no bearing on a real API resource. Make it so.

Change-Id: Ib213ece0ffa3188b805c27040533429987653dd5
Signed-off-by: Stephen Finucane <[email protected]>
  • Loading branch information
stephenfin committed Feb 25, 2025
1 parent 8278952 commit e9f60bb
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 18 deletions.
15 changes: 9 additions & 6 deletions openstack/load_balancer/v2/_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def wait_for_load_balancer(
attribute='provisioning_status',
)

def failover_load_balancer(self, load_balancer, **attrs):
def failover_load_balancer(self, load_balancer):
"""Failover a load balancer
:param load_balancer: The value can be the ID of a load balancer
Expand All @@ -198,7 +198,8 @@ def failover_load_balancer(self, load_balancer, **attrs):
:returns: ``None``
"""
return self._update(_lb.LoadBalancerFailover, lb_id=load_balancer)
lb = self._get_resource(_lb.LoadBalancer, load_balancer)
lb.failover(self)

def create_listener(self, **attrs):
"""Create a new listener from attributes
Expand Down Expand Up @@ -1072,23 +1073,25 @@ def find_amphora(self, amphora_id, ignore_missing=True):
_amphora.Amphora, amphora_id, ignore_missing=ignore_missing
)

def configure_amphora(self, amphora_id, **attrs):
def configure_amphora(self, amphora_id):
"""Update the configuration of an amphora agent
:param amphora_id: The ID of an amphora
:returns: ``None``
"""
return self._update(_amphora.AmphoraConfig, amphora_id=amphora_id)
lb = self._get_resource(_amphora.Amphora, amphora_id)
lb.configure(self)

def failover_amphora(self, amphora_id, **attrs):
def failover_amphora(self, amphora_id):
"""Failover an amphora
:param amphora_id: The ID of an amphora
:returns: ``None``
"""
return self._update(_amphora.AmphoraFailover, amphora_id=amphora_id)
lb = self._get_resource(_amphora.Amphora, amphora_id)
lb.failover(self)

def create_availability_zone_profile(self, **attrs):
"""Create a new availability zone profile from attributes
Expand Down
48 changes: 48 additions & 0 deletions openstack/load_balancer/v2/amphora.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
# License for the specific language governing permissions and limitations
# under the License.

from openstack import exceptions
from openstack import resource
from openstack import utils


class Amphora(resource.Resource):
Expand Down Expand Up @@ -93,7 +95,52 @@ class Amphora(resource.Resource):
#: The ID of the compute flavor used for the amphora.
compute_flavor = resource.Body('compute_flavor')

def configure(self, session):
"""Configure load balancer.
Update the amphora agent configuration. This will push the new
configuration to the amphora agent and will update the configuration
options that are mutatable.
:param session: The session to use for making this request.
:returns: None
"""
session = self._get_session(session)
version = self._get_microversion(session, action='patch')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'config')

response = session.put(
request.url,
headers=request.headers,
microversion=version,
)

msg = f"Failed to configure load balancer {self.id}"
exceptions.raise_from_response(response, error_message=msg)

def failover(self, session):
"""Failover load balancer.
:param session: The session to use for making this request.
:returns: None
"""
session = self._get_session(session)
version = self._get_microversion(session, action='patch')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'failover')

response = session.put(
request.url,
headers=request.headers,
microversion=version,
)

msg = f"Failed to failover load balancer {self.id}"
exceptions.raise_from_response(response, error_message=msg)


# TODO(stephenfin): Delete this: it's useless
class AmphoraConfig(resource.Resource):
base_path = '/octavia/amphorae/%(amphora_id)s/config'

Expand All @@ -119,6 +166,7 @@ def commit(
return super().commit(session, prepend_key, has_body, *args, *kwargs)


# TODO(stephenfin): Delete this: it's useless
class AmphoraFailover(resource.Resource):
base_path = '/octavia/amphorae/%(amphora_id)s/failover'

Expand Down
24 changes: 24 additions & 0 deletions openstack/load_balancer/v2/load_balancer.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.

from openstack.common import tag
from openstack import exceptions
from openstack import resource
from openstack import utils


class LoadBalancer(resource.Resource, tag.TagMixin):
Expand Down Expand Up @@ -99,6 +102,26 @@ def delete(self, session, error_message=None, **kwargs):
)
return self

def failover(self, session):
"""Failover load balancer.
:param session: The session to use for making this request.
:returns: None
"""
session = self._get_session(session)
version = self._get_microversion(session, action='patch')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'failover')

response = session.put(
request.url,
headers=request.headers,
microversion=version,
)

msg = f"Failed to failover load balancer {self.id}"
exceptions.raise_from_response(response, error_message=msg)


class LoadBalancerStats(resource.Resource):
resource_key = 'stats'
Expand Down Expand Up @@ -126,6 +149,7 @@ class LoadBalancerStats(resource.Resource):
total_connections = resource.Body('total_connections', type=int)


# TODO(stephenfin): Delete this: it's useless
class LoadBalancerFailover(resource.Resource):
base_path = '/lbaas/loadbalancers/%(lb_id)s/failover'

Expand Down
21 changes: 9 additions & 12 deletions openstack/tests/unit/load_balancer/v2/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,11 @@ def test_load_balancer_update(self):
self.verify_update(self.proxy.update_load_balancer, lb.LoadBalancer)

def test_load_balancer_failover(self):
self.verify_update(
self._verify(
"openstack.load_balancer.v2.load_balancer.LoadBalancer.failover",
self.proxy.failover_load_balancer,
lb.LoadBalancerFailover,
method_args=[self.LB_ID],
expected_args=[],
expected_kwargs={'lb_id': self.LB_ID},
expected_args=[self.proxy],
)

def test_listeners(self):
Expand Down Expand Up @@ -411,21 +410,19 @@ def test_amphora_find(self):
self.verify_find(self.proxy.find_amphora, amphora.Amphora)

def test_amphora_configure(self):
self.verify_update(
self._verify(
"openstack.load_balancer.v2.amphora.Amphora.configure",
self.proxy.configure_amphora,
amphora.AmphoraConfig,
method_args=[self.AMPHORA_ID],
expected_args=[],
expected_kwargs={'amphora_id': self.AMPHORA_ID},
expected_args=[self.proxy],
)

def test_amphora_failover(self):
self.verify_update(
self._verify(
"openstack.load_balancer.v2.amphora.Amphora.failover",
self.proxy.failover_amphora,
amphora.AmphoraFailover,
method_args=[self.AMPHORA_ID],
expected_args=[],
expected_kwargs={'amphora_id': self.AMPHORA_ID},
expected_args=[self.proxy],
)

def test_availability_zone_profiles(self):
Expand Down

0 comments on commit e9f60bb

Please sign in to comment.