The states "up" and "down" previously would always change state. That
is, specifying them in the playbook will always invoke `ifup` (or
`ifdown`) or the corresponding of `nmcli connection up` (or `nmcli
connection down`).
That was intentional behavior, because the role doesn't really know which
profile is currently active. That is certainly the case for "initscripts",
where the role has almost no information about the current runtime
state. For "nm" provider, the role knows whether the connection is
already active. However, that alone also does not guarantee that the
current runtime state is idential to what would be the result of an
explicit `nmcli connection up`.
Hence, to be sure that the current state is always as expected, the role
would always explicitly issue the commands and report "changed=1".
That is quite harmful, because running the same role multiple times
should not report changes every time. Also, issuing `ifup` may behave
badly, if the interface is already configured.
Now, try to determine whether the desire "up" or "down" state is already
reached and do nothing.
For "nm" provider that is easy and quite safe. There is still the
possibility to trick the role into thinking that the right configuration
is active, when it actually is not. For example via `nmcli device
modify` on the host. But in general, it should work just fine.
Especially, if the admin manually modifies the runtime state, it may be
just desired for "state: up" not to change anything.
For "initscripts" this is much more fragile. There isn't really much
that can be done about it, because the role doesn't know what is currently
configured on the system.
There is also a new option "force_state_change" to restore the previous
behavior.
https://bugzilla.redhat.com/show_bug.cgi?id=1476053
The role targets NetworkManager 1.8.0 API or newer. Exposing the
active connection state was added in 1.8 API, hence the role failed
against older NetworkManager version.
While that is not officially supported, the workaround is simple,
so do it.
https://bugzilla.redhat.com/show_bug.cgi?id=1476053
We delete the connection asynchrnonously using libnm. Even after the
asynchronous request completes, the connection may still in the cache.
That is a bit odd, but maybe not a bug in libnm. Because it can happen
that the connection was active, so it takes time to bring it down.
Maybe the asynchronous request should not complete in libnm before
the connection is truly gone. But note that deactivating a connection
can take arbitrary long, so it's not clear that his would be the best
behavior.
Anyway, work around it by waiting.
The effect of this bug is if you have "state: absent" followed by
"state:up", then the intermediate "present" state will be skipped.
That is, because it appears that the connection still exists, although
it's about to be deleted. The subsequent "up" will then fail as the
connection is gone in the meantime.
https://github.com/linux-system-roles/network/issues/7https://bugzilla.redhat.com/show_bug.cgi?id=1478910
We group logging messages by their connection (@idx). Finally,
when constructing the overall result, reorder the messages
to ensure that their are still in chronological order.
Usually they are anyway, because we handle connections one-by-one,
and wouldn't log messages for previous connections.
We hack the "warnings" JSON result to return logging messages. However,
ansible will suppress duplicate warnings. That is inconvenient. Avoid that
by prefixing each logging message with an index.
This way one will be able to reliably detect when the example playbook was
forgotten to be upated with correct addresses, as the RFC 5737 addresses are
never used in production (the original RFC 1918 ones often are).
https://github.com/linux-system-roles/network/issues/5
We don't care whether a 'ip' or 'bond' setting is
entirely omitted or whether some fields are set by
the user.
Omitting a setting should make use of some default
values, but that's it. No need to track whether the
setting is actually specified by the user.
As it is, a slave profile MUST reference an existing "master"
profile. Hence, the "slave_type" is implicitly known and not
required.
However, don't drop the "slave_type" option entirely. It allows a user
to explicitly set it, and network_connections.py will reject mismatching
configurations.
Also, check that the 'master' profile is actually of a valid master
type for the slave.
- the maximum allowed vlan_id is 4094, not 4095.
- for ifcfg, we must convert the value to str()
- for nm, we don't need to convert the value, it's
already int().
Implement what nmcli does when activating/deactivating
a connection: waits until the device is fully activated
or deactivate.
The 'wait' option is basically like nmcli's --wait option.
If omitted, wait a default duration of time. If the action
does not complete within the specified timeout, it fails.
Note that a failure may be suppressed by setting 'ignore_errors',
either globaly or per-state.
Set 'wait=0' not to wait. This was the previous behavior before
this commit.
- extend the maxiumum wait time to 3600 seconds
- disallow -1 values for 'wait'. It was used to mark
the default value, but that marker should not be
a valid value for user configuration. Instead, internally
mark the missing value with None.
- don't coerce a missing 'wait' parameter to 90 in _validate_post().
Instead, track it as None so we can later decide between an explicitly
set paramter and a default value. Also, the default for the states
'up', 'down', and 'wait' differ.
This also allows us to slience the warning about 'wait' not
being implemented -- in case the user didn't configure it.
- However, still allow zero.
For
- state: wait
wait: 0
allow it for convinience, so a user can disable the statement by
setting the wait value to zero.
For
- name: MyProfile
state: up
wait: 0
'wait' zero makes sense. It is what we currently do -- not wait
at all. Later, when we implement waiting for complete activation, the
default of wait will change (to 90 in this case).
The role already supported a default variable ("network_provider") and
host variables ("network_provider_default", "network_service_name",
"network_packages").
Don't use nested variables under "network" like
network:
provider:
ignore_error:
connections:
instead promote them all to top-level variables like:
network_provider:
network_ignore_error:
network_connections:
This seems more consistent (as we already have multiple top-level
variables), it seems to follow ansible style, and it makes it easier
to overload individual variables via conditional include files.
The library fetches interface info by reading sysfs. However, "/sys/class/net/"
may contain certain files that are not interface names, like "bonding_master"
Skip over these files. Otherwise, the code fails with
File "/tmp/ansible_HkmKOM/ansible_module_network_connections.py", line 347,
in link_infos b = SysUtil._link_infos_fetch()
File "/tmp/ansible_HkmKOM/ansible_module_network_connections.py", line 323,
in _link_infos_fetch ifindex = SysUtil._link_read_ifindex(ifname)
File "/tmp/ansible_HkmKOM/ansible_module_network_connections.py", line 302,
in _link_read_ifindex c = SysUtil._sysctl_read('/sys/class/net/' + ifname + '/ifindex')
File "/tmp/ansible_HkmKOM/ansible_module_network_connections.py", line 293,
in _sysctl_read with open(filename, 'r') as f: IOError: [Errno 20] Not a directory: '/sys/class/net/bonding_masters/ifindex'
This requires to make "library/network_connections.py" more
independent from ansible modules
In the end, the Python code runs on the target machines,
with the python installed there. So, it makes sense to also
run the unit tests as part of the CI tests.
The unit tests should run without requiring the
ansible module.
At least, all currently existing tests can now run.
In the future, we might have tests that optionally
only run when the ansible libraries are available.
As the module runs, it constructs debugging information
in run_result array. That is, because we can only pass
on the debugging messages at the end during json_exit()
or json_fail().
During early stage of run_prepare() we must also ensure
no to loose messages.
