quattor / configuration-modules-core

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

ncm-network: add ipv6 support to the nmstate backend #1687

Closed guillaume-philippon closed 6 days ago

guillaume-philippon commented 3 months ago

Add ipv6 support to the nmstate backend supporting:

jrha commented 2 months ago

Thanks! I'll get this tested at RAL.

guillaume-philippon commented 2 months ago

Last version should allow ipv4 and ipv6 on same interface (was not the case before) but not allow aliases.

aka7 commented 2 months ago

Last version should allow ipv4 and ipv6 on same interface (was not the case before) but not allow aliases.

@guillaume-philippon I wonder if you can pull in the alias bits I got in my pr into this? to support for both ipv4 n ipv6? so can all go into your pr if its not too much trouble? or I can wait until yours is merged to add Alias support. not sure why the tests are failing on both though, unrelated I think.

guillaume-philippon commented 2 months ago

I think it would be easiest to have only one PR. Especially if some code refactoring are needded.

jrha commented 2 months ago

This functions well on test hosts at RAL, with the limitations I've added to the description text.

The tests are failing because no tests seem to be executed by nmstate_ipv6.t, which is odd because it looks reasonable to me.

aka7 commented 2 months ago

@guillaume-philippon can you rebase and publish again, @jrha James has merged my PR https://github.com/quattor/configuration-modules-core/pull/1688 so this will need a rebase.

jrha commented 2 months ago

I'll have a go at rebasing this.

jrha commented 2 months ago

Rebased and re-tested at RAL, looks good. I'll see if I can work out why the tests are unhappy.

jrha commented 2 months ago

I've sorted out the issue with the tests and finished the implementation of secondary IPv6 addresses.

The tests are now only failing due to the pre-existing issue with ncm-sysctl.

These commits should probably be squashed a bit prior to merging, but I've left them as is for now so it is more clear what I've changed.

jrha commented 2 months ago

I'll open another PR to implement additional route support for IPv6.