Feature: Add mac_address parameter

[the-maldridge]

The docker task driver allows specifying the container mac address. When combined with CNI networks this buidls a powerful ability to use administrator assigned MAC ranges to dynamically assign container addresses using DHCP.

If this would be accepted, I can send a patch.

[lgfa29]

Hi @the-maldridge 👋

I think parity with the Docker driver whenever possible is always welcomed.

Thank you for the interest in working on this. Let us know if you have any question!

[the-maldridge]

So the further into this I get the more I'm actually convinced that this is a bug that it isn't working already.

In api/structs.go the field StaticMAC is already defined, and should take a MAC address. From what I can ascertain, this is consumed in driver.go in Driver.StartTask() where when network is defined as bridge something should be putting the mac_address into the createOpts structure. There is no code that performs this mapping, or even reads from the mac_address field for that matter. It seems like in theory there should be something doing a mapping.

I'm not entirely sure why this is restricted to only bridge network mode, as it should work fine in anything except host mode where there would be a pre-existing conflicting address.

I'll see if I have cycles to get to this, but given the generally abysmal support for Podman, both in weird bugs I've noticed when using the driver in Nomad as well as trying to get a not-ancient version of it on Debian based systems, I've gone back to docker for the time being.

[lgfa29]

Thank you for looking into this @the-maldridge.

I suspect Podman API v4 significantly modified the network container configuration to the point where the spec we currently have in api/structs.go no longer works for some container network configuration.

From their v4.0.0 Breaking Changes section:

The Podman APIs for Manifest List and Network operations have been completely rewritten to address issues and inconsistencies in the previous APIs. Incompatible APIs should warn if they are used with an older Podman client.

And looking at the latest content for their specs, the `StaticMAC field is marked as deprecated and only used to maintain database schema backwards compatibility:
https://github.com/containers/podman/blob/e4cdd4b35ad2dc617f5d33bb92abf2d433704ad4/libpod/container_config.go#L260-L261

So now the question becomes: how does it work now? And after looking into it a bit, I'm not sure 😞

The API reference seems to be broken as I can't really see the request schema from https://docs.podman.io/en/latest/_static/api.html#tag/containers/operation/ContainerCreateLibpod.

Looking at the OpenAPI source file (https://storage.googleapis.com/libpod-master-releases/swagger-latest.yaml) SpecGen points back to https://github.com/containers/podman/blob/main/pkg/specgen/specgen.go, which is where our api/structs.go file was sourced from, but it's the updated version where ContainerNetworkConfig does not have StaticMAC field.

But reading through the over values, we see this:

	// Map of networks names or ids that the container should join.
	// You can request additional settings for each network, you can
	// set network aliases, static ips, static mac address  and the
	// network interface name for this container on the specific network.
	// If the map is empty and the bridge network mode is set the container
	// will be joined to the default network.
	Networks map[string]nettypes.PerNetworkOptions

https://github.com/containers/podman/blob/e4cdd4b35ad2dc617f5d33bb92abf2d433704ad4/pkg/specgen/specgen.go#L458-L464

So it seems like static MAC addresses should be set in the Networks map?

At this point I gave up trying to understand the API and went looking for how it's consumed by the podman container run command and indeed the --mac-address flag does seem to be mapped to Networks:

https://github.com/containers/podman/blob/e4cdd4b35ad2dc617f5d33bb92abf2d433704ad4/cmd/podman/common/netflags.go#L250-L263

And yes, this option does seem restricted to only bridge network mode, but I'm not sure why and this requirement doesn't seem to be documented or explained anywhere.

So, I think the way to get this to work would be:

1. Add that Networks field to api/structs.go.
2. Add PerNetworkOptions struct to api/structs.go, which, I think, comes from here?
   https://github.com/containers/common/blob/3cec82a3710580e7e545b23c47534f5cb2682917/libnetwork/types/network.go#L246-L263
3. Create a new task configuration for mac_address.

And now is the weird part. We try to keep as much compatibility as possible, so we would need to detect which version of Podman is being used. Luckily this is already done by the fingerprinter, so I think we could store that apiVersion into the driver itself so we can later access it in StartTask() (since fingerprints and tasks start concurrently, this value would need to be protected by a mutex).

nomad-driver-podman/driver.go

Lines 260 to 264 in 8251cf9

	func (d *Driver) buildFingerprint() *drivers.Fingerprint {
	attrs := map[string]*pstructs.Attribute{}
	// Ping podman api
	apiVersion, err := d.podman.Ping(d.ctx)

Now that we know the Podman version being used we do one of the following:

4.a. If Podman version is 3.x, populate StaticMAC as you did.
4.b. If podman version is 4.x, create the Networks with a "default" key and a PerNetworkOptions with the static mac address set. This part I'm not totally sure would work like this, but I'm going by the CLI code again (https://github.com/containers/podman/blob/e4cdd4b35ad2dc617f5d33bb92abf2d433704ad4/cmd/podman/common/netflags.go#L206-L211)

Now, that's a lot of changes, and I'm probably missing some extra details, so I totally understand if you don't have the time to take on this.

Moreover, it may be good for us to figure out our Podman version compatibility and update to use the API v4, potentially dropping v3 support when incompatible (and hope V5 doesn't break too much stuff again...)

[the-maldridge]

That's kind of what I was afraid of. I currently manage a bunch of podman containers via the ansible collection, and getting static MAC addresses to work in there involved descending pretty far into CNI mutating template parsers, so I'm not surprised that that complexity percolates all the way back up to the top.

I'm a little concerned about blindly setting up a default network unless there's some way to guarantee that in the context that gets parsed in it can only match the network that nomad expects, otherwise this might inadvertently attach the container to an extant and unexpected network.

I haven't got cycles to get that far into the API right now, and I'm largely reconsidering if the static MAC addresses are the right way to achieve what I'm doing anyway due to some really strange boundary conditions that occur when running the CNI dhcp driver (which was the only motivation for having known mac addresses in the first place). I'm increasingly considering just provisioning services behind a proxy which I can advertise via an internal bgp announcement to move its service VIP around. I'll chew on this some, but right now I'm increasingly thinking this will cause more problems than it solves.