-
Notifications
You must be signed in to change notification settings - Fork 123
Add multus_type to cifmw_networking_env_definition variable #2816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add multus_type to cifmw_networking_env_definition variable #2816
Conversation
d8fe43c
to
bdd9a2c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just change commit message and would be good ;)
bdd9a2c
to
113aee0
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/eca194b7b9ac40a388841ed3a32e1b19 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 41m 22s |
113aee0
to
a0dec14
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
c497149
to
b48f41b
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4d4091cfd8da4ea6a3155150d1d83049 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 48m 34s |
b48f41b
to
9164234
Compare
Swtich the scenario to scenarios/centos-9/ironic.yml Update hooks/playbooks/control_plane_ironic.yml to include kustomization patches to configure ironic and Nova Cell with computeDriver: ironic.IronicDriver Depends-On: openstack-k8s-operators/install_yamls#969 Depends-On: openstack-k8s-operators#2816 Conflicts: zuul.d/projects.yaml
ae331fa
to
179c625
Compare
Swtich the scenario to scenarios/centos-9/ironic.yml Update hooks/playbooks/control_plane_ironic.yml to include kustomization patches to configure ironic and Nova Cell with computeDriver: ironic.IronicDriver Depends-On: openstack-k8s-operators#2816 Conflicts: zuul.d/projects.yaml
Swtich the scenario to scenarios/centos-9/ironic.yml Update hooks/playbooks/control_plane_ironic.yml to include kustomization patches to configure ironic and Nova Cell with computeDriver: ironic.IronicDriver Depends-On: openstack-k8s-operators#2816 Conflicts: zuul.d/projects.yaml hooks/playbooks/control_plane_ironic.yml
af1c269
to
a619b47
Compare
recheck |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/5f25172d9dad4b7d95b3a72369a58a2e ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 43m 24s |
recheck |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/9a8f17790bc44ba78f33c85505032618 ✔️ openstack-k8s-operators-content-provider SUCCESS in 26m 03s |
recheck |
Swtich the scenario to scenarios/centos-9/ironic.yml Update hooks/playbooks/control_plane_ironic.yml to include kustomization patches to configure ironic and Nova Cell with computeDriver: ironic.IronicDriver Depends-On: openstack-k8s-operators#2816 Depends-On: openstack-k8s-operators#2816 Conflicts: zuul.d/projects.yaml hooks/playbooks/control_plane_ironic.yml
be22b4a
to
c0863fe
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/e394250534724600bd3bb74e679ce7ea ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 51m 15s |
6c169ae
to
e7e85e1
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/0fbeabc10d6b40649989e3c08f0be55c ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 46m 56s |
e7e85e1
to
74df66b
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a0d7033b6695499ba56dc2658b90fbd7 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 42m 13s |
recheck |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/798458246646492b9135c9071793340b ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 10m 46s |
recheck |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4e891a137daa41be9a8e3d521e66f62d ❌ openstack-k8s-operators-content-provider FAILURE in 16m 26s |
74df66b
to
5c09cf3
Compare
@@ -1044,6 +1044,7 @@ class SubnetBasedNetworkToolDefinition: | |||
__FIELD_ROUTES = "routes" | |||
__FIELD_ROUTES_IPV4 = "routes-v4" | |||
__FIELD_ROUTES_IPV6 = "routes-v6" | |||
__FIELD_TYPE = "type" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Information: This is the input for the network mapper, so this is what we need to supply in order to configure multus tool to be either macvlan or bridge.
@@ -678,12 +678,15 @@ def __build_network_tool_common( | |||
for ip_route in tool_net_def.routes_ipv6 | |||
], | |||
] | |||
multus_type = [] | |||
if tool_type.__name__ == "MappedMultusNetworkConfig": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the tool is multus, then we add the multus_type argument to the class init. This end up in a networking env variable with a multus_type inside the multus tool section either value macvlan or bridge.
@@ -10,7 +10,7 @@ Creates additional networks in a OCP cluster using NetworkAttachmentDefinition | |||
* `cifmw_ci_multus_namespace`: (String) The namespace where OCP resources will be installed. Defaults to `openstack`. | |||
* `cifmw_ci_multus_ocp_hostname`: (String) The OCP inventory hostname. Used to gather network information specific to those nodes, mostly the interfaces. Defaults to `crc`. | |||
* `cifmw_ci_multus_cniversion`: (String) The CNI specification version used when creating the resource. Defaults to `0.3.1`. | |||
* `cifmw_ci_multus_default_nad_type`: (String) Default NAD type used when not specified by the network configuration. Defaults to `macvlan`. | |||
* `cifmw_ci_multus_default_nad_type`: (String) Default NAD type used when not specified by the network configuration. Defaults to `macvlan`. You can select the type of each NAD by "multus_type" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is at the end the output of the networking mapperr. So actually you select the tool type from the networking mapper type variable.
@@ -40,17 +40,6 @@ | |||
ansible.builtin.include_vars: | |||
file: ../resources/vars/shared_vars.yml | |||
|
|||
- name: Override interface name in cifmw_networking_env_definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary. It was used for checking the patch feature of ci_multus, done in the shared_vars section.
@@ -0,0 +1,44 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We check the rendered of nad.yml.j2 after running ci_multus. We are checking the spec section from what we get after attaching a test pod to this network and the expected. We just check the spec section.
@@ -1,4 +1,9 @@ | |||
{% for network_name, network_details in _cifmw_ci_multus_net_info.items() %} | |||
{% if network_details.tools.get('multus', {}).get('multus_type', None) %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If multus type is defined then we configure the multus tool type depends on this, if not as previousle default value.
@@ -191,3 +191,4 @@ | |||
ansible.builtin.assert: | |||
that: | |||
- "_content.networks['internalapi'].vlan_id == 100" | |||
- "_content.networks['internalapi'].tools.multus.multus_type == 'bridge'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're checking the output here to be the expected when we select in the networking mapper the bridge type.
@@ -1190,6 +1200,21 @@ def __parse_raw_route_field( | |||
if ipv6_route: | |||
self.__ipv6_routes.append(ipv6_route) | |||
|
|||
@property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're adding here the new type for the networking mapper input.
@@ -678,12 +678,15 @@ def __build_network_tool_common( | |||
for ip_route in tool_net_def.routes_ipv6 | |||
], | |||
] | |||
multus_type = [] | |||
if tool_type.__name__ == "MappedMultusNetworkConfig": | |||
multus_type.append(tool_net_def.type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushing the value from the input "type" variable into the output "multus_type" variable that would be used for rendering nad.yml.j2 file endind up in NetworkAttachmentDefinition
@@ -36,6 +36,7 @@ cifmw_ci_multus_net_info_patch_1: | |||
ipv4_ranges: | |||
- start: 192.168.122.30 | |||
end: 192.168.122.70 | |||
type: bridge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good that you added that, I was confused
- name: Assert expected Network Attachment Definitions content spec with the expected | ||
ansible.builtin.assert: | ||
that: _ci_multus_out_spec == _ci_multus_expected_spec | ||
vars: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if you can put _ci_multus_expected_spec
before calling assert module (to line 47) would be easier to read. thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, done!
@@ -12,12 +17,11 @@ spec: | |||
{ | |||
"cniVersion": "{{ cifmw_ci_multus_cniversion }}", | |||
"name": "{{ network_name }}", | |||
{% if cifmw_ci_multus_default_nad_type == "macvlan" %} | |||
"type": "macvlan", | |||
"type": "{{ multus_type }}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it not be in condition (L22) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We suppose to always have a multus_type, no matter if the default one (set in the role defaults) or by using multus_type.
Currently we support "bridge" and "macvlan" and no matter which one is the selected both rendered files have type: macvlan or type: bridge.
The differentation here comes because, depending its value, we go "master" with the interface name or "bridge".
So condition L22 it's for that.
We now can select the multus_type between "bridge" and macvlan" so ci_multus would render each NetworkAttachmentDefinition appropiate without the assumption that each NAD has the same multus type defined at cifmw_ci_multus_default_nad_type. For a given NAD, if there's no multus_type variable, then it'd take (as previously) the default one. For adding the multus_type to cifmw_networking_env_definition we need to add the argument "type" for the input of networking_mapper.
5c09cf3
to
0e44750
Compare
We now can select the multus_type between "bridge" and macvlan"
so ci_multus would render each NetworkAttachmentDefinition appropiate
without the assumption that each NAD has the same multus type defined at
cifmw_ci_multus_default_nad_type.
For a given NAD, if there's no multus_type variable, then it'd take
(as previously) the default one.
For adding the multus_type to cifmw_networking_env_definition we need to add
the argument "type" for the input of networking_mapper.