Mirrors/network
1
0
Fork 0
mirror of https://github.com/linux-system-roles/network.git synced 2026-07-28 13:33:57 +00:00
Code Issues Projects Releases Wiki Activity

Separate 'persistent_state' from 'state'

Browse source 
- persistent_state represents whether a profile is stored on disk
- persistent_state defaults to 'present'
- When there is no type specified for the profile, it is enough for a
profile with the same name to be stored on the target's systems file
system. Otherwise the role will fail
- state now represents the runtime state and can be up, down or
unspecified
- translate the state definitions into actions that will be performed.
The actions correspond to the previous states.
- add the possibility to write unit tests to only verify parts of the
resulting connection dictionary to only check for the expected changes
instead of the full connection that can also contain unrelated defaults
This commit is contained in:
Till Maas 2018-07-31 18:01:15 +02:00
parent 987c8d05bb
commit 23605615da
9 changed files with 739 additions and 369 deletions
Download patch file Download diff file Expand all files Collapse all files

136
README.md
View file

@ -68,14 +68,33 @@ for NetworkManager, a connection can only be active at one device at a time.
Note that here too the name doesn't specify the `DEVICE` but a filename. As a consequence
`'/'` is not a valid character for the name.
### `state`
### `state` and `persistent_state`
Each connection profile can have a runtime state, represented by the `state`
setting and a persistent state, represtented by the `persistent_state` setting.
The optional `state` setting supports the following values:
- `up`
- `down`
It defines whether the profile is activated (`up`) or deactivated (`down`). If
it is unset, the profile's runtime state will not be changed.
The `persistent_state` setting is either `present` (default) or `absent`. If
the `persistent_state` setting is `present` and the connection profile contains
a `type` setting, the profile will be created or updated. If the profile is
incomplete (lacks the `type` setting) and `persistent_state` is `present`,
the behavior is undefined. The value `absent` makes the role ensure
that the profile is not present on the target host.
#### Example
```yaml
network_connections:
- name: "eth0"
state: "absent"
- name: eth0
persistent_state: absent
```
Above example ensures the absence of a connection profile. If a profile with `name` `eth0`
@ -90,45 +109,36 @@ exists, it will be deleted.
* For initscripts it results in the deletion of the ifcfg file. Usually that
has no side-effect, unless some component is watching the sysconfig directory.
We already saw that state `absent` before. There are more states:
- `absent`
- `present`
- `up`
- `down`
If the `state` variable is omitted, the default is `up` -- unless a `type` is specified,
in which case the default is `present`.
#### Example
```yaml
network_connections:
- name: "eth0"
#state: present # default, as a type is present
type: "ethernet"
- name: eth0
#persistent_state: present # default
type: ethernet
autoconnect: yes
mac: "00:00:5e:00:53:5d"
mac: 00:00:5e:00:53:5d
ip:
dhcp4: yes
```
Above example creates a new connection profile or ensures that it is present
with the given configuration. It has implicitly `state` `present`, due to the
presence of `type`. On the other hand, the `present` state requires at least a `type`
variable.
with the given configuration. It implies the `persistent_state` setting to be
`present`.
Valid values for `type` are:
- `bond`
- `bridge`
- `ethernet`
- `infiniband`
- `bridge`
- `bond`
- `macvlan`
- `team`
- `vlan`
`state` `present` does not directly result in a change in the network configuration.
That is, the profile is only created or modified, not activated.
The value `present` for the `persistent_state` setting does not directly
result in a change in the network configuration. That is, without `state`
set to `up`, the profile is only created or modified, not activated.
- For NetworkManager, note the new connection profile is created with
`autoconnect` turned on by default. Thus, NetworkManager may very well decide
@ -156,11 +166,11 @@ profile.
### `interface_name`
For type `ethernet` and `infiniband`, this option restricts the profile to the
given interface by name. This argument is optional and by default the profile
name is used unless a mac address is specified using the `mac` key. Specifying
an empty string (`""`) allows to specify that the profile is not restricted to
a network interface.
For the types `ethernet` and `infiniband`, this option restricts the profile to
the given interface by name. This argument is optional and by default the
profile name is used unless a mac address is specified using the `mac` key.
Specifying an empty string (`""`) allows to specify that the profile is not
restricted to a network interface.
**Note:** With [persistent interface naming](https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/ch-Consistent_Network_Device_Naming.html),
@ -185,30 +195,27 @@ Slaves to bridge/bond/team devices cannot specify a zone.
```yaml
network_connections:
- name: "eth0"
#state: up # implicit default, as there is no type specified
wait: 0
- name: eth0
state: up
```
The above example defaults to `state=up` and requires an existing profile to activate.
Note that if neither `type` nor `state` is specifed, `up` is implied. Thus in above
example the `state` is redundant.
The above example requires an existing profile to activate.
- For NetworkManager this results in `nmcli connection id {{name}} up`.
- For initscripts it is the same as `ifup {{name}}`.
`up` also supports an optional integer argument `wait`. `wait=0` will only initiate
the activation but not wait until the device is fully connected. Connection will complete
in the background, for example after a DHCP lease was received.
`wait: <SECONDS>` is a timeout for how long we give the device
to activate. The default is `wait=-1` which uses a suitable timeout. Note that this
argument only makes sense for NetworkManager.
State `up` also supports an optional integer argument `wait`. `wait: 0` will
only initiate the activation but not wait until the device is fully connected.
Connection will complete in the background, for example after a DHCP lease was
received. `wait: <SECONDS>` is a timeout for how long we give the device to
activate. The default is `wait: -1` which uses a suitable timeout. Note that
this argument only makes sense for NetworkManager.
**TODO** `wait` different from zero is not yet implemented.
Note that `up` always re-activates the profile and possibly changes the networking
configuration, even if the profile was already active before. As such, it always
changes the system.
Note that state `up` always re-activates the profile and possibly changes the
networking configuration, even if the profile was already active before. As
such, it always changes the system.
### `state: down`
@ -218,7 +225,6 @@ changes the system.
network_connections:
- name: eth0
state: down
wait: 0
```
Another `state` is `down`.
@ -228,8 +234,8 @@ Another `state` is `down`.
- For initscripts this means to call `ifdown {{name}}`.
This is the opposite of the `up` state. It also will always issue the command
to deactivate the profile, even it if seemingly is currently not active. As such,
`down` always changes the system.
to deactivate the profile, even it if seemingly is currently not active. As
such, `down` always changes the system.
For NetworkManager, a `wait` argument is supported like for `up` state.
@ -239,18 +245,19 @@ For NetworkManager, a `wait` argument is supported like for `up` state.
```yaml
network_connections:
- name: "Wired0"
type: "ethernet"
interface_name: "eth0"
- name: Wired0
type: ethernet
interface_name: eth0
ip:
dhcp4: yes
- name: "Wired0"
- name: Wired0
state: up
```
As said, the `name` identifies a unique profile. However, you can refer to the same
profile multiple times. Thus above example makes perfectly sense to create a profile and
activate it within the same play.
As said, the `name` identifies a unique profile. However, you can refer to the
same profile multiple times. Therefore it is possible to create a profile and
activate it separately.
### `ip`
@ -258,8 +265,8 @@ The IP configuration supports the following options:
```yaml
network_connections:
- name: "eth0"
type: "ethernet"
- name: eth0
type: ethernet
ip:
route_metric4: 100
dhcp4: no
@ -341,8 +348,8 @@ integer giving the speed in Mb/s, the valid values of `duplex` are `half` and `f
```yaml
network_connections:
- name: "eth0"
type: "ethernet"
- name: eth0
type: ethernet
ethernet:
autoneg: no
@ -356,9 +363,9 @@ Device types like `bridge`, `bond`, `team` work similar:
```yaml
network_connections:
- name: "br0"
- name: br0
type: bridge
#interface_name: br0 # defaults to the connection name
#interface_name: br0 # defaults to the connection name
```
Note that `team` is not supported on RHEL6 kernels.
@ -368,7 +375,8 @@ For slaves of these virtual types, the special properites `slave_type` and
```yaml
network_connections:
- name: br0
- name: internal-br0
interface_name: br0
type: bridge
ip:
dhcp4: no
@ -377,7 +385,7 @@ network_connections:
- name: br0-bond0
type: bond
interface_name: bond0
master: br0
master: internal-br0
slave_type: bridge
- name: br0-bond0-eth1
@ -473,7 +481,7 @@ operating system. This is usually `nm` except for RHEL 6 or CentOS 6 systems.
```yaml
network_provider: nm
network_connections:
- name: "eth0"
- name: eth0
#...
```
@ -511,7 +519,7 @@ so that the host is connected to a management LAN or VLAN. It strongly depends o
- It seems difficult to change networking of the target host in a way that breaks the current
SSH connection of ansible. If you want to do that, ansible-pull might be a solution.
Alternatively, a combination of `async`/`poll` with changing the `ansible_host` midway
of the play.
of the play.
**TODO** The current role doesn't yet support to easily split the
play in a pre-configure step, and a second step to activate the new configuration.

1
examples/eth-with-vlan.yml
View file

@ -8,6 +8,7 @@
- name: prod2
type: ethernet
autoconnect: no
state: up
interface_name: "{{ network_interface_name2 }}"
ip:
dhcp4: no

1
examples/infiniband.yml
View file

@ -15,6 +15,7 @@
autoconnect: yes
infiniband_p_key: 10
parent: ib0
state: up
ip:
dhcp4: no
auto6: no

2
examples/macvlan.yml
View file

@ -6,6 +6,7 @@
- name: eth0
type: ethernet
state: up
interface_name: eth0
ip:
address:
@ -14,6 +15,7 @@
# Create a virtual ethernet card bound to eth0
- name: veth0
type: macvlan
state: up
parent: eth0
macvlan:
mode: bridge

103
library/network_connections.py
View file

@ -546,6 +546,9 @@ class IfcfgUtil:
@classmethod
def content_from_file(cls, name, file_type=None):
"""
Return dictionary with all file contents for an initscripts profile
"""
content = {}
for file_type in cls._file_types(file_type):
path = cls.ifcfg_path(name, file_type)
@ -1368,9 +1371,12 @@ class RunEnvironmentAnsible(RunEnvironment):
prefix = "#"
else:
c = connections[idx]
prefix = "#%s, state:%s" % (idx, c["state"])
if c["state"] != "wait":
prefix = prefix + (", '%s'" % (c["name"]))
prefix = "#%s, state:%s persistent_state:%s" % (
idx,
c["state"],
c["persistent_state"],
)
prefix = prefix + (", '%s'" % (c["name"]))
for r in rr["log"]:
yield (r[2], "[%03d] %s %s: %s" % (r[2], LogLevel.fmt(r[0]), prefix, r[1]))
@ -1565,13 +1571,14 @@ class Cmd:
continue
c_state = c["state"]
c_pstate = c["persistent_state"]
if c_state == "up" and "type" not in c:
pass
elif c_state == "down":
return True
elif c_state == "absent":
elif c_pstate == "absent":
return True
elif c_state in ["present", "up"]:
elif c_state == "up" or c_pstate == "present":
if self.connections_data[idx]["changed"]:
return True
@ -1611,28 +1618,17 @@ class Cmd:
while self.check_mode_next() != CheckMode.DONE:
for idx, connection in enumerate(self.connections):
try:
state = connection["state"]
if state == "wait":
w = connection["wait"]
if w is None:
w = 10
self.log_info(idx, "wait for %s seconds" % (w))
if self.check_mode == CheckMode.REAL_RUN:
import time
time.sleep(w)
elif state == "absent":
self.run_state_absent(idx)
elif state == "present":
self.run_state_present(idx)
elif state == "up":
if "type" in connection:
self.run_state_present(idx)
self.run_state_up(idx)
elif state == "down":
self.run_state_down(idx)
else:
assert False
for action in connection["actions"]:
if action == "absent":
self.run_action_absent(idx)
elif action == "present":
self.run_action_present(idx)
elif action == "up":
self.run_action_up(idx)
elif action == "down":
self.run_action_down(idx)
else:
assert False
except Exception as e:
self.log_warn(
idx, "failure: %s [[%s]]" % (e, traceback.format_exc())
@ -1687,16 +1683,16 @@ class Cmd:
% (connection["interface_name"], connection["mac"]),
)
def run_state_absent(self, idx):
def run_action_absent(self, idx):
raise NotImplementedError()
def run_state_present(self, idx):
def run_action_present(self, idx):
raise NotImplementedError()
def run_state_down(self, idx):
def run_action_down(self, idx):
raise NotImplementedError()
def run_state_up(self, idx):
def run_action_up(self, idx):
raise NotImplementedError()
@ -1723,11 +1719,9 @@ class Cmd_nm(Cmd):
Cmd.run_prepare(self)
names = {}
for connection in self.connections:
if connection["state"] not in ["up", "down", "present", "absent"]:
continue
name = connection["name"]
if not name:
assert connection["state"] == "absent"
assert connection["persistent_state"] == "absent"
continue
if name in names:
exists = names[name]["nm.exists"]
@ -1744,7 +1738,7 @@ class Cmd_nm(Cmd):
connection["nm.exists"] = exists
connection["nm.uuid"] = uuid
def run_state_absent(self, idx):
def run_action_absent(self, idx):
seen = set()
name = self.connections[idx]["name"]
black_list_names = None
@ -1769,13 +1763,23 @@ class Cmd_nm(Cmd):
if not seen:
self.log_info(idx, "no connection '%s'" % (name))
def run_state_present(self, idx):
def run_action_present(self, idx):
connection = self.connections[idx]
con_cur = Util.first(
self.nmutil.connection_list(
name=connection["name"], uuid=connection["nm.uuid"]
)
)
if not connection.get("type"):
# if the type is not specified, just check that the connection was
# found
if not con_cur:
self.log_error(
idx, "Connection not found on system and 'type' not present"
)
return
con_new = self.nmutil.connection_create(self.connections, idx, con_cur)
if con_cur is None:
self.log_info(
@ -1829,7 +1833,7 @@ class Cmd_nm(Cmd):
self.log_error(idx, "delete duplicate connection failed: %s" % (e))
seen.add(c)
def run_state_up(self, idx):
def run_action_up(self, idx):
connection = self.connections[idx]
con = Util.first(
@ -1893,7 +1897,7 @@ class Cmd_nm(Cmd):
except MyError as e:
self.log_error(idx, "up connection failed while waiting: %s" % (e))
def run_state_down(self, idx):
def run_action_down(self, idx):
connection = self.connections[idx]
cons = self.nmutil.connection_list(name=connection["name"])
@ -1970,7 +1974,7 @@ class Cmd_initscripts(Cmd):
return None
return f
def run_state_absent(self, idx):
def run_action_absent(self, idx):
n = self.connections[idx]["name"]
name = n
if not name:
@ -2013,7 +2017,7 @@ class Cmd_initscripts(Cmd):
% ("'" + n + "'" if n else "*"),
)
def run_state_present(self, idx):
def run_action_present(self, idx):
if not self.check_name(idx):
return
@ -2022,6 +2026,15 @@ class Cmd_initscripts(Cmd):
old_content = IfcfgUtil.content_from_file(name)
if not connection.get("type"):
# if the type is not specified, just check that the connection was
# found
if not old_content.get("ifcfg"):
self.log_error(
idx, "Connection not found on system and 'type' not present"
)
return
ifcfg_all = IfcfgUtil.ifcfg_create(
self.connections, idx, lambda msg: self.log_warn(idx, msg), old_content
)
@ -2047,7 +2060,7 @@ class Cmd_initscripts(Cmd):
idx, "%s ifcfg-rh profile '%s' failed: %s" % (op, name, e)
)
def _run_state_updown(self, idx, do_up):
def _run_action_updown(self, idx, do_up):
if not self.check_name(idx):
return
@ -2119,11 +2132,11 @@ class Cmd_initscripts(Cmd):
idx, "call '%s %s' failed with exit status %d" % (cmd, name, rc)
)
def run_state_up(self, idx):
self._run_state_updown(idx, True)
def run_action_up(self, idx):
self._run_action_updown(idx, True)
def run_state_down(self, idx):
self._run_state_updown(idx, False)
def run_action_down(self, idx):
self._run_action_updown(idx, False)
###############################################################################

166
module_utils/network_lsr/argument_validator.py
View file

@ -21,10 +21,9 @@ class ArgUtil:
if "name" not in connection or name != connection["name"]:
continue
if connection["state"] == "absent":
if connection["persistent_state"] == "absent":
c = None
elif "type" in connection:
assert connection["state"] in ["up", "present"]
elif connection["persistent_state"] == "present":
c = connection
return c
@ -611,7 +610,8 @@ class ArgValidator_DictMacvlan(ArgValidatorDict):
class ArgValidator_DictConnection(ArgValidatorDict):
VALID_STATES = ["up", "down", "present", "absent", "wait"]
VALID_STATES = ["up", "down"]
VALID_PERSISTENT_STATES = ["absent", "present"]
VALID_TYPES = [
"ethernet",
"infiniband",
@ -632,8 +632,18 @@ class ArgValidator_DictConnection(ArgValidatorDict):
ArgValidatorStr(
"state", enum_values=ArgValidator_DictConnection.VALID_STATES
),
ArgValidatorStr(
"persistent_state",
enum_values=ArgValidator_DictConnection.VALID_PERSISTENT_STATES,
),
ArgValidatorBool("force_state_change", default_value=None),
ArgValidatorNum("wait", val_min=0, val_max=3600, numeric_type=float),
ArgValidatorNum(
"wait",
val_min=0,
val_max=3600,
numeric_type=float,
default_value=None,
),
ArgValidatorStr(
"type", enum_values=ArgValidator_DictConnection.VALID_TYPES
),
@ -681,70 +691,112 @@ class ArgValidator_DictConnection(ArgValidatorDict):
all_missing_during_validate=True,
)
def _validate_post(self, value, name, result):
if "state" not in result:
if "type" in result:
result["state"] = "present"
elif list(result.keys()) == ["wait"]:
result["state"] = "wait"
else:
result["state"] = "up"
# valid field based on specified state, used to set defaults and reject
# bad values
self.VALID_FIELDS = []
if result["state"] == "present" or (
result["state"] == "up" and "type" in result
):
VALID_FIELDS = list(self.nested.keys())
if result["state"] == "present":
VALID_FIELDS.remove("wait")
VALID_FIELDS.remove("force_state_change")
elif result["state"] in ["up", "down"]:
VALID_FIELDS = [
"name",
"state",
"wait",
"ignore_errors",
"force_state_change",
]
elif result["state"] == "absent":
VALID_FIELDS = ["name", "state", "ignore_errors"]
elif result["state"] == "wait":
VALID_FIELDS = ["state", "wait"]
def _validate_post_state(self, value, name, result):
"""
Validate state definitions and create a corresponding list of actions.
"""
actions = []
state = result.get("state")
persistent_state = result.get("persistent_state")
# default persistent_state to present (not done via default_value in the
# ArgValidatorStr, the value will only be set at the end of
# _validate_post()
if not persistent_state:
persistent_state = "present"
# If the profile is present, it should be ensured first
if persistent_state == "present":
actions.append(persistent_state)
# If the profile should be absent at the end, it needs to be present in
# the meantime to allow to (de)activate it
if persistent_state == "absent" and state:
actions.append("present")
# Change the runtime state if necessary
if state:
actions.append(state)
# Remove the profile in the end if requested
if persistent_state == "absent":
actions.append(persistent_state)
result["state"] = state
result["persistent_state"] = persistent_state
result["actions"] = actions
return result
def _validate_post_fields(self, value, name, result):
"""
Validate the allowed fields (settings depending on the requested state).
FIXME: Maybe it should check whether "up"/"down" is present in the
actions instead of checking the runtime state from "state" to switch
from state to actions after the state parsing is done.
"""
state = result.get("state")
persistent_state = result.get("persistent_state")
# minimal settings not related to runtime changes
valid_fields = ["actions", "ignore_errors", "name", "persistent_state", "state"]
# when type is present, a profile is completely specified (using
# defaults or other settings)
if "type" in result:
valid_fields += list(self.nested.keys())
# If there are no runtime changes, "wait" and "force_state_change" do
# not make sense
# FIXME: Maybe this restriction can be removed. Need to make sure that
# defaults for wait or force_state_change do not interfer
if not state:
while "wait" in valid_fields:
valid_fields.remove("wait")
while "force_state_change" in valid_fields:
valid_fields.remove("force_state_change")
else:
assert False
valid_fields += ["force_state_change", "wait"]
VALID_FIELDS = set(VALID_FIELDS)
# FIXME: Maybe just accept all values, even if they are not
# needed/meaningful in the respective context
valid_fields = set(valid_fields)
for k in result:
if k not in VALID_FIELDS:
if k not in valid_fields:
raise ValidationError(
name + "." + k,
"property is not allowed for state '%s'" % (result["state"]),
"property is not allowed for state '%s' and persistent_state '%s'"
% (state, persistent_state),
)
if result["state"] != "wait":
if result["state"] == "absent":
if "name" not in result:
result[
"name"
] = "" # set to empty string to mean *absent all others*
if "name" not in result:
if persistent_state == "absent":
result["name"] = "" # set to empty string to mean *absent all others*
else:
if "name" not in result:
raise ValidationError(name, "missing 'name'")
raise ValidationError(name, "missing 'name'")
if result["state"] in ["wait", "up", "down"]:
if "wait" not in result:
result["wait"] = None
else:
if "wait" in result:
raise ValidationError(
name + ".wait",
"'wait' is not allowed for state '%s'" % (result["state"]),
)
if result["state"] == "present" and "type" not in result:
# FIXME: Seems to be a duplicate check since "wait" will be removed from
# valid_keys when state is considered to be not True
if "wait" in result and not state:
raise ValidationError(
name + ".state", '"present" state requires a "type" argument'
name + ".wait",
"'wait' is not allowed for state '%s'" % (result["state"]),
)
result["state"] = state
result["persistent_state"] = persistent_state
self.VALID_FIELDS = valid_fields
return result
def _validate_post(self, value, name, result):
result = self._validate_post_state(value, name, result)
result = self._validate_post_fields(value, name, result)
if "type" in result:
if "master" in result:
@ -959,7 +1011,7 @@ class ArgValidator_DictConnection(ArgValidatorDict):
% (result["type"]),
)
for k in VALID_FIELDS:
for k in self.VALID_FIELDS:
if k in result:
continue
v = self.nested[k]

2
tests/remove-profile.yml
View file

@ -5,6 +5,6 @@
vars:
network_connections:
- name: "{{ profile }}"
state: absent
persistent_state: absent
roles:
- linux-system-roles.network

690
tests/test_network_connections.py
View file

File diff suppressed because it is too large Load diff

7
tests/tests_unit.yml
View file

@ -3,6 +3,12 @@
- hosts: all
name: Setup for test running
tasks:
- name: Install EPEL on enterprise Linux for python2-mock
command: yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-{{ ansible_distribution_major_version }}.noarch.rpm
when:
- ansible_distribution in ['RedHat', 'CentOS']
- ansible_distribution_major_version in ['6', '7']
# FIXME: change with_items to loop when test-harness is updated
- package:
name: "{{ item }}"
@ -13,6 +19,7 @@
- python2-gobject-base
- python3-gobject-base
- python-gobject-base
- python2-mock
- hosts: all
name: execute python unit tests