provider/maas: conditionally run ifdown/up #6619

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

frobware commented Nov 25, 2016

If the nett result of bridging interfaces results in nothing-to-do,
because they are already bridged, then don't run ifdown/up
unnecessarily.

This state can exist in MAAS 2.1 because you now have the option of
bridging interfaces ahead of deployment or bootstrap. And as we move
to dynamic bridging in Juju we want to invoke this script on an
interface on an ad-hoc basis but don't want it to do anything if that
interface is already bridged.

QA steps:

On a machine run:

$ sudo ./add-juju-bridge.py /etc/network/interfaces
--activate
--interfaces-to-bridge=enp1s0f2

And note that enp1s0f2 gets bridged.

Verify with:

$ brctl show

Run the add-juju-bridge.py script again with the same arguments and
now you'll see no action taken as it is already bridged as
br-enp1s0f2.

I also configured a node in MAAS 2.1 that had two bridged NICs;
bootstrapping a node doesn't additionally run ifdown/up because
there's nothing to do.

provider/maas: conditionally run ifdown/up
If the nett result of bridging interfaces results in nothing-to-do,
because they are already bridged, then don't run ifdown/up
unnecessarily.

This state can exist in MAAS 2.1 because you now have the option of
bridging interfaces ahead of deployment or bootstrap. And as we move
to dynamic bridging in Juju we want to invoke this script on an
interface on an ad-hoc basis but don't want it to do anything if that
interface is already bridged.

QA steps:

On a machine run:

 $ sudo ./add-juju-bridge.py /etc/network/interfaces \
     --activate \
     --interfaces-to-bridge=enp1s0f2

And note that enp1s0f2 gets bridged.

Verify with:

 $ brctl show

Run the add-juju-bridge.py script again with the same arguments and
now you'll see no action taken as it is already bridged as
br-enp1s0f2.

I also configured a node in MAAS 2.1 that had two bridged NICs;
bootstrapping a node doesn't additionally run ifdown/up because
there's nothing to do.

The change LGTM. Shame it can't easily be tested.

Contributor

frobware commented Nov 25, 2016

!!retry!!

Contributor

bz2 commented Nov 25, 2016

!!retry!!

Owner

jameinel commented Nov 27, 2016

!!retry!!

Ideally we could share more of the print_stanza sections, which also helps so that some hiccup doesn't cause us to alter what we write after reconsideration.

+ print_stanzas(stanzas, new)
+ print_stanzas(parser.stanzas(), cur)
+
+ if cur.getvalue() == new.getvalue():
@jameinel

jameinel Nov 27, 2016

Owner

Given you just did all the work to generate the stanzas, shouldn't we just use that and copy the new.getvalue() to the output, instead of calling print_stanzas() again later?

I also see a 'print_stanzas' call up above, seems like we could avoid doing it multiple times.

Contributor

frobware commented Nov 28, 2016

!!retry!!

Contributor

frobware commented Nov 28, 2016

Based on the concern about testing this functionality I am reworking this change here:

https://github.com/frobware/juju/tree/bridge-if-different-strike2

Contributor

frobware commented Dec 5, 2016

Submitting this in a new PR.

@frobware frobware closed this Dec 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment