From 80afa3734e8584382adfcff573f418cf10c012b7 Mon Sep 17 00:00:00 2001 From: Ashley Schuett Date: Thu, 22 Jul 2021 15:15:30 +0200 Subject: [PATCH 1/2] update virt-handler with maxDevices Signed-off-by: Ashley Schuett --- api/openapi-spec/swagger.json | 4 ++++ manifests/generated/kv-resource.yaml | 4 ++++ pkg/virt-operator/resource/apply/apps.go | 15 +++++++++++++++ pkg/virt-operator/resource/apply/reconcile.go | 1 + .../generate/components/validations_generated.go | 2 ++ .../client-go/api/v1/deepcopy_generated.go | 5 +++++ .../client-go/api/v1/openapi_generated.go | 6 ++++++ staging/src/kubevirt.io/client-go/api/v1/types.go | 11 ++++++----- .../apis/snapshot/v1alpha1/openapi_generated.go | 6 ++++++ 9 files changed, 49 insertions(+), 5 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 34e3536e1338..b3e79561cc85 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -10871,6 +10871,10 @@ "items": { "type": "string" } + }, + "virtualMachineInstancesPerNode": { + "type": "integer", + "format": "int32" } } }, diff --git a/manifests/generated/kv-resource.yaml b/manifests/generated/kv-resource.yaml index 2978d95cc4c0..95e4745c8299 100644 --- a/manifests/generated/kv-resource.yaml +++ b/manifests/generated/kv-resource.yaml @@ -295,6 +295,8 @@ spec: items: type: string type: array + virtualMachineInstancesPerNode: + type: integer type: object customizeComponents: properties: @@ -2192,6 +2194,8 @@ spec: items: type: string type: array + virtualMachineInstancesPerNode: + type: integer type: object customizeComponents: properties: diff --git a/pkg/virt-operator/resource/apply/apps.go b/pkg/virt-operator/resource/apply/apps.go index 6cc5c1d0101c..dc29c61d9388 100644 --- a/pkg/virt-operator/resource/apply/apps.go +++ b/pkg/virt-operator/resource/apply/apps.go @@ -11,6 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + v1 "kubevirt.io/client-go/api/v1" "kubevirt.io/client-go/log" "kubevirt.io/kubevirt/pkg/virt-operator/resource/generate/components" ) @@ -89,6 +90,10 @@ func (r *Reconciler) syncDaemonSet(daemonSet *appsv1.DaemonSet) error { injectOperatorMetadata(kv, &daemonSet.Spec.Template.ObjectMeta, imageTag, imageRegistry, id, false) injectPlacementMetadata(kv.Spec.Workloads, &daemonSet.Spec.Template.Spec) + if daemonSet.GetName() == "virt-handler" { + setMaxDevices(r.kv, daemonSet) + } + var cachedDaemonSet *appsv1.DaemonSet obj, exists, _ := r.stores.DaemonSetCache.Get(daemonSet) @@ -140,6 +145,16 @@ func (r *Reconciler) syncDaemonSet(daemonSet *appsv1.DaemonSet) error { return nil } +func setMaxDevices(kv *v1.KubeVirt, vh *appsv1.DaemonSet) { + if kv.Spec.Configuration.VirtualMachineInstancesPerNode == nil { + return + } + + vh.Spec.Template.Spec.Containers[0].Command = append(vh.Spec.Template.Spec.Containers[0].Command, + "--maxDevices", + fmt.Sprintf("%d", *kv.Spec.Configuration.VirtualMachineInstancesPerNode)) +} + func (r *Reconciler) syncPodDisruptionBudgetForDeployment(deployment *appsv1.Deployment) error { kv := r.kv podDisruptionBudget := components.NewPodDisruptionBudgetForDeployment(deployment) diff --git a/pkg/virt-operator/resource/apply/reconcile.go b/pkg/virt-operator/resource/apply/reconcile.go index 6764efd156de..4bf144f9f35f 100644 --- a/pkg/virt-operator/resource/apply/reconcile.go +++ b/pkg/virt-operator/resource/apply/reconcile.go @@ -30,6 +30,7 @@ import ( "github.com/blang/semver" promv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1" secv1 "github.com/openshift/api/security/v1" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" diff --git a/pkg/virt-operator/resource/generate/components/validations_generated.go b/pkg/virt-operator/resource/generate/components/validations_generated.go index 771cc3749c4b..81d13a9bacfe 100644 --- a/pkg/virt-operator/resource/generate/components/validations_generated.go +++ b/pkg/virt-operator/resource/generate/components/validations_generated.go @@ -715,6 +715,8 @@ var CRDsValidation map[string]string = map[string]string{ items: type: string type: array + virtualMachineInstancesPerNode: + type: integer type: object customizeComponents: properties: diff --git a/staging/src/kubevirt.io/client-go/api/v1/deepcopy_generated.go b/staging/src/kubevirt.io/client-go/api/v1/deepcopy_generated.go index 76aa55a19087..45d06b563e4c 100644 --- a/staging/src/kubevirt.io/client-go/api/v1/deepcopy_generated.go +++ b/staging/src/kubevirt.io/client-go/api/v1/deepcopy_generated.go @@ -2015,6 +2015,11 @@ func (in *KubeVirtConfiguration) DeepCopyInto(out *KubeVirtConfiguration) { (*out)[key] = val } } + if in.VirtualMachineInstancesPerNode != nil { + in, out := &in.VirtualMachineInstancesPerNode, &out.VirtualMachineInstancesPerNode + *out = new(int) + **out = **in + } return } diff --git a/staging/src/kubevirt.io/client-go/api/v1/openapi_generated.go b/staging/src/kubevirt.io/client-go/api/v1/openapi_generated.go index f2d900e1e70c..5d5a20e7714a 100644 --- a/staging/src/kubevirt.io/client-go/api/v1/openapi_generated.go +++ b/staging/src/kubevirt.io/client-go/api/v1/openapi_generated.go @@ -21595,6 +21595,12 @@ func schema_kubevirtio_client_go_api_v1_KubeVirtConfiguration(ref common.Referen }, }, }, + "virtualMachineInstancesPerNode": { + SchemaProps: spec.SchemaProps{ + Type: []string{"integer"}, + Format: "int32", + }, + }, }, }, }, diff --git a/staging/src/kubevirt.io/client-go/api/v1/types.go b/staging/src/kubevirt.io/client-go/api/v1/types.go index 1b1b93ee0180..98b479d84be5 100644 --- a/staging/src/kubevirt.io/client-go/api/v1/types.go +++ b/staging/src/kubevirt.io/client-go/api/v1/types.go @@ -1874,11 +1874,12 @@ type KubeVirtConfiguration struct { DefaultRuntimeClass string `json:"defaultRuntimeClass,omitempty"` SMBIOSConfig *SMBiosConfiguration `json:"smbios,omitempty"` // deprecated - SupportedGuestAgentVersions []string `json:"supportedGuestAgentVersions,omitempty"` - MemBalloonStatsPeriod *uint32 `json:"memBalloonStatsPeriod,omitempty"` - PermittedHostDevices *PermittedHostDevices `json:"permittedHostDevices,omitempty"` - MinCPUModel string `json:"minCPUModel,omitempty"` - ObsoleteCPUModels map[string]bool `json:"obsoleteCPUModels,omitempty"` + SupportedGuestAgentVersions []string `json:"supportedGuestAgentVersions,omitempty"` + MemBalloonStatsPeriod *uint32 `json:"memBalloonStatsPeriod,omitempty"` + PermittedHostDevices *PermittedHostDevices `json:"permittedHostDevices,omitempty"` + MinCPUModel string `json:"minCPUModel,omitempty"` + ObsoleteCPUModels map[string]bool `json:"obsoleteCPUModels,omitempty"` + VirtualMachineInstancesPerNode *int `json:"virtualMachineInstancesPerNode,omitempty"` } // diff --git a/staging/src/kubevirt.io/client-go/apis/snapshot/v1alpha1/openapi_generated.go b/staging/src/kubevirt.io/client-go/apis/snapshot/v1alpha1/openapi_generated.go index 5ce3003777d0..95ad285b2581 100644 --- a/staging/src/kubevirt.io/client-go/apis/snapshot/v1alpha1/openapi_generated.go +++ b/staging/src/kubevirt.io/client-go/apis/snapshot/v1alpha1/openapi_generated.go @@ -16653,6 +16653,12 @@ func schema_kubevirtio_client_go_api_v1_KubeVirtConfiguration(ref common.Referen }, }, }, + "virtualMachineInstancesPerNode": { + SchemaProps: spec.SchemaProps{ + Type: []string{"integer"}, + Format: "int32", + }, + }, }, }, }, From dccd8b3e27cb1def081c5f3787ba3c6e6a713c1d Mon Sep 17 00:00:00 2001 From: Ashley Schuett Date: Wed, 28 Jul 2021 18:36:15 +0200 Subject: [PATCH 2/2] add tests for VirtualMachineInstancesPerNode configuration Signed-off-by: Ashley Schuett --- pkg/virt-operator/resource/apply/BUILD.bazel | 1 + pkg/virt-operator/resource/apply/apps_test.go | 142 ++++++++++++++++++ 2 files changed, 143 insertions(+) diff --git a/pkg/virt-operator/resource/apply/BUILD.bazel b/pkg/virt-operator/resource/apply/BUILD.bazel index f460b3b299d2..aa8404169fc2 100644 --- a/pkg/virt-operator/resource/apply/BUILD.bazel +++ b/pkg/virt-operator/resource/apply/BUILD.bazel @@ -83,6 +83,7 @@ go_test( "//pkg/certificates/triple/cert:go_default_library", "//pkg/controller:go_default_library", "//pkg/testutils:go_default_library", + "//pkg/util/types:go_default_library", "//pkg/virt-operator/resource/generate/components:go_default_library", "//pkg/virt-operator/resource/generate/install:go_default_library", "//pkg/virt-operator/resource/generate/rbac:go_default_library", diff --git a/pkg/virt-operator/resource/apply/apps_test.go b/pkg/virt-operator/resource/apply/apps_test.go index 9c0202a6bed2..aec59e4b746f 100644 --- a/pkg/virt-operator/resource/apply/apps_test.go +++ b/pkg/virt-operator/resource/apply/apps_test.go @@ -20,6 +20,7 @@ package apply import ( + "encoding/json" "fmt" "reflect" "strings" @@ -46,6 +47,7 @@ import ( v1 "kubevirt.io/client-go/api/v1" "kubevirt.io/client-go/kubecli" "kubevirt.io/kubevirt/pkg/controller" + utiltypes "kubevirt.io/kubevirt/pkg/util/types" "kubevirt.io/kubevirt/pkg/virt-operator/resource/generate/components" "kubevirt.io/kubevirt/pkg/virt-operator/util" ) @@ -204,6 +206,146 @@ var _ = Describe("Apply Apps", func() { }) }) + Context("setting virt-handler maxDevices flag ", func() { + + var daemonSet *appsv1.DaemonSet + var err error + var clientset *kubecli.MockKubevirtClient + var kv *v1.KubeVirt + var expectations *util.Expectations + var stores util.Stores + var mockDSCacheStore *MockStore + var dsClient *fake.Clientset + + var ctrl *gomock.Controller + + vmiPerNode := 10 + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + kvInterface := kubecli.NewMockKubeVirtInterface(ctrl) + + dsClient = fake.NewSimpleClientset() + + stores = util.Stores{} + mockDSCacheStore = &MockStore{} + stores.DaemonSetCache = mockDSCacheStore + + expectations = &util.Expectations{} + expectations.DaemonSet = controller.NewUIDTrackingControllerExpectations(controller.NewControllerExpectationsWithName("DaemonSet")) + + clientset = kubecli.NewMockKubevirtClient(ctrl) + clientset.EXPECT().KubeVirt(Namespace).Return(kvInterface).AnyTimes() + clientset.EXPECT().AppsV1().Return(dsClient.AppsV1()).AnyTimes() + kv = &v1.KubeVirt{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: Namespace, + }, + } + + daemonSet, err = components.NewHandlerDaemonSet(Namespace, Registry, "", Version, "", "", "", corev1.PullIfNotPresent, "verbosity", map[string]string{}) + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + ctrl.Finish() + }) + + It("should create with maxDevices Set", func() { + kv.Spec.Configuration.VirtualMachineInstancesPerNode = &vmiPerNode + created := false + r := &Reconciler{ + clientset: clientset, + kv: kv, + expectations: expectations, + stores: stores, + } + + dsClient.Fake.PrependReactor("create", "daemonsets", func(action testing.Action) (handled bool, obj runtime.Object, err error) { + update, ok := action.(testing.CreateAction) + Expect(ok).To(BeTrue()) + created = true + + ds := update.GetObject().(*appsv1.DaemonSet) + + command := ds.Spec.Template.Spec.Containers[0].Command + Expect(strings.Join(command, " ")).To(ContainSubstring("--maxDevices 10")) + + return true, update.GetObject(), nil + }) + + err = r.syncDaemonSet(daemonSet) + + Expect(err).ToNot(HaveOccurred()) + Expect(created).To(BeTrue()) + }) + + It("should patch DS with maxDevices and then remove it", func() { + mockDSCacheStore.get = daemonSet + SetGeneration(&kv.Status.Generations, daemonSet) + patched := false + containMaxDeviceFlag := false + + r := &Reconciler{ + clientset: clientset, + kv: kv, + expectations: expectations, + stores: stores, + } + + // add VirtualMachineInstancesPerNode configuration + kv.Spec.Configuration.VirtualMachineInstancesPerNode = &vmiPerNode + containMaxDeviceFlag = true + kv.SetGeneration(2) + + dsClient.Fake.PrependReactor("patch", "daemonsets", func(action testing.Action) (handled bool, obj runtime.Object, err error) { + a, ok := action.(testing.PatchAction) + Expect(ok).To(BeTrue()) + patched = true + + patches := []utiltypes.PatchOperation{} + json.Unmarshal(a.GetPatch(), &patches) + + var dsSpec *appsv1.DaemonSetSpec + for _, v := range patches { + if v.Path == "/spec" && v.Op == "replace" { + dsSpec = &appsv1.DaemonSetSpec{} + template, err := json.Marshal(v.Value) + Expect(err).ToNot(HaveOccurred()) + json.Unmarshal(template, dsSpec) + } + } + + Expect(dsSpec).ToNot(BeNil()) + + command := dsSpec.Template.Spec.Containers[0].Command + if containMaxDeviceFlag { + Expect(strings.Join(command, " ")).To(ContainSubstring("--maxDevices 10")) + } else { + Expect(strings.Join(command, " ")).ToNot(ContainSubstring("--maxDevices 10")) + } + + return true, &appsv1.DaemonSet{}, nil + }) + + err = r.syncDaemonSet(daemonSet) + + Expect(patched).To(BeTrue()) + Expect(err).ToNot(HaveOccurred()) + + // remove VirtualMachineInstancesPerNode configuration + patched = false + kv.Spec.Configuration.VirtualMachineInstancesPerNode = nil + containMaxDeviceFlag = false + kv.SetGeneration(3) + + err = r.syncDaemonSet(daemonSet) + + Expect(patched).To(BeTrue()) + Expect(err).ToNot(HaveOccurred()) + }) + }) + Context("Injecting Metadata", func() { It("should set expected values", func() {