Network: add mtu setting

[squeed]

This plumbs through the "mtu" top-level variable in the network XML.

It's not amazingly useful, as libvirt doesn't pass it through to the DHCP server, but... baby steps!

[MalloZup]

ciao @squeed thx for pr! 🎅 Looks good.

We are missing however to handle a case: if we have an existing network and we want to update the mtu value from an existing network ( example exiting network A mtu 2000 to 1000 fake values via terraform apply)

[MalloZup]

https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/libvirt/resource_libvirt_network.go#L249 this is the function.

If you want to test it via testacc you can look/take inspiration from this ( with some adaption)

https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/libvirt/resource_libvirt_cloud_init_test.go#L73

[MalloZup]

@squeed you will adress the comments in middle-term time? TIA. maybe next year, or when do you have time, feel free to do it. If you need need this urgently, i could help you adding the update call .let me know, ciao enjoy your day

[squeed]

ciao @MalloZup. I should get to this in the next few days, thanks a lot for the pointers!

[MalloZup]

@squeed yw, for any info feel free to ping. ok thx for info and take your time as needed 🍵

[inercia]

We are missing however to handle a case: if we have an existing network and we want to update the mtu value from an existing network ( example exiting network A mtu 2000 to 1000 fake values via terraform apply)

@MalloZup I think we will not be able to change the MTU dynamically, so I guess we will have to set ForceNew: true...

[MalloZup]

@squeed can you rebase on top? @inercia @dmacvicar feel free to review and share your pov on it

[MalloZup]

Closing due to stale. thx for pr feel free to reopen with rebase. Thx.!

[squeed]

@MalloZup Picking this back up, finally. MTU isn't changeable on live networks - how should I handle this?

[MalloZup]

@squeed hi, just rebase with master

[squeed]

Rebased and updated. Thanks!

BTW, I have a change in Libvirt 5.1 that sets mtu as a DHCP option too :-)

[MalloZup]

@dmacvicar cc

[MalloZup]

We have xslt which let you add it without implementing it in go.

[MalloZup]

@squeed we decided following design goals:
https://github.com/dmacvicar/terraform-provider-libvirt#introduction--goals

This goals were made to follow kind of minimalism in term of design/maintenance and also focus on feature which would enable more the community.

I'm thinking that this PR is good per-se, but i am hesitating because of it

thx anyways.

I will let @dmacvicar and @inercia take decision on it

[squeed]

Hi there,

I understand where you are coming from - libvirt is definitely a large API surface.

I would argue MTU makes the cut - it is a single independent scalar property that is easily plumbed through. It doesn't really add any complexity. It is also hard to change on a running network, which makes it difficult to fix up after Terraform. Setting the MTU is definitely critical for Kubernetes clusters, since the vxlan overhead decreases significantly for larger packets. That's my use-case, FWIW.

So, IMHO, this would be a great feature to add.

[MalloZup]

@squeed yep to me your PR is good to merge but i want to reach a consesus with other maintainers

[inercia]

I agree with @squeed: I think the MTU is an important parameter in a network and it should be part of the exposed interface in the provider.

[MalloZup]

thx @squeed @inercia for this 👍