checkup: Introduce vmi under test configmap #219

RamLavi · 2024-01-25T09:59:38Z

This PR introduces a new configmap to the checkup, that will be consumed by the VMI-under-test VMI.
In future PRs this configmap will be used to insert scripts to the guest.

RamLavi · 2024-01-25T10:08:39Z

passes on CNV 4.15 cluster:

make test/e2e
podman run --rm \
           --volume /home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup:/home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup:Z \
           --volume /home/ralavi/.kube/sno01-cnvqe2-rdu2:/root/.kube:Z,ro \
           --workdir /home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup \
           -e KUBECONFIG=/root/.kube/kubeconfig \
           -e TEST_CHECKUP_IMAGE=quay.io/ramlavi/kubevirt-dpdk-checkup:latest \
           -e TEST_NAMESPACE=dpdk-checkup-ns-1 \
           -e NETWORK_ATTACHMENT_DEFINITION_NAME=dpdk-sriovnetwork-ns-1 \
           -e TRAFFIC_GEN_CONTAINER_DISK_IMAGE=quay.io/kiagnose/kubevirt-dpdk-checkup-traffic-gen:main \
           -e VM_UNDER_TEST_CONTAINER_DISK_IMAGE=quay.io/kiagnose/kubevirt-dpdk-checkup-vm:main \
           docker.io/library/golang:1.20.12 go test ./tests/... -test.v -test.timeout=1h -ginkgo.v -ginkgo.timeout=1h
=== RUN   TestKubevirtDpdkCheckup
Running Suite: KubevirtDpdkCheckup Suite - /home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup/tests
==============================================================================================================
Random Seed: 1706176945

Will run 1 of 1 specs
------------------------------
[BeforeSuite] 
/home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup/tests/test_suite_test.go:56
[BeforeSuite] PASSED [0.002 seconds]
------------------------------
Execute the checkup Job should complete successfully
/home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup/tests/checkup_test.go:83
• [308.245 seconds]
------------------------------

Ran 1 of 1 Specs in 308.248 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestKubevirtDpdkCheckup (308.25s)
PASS
ok  	github.com/kiagnose/kubevirt-dpdk-checkup/tests	308.269s

orelmisan

Thank you for the PR @RamLavi.
Please see the inline comments.

orelmisan · 2024-01-25T10:10:34Z

pkg/internal/checkup/checkup.go

+func vmiUnderTestConfigMapName(suffix string) string {
+	return vmiUnderTestConfigMapNamePrefix + "-" + suffix
+}


This function is a duplicate of trafficGenConfigMapName(), please consider adjusting the signature of trafficGenConfigMapName and using a single function for both uses.

Let's do it in a later PR

orelmisan · 2024-01-25T10:12:09Z

pkg/internal/checkup/checkup_test.go

@@ -200,7 +200,7 @@ func TestTeardownShouldFailWhen(t *testing.T) {
 	})
 }

-func TestTrafficGenCMTeardownFailure(t *testing.T) {
+func TestCMTeardownFailure(t *testing.T) {


WDYT about?

Suggested change

func TestCMTeardownFailure(t *testing.T) {

func TestVMConfigMapTeardownFailure(t *testing.T) {

orelmisan · 2024-01-25T10:14:52Z

pkg/internal/checkup/vmi.go

+		vmi.WithConfigMapVolume(configVolumeName, configMapName),
+		vmi.WithConfigMapDisk(configVolumeName, configDiskSerial),


This part could potentially be moved to baseOptions() so it could be shared by the two VMIs.

I thought about it, but ended up not liking the fact that part of the configmap is loaded on the base function, and part here. It makes it less understood. WDYT?

orelmisan · 2024-01-25T10:16:03Z

pkg/internal/checkup/vmi.go

@@ -156,3 +164,12 @@ func trafficGenBootCommands(configDiskSerial string) []string {
 		fmt.Sprintf("cp %s/*.py %s", configMountDirectory, trex.StreamsPyPath),
 	}
 }
+
+func vmiUnderTestBootCommands(configDiskSerial string) []string {
+	const configMountDirectory = "/mnt/app-config"


Please consider increasing the scope of the existing configMountDirectory const, and share it between the functions.

we don't need it anywhere else. Everything is copied from this dir to where it will actually be executed.

orelmisan · 2024-01-25T10:17:08Z

pkg/internal/checkup/vmi.go

-func newVMIUnderTest(name string, checkupConfig config.Config) *kvcorev1.VirtualMachineInstance {
+func newVMIUnderTest(name string, checkupConfig config.Config, configMapName string) *kvcorev1.VirtualMachineInstance {
+	const (
+		configDiskSerial = "DEADBEEF"


Please consider increasing the scope of the original configDiskSerial const, and share it between the VMIs.

I don't see a reason to do that, It would imply that it is connected somehow..
Both VMIs have configmaps, both happen to have the same serial..

orelmisan · 2024-01-25T10:19:26Z

pkg/internal/checkup/checkup.go

+func (c *Checkup) createVMIUnderTestCM(ctx context.Context) error {
+	log.Printf("Creating ConfigMap %q...", ObjectFullName(c.namespace, c.vmiUnderTestConfigMap.Name))
+
+	_, err := c.client.CreateConfigMap(ctx, c.namespace, c.vmiUnderTestConfigMap)
+	return err
+}
+
+func (c *Checkup) deleteVMIUnderTestCM(ctx context.Context) error {
+	log.Printf("Deleting ConfigMap %q...", ObjectFullName(c.namespace, c.vmiUnderTestConfigMap.Name))
+
+	return c.client.DeleteConfigMap(ctx, c.namespace, c.vmiUnderTestConfigMap.Name)
+}


Please consider adjusting the signatures of createTrafficGenCM and deleteTrafficGenCM so they could be shared between the two configMaps.

In a similar way to how createVMI and deleteVMI work.

This commit adds a new configmap that will be consumed by the vmi-under-test in future commits. The create and delete configmap functions are generalized to accept both configmaps. The unit test is also generalized in order to include any configmap deleted/created. Signed-off-by: Ram Lavi <[email protected]>

Currently the configmap is mounted but has no data. Signed-off-by: Ram Lavi <[email protected]>

RamLavi · 2024-01-25T11:31:40Z

make test/e2e
podman run --rm \
           --volume /home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup:/home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup:Z \
           --volume /home/ralavi/.kube/sno01-cnvqe2-rdu2:/root/.kube:Z,ro \
           --workdir /home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup \
           -e KUBECONFIG=/root/.kube/kubeconfig \
           -e TEST_CHECKUP_IMAGE=quay.io/ramlavi/kubevirt-dpdk-checkup:latest \
           -e TEST_NAMESPACE=dpdk-checkup-ns-1 \
           -e NETWORK_ATTACHMENT_DEFINITION_NAME=dpdk-sriovnetwork-ns-1 \
           -e TRAFFIC_GEN_CONTAINER_DISK_IMAGE=quay.io/kiagnose/kubevirt-dpdk-checkup-traffic-gen:main \
           -e VM_UNDER_TEST_CONTAINER_DISK_IMAGE=quay.io/kiagnose/kubevirt-dpdk-checkup-vm:main \
           docker.io/library/golang:1.20.12 go test ./tests/... -test.v -test.timeout=1h -ginkgo.v -ginkgo.timeout=1h
=== RUN   TestKubevirtDpdkCheckup
Running Suite: KubevirtDpdkCheckup Suite - /home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup/tests
==============================================================================================================
Random Seed: 1706181861

Will run 1 of 1 specs
------------------------------
[BeforeSuite] 
/home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup/tests/test_suite_test.go:56
[BeforeSuite] PASSED [0.003 seconds]
------------------------------
Execute the checkup Job should complete successfully
/home/ralavi/go/src/github.com/kiagnose/kubevirt-dpdk-checkup/tests/checkup_test.go:83
• [361.935 seconds]
------------------------------

Ran 1 of 1 Specs in 361.938 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestKubevirtDpdkCheckup (361.94s)
PASS
ok  	github.com/kiagnose/kubevirt-dpdk-checkup/tests	361.971s

RamLavi requested a review from orelmisan January 25, 2024 09:59

orelmisan reviewed Jan 25, 2024

View reviewed changes

RamLavi mentioned this pull request Jan 25, 2024

Move boot scripts to cloud init #220

Merged

RamLavi added 2 commits January 25, 2024 13:20

checkup/vmi: mount configmap to vmi-under-test

d63b317

Currently the configmap is mounted but has no data. Signed-off-by: Ram Lavi <[email protected]>

RamLavi force-pushed the introduce_vmi_under_test_configmap branch from de93b0d to d63b317 Compare January 25, 2024 11:21

orelmisan approved these changes Jan 25, 2024

View reviewed changes

orelmisan merged commit ed5c2cb into kiagnose:main Jan 25, 2024
4 checks passed

RamLavi mentioned this pull request Jan 25, 2024

boot VMIs from the checkup's setup #217

Closed

orelmisan mentioned this pull request Jan 28, 2024

[release-0.3] Fix login race #226

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

checkup: Introduce vmi under test configmap #219

checkup: Introduce vmi under test configmap #219

RamLavi commented Jan 25, 2024

RamLavi commented Jan 25, 2024

orelmisan left a comment

orelmisan Jan 25, 2024

RamLavi Jan 25, 2024

orelmisan Jan 25, 2024

RamLavi Jan 25, 2024

orelmisan Jan 25, 2024

RamLavi Jan 25, 2024

orelmisan Jan 25, 2024

RamLavi Jan 25, 2024

orelmisan Jan 25, 2024

RamLavi Jan 25, 2024

orelmisan Jan 25, 2024 •

edited

Loading

RamLavi Jan 25, 2024

RamLavi commented Jan 25, 2024

	func TestCMTeardownFailure(t *testing.T) {
	func TestVMConfigMapTeardownFailure(t *testing.T) {

		vmi.WithConfigMapVolume(configVolumeName, configMapName),
		vmi.WithConfigMapDisk(configVolumeName, configDiskSerial),

checkup: Introduce vmi under test configmap #219

checkup: Introduce vmi under test configmap #219

Conversation

RamLavi commented Jan 25, 2024

RamLavi commented Jan 25, 2024

orelmisan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orelmisan Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RamLavi commented Jan 25, 2024

orelmisan Jan 25, 2024 •

edited

Loading