Add entry-scale-ironic-flat-network scenario to openstack-ardana #3104
Conversation
.../jenkins/ardana/ansible/roles/input_model_generator/vars/entry-scale-ironic-flat-network.yml
Outdated
Show resolved
Hide resolved
I'd like to add what we decided in the cloud team long ago:
A single feature should be able to be reverted easily without side effects (unrelated changes). Also cherry-picking other features from your PR should be possible if the PR itself is not ready to be merged, but parts of it are and are needed for other PRs. |
mital-suse
force-pushed
the
master
branch
9 times, most recently
from
4a7a78a
to
e86e3a6
Compare
There was a problem hiding this comment.
Thanks Mital, it's awesome that you took the time to understand how the input model generator works and got this new scenario implemented. I'm open to suggestions, but I think we can make this twice as valuable if we can get it to deploy with virtual environments too (that's the way we did with all other scenarios), otherwise we just extend our dependency to QA hardware.
In addition to that, I would argue that Ironic isn't really a scenario, but a feature that you can probably add to some of the existing standard scenarios as well (but there may be limitations, I'm not an expert - for instance, can you have Ironic nodes mixed in with regular SLES and RHEL compute nodes ?) - which is why we shouldn't limit it to one scenario, but allow other scenarios to reuse it, at least in theory.
I hope my suggestions are welcome. If you have a hard time understanding or implementing them, let me know, I can help.
This will make a great addition to the CI. Thanks again for working on it !
scripts/jenkins/ardana/ansible/roles/input_model_generator/defaults/main.yml
Outdated
Show resolved
Hide resolved
| @@ -26,9 +26,14 @@ | |||
| - AZ2 | |||
| - AZ3 | |||
| configuration-data: | |||
| {% if scenario.name == 'entry-scale-ironic-flat-network' %} | |||
There was a problem hiding this comment.
You don't need to condition configuration data on enabled features or services. IIRC, the config-processor already knows how to eliminate or ignore unused configuration data, so you can add them all here. This also helps with my other objection about not explicitly referencing scenario names in the input model generator implementation.
There was a problem hiding this comment.
In that case I can remove the check here and comment out designate, octavia services after input-model is generated in deploy-cloud.yml
- name: Remove designate and octavia features from input model when scenario is ironic
replace:
path: "{{ input_model_path }}/data/control_plane.yml"
regexp: '(.*{{ item }}.*)'
replace: '#\1'
when: scenario_name == "entry-scale-ironic-flat-network"
delegate_to: localhost
loop:
- designate
- octavia
- bind
| @@ -77,7 +77,11 @@ | |||
| {% set ns.routes=network_group.routes|default([]) %} | |||
| {% if 'MANAGEMENT' in network_group.component_endpoints %} | |||
| {% if bm_info is defined %} | |||
| {% if scenario.name != 'entry-scale-ironic-flat-network' %} | |||
There was a problem hiding this comment.
You must not reference scenario names in the input model implementation, because it reduces its flexibility. The input model generator should instead use variables that represent features or node types, which can be (re)used across several scenarios. In this particular case, you can reference the ironic_computes variable and use its value to define conditions (i.e. if it's value is defined and not zero, do X otherwise do Y).
| @@ -196,12 +200,17 @@ | |||
| # This is the network group that will be used to provide | |||
| # external access to VMs (via floating IP Addresses) | |||
| # | |||
| {% if enable_external_network_bridge %} | |||
| {% if enable_external_network_bridge and scenario.name !='entry-scale-ironic-flat-network' %} | |||
There was a problem hiding this comment.
Same comment here about not using the scenario name. Moreover, you can count on the fact that enable_external_network_bridge will be false in most cases - in fact, we're only using it for GM8 builds and we will remove it soon, because its use is deprecated.
| - neutron.l3_agent.external_network_bridge | ||
| {% else %} | ||
| - neutron.networks.flat: | ||
| {% if scenario.name == 'entry-scale-ironic-flat-network' %} | ||
| {% set ns.physnet_id = ns.physnet_id+1 %} | ||
| provider-physical-network: physnet{{ ns.physnet_id }} |
There was a problem hiding this comment.
This doesn't seem to map well to virtual deployments. Can we make this work with virtual Ardana deployments too ? That would be truly awesome, otherwise we continue to depend on QA hardware to test Ironic.
There was a problem hiding this comment.
Please disregard this comment. I realized it won't be possible to test Ironic support using virtual cloud setups. Still, you mustn't use the scenario name in the condition here. You could use the number of ironic compute nodes instead.
...na/ansible/roles/input_model_generator/templates/input_model/data/neutron/neutron_config.yml
Outdated
Show resolved
Hide resolved
|
Since Ironic works directly with the Hardware not sure we gain anything from doing this virtual. We certainly can not emluate boot from SAN or can we in a virtual deployment. Also I am not sure you can mix standard computes and ironic computes without doing the multi tenant stuff which means you do HW configs on the switch. Guang can elaborate in more detail hopefully |
|
Needs rebase |
| @@ -39,6 +39,32 @@ | |||
|
|
|||
| tasks: | |||
| - block: | |||
| - name: Remove versioned features from input model when not enabled | |||
There was a problem hiding this comment.
This was moved to the input_model_generator and input_model_clone roles (see 1), you should not add it back here.
| - heat-api-cloudwatch | ||
| - nova-console-auth | ||
|
|
||
| - name: Remove designate and octavia features from input model when scenario is ironic |
There was a problem hiding this comment.
You should also move this to the input_model_generator role.
Thanks @KeithMnemonic . Now that you mention it, it's quite obvious we can't use virtual nodes to test this, if not because of SAN boot and networking issues, it's because we don't have IPMI. I retract my statement about virtual setups. |
|
@mital-suse, the way that I see it, you're just a couple of short steps away from getting this PR merged. You just need to address @flaviodsr 's comments about the rebase merge operation that you performed and to follow up on my suggestion to stop using the |
| --- | ||
|
|
||
| controllers: 3 | ||
| sles_computes: 0 |
There was a problem hiding this comment.
Since you don't need SLES and RHEL computes with this input model scenario, you can delete these and replace them with a default value (1 ?) for your ironic_computes variable.
No description provided.