From 9501e6f3cb0907f78c7522d0ffad58530106cc92 Mon Sep 17 00:00:00 2001 From: Orel Misan Date: Mon, 8 Apr 2024 11:54:48 +0300 Subject: [PATCH 1/2] Config: Make vmUnderTestContainerDiskImage mandatory Currently, vmUnderTestContainerDiskImage defaults to: `quay.io/kiagnose/kubevirt-realtime-checkup-vm:main`. This may lead to unexpected behavior when using an older checkup version in combination with the latest VM container disk image (tagged as `main`). Make the field mandatory in order for the user to specify it explicitly. Signed-off-by: Orel Misan --- pkg/internal/config/config.go | 12 ++++++------ pkg/internal/config/config_test.go | 25 +++++++++++++++++-------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/pkg/internal/config/config.go b/pkg/internal/config/config.go index aeaea3dd..f05916b5 100644 --- a/pkg/internal/config/config.go +++ b/pkg/internal/config/config.go @@ -37,9 +37,8 @@ const ( const ( VMIPassword = "redhat" // #nosec - VMUnderTestDefaultContainerDiskImage = "quay.io/kiagnose/kubevirt-realtime-checkup-vm:main" - OslatDefaultDuration = 5 * time.Minute - OslatDefaultLatencyThreshold = 40 * time.Microsecond + OslatDefaultDuration = 5 * time.Minute + OslatDefaultLatencyThreshold = 40 * time.Microsecond BootScriptName = "realtime-checkup-boot.sh" BootScriptBinDirectory = "/usr/bin/" @@ -48,6 +47,7 @@ const ( ) var ( + ErrInvalidVMContainerDiskImage = errors.New("invalid VM container disk image") ErrInvalidOslatDuration = errors.New("invalid oslat duration") ErrInvalidOslatLatencyThreshold = errors.New("invalid oslat latency threshold") ) @@ -66,13 +66,13 @@ func New(baseConfig kconfig.Config) (Config, error) { PodName: baseConfig.PodName, PodUID: baseConfig.PodUID, VMUnderTestTargetNodeName: baseConfig.Params[VMUnderTestTargetNodeNameParamName], - VMUnderTestContainerDiskImage: VMUnderTestDefaultContainerDiskImage, + VMUnderTestContainerDiskImage: baseConfig.Params[VMUnderTestContainerDiskImageParamName], OslatDuration: OslatDefaultDuration, OslatLatencyThreshold: OslatDefaultLatencyThreshold, } - if rawVal := baseConfig.Params[VMUnderTestContainerDiskImageParamName]; rawVal != "" { - newConfig.VMUnderTestContainerDiskImage = rawVal + if newConfig.VMUnderTestContainerDiskImage == "" { + return Config{}, ErrInvalidVMContainerDiskImage } if rawOslatDuration := baseConfig.Params[OslatDurationParamName]; rawOslatDuration != "" { diff --git a/pkg/internal/config/config_test.go b/pkg/internal/config/config_test.go index f75b397b..081f347d 100644 --- a/pkg/internal/config/config_test.go +++ b/pkg/internal/config/config_test.go @@ -43,7 +43,9 @@ func TestNewShouldApplyDefaultsWhenOptionalFieldsAreMissing(t *testing.T) { baseConfig := kconfig.Config{ PodName: testPodName, PodUID: testPodUID, - Params: map[string]string{}, + Params: map[string]string{ + config.VMUnderTestContainerDiskImageParamName: testVMContainerDiskImage, + }, } actualConfig, err := config.New(baseConfig) @@ -53,7 +55,7 @@ func TestNewShouldApplyDefaultsWhenOptionalFieldsAreMissing(t *testing.T) { PodName: testPodName, PodUID: testPodUID, VMUnderTestTargetNodeName: "", - VMUnderTestContainerDiskImage: config.VMUnderTestDefaultContainerDiskImage, + VMUnderTestContainerDiskImage: testVMContainerDiskImage, OslatDuration: config.OslatDefaultDuration, OslatLatencyThreshold: config.OslatDefaultLatencyThreshold, } @@ -94,21 +96,28 @@ func TestNewShouldFailWhen(t *testing.T) { } testCases := []failureTestCase{ + { + description: "VM container disk image is missing", + userParameters: map[string]string{}, + expectedError: config.ErrInvalidVMContainerDiskImage, + }, { description: "oslatDuration is invalid", userParameters: map[string]string{ - config.VMUnderTestTargetNodeNameParamName: testVMUnderTestTargetNodeName, - config.OslatDurationParamName: "wrongValue", - config.OslatLatencyThresholdParamName: testOslatLatencyThresholdMicroSeconds, + config.VMUnderTestContainerDiskImageParamName: testVMContainerDiskImage, + config.VMUnderTestTargetNodeNameParamName: testVMUnderTestTargetNodeName, + config.OslatDurationParamName: "wrongValue", + config.OslatLatencyThresholdParamName: testOslatLatencyThresholdMicroSeconds, }, expectedError: config.ErrInvalidOslatDuration, }, { description: "oslatLatencyThresholdMicroSeconds is invalid", userParameters: map[string]string{ - config.VMUnderTestTargetNodeNameParamName: testVMUnderTestTargetNodeName, - config.OslatDurationParamName: testOslatDuration, - config.OslatLatencyThresholdParamName: "wrongValue", + config.VMUnderTestContainerDiskImageParamName: testVMContainerDiskImage, + config.VMUnderTestTargetNodeNameParamName: testVMUnderTestTargetNodeName, + config.OslatDurationParamName: testOslatDuration, + config.OslatLatencyThresholdParamName: "wrongValue", }, expectedError: config.ErrInvalidOslatLatencyThreshold, }, From acd1d1d71555347991c4f73bf1ff98ef9de3a9af Mon Sep 17 00:00:00 2001 From: Orel Misan Date: Mon, 8 Apr 2024 12:01:20 +0300 Subject: [PATCH 2/2] README: Make vmUnderTestContainerDiskImage mandatory Following the previous commit, adjust the documentation accordingly. Signed-off-by: Orel Misan --- README.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index fda55516..c204d035 100644 --- a/README.md +++ b/README.md @@ -65,13 +65,13 @@ roleRef: ## Configuration -| Key | Description | Is Mandatory | Remarks | -|----------------------------------------------|-----------------------------------------------------------------|--------------|------------------------------------------------------------------| -| spec.timeout | How much time before the checkup will try to close itself | True | | -| spec.param.vmUnderTestContainerDiskImage | VM under test container disk image | False | Defaults to `quay.io/kiagnose/kubevirt-realtime-checkup-vm:main` | -| spec.param.vmUnderTestTargetNodeName | Node Name on which the VM under test will be scheduled to | False | Assumed to be configured to nodes that allow realtime traffic | -| spec.param.oslatDuration | How much time will the oslat program run | False | Defaults to TBD | -| spec.param.oslatLatencyThresholdMicroSeconds | A latency higher than this value will cause the checkup to fail | False | Defaults to TBD | +| Key | Description | Is Mandatory | Remarks | +|----------------------------------------------|-----------------------------------------------------------------|--------------|---------------------------------------------------------------| +| spec.timeout | How much time before the checkup will try to close itself | True | | +| spec.param.vmUnderTestContainerDiskImage | VM under test container disk image | True | | +| spec.param.vmUnderTestTargetNodeName | Node Name on which the VM under test will be scheduled to | False | Assumed to be configured to nodes that allow realtime traffic | +| spec.param.oslatDuration | How much time will the oslat program run | False | Defaults to TBD | +| spec.param.oslatLatencyThresholdMicroSeconds | A latency higher than this value will cause the checkup to fail | False | Defaults to TBD | ### Example @@ -82,6 +82,7 @@ metadata: name: realtime-checkup-config data: spec.timeout: 10m + spec.param.vmUnderTestContainerDiskImage: quay.io/kiagnose/kubevirt-realtime-checkup-vm:main spec.param.oslatDuration: 1h ```