diff --git a/pkg/kubevirt/apis/provider_spec.go b/pkg/kubevirt/apis/provider_spec.go index fe5e0c5..5ab6378 100644 --- a/pkg/kubevirt/apis/provider_spec.go +++ b/pkg/kubevirt/apis/provider_spec.go @@ -16,21 +16,24 @@ package api import ( corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" kubevirtv1 "kubevirt.io/client-go/api/v1" ) // KubeVirtProviderSpec is the spec to be used while parsing the calls. type KubeVirtProviderSpec struct { + // Resources defines requests and limits resources of VMI + Resources kubevirtv1.ResourceRequirements `json:"resources"` // SourceURL is the HTTP URL of the source image imported by CDI. SourceURL string `json:"sourceURL"` // StorageClassName is the name which CDI uses to in order to create claims. StorageClassName string `json:"storageClassName"` // PVCSize is the size of the PersistentVolumeClaim that is created during the image import by CDI. - PVCSize string `json:"pvcSize"` - // CPUs is the number of CPUs requested by the VM. - CPUs string `json:"cpus"` - // Memory is the amount of memory requested by the VM. - Memory string `json:"memory"` + PVCSize resource.Quantity `json:"pvcSize"` + // Region is the name of the region for the VM. + Region string `json:"region"` + // Zone is the name of the zone for the VM. + Zone string `json:"zone"` // DNSConfig is the DNS configuration of the VM pod. // The parameters specified here will be merged with the generated DNS configuration based on DNSPolicy. // +optional @@ -47,21 +50,19 @@ type KubeVirtProviderSpec struct { // the pod network won't be added, otherwise it will be added as default. // +optional Networks []NetworkSpec `json:"networks,omitempty"` - // Region is the name of the region for the VM. - Region string `json:"region"` - // Zone is the name of the zone for the VM. - // +optional - Zone string `json:"zone,omitempty"` // Tags is an optional map of tags that is added to the VM as labels. // +optional Tags map[string]string `json:"tags,omitempty"` - // MemoryFeatures allows specifying the VirtualMachineInstance memory features like huge pages and guest memory settings. + // CPU allows specifying the CPU topology of KubeVirt VM. + // +optional + CPU *kubevirtv1.CPU `json:"cpu,omitempty"` + // Memory allows specifying the VirtualMachineInstance memory features like huge pages and guest memory settings. // Each feature might require appropriate FeatureGate enabled. // For hugepages take a look at: // k8s - https://kubernetes.io/docs/tasks/manage-hugepages/scheduling-hugepages/ // okd - https://docs.okd.io/3.9/scaling_performance/managing_hugepages.html#huge-pages-prerequisites // +optional - MemoryFeatures *kubevirtv1.Memory `json:"memoryFeatures,omitempty"` + Memory *kubevirtv1.Memory `json:"memory,omitempty"` } // NetworkSpec contains information about a network. diff --git a/pkg/kubevirt/core/core.go b/pkg/kubevirt/core/core.go index 9be1b17..83a5344 100644 --- a/pkg/kubevirt/core/core.go +++ b/pkg/kubevirt/core/core.go @@ -24,11 +24,9 @@ import ( api "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/apis" clouderrors "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/errors" - "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/util" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" @@ -98,18 +96,7 @@ func (p PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, pr return "", fmt.Errorf("failed to create client: %v", err) } - requestsAndLimits, err := util.ParseResources(providerSpec.CPUs, providerSpec.Memory) - if err != nil { - return "", fmt.Errorf("failed to parse resources fields: %v", err) - } - - pvcSize, err := resource.ParseQuantity(providerSpec.PVCSize) - if err != nil { - return "", fmt.Errorf("failed to parse pvcSize field: %v", err) - } - var ( - pvcRequest = corev1.ResourceList{corev1.ResourceStorage: pvcSize} terminationGracePeriodSeconds = int64(30) userdataSecretName = fmt.Sprintf("userdata-%s-%s", machineName, strconv.Itoa(int(time.Now().Unix()))) ) @@ -160,7 +147,9 @@ func (p PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, pr "ReadWriteOnce", }, Resources: corev1.ResourceRequirements{ - Requests: pvcRequest, + Requests: corev1.ResourceList{ + corev1.ResourceStorage: providerSpec.PVCSize, + }, }, }, Source: cdi.DataVolumeSource{ @@ -196,6 +185,8 @@ func (p PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, pr }, Spec: kubevirtv1.VirtualMachineInstanceSpec{ Domain: kubevirtv1.DomainSpec{ + CPU: providerSpec.CPU, + Memory: providerSpec.Memory, Devices: kubevirtv1.Devices{ Disks: []kubevirtv1.Disk{ { @@ -209,10 +200,7 @@ func (p PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, pr }, Interfaces: interfaces, }, - Resources: kubevirtv1.ResourceRequirements{ - Requests: *requestsAndLimits, - Limits: *requestsAndLimits, - }, + Resources: providerSpec.Resources, }, TerminationGracePeriodSeconds: &terminationGracePeriodSeconds, Volumes: []kubevirtv1.Volume{ @@ -248,12 +236,6 @@ func (p PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, pr }, } - if providerSpec.MemoryFeatures != nil && providerSpec.MemoryFeatures.Hugepages != nil { - virtualMachine.Spec.Template.Spec.Domain.Memory = &kubevirtv1.Memory{ - Hugepages: providerSpec.MemoryFeatures.Hugepages, - } - } - if err := c.Create(ctx, virtualMachine); err != nil { return "", fmt.Errorf("failed to create VirtualMachine: %v", err) } diff --git a/pkg/kubevirt/core/core_test.go b/pkg/kubevirt/core/core_test.go index 085ed31..7cbca79 100644 --- a/pkg/kubevirt/core/core_test.go +++ b/pkg/kubevirt/core/core_test.go @@ -22,8 +22,10 @@ import ( api "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/apis" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/client-go/kubernetes/scheme" "k8s.io/klog" + kubevirtv1 "kubevirt.io/client-go/api/v1" cdi "kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -33,9 +35,17 @@ var ( providerSpec = &api.KubeVirtProviderSpec{ SourceURL: "http://test-image.com", StorageClassName: "test-sc", - PVCSize: "10Gi", - CPUs: "1", - Memory: "4096M", + PVCSize: resource.MustParse("10Gi"), + Resources: kubevirtv1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + corev1.ResourceMemory: resource.MustParse("4096Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + corev1.ResourceMemory: resource.MustParse("8Gi"), + }, + }, } machineName = "kubevirt-machine" namespace = "default" diff --git a/pkg/kubevirt/machine_server_util.go b/pkg/kubevirt/machine_server_util.go index 79a49c3..b40772a 100644 --- a/pkg/kubevirt/machine_server_util.go +++ b/pkg/kubevirt/machine_server_util.go @@ -30,9 +30,14 @@ func decodeProviderSpecAndSecret(machineClass *v1alpha1.MachineClass, secret *co return nil, status.Error(codes.Internal, wrapped.Error()) } - validationErrors := validation.ValidateKubevirtProviderSpecAndSecret(providerSpec, secret) - if validationErrors != nil { - err = fmt.Errorf("could not validate provider spec and secret: %v", validationErrors) + if errs := validation.ValidateKubevirtProviderSpec(providerSpec); errs != nil { + err = fmt.Errorf("could not validate provider spec: %v", errs) + klog.V(2).Infof(err.Error()) + return nil, status.Error(codes.Internal, err.Error()) + } + + if errs := validation.ValidateKubevirtProviderSecrets(secret); errs != nil { + err = fmt.Errorf("could not validate provider secrets: %v", errs) klog.V(2).Infof(err.Error()) return nil, status.Error(codes.Internal, err.Error()) } diff --git a/pkg/kubevirt/util/util.go b/pkg/kubevirt/util/util.go deleted file mode 100644 index 0a74e76..0000000 --- a/pkg/kubevirt/util/util.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package util - -import ( - "fmt" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" -) - -// ParseResources receives cpus and memory parameters and parse them as a ResourceList to be used in the virtual machine. -func ParseResources(cpus, memory string) (*corev1.ResourceList, error) { - memoryResource, err := resource.ParseQuantity(memory) - if err != nil { - return nil, fmt.Errorf("failed to parse memory requests: %v", err) - } - cpuResource, err := resource.ParseQuantity(cpus) - if err != nil { - return nil, fmt.Errorf("failed to parse cpu request: %v", err) - } - return &corev1.ResourceList{ - corev1.ResourceMemory: memoryResource, - corev1.ResourceCPU: cpuResource, - }, nil -} diff --git a/pkg/kubevirt/validation/validation.go b/pkg/kubevirt/validation/validation.go index 25c8927..cb18fe8 100644 --- a/pkg/kubevirt/validation/validation.go +++ b/pkg/kubevirt/validation/validation.go @@ -20,90 +20,92 @@ import ( "fmt" api "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/apis" - "github.com/gardener/machine-controller-manager-provider-kubevirt/pkg/kubevirt/util" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/tools/clientcmd" ) -// ValidateKubevirtProviderSpecAndSecret validates kubevirt spec and secret to check if all fields are present and valid -func ValidateKubevirtProviderSpecAndSecret(spec *api.KubeVirtProviderSpec, secrets *corev1.Secret) []error { - var validationErrors []error +// ValidateKubevirtProviderSpec validates kubevirt spec to check if all fields are present and valid +func ValidateKubevirtProviderSpec(spec *api.KubeVirtProviderSpec) field.ErrorList { + errs := field.ErrorList{} - if spec.CPUs == "" { - validationErrors = append(validationErrors, errors.New("cpus field cannot be empty")) + requestsPath := field.NewPath("resources").Child("requests") + if spec.Resources.Requests.Memory().IsZero() { + errs = append(errs, field.Required(requestsPath.Child("memory"), "cannot be zero")) } - if spec.Memory == "" { - validationErrors = append(validationErrors, errors.New("memory field cannot be empty")) - } - if _, err := util.ParseResources(spec.CPUs, spec.Memory); err != nil { - validationErrors = append(validationErrors, fmt.Errorf("invalid cpus/memory values: %v", err)) + if spec.Resources.Requests.Cpu().IsZero() { + errs = append(errs, field.Required(requestsPath.Child("cpu"), "cannot be zero")) } + if spec.SourceURL == "" { - validationErrors = append(validationErrors, errors.New("sourceURL field cannot be empty")) + errs = append(errs, field.Required(field.NewPath("sourceURL"), "cannot be empty")) } + if spec.StorageClassName == "" { - validationErrors = append(validationErrors, errors.New("storageClassName field cannot be empty")) + errs = append(errs, field.Required(field.NewPath("storageClassName"), "cannot be empty")) } - if spec.PVCSize == "" { - validationErrors = append(validationErrors, errors.New("memory field cannot be empty")) + + if spec.PVCSize.IsZero() { + errs = append(errs, field.Required(field.NewPath("pvcSize"), "cannot be zero")) } - if _, err := resource.ParseQuantity(spec.PVCSize); err != nil { - validationErrors = append(validationErrors, fmt.Errorf("failed to parse value of pvcSize field: %v", err)) + + if spec.Region == "" { + errs = append(errs, field.Required(field.NewPath("region"), "cannot be empty")) + } + + if spec.Zone == "" { + errs = append(errs, field.Required(field.NewPath("zone"), "cannot be empty")) } + if spec.DNSPolicy != "" { + dnsPolicyPath := field.NewPath("dnsPolicy") + dnsConfigPath := field.NewPath("dnsConfig") + switch spec.DNSPolicy { case corev1.DNSDefault, corev1.DNSClusterFirstWithHostNet, corev1.DNSClusterFirst, corev1.DNSNone: break default: - validationErrors = append(validationErrors, fmt.Errorf("invalid dns policy: %v", spec.DNSPolicy)) + errs = append(errs, field.Invalid(dnsPolicyPath, spec.DNSPolicy, "invalid dns policy")) } if spec.DNSPolicy == corev1.DNSNone { if spec.DNSConfig != nil { if len(spec.DNSConfig.Nameservers) == 0 { - validationErrors = append(validationErrors, errors.New("dns config must specify nameservers when dns policy is None")) + errs = append(errs, field.Required(dnsConfigPath.Child("nameservers"), + fmt.Sprintf("cannot be empty when dns policy is %s", corev1.DNSNone))) } } else { - validationErrors = append(validationErrors, errors.New("dns config must be specified when dns policy is None")) + errs = append(errs, field.Required(dnsConfigPath, + fmt.Sprintf("cannot be empty when dns policy is %s", corev1.DNSNone))) } } } - if spec.MemoryFeatures != nil && spec.MemoryFeatures.Hugepages != nil { - _, err := resource.ParseQuantity(spec.MemoryFeatures.Hugepages.PageSize) - if err != nil { - validationErrors = append(validationErrors, fmt.Errorf("invalid value of hugepages size '%v'", - spec.MemoryFeatures.Hugepages.PageSize)) - } - } - - validationErrors = append(validationErrors, validateSecrets(secrets)...) - - return validationErrors + return errs } -func validateSecrets(secret *corev1.Secret) []error { - var validationErrors []error +// ValidateKubevirtProviderSecrets validates kubevirt secrets +func ValidateKubevirtProviderSecrets(secret *corev1.Secret) []error { + var errs []error if secret == nil { - validationErrors = append(validationErrors, errors.New("secret object passed by the MCM is nil")) + errs = append(errs, errors.New("secret object passed by the MCM is nil")) } else { kubeconfig, kubevirtKubeconifgCheck := secret.Data["kubeconfig"] _, userdataCheck := secret.Data["userData"] if !kubevirtKubeconifgCheck { - validationErrors = append(validationErrors, fmt.Errorf("secret kubeconfig is required field")) + errs = append(errs, fmt.Errorf("secret kubeconfig is required field")) } else { _, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) if err != nil { - validationErrors = append(validationErrors, fmt.Errorf("failed to decode kubeconfig: %v", err)) + errs = append(errs, fmt.Errorf("failed to decode kubeconfig: %v", err)) } } if !userdataCheck { - validationErrors = append(validationErrors, fmt.Errorf("secret userData is required field")) + errs = append(errs, fmt.Errorf("secret userData is required field")) } } - return validationErrors + return errs }