Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

libvirt: Enable multiple PodVM image scenario #2061

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

libvirt: Enable multiple PodVM image scenario #2061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d1a356f
libvirt: Enable multiple PodVM image scenario
ajaypvictor Oct 3, 2024
5f5ef7e
test/e2e: Adding tests for multiple PodVM scenario
ajaypvictor Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/cloud-api-adaptor/pkg/adaptor/cloud/cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/cloud-api-adaptor/pkg/util/cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down
39 changes: 39 additions & 0 deletions src/cloud-api-adaptor/pkg/util/cloud_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
28 changes: 28 additions & 0 deletions src/cloud-api-adaptor/test/e2e/assessment_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
15 changes: 15 additions & 0 deletions src/cloud-api-adaptor/test/e2e/assessment_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ type TestCase struct {
testInstanceTypes InstanceValidatorFunctions
isNydusSnapshotter bool
FailReason string
alternateImageName string
}

func (tc *TestCase) WithConfigMap(configMap *v1.ConfigMap) *TestCase {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
38 changes: 38 additions & 0 deletions src/cloud-api-adaptor/test/e2e/common_suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 11 additions & 1 deletion src/cloud-api-adaptor/test/e2e/libvirt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down
127 changes: 69 additions & 58 deletions src/cloud-api-adaptor/test/provisioner/libvirt/provision_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/cloud-providers/libvirt/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ajay, I'd like to understand better how this change is propagated through if needed. My (very limited) understanding is that we set this value initially when we create the libvirt provider client, so it overriding that here sufficient to pick up this custom specified image, or is there any other code we need to skip to stop the previous version being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Steve,
Exactly, that's what my understanding also is..

With the enhancement of CreateVM() function to support getting the image from GetImageFromAnnotation(), we are able to retrieve the same on CreateInstance() and then furthermore override the default Volume Name which is set initially once the CAA comes up for the libvirt provider as you pointed.

I guess with the CreateDomain() flow starting post overriding the volume and initially the volume being picked up from the LIBVIRT_VOL_NAME which is being assigned to libvirtcfg.VolName, we might not have anywhere else to update?

Kindly suggest as I might be missing something here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, there isn't anything I know that is missing, I just wanted to check my understanding.

I'm assuming that you've tested this locally. Is there a way to integrate this into our e2e tests for libvirt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Steve..

I have posted the test results (CAA logs) for these changes on the PR description.

I will start exploring the options of adding e2e tests for this..

}

result, err := CreateDomain(ctx, p.libvirtClient, vm)
if err != nil {
logger.Printf("failed to create an instance : %v", err)
Expand Down
1 change: 1 addition & 0 deletions src/cloud-providers/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,5 @@ type InstanceTypeSpec struct {
Memory int64
Arch string
GPUs int64
Image string
}