Implement macvtap interface configuration#203
Implement macvtap interface configuration#203Freddo3000 wants to merge 1 commit intohashicorp:mainfrom
Conversation
|
I've now tested it with bridge networking and hosts are able to acquire IPs via DHCP. It can also acquire a static IP by passing it through cloud-init. job "server" {
group "virt-group" {
reschedule {
attempts = 0
unlimited = false
}
task "virt-task" {
driver = "virt"
artifact {
source = "http://cloud-images.ubuntu.com/noble/current/noble-server-cloudimg-amd64.img"
destination = "local/noble-server-cloudimg-amd64.img"
mode = "file"
}
config {
user_data =<<EOH
#cloud-config
password: secret
ssh_pwauth: true
EOH
network_interface {
macvtap {
device = "ens1.320"
}
}
disk {
size = "10GiB"
pool = "ceph"
chained = true
source {
image = "local/noble-server-cloudimg-amd64.img"
}
}
}
resources {
cores = 4
memory = 4096
}
}
}
}I have not tested any of the other macvtap modes other than bridge as I don't have any equipment available to test it on properly, but I don't see why it wouldn't work. |
chrisroberts
left a comment
There was a problem hiding this comment.
Hi @Freddo3000, thanks for this PR! Pulled in your changes and everything works as described. This looks great. Left a couple comments, and it would be great to get a couple tests added in here to verify the config. We're lacking tests elsewhere so I'm happy to add coverage related to this as we create those tests.
| if netInterface.Macvtap != nil { | ||
| if netInterface.Macvtap.Device == "" { | ||
| mErr.Errors = append(mErr.Errors, | ||
| fmt.Errorf("network interface macvtap '%v' requires device parameter", i)) |
There was a problem hiding this comment.
So there is a helper for these checks and after importing the github.com/hashicorp/nomad-driver-virt/internal/errs package, the empty string conditional can be removed and this can be updated to:
mErr = multierror.Append(mErr, errs.MissingAttribute("network_interface.macvtap.device", netInterface.Macvtap.Device))However, I see that the validation here isn't using a pointer for the mErr value so using the multierror.Append helper isn't possible! I'll update this validation method in a subsequent PR 🙂
| })), | ||
| "macvtap": hclspec.NewBlock("macvtap", false, hclspec.NewObject(map[string]*hclspec.Spec{ | ||
| "device": hclspec.NewAttr("device", "string", true), | ||
| "mode": hclspec.NewAttr("mode", "string", false), |
There was a problem hiding this comment.
The default mode can be included in the spec:
...
"mode": hclspec.NewDefault(
hclspec.NewAttr("mode", "string", false),
hclspec.NewLiteral(fmt.Sprintf("%q", MacvtapModeBridge)),
),
...| // interfaces. This is used to filter out interface types — such as macvtap — | ||
| // that manage their own network identity and have no interaction with Nomad's | ||
| // host-side port mapping machinery. | ||
| func (n NetworkInterfacesConfig) BridgeOnly() NetworkInterfacesConfig { |
There was a problem hiding this comment.
How about updating this name to Configurable so that it's not limited to only bridge devices?
This pull request implements the macvtap interface configuration. As a macvtap interface typically has it's own IP, it is not really compatible with nomad ports definitions, as such they're omitted for this mode. CNI with a dummy interface could be used instead.
I'm leaving this as a draft to gather feedback until I've tested it a bit.