From d1a356f8152337bd32cd2952c2e6406403500139 Mon Sep 17 00:00:00 2001 From: Ajay Victor Date: Thu, 3 Oct 2024 21:34:14 +0530 Subject: [PATCH 1/2] libvirt: Enable multiple PodVM image scenario Enable support for multiple PodVM images for libvirt provider with the merge of kata-containers/kata-containers#10274 Fixes confidential-containers#1282 Signed-off-by: Ajay Victor --- .../pkg/adaptor/cloud/cloud.go | 4 ++ src/cloud-api-adaptor/pkg/util/cloud.go | 8 ++++ src/cloud-api-adaptor/pkg/util/cloud_test.go | 39 +++++++++++++++++++ src/cloud-providers/libvirt/provider.go | 5 +++ src/cloud-providers/types.go | 1 + 5 files changed, 57 insertions(+) diff --git a/src/cloud-api-adaptor/pkg/adaptor/cloud/cloud.go b/src/cloud-api-adaptor/pkg/adaptor/cloud/cloud.go index 7afadd439..2ff72a7db 100644 --- a/src/cloud-api-adaptor/pkg/adaptor/cloud/cloud.go +++ b/src/cloud-api-adaptor/pkg/adaptor/cloud/cloud.go @@ -203,11 +203,15 @@ func (s *cloudService) CreateVM(ctx context.Context, req *pb.CreateVMRequest) (r // Get Pod VM cpu and memory from annotations vcpus, memory := util.GetCPUAndMemoryFromAnnotation(req.Annotations) + // Get Pod VM image from annotations + image := util.GetImageFromAnnotation(req.Annotations) + // Pod VM spec vmSpec := provider.InstanceTypeSpec{ InstanceType: instanceType, VCPUs: vcpus, Memory: memory, + Image: image, } // TODO: server name is also generated in each cloud provider, and possibly inconsistent diff --git a/src/cloud-api-adaptor/pkg/util/cloud.go b/src/cloud-api-adaptor/pkg/util/cloud.go index 775aa430d..38a651b9c 100644 --- a/src/cloud-api-adaptor/pkg/util/cloud.go +++ b/src/cloud-api-adaptor/pkg/util/cloud.go @@ -35,6 +35,14 @@ func GetInstanceTypeFromAnnotation(annotations map[string]string) string { return annotations[hypannotations.MachineType] } +// Method to get image from annotation +func GetImageFromAnnotation(annotations map[string]string) string { + // The image annotation in Kata refers to image path + // For example image for Kata/Qemu refers to /hypervisor/image.img etc. + // We use the same annotation for Kata/remote to refer to image name + return annotations[hypannotations.ImagePath] +} + // Method to get vCPU and memory from annotations func GetCPUAndMemoryFromAnnotation(annotations map[string]string) (int64, int64) { diff --git a/src/cloud-api-adaptor/pkg/util/cloud_test.go b/src/cloud-api-adaptor/pkg/util/cloud_test.go index c49a809f4..5da0a3693 100644 --- a/src/cloud-api-adaptor/pkg/util/cloud_test.go +++ b/src/cloud-api-adaptor/pkg/util/cloud_test.go @@ -114,3 +114,42 @@ func TestGetInstanceTypeFromAnnotation(t *testing.T) { }) } } + +func TestGetImageFromAnnotation(t *testing.T) { + type args struct { + annotations map[string]string + } + tests := []struct { + name string + args args + want string + }{ + // Add test cases with annotations for only image name + { + name: "image name only", + args: args{ + annotations: map[string]string{ + hypannotations.ImagePath: "rhel9-os", + }, + }, + want: "rhel9-os", + }, + // Add test cases with annotations for only image name with empty value + { + name: "image name only with empty value", + args: args{ + annotations: map[string]string{ + hypannotations.ImagePath: "", + }, + }, + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetImageFromAnnotation(tt.args.annotations); got != tt.want { + t.Errorf("GetImageFromAnnotation() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/src/cloud-providers/libvirt/provider.go b/src/cloud-providers/libvirt/provider.go index dce89a148..f795a384c 100644 --- a/src/cloud-providers/libvirt/provider.go +++ b/src/cloud-providers/libvirt/provider.go @@ -79,6 +79,11 @@ func (p *libvirtProvider) CreateInstance(ctx context.Context, podName, sandboxID } logger.Printf("LaunchSecurityType: %s", vm.launchSecurityType.String()) + if spec.Image != "" { + logger.Printf("Choosing %s as libvirt volume for the PodVM image", spec.Image) + p.libvirtClient.volName = spec.Image + } + result, err := CreateDomain(ctx, p.libvirtClient, vm) if err != nil { logger.Printf("failed to create an instance : %v", err) diff --git a/src/cloud-providers/types.go b/src/cloud-providers/types.go index 55bd8b1a9..82ea5b0c7 100644 --- a/src/cloud-providers/types.go +++ b/src/cloud-providers/types.go @@ -66,4 +66,5 @@ type InstanceTypeSpec struct { Memory int64 Arch string GPUs int64 + Image string } From 5f5ef7eb78cc733a435c232aaec86d0fc1b9615f Mon Sep 17 00:00:00 2001 From: Ajay Victor Date: Thu, 3 Oct 2024 21:38:42 +0530 Subject: [PATCH 2/2] test/e2e: Adding tests for multiple PodVM scenario Negative test for multiple PodVM images on libvirt provider with a non existing volume Fixes confidential-containers#1282 Signed-off-by: Ajay Victor --- .../test/e2e/assessment_helpers.go | 28 ++++ .../test/e2e/assessment_runner.go | 15 +++ .../test/e2e/common_suite.go | 38 ++++++ .../test/e2e/libvirt_test.go | 12 +- .../provisioner/libvirt/provision_common.go | 127 ++++++++++-------- 5 files changed, 161 insertions(+), 59 deletions(-) diff --git a/src/cloud-api-adaptor/test/e2e/assessment_helpers.go b/src/cloud-api-adaptor/test/e2e/assessment_helpers.go index 84334f88b..c587c1de6 100644 --- a/src/cloud-api-adaptor/test/e2e/assessment_helpers.go +++ b/src/cloud-api-adaptor/test/e2e/assessment_helpers.go @@ -525,3 +525,31 @@ func AddImagePullSecretToDefaultServiceAccount(ctx context.Context, client klien } return nil } + +func VerifyCloudAPIAdaptorLogs(ctx context.Context, client klient.Client, t *testing.T, pod *v1.Pod, expectedSuccessMessage string) error { + var podlist v1.PodList + var podLogString string + + if err := client.Resources("confidential-containers-system").List(ctx, &podlist); err != nil { + return err + } + for _, pod := range podlist.Items { + if pod.Labels["app"] == "cloud-api-adaptor" { + podLogString, _ = GetPodLog(ctx, client, pod) + break + } + } + if strings.Contains(podLogString, expectedSuccessMessage) { + t.Logf("cloud-api-adaptor logs are matching with the expected message") + } else { + yamlData, err := yaml.Marshal(pod.Status) + if err != nil { + t.Fatalf("Error marshaling pod.Status to JSON: %s", err.Error()) + } else { + t.Logf("Current Pod State: %v", string(yamlData)) + t.Fatal("failed due to unknown reason") + } + return errors.New("failed due to unknown reason") + } + return nil +} diff --git a/src/cloud-api-adaptor/test/e2e/assessment_runner.go b/src/cloud-api-adaptor/test/e2e/assessment_runner.go index a1d28322b..db293450c 100644 --- a/src/cloud-api-adaptor/test/e2e/assessment_runner.go +++ b/src/cloud-api-adaptor/test/e2e/assessment_runner.go @@ -78,6 +78,7 @@ type TestCase struct { testInstanceTypes InstanceValidatorFunctions isNydusSnapshotter bool FailReason string + alternateImageName string } func (tc *TestCase) WithConfigMap(configMap *v1.ConfigMap) *TestCase { @@ -135,6 +136,11 @@ func (tc *TestCase) WithInstanceTypes(testInstanceTypes InstanceValidatorFunctio return tc } +func (tc *TestCase) WithAlternateImage(alternateImageName string) *TestCase { + tc.alternateImageName = alternateImageName + return tc +} + func (pod *ExtraPod) WithTestCommands(TestCommands []TestCommand) *ExtraPod { pod.testCommands = TestCommands return pod @@ -545,6 +551,15 @@ func (tc *TestCase) Run() { } } + + if tc.alternateImageName != "" { + expectedSuccessMessage := "Choosing " + tc.alternateImageName + err := VerifyCloudAPIAdaptorLogs(ctx, client, t, tc.pod, expectedSuccessMessage) + if err != nil { + t.Fatal(err) + } + t.Logf("PodVM was brought up using the alternate PodVM image %s", tc.alternateImageName) + } return ctx }). Teardown(func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { diff --git a/src/cloud-api-adaptor/test/e2e/common_suite.go b/src/cloud-api-adaptor/test/e2e/common_suite.go index c575c8103..364acc86d 100644 --- a/src/cloud-api-adaptor/test/e2e/common_suite.go +++ b/src/cloud-api-adaptor/test/e2e/common_suite.go @@ -406,6 +406,44 @@ func DoTestPodVMwithAnnotationsLargerCPU(t *testing.T, e env.Environment, assert NewTestCase(t, e, "PodVMwithAnnotationsLargerCPU", assert, "Failed to Create PodVM with Annotations Larger CPU").WithPod(pod).WithInstanceTypes(testInstanceTypes).WithCustomPodState(v1.PodPending).Run() } +func DoTestCreatePeerPodContainerWithValidAlternateImage(t *testing.T, e env.Environment, assert CloudAssert, alternateImageName string) { + podName := "annotations-valid-alternate-image" + containerName := "busybox" + imageName := BUSYBOX_IMAGE + annotationData := map[string]string{ + "io.katacontainers.config.hypervisor.image": alternateImageName, + } + pod := NewPod(E2eNamespace, podName, containerName, imageName, WithCommand([]string{"/bin/sh", "-c", "sleep 3600"}), WithAnnotations(annotationData)) + + NewTestCase(t, e, "PodVMwithAnnotationsValidAlternateImage", assert, "PodVM created with an alternate image").WithPod(pod).WithAlternateImage(alternateImageName).Run() +} + +func DoTestCreatePeerPodContainerWithInvalidAlternateImage(t *testing.T, e env.Environment, assert CloudAssert) { + podName := "annotations-invalid-alternate-image" + containerName := "busybox" + imageName := BUSYBOX_IMAGE + nonExistingImageName := "non-existing-image" + expectedErrorMessage := "Error in creating volume: Can't retrieve volume " + nonExistingImageName + annotationData := map[string]string{ + "io.katacontainers.config.hypervisor.image": nonExistingImageName, + } + pod := NewPod(E2eNamespace, podName, containerName, imageName, WithCommand([]string{"/bin/sh", "-c", "sleep 3600"}), WithAnnotations(annotationData)) + + testInstanceTypes := InstanceValidatorFunctions{ + testSuccessfn: IsStringEmpty, + testFailurefn: func(errorMsg error) bool { + if strings.Contains(errorMsg.Error(), expectedErrorMessage) { + t.Logf("Got Expected Error: %v", errorMsg.Error()) + return true + } else { + t.Logf("Failed to Get Expected Error: %v", errorMsg.Error()) + return false + } + }, + } + NewTestCase(t, e, "PodVMwithAnnotationsInvalidAlternateImage", assert, "Failed to Create PodVM with a non-existent image").WithPod(pod).WithInstanceTypes(testInstanceTypes).WithCustomPodState(v1.PodPending).Run() +} + func DoTestPodToServiceCommunication(t *testing.T, e env.Environment, assert CloudAssert) { clientPodName := "test-client" clientContainerName := "busybox" diff --git a/src/cloud-api-adaptor/test/e2e/libvirt_test.go b/src/cloud-api-adaptor/test/e2e/libvirt_test.go index 0c6e17f31..3990b4209 100644 --- a/src/cloud-api-adaptor/test/e2e/libvirt_test.go +++ b/src/cloud-api-adaptor/test/e2e/libvirt_test.go @@ -9,7 +9,7 @@ import ( "os" "testing" - _ "github.com/confidential-containers/cloud-api-adaptor/src/cloud-api-adaptor/test/provisioner/libvirt" + "github.com/confidential-containers/cloud-api-adaptor/src/cloud-api-adaptor/test/provisioner/libvirt" ) func TestLibvirtCreateSimplePod(t *testing.T) { @@ -35,6 +35,16 @@ func TestLibvirtCreatePeerPodContainerWithExternalIPAccess(t *testing.T) { } +func TestLibvirtCreatePeerPodContainerWithValidAlternateImage(t *testing.T) { + assert := LibvirtAssert{} + DoTestCreatePeerPodContainerWithValidAlternateImage(t, testEnv, assert, libvirt.AlternateVolumeName) +} + +func TestLibvirtCreatePeerPodContainerWithInvalidAlternateImage(t *testing.T) { + assert := LibvirtAssert{} + DoTestCreatePeerPodContainerWithInvalidAlternateImage(t, testEnv, assert) +} + func TestLibvirtCreatePeerPodWithJob(t *testing.T) { assert := LibvirtAssert{} DoTestCreatePeerPodWithJob(t, testEnv, assert) diff --git a/src/cloud-api-adaptor/test/provisioner/libvirt/provision_common.go b/src/cloud-api-adaptor/test/provisioner/libvirt/provision_common.go index 226ec2c3c..b28ed664c 100644 --- a/src/cloud-api-adaptor/test/provisioner/libvirt/provision_common.go +++ b/src/cloud-api-adaptor/test/provisioner/libvirt/provision_common.go @@ -19,6 +19,8 @@ import ( "sigs.k8s.io/e2e-framework/pkg/envconf" ) +const AlternateVolumeName = "another-podvm-base.qcow2" + // LibvirtProvisioner implements the CloudProvisioner interface for Libvirt. type LibvirtProvisioner struct { conn *libvirt.Connect // Libvirt connection @@ -152,30 +154,35 @@ func (l *LibvirtProvisioner) CreateVPC(ctx context.Context, cfg *envconf.Config) return fmt.Errorf("Storage pool '%s' not found. It should be created beforehand", l.storage) } - // Create the podvm storage volume if it does not exist. - if _, err = sPool.LookupStorageVolByName(l.volumeName); err != nil { - volCfg := libvirtxml.StorageVolume{ - Name: l.volumeName, - Capacity: &libvirtxml.StorageVolumeSize{ - Unit: "GiB", - Value: 20, - }, - Allocation: &libvirtxml.StorageVolumeSize{ - Unit: "GiB", - Value: 2, - }, - Target: &libvirtxml.StorageVolumeTarget{ - Format: &libvirtxml.StorageVolumeTargetFormat{ - Type: "qcow2", + // Create two volumes to test the multiple podvm image scenario. + lVolumes := [2]string{l.volumeName, AlternateVolumeName} + + // Create the podvm storage volumes if it does not exist. + for _, volume := range lVolumes { + if _, err = sPool.LookupStorageVolByName(volume); err != nil { + volCfg := libvirtxml.StorageVolume{ + Name: volume, + Capacity: &libvirtxml.StorageVolumeSize{ + Unit: "GiB", + Value: 20, }, - }, - } - xml, err := volCfg.Marshal() - if err != nil { - return err - } - if _, err = sPool.StorageVolCreateXML(xml, libvirt.STORAGE_VOL_CREATE_PREALLOC_METADATA); err != nil { - return err + Allocation: &libvirtxml.StorageVolumeSize{ + Unit: "GiB", + Value: 2, + }, + Target: &libvirtxml.StorageVolumeTarget{ + Format: &libvirtxml.StorageVolumeTargetFormat{ + Type: "qcow2", + }, + }, + } + xml, err := volCfg.Marshal() + if err != nil { + return err + } + if _, err = sPool.StorageVolCreateXML(xml, libvirt.STORAGE_VOL_CREATE_PREALLOC_METADATA); err != nil { + return err + } } } return nil @@ -227,52 +234,56 @@ func (l *LibvirtProvisioner) UploadPodvm(imagePath string, ctx context.Context, } length := fileStat.Size() - sVol, err := sPool.LookupStorageVolByName(l.volumeName) - if err != nil { - return err - } + lVolumes := [2]string{l.volumeName, AlternateVolumeName} - stream, err := l.conn.NewStream(0) - if err != nil { - return err - } - - if err := sVol.Upload(stream, 0, uint64(length), libvirt.STORAGE_VOL_UPLOAD_SPARSE_STREAM); err != nil { - return err - } + for _, volume := range lVolumes { + sVol, err := sPool.LookupStorageVolByName(volume) + if err != nil { + return err + } - fileByteSlice, err := os.ReadFile(imagePath) - if err != nil { - return err - } + stream, err := l.conn.NewStream(0) + if err != nil { + return err + } - sent := 0 - source := func(stream *libvirt.Stream, nbytes int) ([]byte, error) { - tosend := nbytes - if tosend > (len(fileByteSlice) - sent) { - tosend = len(fileByteSlice) - sent + if err := sVol.Upload(stream, 0, uint64(length), libvirt.STORAGE_VOL_UPLOAD_SPARSE_STREAM); err != nil { + return err } - if tosend == 0 { - return []byte{}, nil + fileByteSlice, err := os.ReadFile(imagePath) + if err != nil { + return err } - data := fileByteSlice[sent : sent+tosend] - sent += tosend + sent := 0 + source := func(stream *libvirt.Stream, nbytes int) ([]byte, error) { + tosend := nbytes + if tosend > (len(fileByteSlice) - sent) { + tosend = len(fileByteSlice) - sent + } - return data, nil - } + if tosend == 0 { + return []byte{}, nil + } - if err := stream.SendAll(source); err != nil { - return err - } + data := fileByteSlice[sent : sent+tosend] + sent += tosend - if err := stream.Finish(); err != nil { - return err - } + return data, nil + } - if err := stream.Free(); err != nil { - return err + if err := stream.SendAll(source); err != nil { + return err + } + + if err := stream.Finish(); err != nil { + return err + } + + if err := stream.Free(); err != nil { + return err + } } return nil