From ea2b57a06627e440b82459e6a539d078937497b7 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Mon, 5 Nov 2018 22:49:37 +0200 Subject: [PATCH] Support multiple SR-IOV networks This patch adopts changes from the latest SR-IOV device plugin master [1] that allow us to attach a VMI to multiple SR-IOV Multus networks. (Before the change, devices allocated for multiple networks could end up in different boot order than configured.) Among other things, this patch adds read access to network attachment definitions to virt-controller role. This is to allow the controller to fetch network CRD objects and determine their respective resource names. For this same reason, it extends KubevirtClient object to include NetworkClient(). We do *not* maintain backwards compatibility with older SR-IOV device plugin releases. This is as intended because SR-IOV support is still considered experimental, and there is no good reason for us to support the device plugin release that does not support multiple networks. Updated documentation to reflect the fact that kubevirt is now compatible with device plugin master (and *not* compatible with v1.0). todo: get rid of vendor/* change [1] https://github.com/intel/sriov-network-device-plugin/pull/26 --- docs/sriov.md | 19 ++++-- manifests/dev/rbac.authorization.k8s.yaml.in | 8 +++ manifests/release/kubevirt.yaml.in | 8 +++ pkg/kubecli/generated_mock_kubevirt.go | 17 +++++- pkg/kubecli/kubecli.go | 14 +++++ pkg/kubecli/kubevirt.go | 18 ++++-- pkg/virt-api/rest/subresource_test.go | 4 +- pkg/virt-controller/services/template.go | 43 +++++++++++++- pkg/virt-controller/services/template_test.go | 58 ++++++++++++++++++- pkg/virt-controller/watch/application.go | 8 ++- pkg/virt-controller/watch/migration_test.go | 6 +- pkg/virt-controller/watch/vmi_test.go | 8 ++- pkg/virt-launcher/virtwrap/api/converter.go | 19 +++--- .../virtwrap/api/converter_test.go | 25 +++++--- .../virtwrap/api/deepcopy_generated.go | 11 +++- pkg/virt-launcher/virtwrap/manager.go | 52 ++++++++++++----- pkg/virt-launcher/virtwrap/manager_test.go | 27 +++++---- tests/vmi_multus_test.go | 14 ++++- .../versioned/fake/clientset_generated.go | 7 ++- 19 files changed, 296 insertions(+), 70 deletions(-) diff --git a/docs/sriov.md b/docs/sriov.md index 7366aea3d1a1..c0e1ab20fdb0 100644 --- a/docs/sriov.md +++ b/docs/sriov.md @@ -159,17 +159,24 @@ $ ./cluster/kubectl.sh create -f $GOPATH/src/github.com/intel/multus-cni/images/ Now, deploy SR-IOV device plugin. -Note: as of the time of writing, latest master of SR-IOV device plugin is not -compatible with kubevirt. Please check out the latest version supported by -kubevirt before building the plugin image: - - ``` $ go get -u -d github.com/intel/sriov-network-device-plugin/ $ cd $GOPATH/src/github.com/intel/sriov-network-device-plugin/images -$ git checkout v1.0 $ ./build_docker.sh $ vi images/sriovdp-daemonset.yaml # change to refer to local sriov-network-device-plugin:latest +$ cat < /etc/pcidp/config.json +{ + "resourceList": + [ + { + "resourceName": "sriov", + "rootDevices": ["05:00.0", "05:00.1"], + "sriovMode": true, + "deviceType": "vfio" + } + ] +} +EOF $ ./cluster/kubectl.sh create -f $GOPATH/src/github.com/intel/sriov-network-device-plugin/images/sriovdp-daemonset.yaml ``` diff --git a/manifests/dev/rbac.authorization.k8s.yaml.in b/manifests/dev/rbac.authorization.k8s.yaml.in index 53924a214a58..1f258a4f8e3f 100644 --- a/manifests/dev/rbac.authorization.k8s.yaml.in +++ b/manifests/dev/rbac.authorization.k8s.yaml.in @@ -218,6 +218,14 @@ rules: - '*' verbs: - '*' + - apiGroups: + - k8s.cni.cncf.io + resources: + - network-attachment-definitions + verbs: + - get + - list + - watch --- apiVersion: v1 kind: ServiceAccount diff --git a/manifests/release/kubevirt.yaml.in b/manifests/release/kubevirt.yaml.in index 38a7c02f8f4b..050afd947c20 100644 --- a/manifests/release/kubevirt.yaml.in +++ b/manifests/release/kubevirt.yaml.in @@ -314,6 +314,14 @@ rules: - '*' verbs: - '*' + - apiGroups: + - k8s.cni.cncf.io + resources: + - network-attachment-definitions + verbs: + - get + - list + - watch --- apiVersion: v1 kind: ServiceAccount diff --git a/pkg/kubecli/generated_mock_kubevirt.go b/pkg/kubecli/generated_mock_kubevirt.go index aead4ed7a933..646d5d4b3a0d 100644 --- a/pkg/kubecli/generated_mock_kubevirt.go +++ b/pkg/kubecli/generated_mock_kubevirt.go @@ -7,6 +7,7 @@ import ( time "time" gomock "github.com/golang/mock/gomock" + versioned "github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" types "k8s.io/apimachinery/pkg/types" discovery "k8s.io/client-go/discovery" @@ -41,7 +42,7 @@ import ( v1beta110 "k8s.io/client-go/kubernetes/typed/storage/v1beta1" rest "k8s.io/client-go/rest" - versioned "kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned" + versioned0 "kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned" v19 "kubevirt.io/kubevirt/pkg/api/v1" ) @@ -126,9 +127,9 @@ func (_mr *_MockKubevirtClientRecorder) RestClient() *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "RestClient") } -func (_m *MockKubevirtClient) CdiClient() versioned.Interface { +func (_m *MockKubevirtClient) CdiClient() versioned0.Interface { ret := _m.ctrl.Call(_m, "CdiClient") - ret0, _ := ret[0].(versioned.Interface) + ret0, _ := ret[0].(versioned0.Interface) return ret0 } @@ -136,6 +137,16 @@ func (_mr *_MockKubevirtClientRecorder) CdiClient() *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "CdiClient") } +func (_m *MockKubevirtClient) NetworkClient() versioned.Interface { + ret := _m.ctrl.Call(_m, "NetworkClient") + ret0, _ := ret[0].(versioned.Interface) + return ret0 +} + +func (_mr *_MockKubevirtClientRecorder) NetworkClient() *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "NetworkClient") +} + func (_m *MockKubevirtClient) Discovery() discovery.DiscoveryInterface { ret := _m.ctrl.Call(_m, "Discovery") ret0, _ := ret[0].(discovery.DiscoveryInterface) diff --git a/pkg/kubecli/kubecli.go b/pkg/kubecli/kubecli.go index 90b999ec00bc..12a4f9cb8d27 100644 --- a/pkg/kubecli/kubecli.go +++ b/pkg/kubecli/kubecli.go @@ -33,6 +33,8 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + networkclient "github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned" + cdiclient "kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned" "kubevirt.io/kubevirt/pkg/api/v1" ) @@ -73,12 +75,18 @@ func GetKubevirtSubresourceClientFromFlags(master string, kubeconfig string) (Ku return nil, err } + networkClient, err := networkclient.NewForConfig(config) + if err != nil { + return nil, err + } + return &kubevirt{ master, kubeconfig, restClient, config, cdiClient, + networkClient, coreClient, }, nil } @@ -177,12 +185,18 @@ func GetKubevirtClientFromRESTConfig(config *rest.Config) (KubevirtClient, error return nil, err } + networkClient, err := networkclient.NewForConfig(config) + if err != nil { + return nil, err + } + return &kubevirt{ master, kubeconfig, restClient, config, cdiClient, + networkClient, coreClient, }, nil } diff --git a/pkg/kubecli/kubevirt.go b/pkg/kubecli/kubevirt.go index dfbe617966a2..8e98fa439f8c 100644 --- a/pkg/kubecli/kubevirt.go +++ b/pkg/kubecli/kubevirt.go @@ -34,6 +34,8 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + networkclient "github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned" + cdiclient "kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned" "kubevirt.io/kubevirt/pkg/api/v1" ) @@ -46,15 +48,17 @@ type KubevirtClient interface { ServerVersion() *ServerVersion RestClient() *rest.RESTClient CdiClient() cdiclient.Interface + NetworkClient() networkclient.Interface kubernetes.Interface } type kubevirt struct { - master string - kubeconfig string - restClient *rest.RESTClient - config *rest.Config - cdiClient *cdiclient.Clientset + master string + kubeconfig string + restClient *rest.RESTClient + config *rest.Config + cdiClient *cdiclient.Clientset + networkClient *networkclient.Clientset *kubernetes.Clientset } @@ -62,6 +66,10 @@ func (k kubevirt) CdiClient() cdiclient.Interface { return k.cdiClient } +func (k kubevirt) NetworkClient() networkclient.Interface { + return k.networkClient +} + func (k kubevirt) RestClient() *rest.RESTClient { return k.restClient } diff --git a/pkg/virt-api/rest/subresource_test.go b/pkg/virt-api/rest/subresource_test.go index b5c29ca8097f..b792ae0a1376 100644 --- a/pkg/virt-api/rest/subresource_test.go +++ b/pkg/virt-api/rest/subresource_test.go @@ -84,7 +84,7 @@ var _ = Describe("VirtualMachineInstance Subresources", func() { vmi := v1.NewMinimalVMI("testvmi") vmi.Status.Phase = v1.Running vmi.ObjectMeta.SetUID(uuid.NewUUID()) - templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", configCache, pvcCache) + templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", configCache, pvcCache, app.VirtCli) pod, err := templateService.RenderLaunchManifest(vmi) Expect(err).ToNot(HaveOccurred()) @@ -256,7 +256,7 @@ var _ = Describe("VirtualMachineInstance Subresources", func() { vmi.Status.Phase = v1.Running vmi.ObjectMeta.SetUID(uuid.NewUUID()) - templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", configCache, pvcCache) + templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", configCache, pvcCache, app.VirtCli) pod, err := templateService.RenderLaunchManifest(vmi) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/virt-controller/services/template.go b/pkg/virt-controller/services/template.go index 5ce4775b339d..692a8e82a024 100644 --- a/pkg/virt-controller/services/template.go +++ b/pkg/virt-controller/services/template.go @@ -30,9 +30,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" + networkv1 "github.com/phoracek/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + "kubevirt.io/kubevirt/pkg/api/v1" "kubevirt.io/kubevirt/pkg/config" "kubevirt.io/kubevirt/pkg/hooks" + "kubevirt.io/kubevirt/pkg/kubecli" "kubevirt.io/kubevirt/pkg/log" "kubevirt.io/kubevirt/pkg/precond" registrydisk "kubevirt.io/kubevirt/pkg/registry-disk" @@ -52,6 +55,8 @@ const VhostNetDevice = "devices.kubevirt.io/vhost-net" const CAP_NET_ADMIN = "NET_ADMIN" const CAP_SYS_NICE = "SYS_NICE" +const MULTUS_RESOURCE_NAME_ANNOTATION = "k8s.v1.cni.cncf.io/resourceName" + type TemplateService interface { RenderLaunchManifest(*v1.VirtualMachineInstance) (*k8sv1.Pod, error) } @@ -63,6 +68,7 @@ type templateService struct { imagePullSecret string configMapStore cache.Store persistentVolumeClaimStore cache.Store + virtClient kubecli.KubevirtClient } type PvcNotFoundError error @@ -592,6 +598,15 @@ func (t *templateService) RenderLaunchManifest(vmi *v1.VirtualMachineInstance) ( podLabels[v1.AppLabel] = "virt-launcher" podLabels[v1.CreatedByLabel] = string(vmi.UID) + networkToResourceMap, err := getNetworkToResourceMap(t.virtClient, vmi) + if err != nil { + return nil, err + } + for networkName, resourceName := range networkToResourceMap { + varName := fmt.Sprintf("KUBEVIRT_RESOURCE_NAME_%s", networkName) + container.Env = append(container.Env, k8sv1.EnvVar{Name: varName, Value: resourceName}) + + } containers = append(containers, container) for i, requestedHookSidecar := range requestedHookSidecarList { @@ -796,6 +811,30 @@ func getPortsFromVMI(vmi *v1.VirtualMachineInstance) []k8sv1.ContainerPort { return ports } +func getResourceNameForNetwork(network *networkv1.NetworkAttachmentDefinition) string { + resourceName, ok := network.Annotations[MULTUS_RESOURCE_NAME_ANNOTATION] + if ok { + return resourceName + } + return "" // meaning the network is not served by resources +} + +func getNetworkToResourceMap(virtClient kubecli.KubevirtClient, vmi *v1.VirtualMachineInstance) (networkToResourceMap map[string]string, err error) { + networkToResourceMap = make(map[string]string) + for _, network := range vmi.Spec.Networks { + if network.Multus != nil { + networkName := network.Multus.NetworkName + namespace := precond.MustNotBeEmpty(vmi.GetObjectMeta().GetNamespace()) + crd, err := virtClient.NetworkClient().K8sCniCncfIo().NetworkAttachmentDefinitions(namespace).Get(networkName, metav1.GetOptions{}) + if err != nil { + return map[string]string{}, fmt.Errorf("Failed to locate network attachment definition %s", networkName) + } + networkToResourceMap[network.Name] = getResourceNameForNetwork(crd) + } + } + return +} + func getCniInterfaceList(vmi *v1.VirtualMachineInstance) (ifaceListString string, cniAnnotation string) { ifaceList := make([]string, 0) @@ -824,7 +863,8 @@ func NewTemplateService(launcherImage string, ephemeralDiskDir string, imagePullSecret string, configMapCache cache.Store, - persistentVolumeClaimCache cache.Store) TemplateService { + persistentVolumeClaimCache cache.Store, + virtClient kubecli.KubevirtClient) TemplateService { precond.MustNotBeEmpty(launcherImage) svc := templateService{ @@ -834,6 +874,7 @@ func NewTemplateService(launcherImage string, imagePullSecret: imagePullSecret, configMapStore: configMapCache, persistentVolumeClaimStore: persistentVolumeClaimCache, + virtClient: virtClient, } return &svc } diff --git a/pkg/virt-controller/services/template_test.go b/pkg/virt-controller/services/template_test.go index 7e6af0db4537..ce8f2dc6d0d3 100644 --- a/pkg/virt-controller/services/template_test.go +++ b/pkg/virt-controller/services/template_test.go @@ -17,25 +17,31 @@ * */ -package services_test +package services import ( "errors" "strconv" "testing" + "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo" "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" + networkv1 "github.com/phoracek/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + fakenetworkclient "github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned/fake" kubev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/tools/cache" "kubevirt.io/kubevirt/pkg/api/v1" "kubevirt.io/kubevirt/pkg/hooks" + "kubevirt.io/kubevirt/pkg/kubecli" "kubevirt.io/kubevirt/pkg/log" - . "kubevirt.io/kubevirt/pkg/virt-controller/services" ) const namespaceKubevirt = "kubevirt" @@ -48,6 +54,9 @@ var _ = Describe("Template", func() { var cmCache cache.Indexer var svc TemplateService + ctrl := gomock.NewController(GinkgoT()) + virtClient := kubecli.NewMockKubevirtClient(ctrl) + BeforeEach(func() { cmCache = cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, nil) svc = NewTemplateService("kubevirt/virt-launcher", @@ -55,7 +64,32 @@ var _ = Describe("Template", func() { "/var/run/kubevirt-ephemeral-disks", "pull-secret-1", cmCache, - pvcCache) + pvcCache, + virtClient) + + // Set up mock clients + networkClient := fakenetworkclient.NewSimpleClientset() + virtClient.EXPECT().NetworkClient().Return(networkClient).AnyTimes() + // Sadly, we cannot pass desired attachment objects into + // Clientset constructor because UnsafeGuessKindToResource + // calculates incorrect object kind (without dashes). Instead + // of that, we use tracker Create function to register objects + // under explicitly defined schema name + gvr := schema.GroupVersionResource{ + Group: "k8s.cni.cncf.io", + Version: "v1", + Resource: "network-attachment-definitions", + } + for _, name := range []string{"default", "test1"} { + network := &networkv1.NetworkAttachmentDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + } + err := networkClient.Tracker().Create(gvr, network, "default") + Expect(err).To(Not(HaveOccurred())) + } }) Describe("Rendering", func() { @@ -1224,6 +1258,24 @@ var _ = Describe("Template", func() { }) }) +var _ = Describe("getResourceNameForNetwork", func() { + It("should return empty string when resource name is not specified", func() { + network := &networkv1.NetworkAttachmentDefinition{} + Expect(getResourceNameForNetwork(network)).To(Equal("")) + }) + + It("should return resource name if specified", func() { + network := &networkv1.NetworkAttachmentDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + MULTUS_RESOURCE_NAME_ANNOTATION: "fake.com/fakeResource", + }, + }, + } + Expect(getResourceNameForNetwork(network)).To(Equal("fake.com/fakeResource")) + }) +}) + func True() *bool { b := true return &b diff --git a/pkg/virt-controller/watch/application.go b/pkg/virt-controller/watch/application.go index e18cebbc5621..cafe6da51358 100644 --- a/pkg/virt-controller/watch/application.go +++ b/pkg/virt-controller/watch/application.go @@ -264,6 +264,11 @@ func (vca *VirtControllerApp) getNewRecorder(namespace string, componentName str func (vca *VirtControllerApp) initCommon() { var err error + virtClient, err := kubecli.GetKubevirtClient() + if err != nil { + golog.Fatal(err) + } + registrydisk.SetLocalDirectory(vca.ephemeralDiskDir + "/registry-disk-data") if err != nil { golog.Fatal(err) @@ -273,7 +278,8 @@ func (vca *VirtControllerApp) initCommon() { vca.ephemeralDiskDir, vca.imagePullSecret, vca.configMapCache, - vca.persistentVolumeClaimCache) + vca.persistentVolumeClaimCache, + virtClient) vca.vmiController = NewVMIController(vca.templateService, vca.vmiInformer, vca.podInformer, vca.vmiRecorder, vca.clientSet, vca.configMapInformer, vca.dataVolumeInformer) recorder := vca.getNewRecorder(k8sv1.NamespaceAll, "node-controller") diff --git a/pkg/virt-controller/watch/migration_test.go b/pkg/virt-controller/watch/migration_test.go index 965be2543756..5448d41004e4 100644 --- a/pkg/virt-controller/watch/migration_test.go +++ b/pkg/virt-controller/watch/migration_test.go @@ -24,6 +24,7 @@ import ( . "github.com/onsi/ginkgo" "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" + fakenetworkclient "github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned/fake" k8sv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -60,6 +61,7 @@ var _ = Describe("Migration watcher", func() { var podFeeder *testutils.PodFeeder var virtClient *kubecli.MockKubevirtClient var kubeClient *fake.Clientset + var networkClient *fakenetworkclient.Clientset var configMapInformer cache.SharedIndexInformer var pvcInformer cache.SharedIndexInformer @@ -163,7 +165,7 @@ var _ = Describe("Migration watcher", func() { pvcInformer, _ = testutils.NewFakeInformerFor(&k8sv1.PersistentVolumeClaim{}) controller = NewMigrationController( - services.NewTemplateService("a", "b", "c", "d", configMapInformer.GetStore(), pvcInformer.GetStore()), + services.NewTemplateService("a", "b", "c", "d", configMapInformer.GetStore(), pvcInformer.GetStore(), virtClient), vmiInformer, podInformer, migrationInformer, @@ -179,6 +181,8 @@ var _ = Describe("Migration watcher", func() { virtClient.EXPECT().VirtualMachineInstanceMigration(k8sv1.NamespaceDefault).Return(migrationInterface).AnyTimes() virtClient.EXPECT().VirtualMachineInstance(k8sv1.NamespaceDefault).Return(vmiInterface).AnyTimes() virtClient.EXPECT().CoreV1().Return(kubeClient.CoreV1()).AnyTimes() + networkClient = fakenetworkclient.NewSimpleClientset() + virtClient.EXPECT().NetworkClient().Return(networkClient).AnyTimes() // Make sure that all unexpected calls to kubeClient will fail kubeClient.Fake.PrependReactor("*", "*", func(action testing.Action) (handled bool, obj runtime.Object, err error) { diff --git a/pkg/virt-controller/watch/vmi_test.go b/pkg/virt-controller/watch/vmi_test.go index d7ff7f533722..c3a9a6d67ed6 100644 --- a/pkg/virt-controller/watch/vmi_test.go +++ b/pkg/virt-controller/watch/vmi_test.go @@ -37,7 +37,10 @@ import ( "k8s.io/client-go/tools/cache/testing" "k8s.io/client-go/tools/record" + fakenetworkclient "github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned/fake" + cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/datavolumecontroller/v1alpha1" + "kubevirt.io/kubevirt/pkg/api/v1" "kubevirt.io/kubevirt/pkg/kubecli" "kubevirt.io/kubevirt/pkg/log" @@ -61,6 +64,7 @@ var _ = Describe("VirtualMachineInstance watcher", func() { var podFeeder *testutils.PodFeeder var virtClient *kubecli.MockKubevirtClient var kubeClient *fake.Clientset + var networkClient *fakenetworkclient.Clientset var configMapInformer cache.SharedIndexInformer var pvcInformer cache.SharedIndexInformer @@ -163,7 +167,7 @@ var _ = Describe("VirtualMachineInstance watcher", func() { configMapInformer, _ = testutils.NewFakeInformerFor(&k8sv1.ConfigMap{}) pvcInformer, _ = testutils.NewFakeInformerFor(&k8sv1.PersistentVolumeClaim{}) controller = NewVMIController( - services.NewTemplateService("a", "b", "c", "d", configMapInformer.GetStore(), pvcInformer.GetStore()), + services.NewTemplateService("a", "b", "c", "d", configMapInformer.GetStore(), pvcInformer.GetStore(), virtClient), vmiInformer, podInformer, recorder, @@ -180,6 +184,8 @@ var _ = Describe("VirtualMachineInstance watcher", func() { virtClient.EXPECT().VirtualMachineInstance(k8sv1.NamespaceDefault).Return(vmiInterface).AnyTimes() kubeClient = fake.NewSimpleClientset() virtClient.EXPECT().CoreV1().Return(kubeClient.CoreV1()).AnyTimes() + networkClient = fakenetworkclient.NewSimpleClientset() + virtClient.EXPECT().NetworkClient().Return(networkClient).AnyTimes() // Make sure that all unexpected calls to kubeClient will fail kubeClient.Fake.PrependReactor("*", "*", func(action testing.Action) (handled bool, obj runtime.Object, err error) { diff --git a/pkg/virt-launcher/virtwrap/api/converter.go b/pkg/virt-launcher/virtwrap/api/converter.go index de130a178540..05a81b7758e9 100644 --- a/pkg/virt-launcher/virtwrap/api/converter.go +++ b/pkg/virt-launcher/virtwrap/api/converter.go @@ -57,7 +57,7 @@ type ConverterContext struct { VirtualMachine *v1.VirtualMachineInstance CPUSet []int IsBlockPVC map[string]bool - SRIOVDevices []string + SRIOVDevices map[string][]string } func Convert_v1_Disk_To_api_Disk(diskDevice *v1.Disk, disk *Disk, devicePerBus map[string]int, numQueues *uint) error { @@ -508,11 +508,13 @@ func Convert_v1_FeatureHyperv_To_api_FeatureHyperv(source *v1.FeatureHyperv, hyp return nil } -func popSRIOVPCIAddress(addrs []string) (string, []string, error) { - if len(addrs) > 0 { - return addrs[0], addrs[1:], nil +func popSRIOVPCIAddress(networkName string, addrsMap map[string][]string) (string, map[string][]string, error) { + if len(addrsMap[networkName]) > 0 { + addr := addrsMap[networkName][0] + addrsMap[networkName] = addrsMap[networkName][1:] + return addr, addrsMap, nil } - return "", addrs, fmt.Errorf("no more SR-IOV PCI addresses to allocate") + return "", addrsMap, fmt.Errorf("no more SR-IOV PCI addresses to allocate") } func Convert_v1_VirtualMachine_To_api_Domain(vmi *v1.VirtualMachineInstance, domain *Domain, c *ConverterContext) (err error) { @@ -869,7 +871,10 @@ func Convert_v1_VirtualMachine_To_api_Domain(vmi *v1.VirtualMachineInstance, dom networks[network.Name] = network.DeepCopy() } - sriovPciAddresses := append([]string{}, c.SRIOVDevices...) + sriovPciAddresses := make(map[string][]string) + for key, value := range c.SRIOVDevices { + sriovPciAddresses[key] = append([]string{}, value...) + } for _, iface := range vmi.Spec.Domain.Devices.Interfaces { net, isExist := networks[iface.Name] @@ -879,7 +884,7 @@ func Convert_v1_VirtualMachine_To_api_Domain(vmi *v1.VirtualMachineInstance, dom if iface.SRIOV != nil { var pciAddr string - pciAddr, sriovPciAddresses, err = popSRIOVPCIAddress(sriovPciAddresses) + pciAddr, sriovPciAddresses, err = popSRIOVPCIAddress(iface.Name, sriovPciAddresses) if err != nil { return err } diff --git a/pkg/virt-launcher/virtwrap/api/converter_test.go b/pkg/virt-launcher/virtwrap/api/converter_test.go index 21471b48210d..baf530825604 100644 --- a/pkg/virt-launcher/virtwrap/api/converter_test.go +++ b/pkg/virt-launcher/virtwrap/api/converter_test.go @@ -536,7 +536,7 @@ var _ = Describe("Converter", func() { }, UseEmulation: true, IsBlockPVC: isBlockPVCMap, - SRIOVDevices: []string{}, + SRIOVDevices: map[string][]string{}, } }) @@ -1372,10 +1372,13 @@ var _ = Describe("Converter", func() { } vmi.Spec.Networks = append(vmi.Spec.Networks, sriovNetwork2) - It("should convert sriov interface into host device", func() { + It("should convert sriov interfaces into host devices", func() { c := &ConverterContext{ UseEmulation: true, - SRIOVDevices: []string{"0000:81:11.1", "0000:81:11.2"}, + SRIOVDevices: map[string][]string{ + "sriov": []string{"0000:81:11.1"}, + "sriov2": []string{"0000:81:11.2"}, + }, } domain := vmiToDomain(vmi, c) @@ -1399,17 +1402,21 @@ var _ = Describe("Converter", func() { }) var _ = Describe("popSRIOVPCIAddress", func() { - It("fails on empty slice", func() { - _, _, err := popSRIOVPCIAddress([]string{}) + It("fails on empty map", func() { + _, _, err := popSRIOVPCIAddress("testnet", map[string][]string{}) + Expect(err).To(HaveOccurred()) + }) + It("fails on empty map entry", func() { + _, _, err := popSRIOVPCIAddress("testnet", map[string][]string{"testnet": []string{}}) Expect(err).To(HaveOccurred()) }) It("pops the next address from a non-empty slice", func() { - addrs := []string{"0000:81:11.1", "0001:02:00.0"} - addr, rest, err := popSRIOVPCIAddress(addrs) + addrsMap := map[string][]string{"testnet": []string{"0000:81:11.1", "0001:02:00.0"}} + addr, rest, err := popSRIOVPCIAddress("testnet", addrsMap) Expect(err).ToNot(HaveOccurred()) Expect(addr).To(Equal("0000:81:11.1")) - Expect(len(rest)).To(Equal(1)) - Expect(rest[0]).To(Equal("0001:02:00.0")) + Expect(len(rest["testnet"])).To(Equal(1)) + Expect(rest["testnet"][0]).To(Equal("0001:02:00.0")) }) }) diff --git a/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go b/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go index a93518699ec0..361a3f1403b4 100644 --- a/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go +++ b/pkg/virt-launcher/virtwrap/api/deepcopy_generated.go @@ -623,8 +623,15 @@ func (in *ConverterContext) DeepCopyInto(out *ConverterContext) { } if in.SRIOVDevices != nil { in, out := &in.SRIOVDevices, &out.SRIOVDevices - *out = make([]string, len(*in)) - copy(*out, *in) + *out = make(map[string][]string, len(*in)) + for key, val := range *in { + if val == nil { + (*out)[key] = nil + } else { + (*out)[key] = make([]string, len(val)) + copy((*out)[key], val) + } + } } return } diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index 31236c85b53e..79e806683878 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -381,26 +381,46 @@ func (l *LibvirtDomainManager) preStartHook(vmi *v1.VirtualMachineInstance, doma return domain, err } -// This function parses the SRIOV-VF-PCI-ADDR variable that is set by SR-IOV -// device plugin listing PCI IDs for devices allocated to the pod. The format -// is as follows: +// This function parses variables that are set by SR-IOV device plugin listing +// PCI IDs for devices allocated to the pod. It also parses variables that +// virt-controller sets mapping network names to their respective resource +// names (if any). // +// Format for PCI ID variables set by SR-IOV DP is: // "": for no allocated devices -// "0000:81:11.1,": for a single device -// "0000:81:11.1,0000:81:11.2[,...]": for multiple devices -func getSRIOVPCIAddresses() []string { - pciAddrString, isSet := os.LookupEnv("SRIOV-VF-PCI-ADDR") - if isSet { - addrs := strings.Split(pciAddrString, ",") - naddrs := len(addrs) - if naddrs > 0 { - if addrs[naddrs-1] == "" { - addrs = addrs[:naddrs-1] +// ="0000:81:11.1,": for a single device +// ="0000:81:11.1,0000:81:11.2[,...]": for multiple devices +// +// Format for network to resource mapping variables is: +// KUBEVIRT_RESOURCE_NAME_= +// +func getSRIOVPCIAddresses(ifaces []v1.Interface) map[string][]string { + networkToAddressesMap := map[string][]string{} + for _, iface := range ifaces { + networkToAddressesMap[iface.Name] = []string{} + varName := fmt.Sprintf("KUBEVIRT_RESOURCE_NAME_%s", iface.Name) + resourceName, isSet := os.LookupEnv(varName) + if isSet { + // Intel SR-IOV device plugin truncates resource name + // by cutting off the 'intel.com/' suffix + varName := resourceName + if strings.Contains(varName, "/") { + varName = strings.Split(resourceName, "/")[1] + } + pciAddrString, isSet := os.LookupEnv(varName) + if isSet { + addrs := strings.Split(pciAddrString, ",") + naddrs := len(addrs) + if naddrs > 0 { + if addrs[naddrs-1] == "" { + addrs = addrs[:naddrs-1] + } + } + networkToAddressesMap[iface.Name] = addrs } } - return addrs } - return []string{} + return networkToAddressesMap } func (l *LibvirtDomainManager) SyncVMI(vmi *v1.VirtualMachineInstance, useEmulation bool) (*api.DomainSpec, error) { @@ -436,7 +456,7 @@ func (l *LibvirtDomainManager) SyncVMI(vmi *v1.VirtualMachineInstance, useEmulat UseEmulation: useEmulation, CPUSet: podCPUSet, IsBlockPVC: isBlockPVCMap, - SRIOVDevices: getSRIOVPCIAddresses(), + SRIOVDevices: getSRIOVPCIAddresses(vmi.Spec.Domain.Devices.Interfaces), } if err := api.Convert_v1_VirtualMachine_To_api_Domain(vmi, domain, c); err != nil { logger.Error("Conversion failed.") diff --git a/pkg/virt-launcher/virtwrap/manager_test.go b/pkg/virt-launcher/virtwrap/manager_test.go index 818f9097ea97..d72e76a4ed84 100644 --- a/pkg/virt-launcher/virtwrap/manager_test.go +++ b/pkg/virt-launcher/virtwrap/manager_test.go @@ -259,21 +259,28 @@ var _ = Describe("Manager", func() { }) var _ = Describe("getSRIOVPCIAddresses", func() { - It("returns empty slice", func() { - Expect(len(getSRIOVPCIAddresses())).To(Equal(0)) + It("returns empty map when empty interfaces", func() { + Expect(len(getSRIOVPCIAddresses([]v1.Interface{}))).To(Equal(0)) + }) + It("returns map with empty device id list when variables are not set", func() { + addrs := getSRIOVPCIAddresses([]v1.Interface{v1.Interface{Name: "testnet"}}) + Expect(len(addrs)).To(Equal(1)) + Expect(len(addrs["testnet"])).To(Equal(0)) }) It("gracefully handles trailing comma", func() { - os.Setenv("SRIOV-VF-PCI-ADDR", "0000:81:11.1,") - addrs := getSRIOVPCIAddresses() + os.Setenv("testnet_pool", "0000:81:11.1,") + os.Setenv("KUBEVIRT_RESOURCE_NAME_testnet", "testnet_pool") + addrs := getSRIOVPCIAddresses([]v1.Interface{v1.Interface{Name: "testnet"}}) Expect(len(addrs)).To(Equal(1)) - Expect(addrs[0]).To(Equal("0000:81:11.1")) + Expect(addrs["testnet"][0]).To(Equal("0000:81:11.1")) }) It("returns multiple PCI addresses", func() { - os.Setenv("SRIOV-VF-PCI-ADDR", "0000:81:11.1,0001:02:00.0") - addrs := getSRIOVPCIAddresses() - Expect(len(addrs)).To(Equal(2)) - Expect(addrs[0]).To(Equal("0000:81:11.1")) - Expect(addrs[1]).To(Equal("0001:02:00.0")) + os.Setenv("testnet_pool", "0000:81:11.1,0001:02:00.0") + os.Setenv("KUBEVIRT_RESOURCE_NAME_testnet", "testnet_pool") + addrs := getSRIOVPCIAddresses([]v1.Interface{v1.Interface{Name: "testnet"}}) + Expect(len(addrs["testnet"])).To(Equal(2)) + Expect(addrs["testnet"][0]).To(Equal("0000:81:11.1")) + Expect(addrs["testnet"][1]).To(Equal("0001:02:00.0")) }) }) diff --git a/tests/vmi_multus_test.go b/tests/vmi_multus_test.go index 793d1210717d..bb91f3c99b7a 100644 --- a/tests/vmi_multus_test.go +++ b/tests/vmi_multus_test.go @@ -208,9 +208,19 @@ var _ = Describe("Multus Networking", func() { Expect(err).ToNot(HaveOccurred()) tests.WaitUntilVMIReady(vmiOne, tests.LoggedInFedoraExpecter) - By("checking default interface is present") + By("checking KUBEVIRT_RESOURCE_NAME_ variable is defined in pod") vmiPod := tests.GetRunningPodByVirtualMachineInstance(vmiOne, tests.NamespaceTestDefault) - _, err := tests.ExecuteCommandOnPod( + out, err := tests.ExecuteCommandOnPod( + virtClient, + vmiPod, + "compute", + []string{"sh", "-c", "echo $KUBEVIRT_RESOURCE_NAME_sriov"}, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(out).To(Equal("intel.com/sriov\n")) + + By("checking default interface is present") + _, err = tests.ExecuteCommandOnPod( virtClient, vmiPod, "compute", diff --git a/vendor/github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned/fake/clientset_generated.go b/vendor/github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned/fake/clientset_generated.go index 75cff3856794..39fa538d7efe 100644 --- a/vendor/github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned/fake/clientset_generated.go +++ b/vendor/github.com/phoracek/network-attachment-definition-client/pkg/client/clientset/versioned/fake/clientset_generated.go @@ -41,7 +41,7 @@ func NewSimpleClientset(objects ...runtime.Object) *Clientset { } } - cs := &Clientset{} + cs := &Clientset{tracker: o} cs.discovery = &fakediscovery.FakeDiscovery{Fake: &cs.Fake} cs.AddReactor("*", "*", testing.ObjectReaction(o)) cs.AddWatchReactor("*", func(action testing.Action) (handled bool, ret watch.Interface, err error) { @@ -63,12 +63,17 @@ func NewSimpleClientset(objects ...runtime.Object) *Clientset { type Clientset struct { testing.Fake discovery *fakediscovery.FakeDiscovery + tracker testing.ObjectTracker } func (c *Clientset) Discovery() discovery.DiscoveryInterface { return c.discovery } +func (c *Clientset) Tracker() testing.ObjectTracker { + return c.tracker +} + var _ clientset.Interface = &Clientset{} // K8sCniCncfIoV1 retrieves the K8sCniCncfIoV1Client