Merge pull request kubevirt#2245 from ahadas/default_memory_request2

Make memory request configurable again

cmd/virt-handler/virt-handler.go

@@ -271,7 +271,7 @@ func (app *virtHandlerApp) Run() {
 		gracefulShutdownInformer,
 		int(app.WatchdogTimeoutDuration.Seconds()),
 		app.MaxDevices,
-		virtconfig.NewClusterConfig(factory.ConfigMap().GetStore(), app.namespace),
+		virtconfig.NewClusterConfig(factory.ConfigMap(), app.namespace),
 		app.migrationTLSConfig,
 	)
 

pkg/api/v1/defaults.go

@@ -2,8 +2,6 @@ package v1
 
 import (
 	"github.com/pborman/uuid"
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -110,13 +108,6 @@ func SetDefaults_Firmware(obj *Firmware) {
 }
 
 func SetDefaults_VirtualMachineInstance(obj *VirtualMachineInstance) {
-	// FIXME we need proper validation and configurable defaulting instead of this
-	if _, exists := obj.Spec.Domain.Resources.Requests[v1.ResourceMemory]; !exists {
-		if obj.Spec.Domain.Resources.Requests == nil {
-			obj.Spec.Domain.Resources.Requests = v1.ResourceList{}
-		}
-		obj.Spec.Domain.Resources.Requests[v1.ResourceMemory] = resource.MustParse("8192Ki")
-	}
 	if obj.Spec.Domain.Firmware == nil {
 		obj.Spec.Domain.Firmware = &Firmware{}
 	}

pkg/testutils/BUILD.bazel

@@ -25,7 +25,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library",
         "//vendor/k8s.io/client-go/rest:go_default_library",
         "//vendor/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/k8s.io/client-go/tools/cache/testing:go_default_library",

pkg/testutils/matchers.go

@@ -241,3 +241,11 @@ func BeIn(elements ...interface{}) types.GomegaMatcher {
 		Elements: elements,
 	}
 }
+
+func SatisfyAnyRegexp(regexps []string) types.GomegaMatcher {
+	matchers := []types.GomegaMatcher{}
+	for _, regexp := range regexps {
+		matchers = append(matchers, gomega.MatchRegexp(regexp))
+	}
+	return gomega.SatisfyAny(matchers...)
+}

pkg/testutils/mock_config.go

@@ -3,28 +3,36 @@ package testutils
 import (
 	v1 "k8s.io/api/core/v1"
 	v12 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/watch"
+	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/client-go/tools/cache"
 
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 )
 
-func MakeFakeClusterConfig(configMaps []v1.ConfigMap, stopChan chan struct{}) *virtconfig.ClusterConfig {
-	cmListWatch := &cache.ListWatch{
-		ListFunc: func(options v12.ListOptions) (runtime.Object, error) {
-			return &v1.ConfigMapList{Items: configMaps}, nil
-		},
-		WatchFunc: func(options v12.ListOptions) (watch.Interface, error) {
-			fakeWatch := watch.NewFake()
-			for _, cfgMap := range configMaps {
-				fakeWatch.Add(&cfgMap)
-			}
-			return fakeWatch, nil
-		},
+const (
+	configMapName = "kubevirt-config"
+	namespace     = "kubevirt"
+)
+
+func NewFakeClusterConfig(cfgMap *v1.ConfigMap) (*virtconfig.ClusterConfig, cache.SharedIndexInformer) {
+	informer, _ := NewFakeInformerFor(&v1.ConfigMap{})
+	copy := copy(cfgMap)
+	informer.GetStore().Add(copy)
+	return virtconfig.NewClusterConfig(informer, namespace), informer
+}
+
+func UpdateFakeClusterConfig(informer cache.SharedIndexInformer, cfgMap *v1.ConfigMap) {
+	copy := copy(cfgMap)
+	informer.GetStore().Update(copy)
+}
+
+func copy(cfgMap *v1.ConfigMap) *v1.ConfigMap {
+	copy := cfgMap.DeepCopy()
+	copy.ObjectMeta = v12.ObjectMeta{
+		Namespace: namespace,
+		Name:      configMapName,
+		// Change the resource version, or the config will not be updated
+		ResourceVersion: rand.String(10),
 	}
-	cmInformer := cache.NewSharedIndexInformer(cmListWatch, &v1.ConfigMap{}, 0, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
-	go cmInformer.Run(stopChan)
-	cache.WaitForCacheSync(stopChan, cmInformer.HasSynced)
-	return virtconfig.NewClusterConfig(cmInformer.GetStore(), "kubevirt")
+	return copy
 }

pkg/virt-api/api.go

@@ -107,6 +107,7 @@ type virtAPIApp struct {
 	aggregatorClient *aggregatorclient.Clientset
 	authorizor       rest.VirtApiAuthorizor
 	certsDirectory   string
+	clusterConfig    *virtconfig.ClusterConfig
 
 	signingCertBytes []byte
 	certBytes        []byte
@@ -127,8 +128,6 @@ func NewVirtApi() VirtApi {
 }
 
 func (app *virtAPIApp) Execute() {
-	virtconfig.Init()
-
 	virtCli, err := kubecli.GetKubevirtClient()
 	if err != nil {
 		panic(err)
@@ -742,22 +741,22 @@ func (app *virtAPIApp) createValidatingWebhook() error {
 	}
 
 	http.HandleFunc(vmiCreateValidatePath, func(w http.ResponseWriter, r *http.Request) {
-		validating_webhook.ServeVMICreate(w, r)
+		validating_webhook.ServeVMICreate(w, r, app.clusterConfig)
 	})
 	http.HandleFunc(vmiUpdateValidatePath, func(w http.ResponseWriter, r *http.Request) {
 		validating_webhook.ServeVMIUpdate(w, r)
 	})
 	http.HandleFunc(vmValidatePath, func(w http.ResponseWriter, r *http.Request) {
-		validating_webhook.ServeVMs(w, r)
+		validating_webhook.ServeVMs(w, r, app.clusterConfig)
 	})
 	http.HandleFunc(vmirsValidatePath, func(w http.ResponseWriter, r *http.Request) {
-		validating_webhook.ServeVMIRS(w, r)
+		validating_webhook.ServeVMIRS(w, r, app.clusterConfig)
 	})
 	http.HandleFunc(vmipresetValidatePath, func(w http.ResponseWriter, r *http.Request) {
 		validating_webhook.ServeVMIPreset(w, r)
 	})
 	http.HandleFunc(migrationCreateValidatePath, func(w http.ResponseWriter, r *http.Request) {
-		validating_webhook.ServeMigrationCreate(w, r)
+		validating_webhook.ServeMigrationCreate(w, r, app.clusterConfig)
 	})
 	http.HandleFunc(migrationUpdateValidatePath, func(w http.ResponseWriter, r *http.Request) {
 		validating_webhook.ServeMigrationUpdate(w, r)
@@ -860,7 +859,7 @@ func (app *virtAPIApp) createMutatingWebhook() error {
 	}
 
 	http.HandleFunc(vmiMutatePath, func(w http.ResponseWriter, r *http.Request) {
-		mutating_webhook.ServeVMIs(w, r)
+		mutating_webhook.ServeVMIs(w, r, app.clusterConfig)
 	})
 	http.HandleFunc(migrationMutatePath, func(w http.ResponseWriter, r *http.Request) {
 		mutating_webhook.ServeMigrationCreate(w, r)
@@ -1030,19 +1029,23 @@ func (app *virtAPIApp) Run() {
 
 	// Run informers for webhooks usage
 	webhookInformers := webhooks.GetInformers()
+	kubeInformerFactory := controller.NewKubeInformerFactory(app.virtCli.RestClient(), app.virtCli, app.namespace)
+	configMapInformer := kubeInformerFactory.ConfigMap()
 
 	stopChan := make(chan struct{}, 1)
 	defer close(stopChan)
 	go webhookInformers.VMIInformer.Run(stopChan)
 	go webhookInformers.VMIPresetInformer.Run(stopChan)
 	go webhookInformers.NamespaceLimitsInformer.Run(stopChan)
-	go webhookInformers.ConfigMapInformer.Run(stopChan)
+	go configMapInformer.Run(stopChan)
 
 	cache.WaitForCacheSync(stopChan,
 		webhookInformers.VMIInformer.HasSynced,
 		webhookInformers.VMIPresetInformer.HasSynced,
 		webhookInformers.NamespaceLimitsInformer.HasSynced,
-		webhookInformers.ConfigMapInformer.HasSynced)
+		configMapInformer.HasSynced)
+
+	app.clusterConfig = virtconfig.NewClusterConfig(configMapInformer, app.namespace)
 
 	// Verify/create webhook endpoint.
 	err = app.createWebhook()

pkg/virt-api/rest/BUILD.bazel

@@ -49,6 +49,7 @@ go_test(
         "//pkg/api/v1:go_default_library",
         "//pkg/kubecli:go_default_library",
         "//pkg/log:go_default_library",
+        "//pkg/testutils:go_default_library",
         "//pkg/virt-controller/services:go_default_library",
         "//vendor/github.com/emicklei/go-restful:go_default_library",
         "//vendor/github.com/gorilla/websocket:go_default_library",

pkg/virt-api/rest/subresource_test.go

@@ -42,6 +42,7 @@ import (
 	v1 "kubevirt.io/kubevirt/pkg/api/v1"
 	"kubevirt.io/kubevirt/pkg/kubecli"
 	"kubevirt.io/kubevirt/pkg/log"
+	"kubevirt.io/kubevirt/pkg/testutils"
 	"kubevirt.io/kubevirt/pkg/virt-controller/services"
 )
 
@@ -57,7 +58,6 @@ var _ = Describe("VirtualMachineInstance Subresources", func() {
 
 	log.Log.SetIOWriter(GinkgoWriter)
 
-	configCache := cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, nil)
 	pvcCache := cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, nil)
 	app := SubresourceAPIApp{}
 	BeforeEach(func() {
@@ -89,7 +89,9 @@ 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, app.VirtCli)
+
+			config, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+			templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", pvcCache, app.VirtCli, config)
 
 			pod, err := templateService.RenderLaunchManifest(vmi)
 			Expect(err).ToNot(HaveOccurred())
@@ -261,7 +263,8 @@ var _ = Describe("VirtualMachineInstance Subresources", func() {
 			vmi.Status.Phase = v1.Running
 			vmi.ObjectMeta.SetUID(uuid.NewUUID())
 
-			templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", configCache, pvcCache, app.VirtCli)
+			config, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+			templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", pvcCache, app.VirtCli, config)
 
 			pod, err := templateService.RenderLaunchManifest(vmi)
 			Expect(err).ToNot(HaveOccurred())

pkg/virt-api/webhooks/mutating-webhook/BUILD.bazel

@@ -12,7 +12,6 @@ go_library(
     deps = [
         "//pkg/api/v1:go_default_library",
         "//pkg/log:go_default_library",
-        "//pkg/util:go_default_library",
         "//pkg/virt-api/webhooks:go_default_library",
         "//pkg/virt-config:go_default_library",
         "//vendor/k8s.io/api/admission/v1beta1:go_default_library",
@@ -40,6 +39,7 @@ go_test(
         "//pkg/log:go_default_library",
         "//pkg/testutils:go_default_library",
         "//pkg/virt-api/webhooks:go_default_library",
+        "//pkg/virt-config:go_default_library",
         "//vendor/github.com/onsi/ginkgo:go_default_library",
         "//vendor/github.com/onsi/gomega:go_default_library",
         "//vendor/k8s.io/api/admission/v1beta1:go_default_library",

pkg/virt-api/webhooks/mutating-webhook/mutating-webhook.go

@@ -32,7 +32,6 @@ import (
 
 	v1 "kubevirt.io/kubevirt/pkg/api/v1"
 	"kubevirt.io/kubevirt/pkg/log"
-	"kubevirt.io/kubevirt/pkg/util"
 	"kubevirt.io/kubevirt/pkg/virt-api/webhooks"
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 )
@@ -43,9 +42,11 @@ type patchOperation struct {
 	Value interface{} `json:"value,omitempty"`
 }
 
-type mutateFunc func(*v1beta1.AdmissionReview) *v1beta1.AdmissionResponse
+type mutator interface {
+	mutate(*v1beta1.AdmissionReview) *v1beta1.AdmissionResponse
+}
 
-func serve(resp http.ResponseWriter, req *http.Request, mutate mutateFunc) {
+func serve(resp http.ResponseWriter, req *http.Request, m mutator) {
 	response := v1beta1.AdmissionReview{}
 	review, err := webhooks.GetAdmissionReview(req)
 
@@ -54,7 +55,7 @@ func serve(resp http.ResponseWriter, req *http.Request, mutate mutateFunc) {
 		return
 	}
 
-	reviewResponse := mutate(review)
+	reviewResponse := m.mutate(review)
 	if reviewResponse != nil {
 		response.Response = reviewResponse
 		response.Response.UID = review.Request.UID
@@ -77,8 +78,11 @@ func serve(resp http.ResponseWriter, req *http.Request, mutate mutateFunc) {
 	resp.WriteHeader(http.StatusOK)
 }
 
-func mutateVMIs(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
+type VMIsMutator struct {
+	clusterConfig *virtconfig.ClusterConfig
+}
 
+func (mutator *VMIsMutator) mutate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
 	if ar.Request.Resource != webhooks.VirtualMachineInstanceGroupVersionResource {
 		err := fmt.Errorf("expect resource to be '%s'", webhooks.VirtualMachineInstanceGroupVersionResource.Resource)
 		return webhooks.ToAdmissionResponseError(err)
@@ -97,11 +101,7 @@ func mutateVMIs(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
 	}
 
 	informers := webhooks.GetInformers()
-	namespace, err := util.GetNamespace()
-	if err != nil {
-		return webhooks.ToAdmissionResponseError(err)
-	}
-	config := virtconfig.NewClusterConfig(informers.ConfigMapInformer.GetStore(), namespace)
+	config := mutator.clusterConfig
 
 	// Apply presets
 	err = applyPresets(&vmi, informers.VMIPresetInformer)
@@ -121,10 +121,8 @@ func mutateVMIs(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
 	log.Log.Object(&vmi).V(4).Info("Apply defaults")
 	setDefaultCPUModel(&vmi, config.GetCPUModel())
 	setDefaultMachineType(&vmi, config.GetMachineType())
+	setDefaultResourceRequests(&vmi, config.GetMemoryRequest(), config.GetCPURequest())
 	v1.SetObjectDefaults_VirtualMachineInstance(&vmi)
-	// Default CPU request is done after the Resources section is initialized, if needed,
-	// in v1.SetObjectDefaults_VirtualMachineInstance. TODO: set default memory here
-	setDefaultCPURequest(&vmi, config.GetCPURequest())
 
 	// Add foreground finalizer
 	vmi.Finalizers = append(vmi.Finalizers, v1.VirtualMachineInstanceFinalizer)
@@ -159,8 +157,10 @@ func mutateVMIs(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
 
 }
 
-func mutateMigrationCreate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
+type MigrationCreateMutator struct {
+}
 
+func (mutator *MigrationCreateMutator) mutate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
 	if ar.Request.Resource != webhooks.MigrationGroupVersionResource {
 		err := fmt.Errorf("expect resource to be '%s'", webhooks.MigrationGroupVersionResource.Resource)
 		return webhooks.ToAdmissionResponseError(err)
@@ -230,7 +230,14 @@ func setDefaultMachineType(vmi *v1.VirtualMachineInstance, defaultMachineType st
 	}
 }
 
-func setDefaultCPURequest(vmi *v1.VirtualMachineInstance, defaultCPURequest resource.Quantity) {
+func setDefaultResourceRequests(vmi *v1.VirtualMachineInstance, defaultMemoryRequest resource.Quantity, defaultCPURequest resource.Quantity) {
+	if _, exists := vmi.Spec.Domain.Resources.Requests[k8sv1.ResourceMemory]; !exists {
+		if vmi.Spec.Domain.Resources.Requests == nil {
+			vmi.Spec.Domain.Resources.Requests = k8sv1.ResourceList{}
+		}
+		vmi.Spec.Domain.Resources.Requests[k8sv1.ResourceMemory] = defaultMemoryRequest
+	}
+
 	if _, exists := vmi.Spec.Domain.Resources.Requests[k8sv1.ResourceCPU]; !exists {
 		if vmi.Spec.Domain.CPU != nil && vmi.Spec.Domain.CPU.DedicatedCPUPlacement {
 			return
@@ -239,10 +246,10 @@ func setDefaultCPURequest(vmi *v1.VirtualMachineInstance, defaultCPURequest reso
 	}
 }
 
-func ServeVMIs(resp http.ResponseWriter, req *http.Request) {
-	serve(resp, req, mutateVMIs)
+func ServeVMIs(resp http.ResponseWriter, req *http.Request, clusterConfig *virtconfig.ClusterConfig) {
+	serve(resp, req, &VMIsMutator{clusterConfig: clusterConfig})
 }
 
 func ServeMigrationCreate(resp http.ResponseWriter, req *http.Request) {
-	serve(resp, req, mutateMigrationCreate)
+	serve(resp, req, &MigrationCreateMutator{})
 }