Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .ansible-lint
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ exclude_paths:
- roles/kustomize_deploy/molecule/flexible_loop/files/networking-environment-definition.yml # Generated
- roles/kustomize_deploy/molecule/flexible_loop/prepare.yml # import_playbook
- roles/*/molecule/*/side_effect.yml # syntax-check[empty-playbook] https://github.com/ansible/molecule/issues/3617
- roles/ci_multus/molecule/*/nads_output.yml # internal-error due to "---" characters
strict: true
quiet: false
verbosity: 1
Expand Down
25 changes: 25 additions & 0 deletions plugins/module_utils/net_map/networking_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,7 @@ class SubnetBasedNetworkToolDefinition:
__FIELD_ROUTES = "routes"
__FIELD_ROUTES_IPV4 = "routes-v4"
__FIELD_ROUTES_IPV6 = "routes-v6"
__FIELD_TYPE = "type"
Copy link
Author

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.


def __init__(
self,
Expand All @@ -1067,6 +1068,7 @@ def __init__(
self.__ipv6_ranges: typing.List[HostNetworkRange] = []
self.__ipv4_routes: typing.List[HostNetworkRoute] = []
self.__ipv6_routes: typing.List[HostNetworkRoute] = []
self.__type: typing.Optional[str] = None

self.__parse_raw(raw_config)

Expand All @@ -1092,6 +1094,13 @@ def __parse_raw(self, raw_definition: typing.Dict[str, typing.Any]):
alone_field=self.__FIELD_ROUTES,
)

_validate_fields_one_of(
[
self.__FIELD_TYPE,
],
raw_definition,
parent_name=self.__object_name,
)
self.__parse_raw_range_field(raw_definition, self.__FIELD_RANGES)
self.__parse_raw_range_field(
raw_definition, self.__FIELD_RANGES_IPV4, ip_version=4
Expand All @@ -1107,6 +1116,7 @@ def __parse_raw(self, raw_definition: typing.Dict[str, typing.Any]):
self.__parse_raw_route_field(
raw_definition, self.__FIELD_ROUTES_IPV6, ip_version=6
)
self.__parse_raw_type_field(raw_definition, self.__FIELD_TYPE)

def __parse_raw_range_field(
self,
Expand Down Expand Up @@ -1190,6 +1200,21 @@ def __parse_raw_route_field(
if ipv6_route:
self.__ipv6_routes.append(ipv6_route)

@property
Copy link
Author

@evallesp evallesp Apr 15, 2025

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.

def type(self) -> str:
"""The type of the tool for multus."""
return self.__type

def __parse_raw_type_field(self, raw_definition, field_name: str):
if field_name in raw_definition:
type = _validate_parse_field_type(
field_name,
raw_definition,
str,
parent_name=self.__object_name,
)
self.__type = type


class MultusNetworkDefinition(SubnetBasedNetworkToolDefinition):
"""Parses and holds Multus configuration for a given network."""
Expand Down
2 changes: 2 additions & 0 deletions plugins/module_utils/net_map/networking_env_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,15 @@ class MappedMultusNetworkConfig:
ipv6_ranges: IPv6 ranges assigned to Multus.
ipv4_routes: IPv4 routes assigned to Multus.
ipv6_routes: IPv6 routes assigned to Multus.
multus_type: The type of the multus network.

"""

ipv4_ranges: typing.List[MappedIpv4NetworkRange]
ipv6_ranges: typing.List[MappedIpv6NetworkRange]
ipv4_routes: typing.List[MappedIpv4NetworkRoute]
ipv6_routes: typing.List[MappedIpv6NetworkRoute]
multus_type: typing.Optional[str] = None


@dataclasses.dataclass(frozen=True)
Expand Down
5 changes: 4 additions & 1 deletion plugins/module_utils/net_map/networking_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Copy link
Author

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.

multus_type.append(tool_net_def.type)
Copy link
Author

@evallesp evallesp Apr 15, 2025

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


if any(
route_field in tool_type.__dataclass_fields__
for route_field in ["ipv4_routes", "ipv6_routes"]
):
args_list = args_list + route_args_list
args_list = args_list + route_args_list + multus_type
return tool_type(*args_list)


Expand Down
4 changes: 3 additions & 1 deletion roles/ci_multus/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Author

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.

* `cifmw_ci_multus_default_nad_ipam_type`: (String) Default NAD IPAM type to be used when not specified by the network configuration. Defaults to `whereabouts`.
* `cifmw_ci_multus_default_nad_ipam_type_ip_version``: (String) Default IP version to use in IPAM config. Defaults to `v4`.
* `cifmw_ci_multus_dryrun`: (Bool) When enabled, tasks that require an OCP environment are skipped. Defaults to `false`.
Expand All @@ -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
Copy link
Contributor

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

```

## Limitations
Expand Down Expand Up @@ -70,6 +71,7 @@ cifmw_ci_multus_net_info_patch_1:
ipv4_ranges:
- start: 192.168.122.30
end: 192.168.122.70
type: macvlan
ansible.builtin.include_role:
name: "ci_multus"
```
Expand Down
11 changes: 0 additions & 11 deletions roles/ci_multus/molecule/default/converge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@
ansible.builtin.include_vars:
file: ../resources/vars/shared_vars.yml

- name: Override interface name in cifmw_networking_env_definition
Copy link
Author

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.

vars:
_cifmw_networking_env_definition_patch:
instances:
crc:
networks:
default:
interface_name: "{{ hostvars.crc.ansible_default_ipv4.interface }}"
ansible.builtin.set_fact:
cifmw_networking_env_definition: "{{ cifmw_networking_env_definition | combine(_cifmw_networking_env_definition_patch, recursive=True) }}"

- name: Call ci_multus role
ansible.builtin.include_role:
name: "ci_multus"
1 change: 1 addition & 0 deletions roles/ci_multus/molecule/default/molecule.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ provisioner:
ipv4_ranges:
- start: 192.168.122.30
end: 192.168.122.70
multus_type: macvlan

cifmw_path: "{{ ansible_user_dir }}/.crc/bin:{{ ansible_user_dir }}/.crc/bin/oc:{{ ansible_user_dir }}/bin:{{ ansible_env.PATH }}"
cifmw_openshift_kubeconfig: "{{ ansible_user_dir }}/.crc/machines/crc/kubeconfig"
Expand Down
44 changes: 44 additions & 0 deletions roles/ci_multus/molecule/default/nads_output.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
Copy link
Author

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.

apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
labels:
osp/net: default
name: default
namespace: openstack
spec:
config: |
{
"cniVersion": "0.3.1",
"name": "default",
"type": "bridge",
"bridge": "eth0",
"ipam": {
"type": "whereabouts",
"range": "192.168.122.0/24",
"range_start": "192.168.122.30",
"range_end": "192.168.122.70"
}
}
---
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
labels:
osp/net: patchnetwork
name: patchnetwork
namespace: openstack
spec:
config: |
{
"cniVersion": "0.3.1",
"name": "patchnetwork",
"type": "macvlan",
"master": "eth2",
"ipam": {
"type": "whereabouts",
"range": "192.168.122.0/24",
"range_start": "192.168.122.30",
"range_end": "192.168.122.70"
}
}
14 changes: 14 additions & 0 deletions roles/ci_multus/molecule/default/verify_crc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,20 @@
(_ci_multus_molecule_nads_out is failed) or
(_ci_multus_molecule_nads_out.resources | length == 0)

- name: Store output spec
ansible.builtin.set_fact:
_ci_multus_out_spec: >-
{{
_ci_multus_molecule_nads_out.resources |
map(attribute='spec.config')
}}

- name: Assert expected Network Attachment Definitions content spec with the expected
vars:
Copy link
Contributor

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, done!

_ci_multus_expected_spec: "{{ lookup('file', 'nads_output.yml', rstrip=True) | from_yaml_all | map(attribute='spec.config') }}"
ansible.builtin.assert:
that: _ci_multus_out_spec == _ci_multus_expected_spec

- name: Create a test pod to attach a network
kubernetes.core.k8s:
kubeconfig: "{{ cifmw_openshift_kubeconfig }}"
Expand Down
5 changes: 4 additions & 1 deletion roles/ci_multus/molecule/resources/vars/shared_vars.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ cifmw_networking_env_definition:
name: crc
networks:
default:
interface_name: "eth1"
interface_name: "eth0"
network_name: default
networks:
default:
Expand All @@ -32,6 +32,7 @@ cifmw_networking_env_definition:
ipv4_ranges:
- start: 192.168.122.30
end: 192.168.122.70
multus_type: bridge
deny_network:
gw_v4: 192.168.122.1
network_name: deny_network
Expand All @@ -41,6 +42,7 @@ cifmw_networking_env_definition:
ipv4_ranges:
- start: 192.168.122.30
end: 192.168.122.70
multus_type: bridge
not_allowed_network:
gw_v4: 192.168.122.1
network_name: not_allowed_network
Expand All @@ -50,6 +52,7 @@ cifmw_networking_env_definition:
ipv4_ranges:
- start: 192.168.122.30
end: 192.168.122.70
multus_type: bridge
no_multus_network:
gw_v4: 192.168.122.1
network_name: patchnetwork
Expand Down
12 changes: 8 additions & 4 deletions roles/ci_multus/templates/nad.yml.j2
Original file line number Diff line number Diff line change
@@ -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) %}
Copy link
Author

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.

{% set multus_type = network_details.tools.multus.multus_type %}
{% else %}
{% set multus_type = cifmw_ci_multus_default_nad_type %}
{% endif %}
---
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
Expand All @@ -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 }}",
Copy link
Contributor

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) ?

Copy link
Author

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.

{% if multus_type == "macvlan" %}
"master": "{{ network_details.interface_name }}",
{% endif %}
{% if cifmw_ci_multus_default_nad_type == "bridge" %}
"type": "bridge",
{% if multus_type == "bridge" %}
"bridge": "{{ network_details.interface_name }}",
{% endif %}
"ipam": {
Expand Down
1 change: 1 addition & 0 deletions roles/networking_mapper/molecule/default/converge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,4 @@
ansible.builtin.assert:
that:
- "_content.networks['internalapi'].vlan_id == 100"
- "_content.networks['internalapi'].tools.multus.multus_type == 'bridge'"
Copy link
Author

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.

1 change: 1 addition & 0 deletions roles/networking_mapper/molecule/default/vars/input.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ networks:
ranges:
- start: 50
end: 59
type: "bridge"
storage:
network: "172.18.0.0/24"
vlan: 21
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
],
"ipv6_ranges": [],
"ipv4_routes": [],
"ipv6_routes": []
"ipv6_routes": [],
"multus_type": "macvlan"
},
"netconfig": {
"ipv4_ranges": [
Expand Down Expand Up @@ -88,7 +89,8 @@
"gateway": "192.168.122.1"
}
],
"ipv6_routes": []
"ipv6_routes": [],
"multus_type": "bridge"
},
"netconfig": {
"ipv4_ranges": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
],
"ipv6_ranges": [],
"ipv4_routes": [],
"ipv6_routes": []
"ipv6_routes": [],
"multus_type": "macvlan"
},
"netconfig": {
"ipv4_ranges": [
Expand Down Expand Up @@ -87,7 +88,8 @@
"gateway": "192.168.122.1"
}
],
"ipv6_routes": []
"ipv6_routes": [],
"multus_type": "bridge"
},
"netconfig": {
"ipv4_ranges": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
],
"ipv6_ranges": [],
"ipv4_routes": [],
"ipv6_routes": []
"ipv6_routes": [],
"multus_type": "macvlan"
},
"netconfig": {
"ipv4_ranges": [
Expand Down Expand Up @@ -88,7 +89,8 @@
"gateway": "192.168.122.1"
}
],
"ipv6_routes": []
"ipv6_routes": [],
"multus_type": "bridge"
},
"netconfig": {
"ipv4_ranges": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ networks:
mtu: 1500
tools:
multus:
type: macvlan
ranges:
- start: 30
end: 39
Expand All @@ -26,6 +27,7 @@ networks:
ranges:
- start: 30
end: 39
type: bridge
routes:
- destination: "192.168.121.0/24"
gateway: "192.168.122.1"
Expand Down