Skip to content

Commit

Permalink
Merge pull request #10568 from ormergi/net-bind-api-net-attach-def
Browse files Browse the repository at this point in the history
Network plugin binding API: Support CNI plugins
  • Loading branch information
kubevirt-bot committed Oct 17, 2023
2 parents 0488861 + 9712819 commit e056113
Show file tree
Hide file tree
Showing 13 changed files with 195 additions and 27 deletions.
4 changes: 4 additions & 0 deletions api/openapi-spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -17343,6 +17343,10 @@
"v1.InterfaceBindingPlugin": {
"type": "object",
"properties": {
"networkAttachmentDefinition": {
"description": "NetworkAttachmentDefinition references to a NetworkAttachmentDefinition CR object. Format: \u003cname\u003e, \u003cnamespace\u003e/\u003cname\u003e. If namespace is not specified, VMI namespace is assumed. version: 1alphav1",
"type": "string"
},
"sidecarImage": {
"description": "SidecarImage references a container image that runs in the virt-launcher pod. The sidecar handles (libvirt) domain configuration and optional services. version: 1alphav1",
"type": "string"
Expand Down
12 changes: 12 additions & 0 deletions manifests/generated/kv-resource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,12 @@ spec:
binding:
additionalProperties:
properties:
networkAttachmentDefinition:
description: 'NetworkAttachmentDefinition references
to a NetworkAttachmentDefinition CR object. Format:
<name>, <namespace>/<name>. If namespace is not specified,
VMI namespace is assumed. version: 1alphav1'
type: string
sidecarImage:
description: 'SidecarImage references a container image
that runs in the virt-launcher pod. The sidecar handles
Expand Down Expand Up @@ -3686,6 +3692,12 @@ spec:
binding:
additionalProperties:
properties:
networkAttachmentDefinition:
description: 'NetworkAttachmentDefinition references
to a NetworkAttachmentDefinition CR object. Format:
<name>, <namespace>/<name>. If namespace is not specified,
VMI namespace is assumed. version: 1alphav1'
type: string
sidecarImage:
description: 'SidecarImage references a container image
that runs in the virt-launcher pod. The sidecar handles
Expand Down
4 changes: 2 additions & 2 deletions pkg/network/netbinding/netbinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func slirpNetBindingPluginSidecar(vmi *v1.VirtualMachineInstance, kvConfig *v1.K
}

var slirpSidecarImage string
if plugin := readNetBindingPluginConfiguration(kvConfig, SlirpNetworkBindingPluginName); plugin == nil {
if plugin := ReadNetBindingPluginConfiguration(kvConfig, SlirpNetworkBindingPluginName); plugin == nil {
// In case no Slirp network binding plugin is registered (i.e.: specified in in Kubevirt config) use default image
// to prevent newly created Slirp VMs from hanging, and reduce friction for users who didn't register an image yet.
// TODO: remove this workaround by next Kubevirt release v1.2.0.
Expand All @@ -114,7 +114,7 @@ func slirpNetBindingPluginSidecar(vmi *v1.VirtualMachineInstance, kvConfig *v1.K
}
}

func readNetBindingPluginConfiguration(kvConfig *v1.KubeVirtConfiguration, pluginName string) *v1.InterfaceBindingPlugin {
func ReadNetBindingPluginConfiguration(kvConfig *v1.KubeVirtConfiguration, pluginName string) *v1.InterfaceBindingPlugin {
if kvConfig != nil && kvConfig.NetworkConfiguration != nil && kvConfig.NetworkConfiguration.Binding != nil {
if plugin, exist := kvConfig.NetworkConfiguration.Binding[pluginName]; exist {
return &plugin
Expand Down
1 change: 1 addition & 0 deletions pkg/virt-controller/services/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ go_library(
"//pkg/host-disk:go_default_library",
"//pkg/network/istio:go_default_library",
"//pkg/network/namescheme:go_default_library",
"//pkg/network/netbinding:go_default_library",
"//pkg/network/sriov:go_default_library",
"//pkg/network/vmispec:go_default_library",
"//pkg/storage/backend-storage:go_default_library",
Expand Down
40 changes: 37 additions & 3 deletions pkg/virt-controller/services/multus_annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ import (
"kubevirt.io/client-go/log"

"kubevirt.io/kubevirt/pkg/network/namescheme"
"kubevirt.io/kubevirt/pkg/network/netbinding"
"kubevirt.io/kubevirt/pkg/network/vmispec"
virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
)

type multusNetworkAnnotationPool struct {
Expand All @@ -54,11 +56,11 @@ func (mnap multusNetworkAnnotationPool) toString() (string, error) {
return string(multusNetworksAnnotation), nil
}

func GenerateMultusCNIAnnotation(namespace string, interfaces []v1.Interface, networks []v1.Network) (string, error) {
return GenerateMultusCNIAnnotationFromNameScheme(namespace, interfaces, networks, namescheme.CreateHashedNetworkNameScheme(networks))
func GenerateMultusCNIAnnotation(namespace string, interfaces []v1.Interface, networks []v1.Network, config *virtconfig.ClusterConfig) (string, error) {
return GenerateMultusCNIAnnotationFromNameScheme(namespace, interfaces, networks, namescheme.CreateHashedNetworkNameScheme(networks), config)
}

func GenerateMultusCNIAnnotationFromNameScheme(namespace string, interfaces []v1.Interface, networks []v1.Network, networkNameScheme map[string]string) (string, error) {
func GenerateMultusCNIAnnotationFromNameScheme(namespace string, interfaces []v1.Interface, networks []v1.Network, networkNameScheme map[string]string, config *virtconfig.ClusterConfig) (string, error) {
multusNetworkAnnotationPool := multusNetworkAnnotationPool{}

for _, network := range networks {
Expand All @@ -67,6 +69,17 @@ func GenerateMultusCNIAnnotationFromNameScheme(namespace string, interfaces []v1
multusNetworkAnnotationPool.add(
newMultusAnnotationData(namespace, interfaces, network, podInterfaceName))
}

if config != nil && config.NetworkBindingPlugingsEnabled() {
if iface := vmispec.LookupInterfaceByName(interfaces, network.Name); iface.Binding != nil {
bindingPluginAnnotationData, err := newBindingPluginMultusAnnotationData(
config.GetConfig(), iface.Binding.Name, namespace, network.Name)
if err != nil {
return "", err
}
multusNetworkAnnotationPool.add(*bindingPluginAnnotationData)
}
}
}

if !multusNetworkAnnotationPool.isEmpty() {
Expand All @@ -75,6 +88,27 @@ func GenerateMultusCNIAnnotationFromNameScheme(namespace string, interfaces []v1
return "", nil
}

func newBindingPluginMultusAnnotationData(kvConfig *v1.KubeVirtConfiguration, pluginName, namespace, networkName string) (*networkv1.NetworkSelectionElement, error) {
plugin := netbinding.ReadNetBindingPluginConfiguration(kvConfig, pluginName)
if plugin == nil {
return nil, fmt.Errorf("unable to find the network binding plugin '%s' in Kubevirt configuration", pluginName)
}

netAttachDefNamespace, netAttachDefName := getNamespaceAndNetworkName(namespace, plugin.NetworkAttachmentDefinition)

// cniArgNetworkName is the CNI arg name for the VM spec network logical name.
// The binding plugin CNI should read this arg and realize which logical network it should modify.
const cniArgNetworkName = "logic-network-name"

return &networkv1.NetworkSelectionElement{
Namespace: netAttachDefNamespace,
Name: netAttachDefName,
CNIArgs: &map[string]interface{}{
cniArgNetworkName: networkName,
},
}, nil
}

func newMultusAnnotationData(namespace string, interfaces []v1.Interface, network v1.Network, podInterfaceName string) networkv1.NetworkSelectionElement {
multusIface := vmispec.LookupInterfaceByName(interfaces, network.Name)
namespace, networkName := getNamespaceAndNetworkName(namespace, network.Multus.NetworkName)
Expand Down
80 changes: 80 additions & 0 deletions pkg/virt-controller/services/multus_annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import (
networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"

v1 "kubevirt.io/api/core/v1"

"kubevirt.io/kubevirt/pkg/testutils"
virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
)

var _ = Describe("Multus annotations", func() {
Expand Down Expand Up @@ -86,4 +89,81 @@ var _ = Describe("Multus annotations", func() {
Expect(multusAnnotationPool.toString()).To(BeIdenticalTo(expectedString))
})
})

Context("Generate Multus network selection annotation", func() {
When("NetworkBindingPlugins feature enabled", func() {
It("should fail if the specified network binding plugin is not registered (specified in Kubevirt config)", func() {
vmi := &v1.VirtualMachineInstance{ObjectMeta: metav1.ObjectMeta{Name: "testvmi", Namespace: "default"}}
vmi.Spec.Networks = []v1.Network{
{Name: "default", NetworkSource: v1.NetworkSource{Pod: &v1.PodNetwork{}}}}
vmi.Spec.Domain.Devices.Interfaces = []v1.Interface{
{Name: "default", Binding: &v1.PluginBinding{Name: "test-binding"}}}

config := testsClusterConfig(map[string]v1.InterfaceBindingPlugin{
"another-test-binding": {NetworkAttachmentDefinition: "another-test-binding-net"},
})

_, err := GenerateMultusCNIAnnotation(vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, config)

Expect(err).To(HaveOccurred())
})

It("should add network binding plugin net-attach-def to multus annotation", func() {
vmi := &v1.VirtualMachineInstance{ObjectMeta: metav1.ObjectMeta{Name: "testvmi", Namespace: "default"}}
vmi.Spec.Networks = []v1.Network{
{Name: "default", NetworkSource: v1.NetworkSource{Pod: &v1.PodNetwork{}}},
{Name: "blue", NetworkSource: v1.NetworkSource{Multus: &v1.MultusNetwork{NetworkName: "test1"}}},
{Name: "red", NetworkSource: v1.NetworkSource{Multus: &v1.MultusNetwork{NetworkName: "other-namespace/test1"}}},
}
vmi.Spec.Domain.Devices.Interfaces = []v1.Interface{
{Name: "default", Binding: &v1.PluginBinding{Name: "test-binding"}},
{Name: "blue"}, {Name: "red"}}

config := testsClusterConfig(map[string]v1.InterfaceBindingPlugin{
"test-binding": {NetworkAttachmentDefinition: "test-binding-net"},
})

Expect(GenerateMultusCNIAnnotation(vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, config)).To(MatchJSON(
`[
{"name": "test-binding-net","namespace": "default", "cni-args": {"logic-network-name": "default"}},
{"name": "test1","namespace": "default","interface": "pod16477688c0e"},
{"name": "test1","namespace": "other-namespace","interface": "podb1f51a511f1"}
]`,
))
})

DescribeTable("should parse NetworkAttachmentDefinition name and namespace correctly, given",
func(netAttachDefRawName, expectedAnnot string) {
vmi := &v1.VirtualMachineInstance{ObjectMeta: metav1.ObjectMeta{Name: "testvmi", Namespace: "default"}}
vmi.Spec.Networks = []v1.Network{*v1.DefaultPodNetwork()}
vmi.Spec.Domain.Devices.Interfaces = []v1.Interface{
{Name: "default", Binding: &v1.PluginBinding{Name: "test-binding"}}}

config := testsClusterConfig(map[string]v1.InterfaceBindingPlugin{
"test-binding": {NetworkAttachmentDefinition: netAttachDefRawName},
})

Expect(GenerateMultusCNIAnnotation(vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, config)).To(MatchJSON(expectedAnnot))
},
Entry("name with no namespace", "my-binding",
`[{"namespace": "default", "name": "my-binding", "cni-args": {"logic-network-name": "default"}}]`),
Entry("name with namespace", "namespace1/my-binding",
`[{"namespace": "namespace1", "name": "my-binding", "cni-args": {"logic-network-name": "default"}}]`),
)
})
})
})

func testsClusterConfig(plugins map[string]v1.InterfaceBindingPlugin) *virtconfig.ClusterConfig {
kvConfig := &v1.KubeVirtConfiguration{
DeveloperConfiguration: &v1.DeveloperConfiguration{
FeatureGates: []string{"NetworkBindingPlugins"},
},
NetworkConfiguration: &v1.NetworkConfiguration{
Binding: plugins,
},
}
config, _, _ := testutils.NewFakeClusterConfigUsingKVConfig(kvConfig)

return config
}
8 changes: 4 additions & 4 deletions pkg/virt-controller/services/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (t *templateService) RenderMigrationManifest(vmi *v1.VirtualMachineInstance
if namescheme.PodHasOrdinalInterfaceName(NonDefaultMultusNetworksIndexedByIfaceName(pod)) {
ordinalNameScheme := namescheme.CreateOrdinalNetworkNameScheme(vmi.Spec.Networks)
multusNetworksAnnotation, err := GenerateMultusCNIAnnotationFromNameScheme(
vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, ordinalNameScheme)
vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, ordinalNameScheme, t.clusterConfig)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -483,7 +483,7 @@ func (t *templateService) renderLaunchManifest(vmi *v1.VirtualMachineInstance, i
sidecarContainerName(i), vmi, sidecarResources(vmi, t.clusterConfig), requestedHookSidecar, userId).Render(requestedHookSidecar.Command))
}

podAnnotations, err := generatePodAnnotations(vmi)
podAnnotations, err := generatePodAnnotations(vmi, t.clusterConfig)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1268,7 +1268,7 @@ func generateContainerSecurityContext(selinuxType string, container *k8sv1.Conta
container.SecurityContext.SELinuxOptions.Level = "s0"
}

func generatePodAnnotations(vmi *v1.VirtualMachineInstance) (map[string]string, error) {
func generatePodAnnotations(vmi *v1.VirtualMachineInstance, config *virtconfig.ClusterConfig) (map[string]string, error) {
annotationsSet := map[string]string{
v1.DomainAnnotation: vmi.GetObjectMeta().GetName(),
}
Expand All @@ -1282,7 +1282,7 @@ func generatePodAnnotations(vmi *v1.VirtualMachineInstance) (map[string]string,
return iface.State != v1.InterfaceStateAbsent
})
nonAbsentNets := vmispec.FilterNetworksByInterfaces(vmi.Spec.Networks, nonAbsentIfaces)
multusAnnotation, err := GenerateMultusCNIAnnotation(vmi.Namespace, nonAbsentIfaces, nonAbsentNets)
multusAnnotation, err := GenerateMultusCNIAnnotation(vmi.Namespace, nonAbsentIfaces, nonAbsentNets, config)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/virt-controller/watch/vmi.go
Original file line number Diff line number Diff line change
Expand Up @@ -2292,7 +2292,7 @@ func (c *VMIController) updateMultusAnnotation(namespace string, interfaces []vi

indexedMultusStatusIfaces := services.NonDefaultMultusNetworksIndexedByIfaceName(pod)
networkToPodIfaceMap := namescheme.CreateNetworkNameSchemeByPodNetworkStatus(networks, indexedMultusStatusIfaces)
multusAnnotations, err := services.GenerateMultusCNIAnnotationFromNameScheme(namespace, interfaces, networks, networkToPodIfaceMap)
multusAnnotations, err := services.GenerateMultusCNIAnnotationFromNameScheme(namespace, interfaces, networks, networkToPodIfaceMap, c.clusterConfig)
if err != nil {
return err
}
Expand Down
50 changes: 34 additions & 16 deletions pkg/virt-controller/watch/vmi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type PodVmIfaceStatus struct {
}

var _ = Describe("VirtualMachineInstance watcher", func() {
var config *virtconfig.ClusterConfig

var ctrl *gomock.Controller
var vmiInterface *kubecli.MockVirtualMachineInstanceInterface
Expand Down Expand Up @@ -262,7 +263,6 @@ var _ = Describe("VirtualMachineInstance watcher", func() {
},
}

var config *virtconfig.ClusterConfig
config, _, kvInformer = testutils.NewFakeClusterConfigUsingKVConfig(kubevirtFakeConfig)
pvcInformer, _ = testutils.NewFakeInformerFor(&k8sv1.PersistentVolumeClaim{})
cdiInformer, _ = testutils.NewFakeInformerFor(&cdiv1.CDIConfig{})
Expand Down Expand Up @@ -3284,8 +3284,11 @@ var _ = Describe("VirtualMachineInstance watcher", func() {
appendNetworkToVMI(
api.NewMinimalVMI(vmName), firstVMNetwork, firstVMInterface),
secondVMNetwork, secondVMInterface)
pod = NewPodForVirtualMachine(vmi, k8sv1.PodRunning)
Expect(pod.Annotations).To(HaveKey(networkv1.NetworkAttachmentAnnot))

var err error
pod, err = NewPodForVirtualMachineWithMultusAnnotations(vmi, k8sv1.PodRunning, config)
Expect(err).ToNot(HaveOccurred())

expectPodStatusUpdateFailed(pod)
})

Expand Down Expand Up @@ -3359,7 +3362,9 @@ var _ = Describe("VirtualMachineInstance watcher", func() {
Name: "blue",
NetworkSource: v1.NetworkSource{Multus: &virtv1.MultusNetwork{NetworkName: "blue-net"}}}}

pod = NewPodForVirtualMachine(vmi, k8sv1.PodRunning, testPodNetworkStatus...)
pod, err := NewPodForVirtualMachineWithMultusAnnotations(vmi, k8sv1.PodRunning, config, testPodNetworkStatus...)
Expect(err).ToNot(HaveOccurred())

prependInjectPodPatch(pod)

Expect(controller.updateMultusAnnotation(vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, pod)).To(Succeed())
Expand Down Expand Up @@ -3413,8 +3418,10 @@ var _ = Describe("VirtualMachineInstance watcher", func() {
vmIfaceStatus = append(vmIfaceStatus, *ifaceStatus[i].vmIfaceStatus)
}
}
pod, err := NewPodForVirtualMachineWithMultusAnnotations(vmi, k8sv1.PodRunning, config, podIfaceStatus...)
Expect(err).ToNot(HaveOccurred())

Expect(controller.updateInterfaceStatus(vmi, NewPodForVirtualMachine(vmi, k8sv1.PodRunning, podIfaceStatus...))).To(Succeed())
Expect(controller.updateInterfaceStatus(vmi, pod)).To(Succeed())
Expect(vmi.Status.Interfaces).To(ConsistOf(vmIfaceStatus))
},
Entry("VMI without interfaces on spec does not generate new interface status", api.NewMinimalVMI(vmName)),
Expand Down Expand Up @@ -3527,20 +3534,11 @@ func setReadyCondition(vmi *virtv1.VirtualMachineInstance, status k8sv1.Conditio
Reason: reason,
})
}
func NewPodForVirtualMachine(vmi *virtv1.VirtualMachineInstance, phase k8sv1.PodPhase, podNetworkStatus ...networkv1.NetworkStatus) *k8sv1.Pod {
multusAnnotations, _ := services.GenerateMultusCNIAnnotation(vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks)

func NewPodForVirtualMachine(vmi *virtv1.VirtualMachineInstance, phase k8sv1.PodPhase) *k8sv1.Pod {
podAnnotations := map[string]string{
virtv1.DomainAnnotation: vmi.Name,
}
if multusAnnotations != "" {
podAnnotations[networkv1.NetworkAttachmentAnnot] = multusAnnotations
}
if len(podNetworkStatus) > 0 {
podCurrentNetworks, err := json.Marshal(podNetworkStatus)
if err == nil {
podAnnotations[networkv1.NetworkStatusAnnot] = string(podCurrentNetworks)
}
}
return &k8sv1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Expand All @@ -3567,6 +3565,26 @@ func NewPodForVirtualMachine(vmi *virtv1.VirtualMachineInstance, phase k8sv1.Pod
}
}

func NewPodForVirtualMachineWithMultusAnnotations(vmi *virtv1.VirtualMachineInstance, phase k8sv1.PodPhase, config *virtconfig.ClusterConfig, podNetworkStatus ...networkv1.NetworkStatus) (*k8sv1.Pod, error) {
pod := NewPodForVirtualMachine(vmi, phase)

multusAnnotations, err := services.GenerateMultusCNIAnnotation(vmi.Namespace, vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, config)
if err != nil {
return nil, err
}
pod.Annotations[networkv1.NetworkAttachmentAnnot] = multusAnnotations

if len(podNetworkStatus) > 0 {
podCurrentNetworksJSON, err := json.Marshal(podNetworkStatus)
if err != nil {
return nil, err
}
pod.Annotations[networkv1.NetworkStatusAnnot] = string(podCurrentNetworksJSON)
}

return pod, nil
}

func NewHotplugPVC(name, namespace string, phase k8sv1.PersistentVolumeClaimPhase) *k8sv1.PersistentVolumeClaim {
t := true
return &k8sv1.PersistentVolumeClaim{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,12 @@ var CRDsValidation map[string]string = map[string]string{
binding:
additionalProperties:
properties:
networkAttachmentDefinition:
description: 'NetworkAttachmentDefinition references to a
NetworkAttachmentDefinition CR object. Format: <name>, <namespace>/<name>.
If namespace is not specified, VMI namespace is assumed.
version: 1alphav1'
type: string
sidecarImage:
description: 'SidecarImage references a container image that
runs in the virt-launcher pod. The sidecar handles (libvirt)
Expand Down
5 changes: 5 additions & 0 deletions staging/src/kubevirt.io/api/core/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2678,6 +2678,11 @@ type InterfaceBindingPlugin struct {
// The sidecar handles (libvirt) domain configuration and optional services.
// version: 1alphav1
SidecarImage string `json:"sidecarImage,omitempty"`
// NetworkAttachmentDefinition references to a NetworkAttachmentDefinition CR object.
// Format: <name>, <namespace>/<name>.
// If namespace is not specified, VMI namespace is assumed.
// version: 1alphav1
NetworkAttachmentDefinition string `json:"networkAttachmentDefinition,omitempty"`
}

// GuestAgentPing configures the guest-agent based ping probe
Expand Down

0 comments on commit e056113

Please sign in to comment.