Skip to content

Commit

Permalink
nm: Fix the incorrect change indication when apply the same config twice
Browse files Browse the repository at this point in the history
When applying the same network connections twice, the second apply still
shows `changed: true`.

The root cause:
 * When user never asked about ethtool configuration, network-role
   will generate an all-default `NM.SettingEthtool` and pass it to
   NetworkManager daemon. But NetworkManager discard it when saving to
   ifcfg plugin as ifcfg plugin will not keep empty ethtool option.

 * During second apply, the `NM.SimpleConnection.compare` will return
   False indicating configuration changed because of on-disk connection
   has no ethtool setting while pending connection does.

To fix it, we just remove the all-default `NM.SettingEthtool`.

Signed-off-by: Gris Ge <fge@redhat.com>
  • Loading branch information
cathay4t committed Jun 29, 2021
1 parent 85b75b6 commit adf68bd
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 0 deletions.
10 changes: 10 additions & 0 deletions library/network_connections.py
Expand Up @@ -976,6 +976,16 @@ def connection_create(self, connections, idx, connection_current=None):
else:
s_ethtool.option_set_uint32(nm_ring, setting)

# * When user never asked about ethtool configuration, network-role
# will generate an all-default `NM.SettingEthtool` and pass it to
# NetworkManager daemon. But NetworkManager discard it when saving to
# ifcfg plugin as ifcfg plugin will not keep empty ethtool option.
# * The follow up `NM.SimpleConnection.compare()` will indicate
# configuration changed as new ethtool setting found.
# * To workaround this, we just remove the all-default NM.SettingEthtool.
if s_ethtool.compare(NM.SettingEthtool.new(), NM.SettingCompareFlags.EXACT):
con.remove_setting(NM.SettingEthtool)

if connection["mtu"]:
if connection["type"] == "infiniband":
s_infiniband = self.connection_ensure_setting(con, NM.SettingInfiniband)
Expand Down
57 changes: 57 additions & 0 deletions tests/tests_change_indication_on_repeat_run.yml
@@ -0,0 +1,57 @@
# SPDX-License-Identifier: BSD-3-Clause
# This file was generated by ensure_provider_tests.py
---
- hosts: all
vars:
interface: testnic1
type: veth
name: Test change indication on repeat run
tasks:
- include_tasks: tasks/manage_test_interface.yml
vars:
state: present
- include_tasks: tasks/assert_device_present.yml
- block:
- import_role:
name: linux-system-roles.network
vars:
network_connections:
- name: "{{ interface }}"
state: up
type: ethernet
ip:
dhcp4: "no"
auto6: "no"
register: __network_connections_result
- name: Assert change:true
assert:
that: __network_connections_result.changed
- import_role:
name: linux-system-roles.network
vars:
network_connections:
- name: "{{ interface }}"
state: up
type: ethernet
ip:
dhcp4: "no"
auto6: "no"
register: __network_connections_result
- name: Assert change:false
assert:
that: not __network_connections_result.changed
always:
- block:
- import_role:
name: linux-system-roles.network
vars:
network_connections:
- name: "{{ interface }}"
persistent_state: absent
state: down
ignore_errors: true
- include_tasks: tasks/manage_test_interface.yml
vars:
state: absent
tags:
- "tests::cleanup"

0 comments on commit adf68bd

Please sign in to comment.