Global network settings always report changes when test=True

[jleroy]

Description of Issue

When a state containing network.system is applied in test mode, Salt always return changes to apply even if the current configuration is up-to-date.

Setup

system:
  network.system:
    - enabled: True
    - hostname: myhostname
    - apply_hostname: True

Steps to Reproduce Issue

Execute the state file bellow in test mode:

$ salt 'myhostname' state.highstate test=True

jid: 20200312054102028991
myhostname:
----------
          ID: system
    Function: network.system
      Result: None
     Comment: Global network settings are set to be updated:
              ---
              +++
              @@ -1,2 +1,11 @@
              -NETWORKING=yes

              -HOSTNAME=myhostname

              +# Configuration for networking init script being run during

              +# the boot sequence

              +

              +# Set to 'no' to skip interfaces configuration on boot

              +CONFIGURE_INTERFACES=yes

              +

              +# Don't configure these interfaces. Shell wildcards supported/

              +#EXCLUDE_INTERFACES=

              +

              +# Set to 'yes' to enable additional verbosity

              +#VERBOSE=no
     Started: 05:41:04.898364
    Duration: 2.225 ms
     Changes:

Summary for myhostname
-------------
Succeeded: 56 (unchanged=1)
Failed:     0
-------------
Total states run:     56
Total run time:  474.120 ms

Salt return changes to be made.

Then run the same state without test mode enabled:

$ salt 'myhostname' state.highstate

jid: 20200312055319027869
myhostname:

Summary for myhostname
-------------
Succeeded: 56
Failed:     0
-------------
Total states run:     56
Total run time:  808.170 ms

Versions Report

Salt Version:
           Salt: 2019.2.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: 2.0.5
      gitpython: 2.1.11
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.7.3 (default, Dec 20 2019, 18:57:59)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.1.2
           RAET: Not Installed
          smmap: 2.0.5
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1

System Versions:
           dist: debian 10.3
         locale: UTF-8
        machine: x86_64
        release: 4.19.0-8-cloud-amd64
         system: Linux
        version: debian 10.3

[Ch3LL]

im having a hard time replicating this. can you paste a sanitized version of what is old and new here?

diff --git a/salt/states/network.py b/salt/states/network.py
index 3d8cc46a1c..70e7790f6e 100644
--- a/salt/states/network.py
+++ b/salt/states/network.py
@@ -393,6 +393,8 @@ def managed(name, type, enabled=True, **kwargs):
     try:
         old = __salt__['ip.get_interface'](name)
         new = __salt__['ip.build_interface'](name, type, enabled, **kwargs)
+        log.critical(old)
+        log.critical(new)
         if kwargs['test']:
             if old == new:
                 pass

[max-arnold]

I'm not sure that this information is relevant, but there are tons of Debian network fixes that were merged to the old Neon branch but weren't backported: #56353

[jleroy]

@Ch3LL Here is it.

old:

['auto eno1\n',
'iface eno1 inet static\n', 
'    address 51.178.88.XXX\n', 
'    netmask 255.255.255.0\n', 
'    gateway 51.178.88.254\n', 
'    post-up /sbin/ip -f inet6 route add 2001:41d0:XXX:7bff:ff:ff:ff:ff dev eno1\n', 
'    post-up /sbin/ip -f inet6 route add default via 2001:41d0:XXX:7bff:ff:ff:ff:ff\n', 
'    pre-down /sbin/ip -f inet6 route del default via 2001:41d0:XXX:7bff:ff:ff:ff:ff\n', 
'    pre-down /sbin/ip -f inet6 route del 2001:41d0:XXX:7bff:ff:ff:ff:ff dev eno1\n', 
'iface eno1 inet6 static\n', 
'    address 2001:41d0:XXX:7be4::1\n', 
'    netmask 64\n', 
'\n']

new:

['auto eno1\n',
'iface eno1 inet static\n', 
'    address 51.178.88.XXX\n', 
'    netmask 255.255.255.0\n', 
'    gateway 51.178.88.254\n', 
'    post-up /sbin/ip -f inet6 route add 2001:41d0:XXX:7bff:ff:ff:ff:ff dev eno1\n', 
'    post-up /sbin/ip -f inet6 route add default via 2001:41d0:XXX:7bff:ff:ff:ff:ff\n', 
'    pre-down /sbin/ip -f inet6 route del default via 2001:41d0:XXX:7bff:ff:ff:ff:ff\n', 
'    pre-down /sbin/ip -f inet6 route del 2001:41d0:XXX:7bff:ff:ff:ff:ff dev eno1\n', 
'iface eno1 inet6 static\n', 
'    address 2001:41d0:XXX:7be4::1\n', 
'    netmask 64\n', 
'\n']

The value of old and new variables are strictly identical.

The issue seems to be in the build_network_settings function:

salt/salt/modules/debian_ip.py

Lines 2024 to 2029 in b4dd235

	network = template.render(opts)
	if 'test' in settings and settings['test']:
	return _read_temp(network)
	# Write settings
	_write_file_network(network, _DEB_NETWORKING_FILE, True)

BTW the content of /etc/default/networking (which is the value of _DEB_NETWORKING_FILE) do not match the output displayed by Salt:

# Configuration for networking init script being run during
# the boot sequence

# Set to 'no' to skip interfaces configuration on boot
CONFIGURE_INTERFACES=yes

# Don't configure these interfaces. Shell wildcards supported/
#EXCLUDE_INTERFACES=

# Set to 'yes' to enable additional verbosity
#VERBOSE=no

AFAIK there's no need to update this file (yes is the default value for CONFIGURE_INTERFACES), and the network.jinja template is out-of-date.
Here's the default content for this file on Debian 10 (Buster):

# Configuration for networking init script being run during
# the boot sequence

# Set to 'no' to skip interfaces configuration on boot
#CONFIGURE_INTERFACES=yes

# Don't configure these interfaces. Shell wildcards supported/
#EXCLUDE_INTERFACES=

# Set to 'yes' to enable additional verbosity
#VERBOSE=no

# Method to wait for the network to become online,
# for services that depend on a working network:
# - ifup: wait for ifup to have configured an interface.
# - route: wait for a route to a given address to appear.
# - ping/ping6: wait for a host to respond to ping packets.
# - none: don't wait.
#WAIT_ONLINE_METHOD=ifup

# Which interface to wait for.
# If none given, wait for all auto interfaces, or if there are none,
# wait for at least one hotplug interface.
#WAIT_ONLINE_IFACE=

# Which address to wait for for route, ping and ping6 methods.
# If none is given for route, it waits for a default gateway.
#WAIT_ONLINE_ADDRESS=

# Timeout in seconds for waiting for the network to come online.
#WAIT_ONLINE_TIMEOUT=300

[Ch3LL]

hmmm if both old and new are identical it should be returning no changes according to the code. Thats what its returning when you run with test=True and its still returning changes?

[jleroy]

You asked me to log the output of ip.get_interface and ip.build_interface, but the issue seems to be in ip.get_network_settings and ip.build_network_settings.
Here's the output of these functions.

old (ip.get_network_settings):

[
'NETWORKING=yes\n',
'HOSTNAME=obst-web3-l\n'
]

new (ip.build_network_settings):

[
'# Configuration for networking init script being run during\n',
'# the boot sequence\n',
'\n',
"# Set to 'no' to skip interfaces configuration on boot\n",
'CONFIGURE_INTERFACES=yes\n',
'\n',
"# Don't configure these interfaces. Shell wildcards supported/\n",
'#EXCLUDE_INTERFACES=\n',
'\n',
"# Set to 'yes' to enable additional verbosity\n",
'#VERBOSE=no\n'
]

[Ch3LL]

oh your totally right. i asked for the wrong log output for managed instead of system which is what your using. Sorry about that. Thanks for pointing it out. Looks like you are probably right, that when test=True we are just returning that rendered jinja template but thats not what is actually included int the file.

[jleroy]

FYI this issue is also present in version 3000.3.