quattor / configuration-modules-core

Node Configuration Manager Components for Everyone
www.quattor.org
Other
6 stars 54 forks source link

ncm-network: mirror "old" bootproto behaviour in nmstate #1653

Closed wdpypere closed 7 months ago

wdpypere commented 7 months ago

I spent a good time figuring out why nmstate.pm was not changing IP's. The code of nmstate is:

    if (defined($eth_bootproto)) {
        if ($eth_bootproto eq 'static') {

In our templates we don't set bootproto.

but if I look in network.pm it does:

    # set the bootprotocol
    &$makeline('bootproto', def => 'static');

    my $bootproto = $iface->{bootproto} || 'static';

So I think nmstate.pm should mimic that behavior.

wdpypere commented 7 months ago

tests fail because other parts of the code (vlans and bridges) seem to assume bootproto to be unset. So this would need more fixing. But I do think nmstate should mimic network.pm, to match admin expectations.

aka7 commented 7 months ago

tests fail because other parts of the code (vlans and bridges) seem to assume bootproto to be unset. So this would need >>more fixing. But I do think nmstate should mimic network.pm, to match admin expectations.

this PR should fix the failures you are seeing. https://github.com/quattor/configuration-modules-core/pull/1647

wdpypere commented 7 months ago

ah, I missed that. I'll close this PR. Thanks @aka7

aka7 commented 7 months ago

@wdpypere I think you will stiill need your PR if you want bootproto to be static by default. but I guess either after mine is merged and you rebase? or I can add it with that pr?

aka7 commented 7 months ago

@wdpypere I think you will stiill need your PR if you want bootproto to be static by default. but I guess either after mine is merged and you rebase? or I can add it with that pr?

I've made changes to the pr to include your suggestion and as a result made that part of the code a bit more simpler. Please pull this in to test if you like. https://github.com/quattor/configuration-modules-core/pull/1647 @wdpypere and a review would be welcomed.

wdpypere commented 7 months ago

@aka7 thanks for adding to the other PR. Ill read it, but I don't think you want my review, I'm very bad at Perl. :D