Skip to content

Commit f3a6775

Browse files
nunnatsanunnatsa
authored
Few last fixes in v1 API (#4113)
* Several fixes in v1 API * make the `spec.deployment` field optional * rename `spec.storage.storageWorkloads` => `spec.storage.workloadResourceRequirements` * move `spec.tuningPolicy` to `spec.virtualization.tuningPolicy` * do not convert the v1beta1 spec.tuningPolicy, if the value is "highBurst", as it's not supported in v1. Signed-off-by: Nahshon Unna Tsameret <nunnatsa@redhat.com> * fix the application code to match the API changes Also, the v1 validator now rejects the "highBurst" value for the tuningPolicy field, instead of just return a warning. The v1 mutator removes the tuningPolicy field, if its value is "highBurst". Signed-off-by: Nahshon Unna Tsameret <nunnatsa@redhat.com> * fix e2e Signed-off-by: Nahshon Unna Tsameret <nunnatsa@redhat.com> * update CRD and docs Signed-off-by: Nahshon Unna Tsameret <nunnatsa@redhat.com> --------- Signed-off-by: Nahshon Unna Tsameret <nunnatsa@redhat.com>
1 parent 1f2769f commit f3a6775

File tree

22 files changed

+289
-172
lines changed

22 files changed

+289
-172
lines changed

‎api/v1/hyperconverged_types.go‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,11 @@ type HyperConvergedTuningPolicy string
2929
// through annotation values.
3030
const (
3131
HyperConvergedAnnotationTuningPolicy HyperConvergedTuningPolicy = "annotation"
32-
// Deprecated: The highBurst profile is deprecated as of v1.16.0 ahead of removal in a future release
33-
HyperConvergedHighBurstProfile HyperConvergedTuningPolicy = "highBurst"
3432
)
3533

3634
// HyperConvergedSpec defines the desired state of HyperConverged
3735
// +k8s:openapi-gen=true
3836
type HyperConvergedSpec struct {
39-
// TuningPolicy allows to configure the mode in which the RateLimits of kubevirt are set.
40-
// If TuningPolicy is not present the default kubevirt values are used.
41-
// It can be set to `annotation` for fine-tuning the kubevirt queryPerSeconds (qps) and burst values.
42-
// Qps and burst values are taken from the annotation hco.kubevirt.io/tuningPolicy
43-
// +kubebuilder:validation:Enum=annotation;highBurst
44-
// +optional
45-
TuningPolicy HyperConvergedTuningPolicy `json:"tuningPolicy,omitempty"`
46-
4737
// NodePlacements defines the node scheduling configuration for infrastructure or workload entities
4838
// +optional
4939
// +k8s:conversion-gen=false
@@ -224,12 +214,21 @@ type HyperConvergedSpec struct {
224214

225215
// Deployment contains all the configurations related to deployment of KubeVirt components
226216
// +kubebuilder:default={"uninstallStrategy": "BlockUninstallIfWorkloadsExist", "deployVmConsoleProxy": false, "applicationAwareConfig": {"enable": false}}
217+
// +optional
227218
// +k8s:conversion-gen=false
228-
Deployment DeploymentConfig `json:"deployment"`
219+
Deployment DeploymentConfig `json:"deployment,omitempty"`
229220
}
230221

231222
// VirtualizationConfig contains all the virtualization configurations
232223
type VirtualizationConfig struct {
224+
// TuningPolicy allows configuring the mode in which the RateLimits of kubevirt are set.
225+
// If TuningPolicy is not present the default kubevirt values are used.
226+
// It can be set to `annotation` for fine-tuning the kubevirt queryPerSeconds (QPS) and burst values.
227+
// QPS and burst values are taken from the annotation hco.kubevirt.io/tuningPolicy
228+
// +kubebuilder:validation:Enum=annotation;highBurst
229+
// +optional
230+
TuningPolicy HyperConvergedTuningPolicy `json:"tuningPolicy,omitempty"`
231+
233232
// Live migration limits and timeouts are applied so that migration processes do not
234233
// overwhelm the cluster.
235234
// +kubebuilder:default={"completionTimeoutPerGiB": 150, "parallelMigrationsPerCluster": 5, "parallelOutboundMigrationsPerNode": 2, "progressTimeout": 150, "allowAutoConverge": false, "allowPostCopy": false}
@@ -354,10 +353,10 @@ type StorageConfig struct {
354353
// +optional
355354
FilesystemOverhead *cdiv1beta1.FilesystemOverhead `json:"filesystemOverhead,omitempty"`
356355

357-
// StorageWorkloads defines the resources requirements for storage workloads. It will propagate to the CDI custom
356+
// WorkloadResourceRequirements defines the resource requirements for storage workloads. It will propagate to the CDI custom
358357
// resource
359358
// +optional
360-
StorageWorkloads *corev1.ResourceRequirements `json:"storageWorkloads,omitempty"`
359+
WorkloadResourceRequirements *corev1.ResourceRequirements `json:"workloadResourceRequirements,omitempty"`
361360
}
362361

363362
// NetworkingConfig contains all the networking configurations

‎api/v1/zz_generated.deepcopy.go‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1/zz_generated.openapi.go‎

Lines changed: 0 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1beta1/conversion.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ func convertNodePlacementsV1beta1ToV1(v1beta1Spec HyperConvergedSpec, v1Spec *hc
108108
}
109109

110110
func convertVirtualizationV1ToV1beta1(v1VirtConfig hcov1.VirtualizationConfig, v1beta1Spec *HyperConvergedSpec) error {
111+
v1beta1Spec.TuningPolicy = v1VirtConfig.TuningPolicy
112+
111113
v1VirtConfig.LiveMigrationConfig.DeepCopyInto(&v1beta1Spec.LiveMigrationConfig)
112114

113115
if v1VirtConfig.PermittedHostDevices != nil {
@@ -191,6 +193,12 @@ func convertVMOptionsV1ToV1beta1(v1VMOptions *hcov1.VirtualMachineOptions, v1bet
191193
}
192194

193195
func convertVirtualizationV1beta1ToV1(v1beta1Spec HyperConvergedSpec, v1VirtConfig *hcov1.VirtualizationConfig) error {
196+
// the "highBurst" policy is deprecated in v1. We don't need to copy it because it's now the
197+
// default value in KubeVirt.
198+
if v1beta1Spec.TuningPolicy == hcov1.HyperConvergedAnnotationTuningPolicy {
199+
v1VirtConfig.TuningPolicy = hcov1.HyperConvergedAnnotationTuningPolicy
200+
}
201+
194202
v1beta1Spec.LiveMigrationConfig.DeepCopyInto(&v1VirtConfig.LiveMigrationConfig)
195203

196204
if v1beta1Spec.PermittedHostDevices != nil {
@@ -283,14 +291,12 @@ func convertStorageV1ToV1beta1(v1StorageConfig *hcov1.StorageConfig, v1beta1Spec
283291
v1beta1Spec.FilesystemOverhead = v1StorageConfig.FilesystemOverhead.DeepCopy()
284292
}
285293

286-
if v1StorageConfig.StorageWorkloads != nil {
294+
if v1StorageConfig.WorkloadResourceRequirements != nil {
287295
if v1beta1Spec.ResourceRequirements == nil {
288296
v1beta1Spec.ResourceRequirements = &OperandResourceRequirements{}
289297
}
290298

291-
if v1StorageConfig.StorageWorkloads != nil {
292-
v1beta1Spec.ResourceRequirements.StorageWorkloads = v1StorageConfig.StorageWorkloads.DeepCopy()
293-
}
299+
v1beta1Spec.ResourceRequirements.StorageWorkloads = v1StorageConfig.WorkloadResourceRequirements.DeepCopy()
294300
}
295301
}
296302

@@ -314,7 +320,7 @@ func convertStorageV1beta1ToV1(v1beta1Spec HyperConvergedSpec) *hcov1.StorageCon
314320
}
315321

316322
if v1beta1Spec.ResourceRequirements != nil && v1beta1Spec.ResourceRequirements.StorageWorkloads != nil {
317-
v1StorageConfig.StorageWorkloads = v1beta1Spec.ResourceRequirements.StorageWorkloads.DeepCopy()
323+
v1StorageConfig.WorkloadResourceRequirements = v1beta1Spec.ResourceRequirements.StorageWorkloads.DeepCopy()
318324
}
319325

320326
return v1StorageConfig

‎api/v1beta1/conversion_test.go‎

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,18 @@ var _ = Describe("api/v1beta1", func() {
513513
})
514514

515515
Context("virtualization", func() {
516-
Context("v1 ==> v1beta2", func() {
516+
Context("v1 ==> v1beta1", func() {
517+
It("should convert tuningPolicy", func() {
518+
v1VirtConfig := hcov1.VirtualizationConfig{
519+
TuningPolicy: hcov1.HyperConvergedAnnotationTuningPolicy,
520+
}
521+
522+
var v1beta1Spec = HyperConvergedSpec{}
523+
Expect(convertVirtualizationV1ToV1beta1(v1VirtConfig, &v1beta1Spec)).To(Succeed())
524+
525+
Expect(v1beta1Spec.TuningPolicy).To(Equal(hcov1.HyperConvergedAnnotationTuningPolicy))
526+
})
527+
517528
It("should convert LiveMigrationConfig", func() {
518529
v1VirtConfig := hcov1.VirtualizationConfig{
519530
LiveMigrationConfig: hcov1.LiveMigrationConfigurations{
@@ -860,6 +871,28 @@ var _ = Describe("api/v1beta1", func() {
860871
})
861872

862873
Context("v1beta1 ==> v1", func() {
874+
It("should convert TuningPolicy if it's 'annotation'", func() {
875+
v1beta1Spec := HyperConvergedSpec{
876+
TuningPolicy: hcov1.HyperConvergedAnnotationTuningPolicy,
877+
}
878+
879+
var v1VirtConfig hcov1.VirtualizationConfig
880+
Expect(convertVirtualizationV1beta1ToV1(v1beta1Spec, &v1VirtConfig)).To(Succeed())
881+
882+
Expect(v1VirtConfig.TuningPolicy).To(Equal(hcov1.HyperConvergedAnnotationTuningPolicy))
883+
})
884+
885+
It("should not convert tuningPolicy if it's 'highBurst'", func() {
886+
v1beta1Spec := HyperConvergedSpec{
887+
TuningPolicy: HyperConvergedHighBurstProfile, //nolint SA1019
888+
}
889+
890+
var v1VirtConfig hcov1.VirtualizationConfig
891+
Expect(convertVirtualizationV1beta1ToV1(v1beta1Spec, &v1VirtConfig)).To(Succeed())
892+
893+
Expect(v1VirtConfig.TuningPolicy).To(BeEmpty())
894+
})
895+
863896
It("should convert LiveMigrationConfig", func() {
864897
v1beta1Spec := HyperConvergedSpec{
865898
LiveMigrationConfig: hcov1.LiveMigrationConfigurations{
@@ -1310,7 +1343,7 @@ var _ = Describe("api/v1beta1", func() {
13101343

13111344
It("should convert StorageWorkloads", func() {
13121345
v1Storage := &hcov1.StorageConfig{
1313-
StorageWorkloads: &corev1.ResourceRequirements{
1346+
WorkloadResourceRequirements: &corev1.ResourceRequirements{
13141347
Limits: corev1.ResourceList{
13151348
corev1.ResourceCPU: resource.MustParse("100m"),
13161349
},
@@ -1340,7 +1373,7 @@ var _ = Describe("api/v1beta1", func() {
13401373
"class-2": "0.2",
13411374
},
13421375
},
1343-
StorageWorkloads: &corev1.ResourceRequirements{
1376+
WorkloadResourceRequirements: &corev1.ResourceRequirements{
13441377
Limits: corev1.ResourceList{
13451378
corev1.ResourceCPU: resource.MustParse("100m"),
13461379
},
@@ -1458,9 +1491,9 @@ var _ = Describe("api/v1beta1", func() {
14581491
v1Storage := convertStorageV1beta1ToV1(v1beta1Spec)
14591492

14601493
Expect(v1Storage).ToNot(BeNil())
1461-
Expect(v1Storage.StorageWorkloads).ToNot(BeNil())
1462-
Expect(v1Storage.StorageWorkloads.Limits).To(HaveLen(1))
1463-
Expect(v1Storage.StorageWorkloads.Limits).To(HaveKeyWithValue(corev1.ResourceCPU, resource.MustParse("100m")))
1494+
Expect(v1Storage.WorkloadResourceRequirements).ToNot(BeNil())
1495+
Expect(v1Storage.WorkloadResourceRequirements.Limits).To(HaveLen(1))
1496+
Expect(v1Storage.WorkloadResourceRequirements.Limits).To(HaveKeyWithValue(corev1.ResourceCPU, resource.MustParse("100m")))
14641497
})
14651498

14661499
It("should convert all fields together", func() {
@@ -1502,9 +1535,9 @@ var _ = Describe("api/v1beta1", func() {
15021535
Expect(v1Storage.FilesystemOverhead.StorageClass).To(HaveKeyWithValue("class-1", cdiv1beta1.Percent("0.3")))
15031536
Expect(v1Storage.FilesystemOverhead.StorageClass).To(HaveKeyWithValue("class-2", cdiv1beta1.Percent("0.2")))
15041537

1505-
Expect(v1Storage.StorageWorkloads).ToNot(BeNil())
1506-
Expect(v1Storage.StorageWorkloads.Limits).To(HaveLen(1))
1507-
Expect(v1Storage.StorageWorkloads.Limits).To(HaveKeyWithValue(corev1.ResourceCPU, resource.MustParse("100m")))
1538+
Expect(v1Storage.WorkloadResourceRequirements).ToNot(BeNil())
1539+
Expect(v1Storage.WorkloadResourceRequirements.Limits).To(HaveLen(1))
1540+
Expect(v1Storage.WorkloadResourceRequirements.Limits).To(HaveKeyWithValue(corev1.ResourceCPU, resource.MustParse("100m")))
15081541
})
15091542
})
15101543

‎api/v1beta1/hyperconverged_types.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,9 @@ import (
2020
// HyperConvergedName is the name of the HyperConverged resource that will be reconciled
2121
const HyperConvergedName = "kubevirt-hyperconverged"
2222

23-
type HyperConvergedTuningPolicy string
24-
25-
// HyperConvergedAnnotationTuningPolicy defines a static configuration of the kubevirt query per seconds (qps) and burst values
26-
// through annotation values.
2723
const (
28-
HyperConvergedAnnotationTuningPolicy HyperConvergedTuningPolicy = "annotation"
2924
// Deprecated: The highBurst profile is deprecated as of v1.16.0 ahead of removal in a future release
30-
HyperConvergedHighBurstProfile HyperConvergedTuningPolicy = "highBurst"
25+
HyperConvergedHighBurstProfile hcov1.HyperConvergedTuningPolicy = "highBurst"
3126
)
3227

3328
// HyperConvergedSpec defines the desired state of HyperConverged
@@ -47,7 +42,8 @@ type HyperConvergedSpec struct {
4742
// Qps and burst values are taken from the annotation hco.kubevirt.io/tuningPolicy
4843
// +kubebuilder:validation:Enum=annotation;highBurst
4944
// +optional
50-
TuningPolicy HyperConvergedTuningPolicy `json:"tuningPolicy,omitempty"`
45+
// +k8s:conversion-gen=false
46+
TuningPolicy hcov1.HyperConvergedTuningPolicy `json:"tuningPolicy,omitempty"`
5147

5248
// infra HyperConvergedConfig influences the pod configuration (currently only placement)
5349
// for all the infra components needed on the virtualization enabled cluster

‎api/v1beta1/zz_generated.conversion.go‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2765,9 +2765,13 @@ spec:
27652765
type: array
27662766
x-kubernetes-list-type: set
27672767
type: object
2768-
storageWorkloads:
2768+
vmStateStorageClass:
2769+
description: VMStateStorageClass is the name of the storage class
2770+
to use for the PVCs created to preserve VM state, like TPM.
2771+
type: string
2772+
workloadResourceRequirements:
27692773
description: |-
2770-
StorageWorkloads defines the resources requirements for storage workloads. It will propagate to the CDI custom
2774+
WorkloadResourceRequirements defines the resource requirements for storage workloads. It will propagate to the CDI custom
27712775
resource
27722776
properties:
27732777
claims:
@@ -2826,21 +2830,7 @@ spec:
28262830
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
28272831
type: object
28282832
type: object
2829-
vmStateStorageClass:
2830-
description: VMStateStorageClass is the name of the storage class
2831-
to use for the PVCs created to preserve VM state, like TPM.
2832-
type: string
28332833
type: object
2834-
tuningPolicy:
2835-
description: |-
2836-
TuningPolicy allows to configure the mode in which the RateLimits of kubevirt are set.
2837-
If TuningPolicy is not present the default kubevirt values are used.
2838-
It can be set to `annotation` for fine-tuning the kubevirt queryPerSeconds (qps) and burst values.
2839-
Qps and burst values are taken from the annotation hco.kubevirt.io/tuningPolicy
2840-
enum:
2841-
- annotation
2842-
- highBurst
2843-
type: string
28442834
virtualization:
28452835
default:
28462836
liveMigrationConfig:
@@ -3279,6 +3269,16 @@ spec:
32793269
- AggregateToDefault
32803270
- Manual
32813271
type: string
3272+
tuningPolicy:
3273+
description: |-
3274+
TuningPolicy allows configuring the mode in which the RateLimits of kubevirt are set.
3275+
If TuningPolicy is not present the default kubevirt values are used.
3276+
It can be set to `annotation` for fine-tuning the kubevirt queryPerSeconds (QPS) and burst values.
3277+
QPS and burst values are taken from the annotation hco.kubevirt.io/tuningPolicy
3278+
enum:
3279+
- annotation
3280+
- highBurst
3281+
type: string
32823282
virtualMachineOptions:
32833283
default:
32843284
disableFreePageReporting: false
@@ -4254,8 +4254,6 @@ spec:
42544254
type: string
42554255
type: object
42564256
type: object
4257-
required:
4258-
- deployment
42594257
type: object
42604258
status:
42614259
description: HyperConvergedStatus defines the observed state of HyperConverged

‎controllers/handlers/kubevirt.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ func getHcoHighBurstProfileTuningValues(hc *hcov1beta1.HyperConverged) (*kubevir
390390

391391
func hcoTuning2Kv(hc *hcov1beta1.HyperConverged) (*kubevirtcorev1.ReloadableComponentConfiguration, error) {
392392
switch hc.Spec.TuningPolicy {
393-
case hcov1beta1.HyperConvergedAnnotationTuningPolicy:
393+
case hcov1.HyperConvergedAnnotationTuningPolicy:
394394
return getHcoAnnotationTuning(hc)
395395
case hcov1beta1.HyperConvergedHighBurstProfile: //nolint SA1019
396396
return getHcoHighBurstProfileTuningValues(hc)

‎controllers/handlers/kubevirt_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3809,7 +3809,7 @@ Version: 1.2.3`)
38093809

38103810
Context("with annotations", func() {
38113811
It("Should return error if annotation is not present", func() {
3812-
hco.Spec.TuningPolicy = hcov1beta1.HyperConvergedAnnotationTuningPolicy
3812+
hco.Spec.TuningPolicy = hcov1.HyperConvergedAnnotationTuningPolicy
38133813

38143814
kv, err := NewKubeVirt(hco)
38153815
Expect(err).To(MatchError("tuning policy set but annotation not present or wrong"))
@@ -3819,7 +3819,7 @@ Version: 1.2.3`)
38193819
})
38203820
It("Should return error if the annotation is present but the parameters are wrong", func() {
38213821

3822-
hco.Spec.TuningPolicy = hcov1beta1.HyperConvergedAnnotationTuningPolicy
3822+
hco.Spec.TuningPolicy = hcov1.HyperConvergedAnnotationTuningPolicy
38233823
hco.Annotations = make(map[string]string, 1)
38243824
//burst is missing
38253825
hco.Annotations[common.TuningPolicyAnnotationName] = `{"qps": 100}`
@@ -3831,7 +3831,7 @@ Version: 1.2.3`)
38313831
})
38323832

38333833
It("Should return error if the json annotation is corrupted", func() {
3834-
hco.Spec.TuningPolicy = hcov1beta1.HyperConvergedAnnotationTuningPolicy
3834+
hco.Spec.TuningPolicy = hcov1.HyperConvergedAnnotationTuningPolicy
38353835
hco.Annotations = make(map[string]string, 1)
38363836
// qps field is missing a "
38373837
hco.Annotations[common.TuningPolicyAnnotationName] = `{"qps: 100, "burst": 200}`
@@ -3843,7 +3843,7 @@ Version: 1.2.3`)
38433843
})
38443844

38453845
It("Should create the fields and populate them when requested", func() {
3846-
hco.Spec.TuningPolicy = hcov1beta1.HyperConvergedAnnotationTuningPolicy
3846+
hco.Spec.TuningPolicy = hcov1.HyperConvergedAnnotationTuningPolicy
38473847
hco.Annotations = make(map[string]string, 1)
38483848
hco.Annotations[common.TuningPolicyAnnotationName] = `{"qps": 100, "burst": 200}`
38493849

0 commit comments

Comments
 (0)