Add LAUNCH_EXT_NETWORKS_IPV4

[beornf]

This PR adds two useful options as static ip addresses are not supported in swarm mode: moby/moby#24170.

[tlex]

First of all, thank you for your merge request.

Looking at the documentation, I see a series of problems with this:

• ipv6_address can't work anyhow, since on line 116 the version defined is 3.8 (and ipv6_address requires v2)
• ipv4_address requires that the networks section on line 261 has an ipam block
• for compose file v3.3 and higher, external can't be combined with other network configuration keys (like the ipam from above) (source)

The more I read the docs, the more I realize that this would require a major rework (probably among the lines of deciding between v2 and v3 and then ensuring that the remaining options are actually compatible with the selected version).

[beornf]

It's true the ipam section is required to define the subnet when creating new networks. This PR only addresses external networks which works without ipam as the subnet can be defined externally.

I can drop the ipv6_address as I only added that for consistency and didn't realize the lack of support in compose v3.

If you want I can add support for static ips on the internal bridge networks by adding an ipv4_subnet variable. The logic being for internal networks ipv4_address and ipv4_subnet is required and external networks only ipv4_address is needed.

Maintaining and testing multiple versions of compose sounds like adding too much complexity to me?

[tlex]

Yeah, would be a nightmare to keep tracking what gets implemented when.

I can accept ipv4_address, however there's one case that needs to be covered: combining LAUNCH_NETWORKS (see L225) and LAUNCH_EXT_NETWORKS (see L258) needs to be handled, since you can't mix a list with a dict. So I suggest changing L225 similarly to L261 in your PR.

[tlex]

I've finished now the review, however I worry about setting the same IP address on multiple LAUNCH_EXT_NETWORKS. I see a few possibilities here: either the IP address is set only for the first network in the list (and this needs to be documented in the README), or we find a way to include as a separate variable (e.g. in the form LAUNCH_EXT_NETWORKS_IPV4=mynetwork:10.0.0.1)

[beornf]

I've used a couple of bashisms to split the separators in LAUNCH_EXT_NETWORKS_IPV4. Can change them to unix tools like cut, sed or awk if you have a preference. I could rewrite the inner loop with associative arrays but most of the time it would only be a few elements. 🤔

MR with changes open: https://gitlab.com/ix.ai/swarm-launcher/-/merge_requests/25

[tlex]

Thanks for your work.

I've built a new version of registry.gitlab.com/ix.ai/swarm-launcher:dev-branch with the changes included. I've also added one more commit (bb71cee) that takes a similar approach to converting Network:IP Network2:IP2 as it was implemented with the LAUNCH_ULIMITS.

You can also take it for a spin and see if it works.

Edit:
Added one more functional commit (615db94) since the network block definition was wrong. Since only issues are tracked in GitHub and all the code is actually on GitLab, this is the MR that I plan to merge:
https://gitlab.com/ix.ai/swarm-launcher/-/merge_requests/25

[beornf]

The MR looks much better than mine especially making the LAUNCH_EXT_NETWORKS and LAUNCH_EXT_NETWORKS_IPV4 variables independent. Checked out the Gitlab code and confirmed its working for my swarm cluster. 👍

[tlex]

Thanks for the feedback. I've release v0.14.0 with the new feature.