dresden-weekly / ansible-network-interfaces

Ansible roles to manage Ubuntu network interface configuration
MIT License
86 stars 62 forks source link

please make networking restart optional #3

Closed gfa closed 8 years ago

gfa commented 8 years ago

hello, thanks for your module I can tell it works fine on debian jessie :+1: please make optional the network service restart as it cut all my other interfaces even when network_manage_devices: no is configured

arBmind commented 8 years ago

Hey, I am pleased that you like this little module. If we suppress the network service restart how do we get the configuration changes into effect? Thinking a bit about it, it seems very rude to restart the service - but we could not figure out a better way so far.

gfa commented 8 years ago

in my playbook i manually start the interfaces

  roles:
    - role: dresden-weekly.network-interfaces
      network_manage_devices: no #if yes all other interfaces are deleted
      network_interfaces:

       - device: eth1
         auto: true
         family: inet
         method: manual
         pre-up: [ifconfig eth1 up]

       - device: eth2
         auto: true
         family: inet
         method: manual
         pre-up: [ifconfig eth2 up]

  tasks:
    # start the nics
    - command: '/sbin/ifup eth1'
    - command: '/sbin/ifup eth2'

you can do the same in the module

starkers commented 8 years ago

Thats not idempotent Simplest way is to use a handler (or chained handler but I don't recommend it should you loose connection and not be using fireball/localhost)..

I recommend a handler gets triggered if the config file changes.. the handler simply reloads all NICs using shell: ifdown -a ; ifup -a

I'm using ansible-pull on my hosts (which connects on localhost) so I don't run any risk of loosing connection but I prefer to reload all the network.. if its gonna break the networking on next boot.. I want to know now.

I would quite happily make that not the default but provide a variable for people like me who do want it. I have made a few other enhancements to this repo and will do a PR on this if you approve? (testing on trusty).. very cool role I have to say!

arBmind commented 8 years ago

@gfa Thank you :+1: for pointing out your use case. I guess we need a way to restart the devices individually. Also we do not have to trigger them if auto is false. Unfortunately I do not know how to do that. Handlers do not have options.

@starkers Welcome to the discussion. I never used Ansible-pull. I could not find your fork but I am looking forward to your pull request. Can you give me an estimation?

I will try to do some tests to find a better solution to the restarting issue.

gfa commented 8 years ago

@starkers no, don't shutdown all my interfaces (ifdown -a) that's the same as restarting networking service.

@arBmind I'm just starting with ansible, so I can't comment on handlers. I would do ifdown $INTERFACE && ifup $INTERFACE for each modified interface.

arBmind commented 8 years ago

I played with the code a bit... no time for real tests.

It seems that the role is already quite defensive with restarts. It restarts only the changed network devices. Of cause the first application will trigger the restart of all the configured devices. I proposed a pull request that delays the restarts to the end of the play and also to suppress them or adapt them to your needs. All untested so far. More an idea, PR to preserve my thoughts.

arBmind commented 8 years ago

@gfa @starkers I created and did some basic tests.

Please take a look at pull request #19 - would you use this?

arBmind commented 8 years ago

I further improved and merged #19