cni: set hostname as K8S_POD_NAME

[vito]

This PR passes the hostname along to the network provider. The CNI provider sets it as K8S_POD_NAME, the rest ignore it.

My goal is to be able to use the dnsname CNI plugin for inter-container networking in Buildkit. Using DNS works great because it lets me keep non-reproducible values like IP addresses out of LLB, maximizing cache reuse for builds that depend on services.

The dnsname plugin uses the "container name" as its address, which it reads from an arg called K8S_POD_NAME. This arg seems to be used by other CNI plugins too. It's kind of a bummer that this arg name isn't something more generic. This plugin is actually designed for podman which sets the same arg; it seems to be a de-facto standard at this point. :/

The hostname seems like the closest thing to a "container name," and it seems reasonable for a network provider to be interested in the hostname, so I threaded the value through and called it done. I'm not married to this approach, but if it seems reasonable I can start working on a test for it.

I also considered adding support for arbitrary CNI options, since folks like me might want port mapping someday, but I'm not sure if y'all would want to commit to that (or even this, frankly).

[tonistiigi]

I'm not sure I understand how this works. meta.Hostname should be constant "buildkitsandbox" by default for all containers so I'm not sure what networking info it carries.

[vito]

I'm not sure I understand how this works. meta.Hostname should be constant "buildkitsandbox" by default for all containers so I'm not sure what networking info it carries.

In my case I'm already setting the hostname using llb.Hostname and I would like that value to double as the DNS name:

https://github.com/vito/bass/blob/4a1b64a05cc8fcfe93a0d46fde9d94cee079cd56/pkg/runtimes/buildkit.go#L707-L708

The provided value is a base32-encoded hash of the data value used to generate the LLB.

This PR should only affect folks who a) maintain their own Buildkit config with CNI + plugins that use K8S_POD_NAME, and b) are already using llb.Hostname to set canonical names for the things they run against it. To your point, if you do a) but not b) you may end up with strange behavior since everything will have the same name (e.g. one DNS entry mapped to a bunch of different IPs).

This change shouldn't affect Buildkit's default behavior, and if moby/buildkit someday enables bridge networking out of the box it still shouldn't affect it as the bridge plugin does not use K8S_POD_NAME.

Hope this helps it make more sense. :)

[tonistiigi]

Ok, but make it so that these new calls only happen when the custom hostname is set and not by default.

[vito]

Sounds good. The latest commit should address all feedback, but I haven't added tests yet.

I have to dash out for today, but I'm leaning towards adding unit tests for the CNI network provider. I'm interested in integration testing but it might be a bit invasive unless I figure out a way to use an alternative stage to integration-tests with dnsname installed and configured.

Let me know if you have a preferred approach.

edit: hm, unit testing this is tricky; I can't assert what options are passed since they're just functions that run against a *Namespace which can't be initialized beforehand. Looking into integration tests now.

[vito]

Added an integration test. It was less invasive than I thought. I was worried all the other tests would start running dnsmasq but the existing sandbox/matrix test infra made it pretty easy to localize it to just the relevant test. 🎉