Interfaces_file - improvements #3328

russoz · 2021-09-03T23:28:33Z

SUMMARY

General improvements, simplifications and pythonification of the code.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

plugins/modules/system/interfaces_file.py
tests/unit/plugins/modules/system/interfaces_file/test_interfaces_file.py

ansibullbot · 2021-09-04T01:32:45Z

cc @hryamzik @obourdon
click here for bot help

plugins/modules/system/interfaces_file.py

russoz · 2021-09-07T06:34:39Z

@felixfontein I have not opened it yet, but I bet it is the sanity import moaning about coverage ;-)

UPDATE: Bingo!!!!

felixfontein · 2021-09-07T20:46:21Z

@russoz tests now all pass :)

plugins/modules/system/interfaces_file.py

russoz · 2021-09-18T09:24:01Z

@felixfontein @hryamzik just added an integration test for this scenario Felix described - it failed on the old code and worked on the new one ;-)

felixfontein

LGTM as far as I can judge.

hryamzik · 2021-09-18T20:30:04Z

LGTM

patchback · 2021-09-19T11:44:40Z

Backport to stable-3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-3/7aae8d5386e0ddb944f171b1847e3e16e981d635/pr-3328

Backported as #3396

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* pythonific!! no camel cases, bitte * simplified iface attributes parsing * some improvements, passing tests * simplified set_interface_option() * further simplifications * remove unreachable stmt * pythonified a file open * added changelog fragment * adjustment per PR * PR: fixed the auto- case * PR: added testcase and chglog frag for the misleading change report * extra line removed * integration is not destructive (cherry picked from commit 7aae8d5)

felixfontein · 2021-09-19T11:44:55Z

@russoz thanks for this improvement!
@hryamzik thanks for reviewing!

* pythonific!! no camel cases, bitte * simplified iface attributes parsing * some improvements, passing tests * simplified set_interface_option() * further simplifications * remove unreachable stmt * pythonified a file open * added changelog fragment * adjustment per PR * PR: fixed the auto- case * PR: added testcase and chglog frag for the misleading change report * extra line removed * integration is not destructive (cherry picked from commit 7aae8d5) Co-authored-by: Alexei Znamensky <[email protected]>

pgassmann · 2021-11-26T23:46:25Z

Regression: The new version repeatedly adds new lines for up/down option. The previous version (ansible 3.0) only added the option if it was not yet present.

- name: floating_ip_interface_up_ip 2a01:a:b:c::1/64 dev eth0
  interfaces_file:
    option: up
    iface: eth0
    dest: '/etc/network/interfaces.d/50-cloud-init.cfg'
    value: 'ip addr add 2a01:a:b:c::1/64 dev eth0'
    state: present

result: ansible-playbook --diff --check -vvv

TASK [floating_ip : floating_ip_interface_down_ip 2a01:a:b:c::1/64 dev eth0] ***************************************************
task path: /../roles/floating_ip/tasks/floating_ip.yml:12
changed: [mail01p.example.org] => changed=true 
  dest: /etc/network/interfaces.d/50-cloud-init.cfg
  gid: 0
  group: root
  ifaces:
    eth0:
      address: 2a01:d:e:f::1/64
      address_family: inet6
      down:
      - ip addr del 88.1.2.332 dev eth0
      - ip addr del 49.1.2.3/32 dev eth0
      - ip addr del 2a01:a:b:c::1/64 dev eth0
      - ip addr del 2a01:a:b:c::1/64 dev eth0
      gateway: fe80::1
      method: static
      post-up: []
      pre-up: []
      up:
      - ip addr add 88.1.2.332 dev eth0
      - ip addr add 49.1.2.3/32 dev eth0
      - ip addr add 2a01:a:b:c::1/64 dev eth0
    lo:
      address_family: inet
      down: []
      method: loopback
      post-up: []
      pre-up: []
      up: []
  invocation:
    module_args:
      address_family: null
      attributes: null
      backup: false
      dest: /etc/network/interfaces.d/50-cloud-init.cfg
      group: null
      iface: eth0
      mode: null
      option: down
      owner: null
      selevel: null
      serole: null
      setype: null
      seuser: null
      state: present
      unsafe_writes: false
      value: ip addr del 2a01:a:b:c::1/64 dev eth0
  mode: '0644'
  owner: root
  size: 823
  state: file
  uid: 0

resulting interface file after multiple runs:

auto lo
iface lo inet loopback

auto eth0
iface eth0 inet dhcp
    dns-nameservers 213.133.99.99 213.133.98.98 213.133.100.100

# control-alias eth0
iface eth0 inet6 static
    address 2a01:d:e:f::1/64
    gateway fe80::1
    up ip addr add 88.1.2.3/32 dev eth0
    down ip addr del 88.1.2.3/32 dev eth0
    # ipv6
    up ip addr add 2a01:a:b:c::1/64 dev eth0
    up ip addr add 49.12.115.122/32 dev eth0
    up ip addr add 88.1.2.3/32 dev eth0
    up ip addr add 2a01:a:b:c::1/64 dev eth0
    up ip addr add 88.1.2.3/32 dev eth0
    up ip addr add 2a01:a:b:c::1/64 dev eth0
    up ip addr add 88.1.2.3/32 dev eth0
    up ip addr add 2a01:a:b:c::1/64 dev eth0
    down ip addr del 2a01:a:b:c::1/64 dev eth0
    down ip addr del 49.12.115.122/32 dev eth0
    down ip addr del 88.1.2.3/32 dev eth0
    down ip addr del 2a01:a:b:c::1/64 dev eth0
    down ip addr del 88.1.2.3/32 dev eth0
    down ip addr del 2a01:a:b:c::1/64 dev eth0
    down ip addr del 88.1.2.3/32 dev eth0
    down ip addr del 2a01:a:b:c::1/64 dev eth0

felixfontein · 2021-11-28T20:09:22Z

@russoz ^

hryamzik · 2021-11-29T23:35:26Z

We also need a test case for this!

pgassmann · 2021-12-02T22:38:47Z

@felixfontein @russoz should I create a separate Ticket for that case?

It would be optimal if the update would remove duplicates. At least state: absent should remove all items at once. (didn't test that)

russoz · 2021-12-02T22:44:19Z

@pgassmann yes please, that would be great

russoz added 8 commits September 2, 2021 21:31

pythonific!! no camel cases, bitte

d314f94

simplified iface attributes parsing

d34d812

some improvements, passing tests

d84f41e

simplified set_interface_option()

d552c8f

further simplifications

ea63d0c

remove unreachable stmt

295b471

pythonified a file open

1c0f33a

added changelog fragment

9eac8a8

felixfontein added backport-3 check-before-release PR will be looked at again shortly before release and merged if possible. labels Sep 5, 2021

ansibullbot removed the needs_triage label Sep 5, 2021

hryamzik reviewed Sep 6, 2021

View reviewed changes

plugins/modules/system/interfaces_file.py Outdated Show resolved Hide resolved

adjustment per PR

812fcf3

felixfontein reviewed Sep 10, 2021

View reviewed changes

plugins/modules/system/interfaces_file.py Outdated Show resolved Hide resolved

plugins/modules/system/interfaces_file.py Show resolved Hide resolved

ansibullbot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI label Sep 13, 2021

PR: added testcase and chglog frag for the misleading change report

d99ef1d

extra line removed

8ee43ba

felixfontein approved these changes Sep 18, 2021

View reviewed changes

ansibullbot added the shipit label Sep 18, 2021

integration is not destructive

8ad43d3

ansibullbot removed the shipit label Sep 19, 2021

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 19, 2021

felixfontein merged commit 7aae8d5 into ansible-collections:main Sep 19, 2021

patchback bot mentioned this pull request Sep 19, 2021

[PR #3328/7aae8d53 backport][stable-3] Interfaces_file - improvements #3396

Merged

russoz deleted the interfaces_file-improvement branch September 20, 2021 13:11

pgassmann mentioned this pull request Dec 2, 2021

interfaces_file regression. not idempotent with down/up option. #3841

Closed

1 task

This was referenced Mar 2, 2023

Module interfaces_file ignore option line not starting with a space #6120

Closed

Interfaces file spaces fix #6131

Merged

patchback bot mentioned this pull request Mar 9, 2023

[PR #6131/abcba9db backport][stable-6] Interfaces file spaces fix #6169

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interfaces_file - improvements #3328

Interfaces_file - improvements #3328

russoz commented Sep 3, 2021

ansibullbot commented Sep 4, 2021

russoz commented Sep 7, 2021 •

edited

Loading

felixfontein commented Sep 7, 2021

russoz commented Sep 18, 2021

felixfontein left a comment

hryamzik commented Sep 18, 2021

patchback bot commented Sep 19, 2021 •

edited

Loading

felixfontein commented Sep 19, 2021

pgassmann commented Nov 26, 2021

felixfontein commented Nov 28, 2021

hryamzik commented Nov 29, 2021

pgassmann commented Dec 2, 2021 •

edited

Loading

russoz commented Dec 2, 2021

Interfaces_file - improvements #3328

Interfaces_file - improvements #3328

Conversation

russoz commented Sep 3, 2021

SUMMARY

ISSUE TYPE

COMPONENT NAME

ansibullbot commented Sep 4, 2021

russoz commented Sep 7, 2021 • edited Loading

felixfontein commented Sep 7, 2021

russoz commented Sep 18, 2021

felixfontein left a comment

Choose a reason for hiding this comment

hryamzik commented Sep 18, 2021

patchback bot commented Sep 19, 2021 • edited Loading

Backport to stable-3: 💚 backport PR created

felixfontein commented Sep 19, 2021

pgassmann commented Nov 26, 2021

felixfontein commented Nov 28, 2021

hryamzik commented Nov 29, 2021

pgassmann commented Dec 2, 2021 • edited Loading

russoz commented Dec 2, 2021

russoz commented Sep 7, 2021 •

edited

Loading

patchback bot commented Sep 19, 2021 •

edited

Loading

pgassmann commented Dec 2, 2021 •

edited

Loading