Neutron gateway actions #611
Conversation
There was a problem hiding this comment.
- I'd recommend removing the backslashes due to col length.
- Could some comments be added, specially for the
run_actioncalls? - The 3 tests have a repeating pattern to check if
result.statusiscompletedand compare data from the client and the one returned from the action. Maybe this code could be wrapped into a helper function?
Other than the above, tests lgtm.
The last mitaka test bundle (xenial-mitaka) does not have loadbalancer services enabled.
| def tearDown(self): | ||
| """Cleanup loadbalancers if there are any left over.""" | ||
| super(NeutronGatewayStatusActionsTest, self).tearDown() | ||
| if not self.SKIP_LBAAS_TESTS: | ||
| load_balancers = self.neutron_client.list_loadbalancers().get( | ||
| 'loadbalancers', []) | ||
| for lbaas in load_balancers: | ||
| self.neutron_client.delete_loadbalancer(lbaas['id']) |
There was a problem hiding this comment.
This gets run after each test in the class? Is this what you want? I only see one test that uses load balancers and it might be more "obvious" and less resource hungry to just wrap the majority of a test in a try: except: and put this code in a finally: to ensure it gets run?
There was a problem hiding this comment.
This is a good point. Indeed the original purpose of this tearDown was to ensure that the loadbalancers are removed even if the test that created them failed. I'll fix it.
| if not routers_from_client: | ||
| self.fail('At least one router must be configured for this test ' | ||
| 'to pass.') |
There was a problem hiding this comment.
Do you want the test to fail, or just to be skipped?
There was a problem hiding this comment.
Intention was for tests to fail because the model is not properly configured. Unless there's a reason for some test models to not have routers. But none of the func test models had problem with this so far.
Edit: My reasoning was that without the proper resources present on the neutron agents (routers, networks, loadbalancers), we can not verify that these actions return correct data. If we skip these tests, errors may silently creep in, in the future.
| if not networks_from_client: | ||
| self.fail('At least one network must be configured for this test ' | ||
| 'to pass.') |
There was a problem hiding this comment.
As above; should this test be skipped if there is no resource to test against rather than failing?
| if not subnet_list: | ||
| raise RuntimeError('Expected subnet "private_subnet" is not ' | ||
| 'configured.') |
There was a problem hiding this comment.
As above, skip or fail? The reason for asking, is whether this can be a generic test or is very specific to this environment.
| cls.SKIP_LBAAS_TESTS = (current_release <= xenial_mitaka or | ||
| current_release >= bionic_train) |
There was a problem hiding this comment.
An alternative option/syntax:
cls.SKIP_LBAAS_TESTS = not (xenial_mitaka > current_release < bionic_train)There was a problem hiding this comment.
thanks, that does look better.
| except Exception as exc: | ||
| raise exc |
There was a problem hiding this comment.
This isn't needed; the exception with happen if no exceptions are caught which is essentially the same thing.
New actions: * show-routers * show-dhcp-networks * show-loadbalancers Partial-Bug: #1916231 Closes-Bug: #1917401 Closes-Bug: #1917403 Closes-Bug: #1917405 Change-Id: Ie59c2a7d5c1ee9c51a0f7db4e8f38229812ac84a func-test-pr: openstack-charmers/zaza-openstack-tests#611
* Update charm-neutron-gateway from branch 'master'
to c7566f4e47e9203e80a83c579af179e26324937d
- Merge "Actions that expose various neutron resources"
- Actions that expose various neutron resources
New actions:
* show-routers
* show-dhcp-networks
* show-loadbalancers
Partial-Bug: #1916231
Closes-Bug: #1917401
Closes-Bug: #1917403
Closes-Bug: #1917405
Change-Id: Ie59c2a7d5c1ee9c51a0f7db4e8f38229812ac84a
func-test-pr: openstack-charmers/zaza-openstack-tests#611
Functional tests for neutron-gateway actions needed by juju-verify.
Related LP issues (1 for each action):
Neutron gateway charm PR: https://review.opendev.org/c/openstack/charm-neutron-gateway/+/782896