From 5d0281d549797fde710a020cec82084089ab5237 Mon Sep 17 00:00:00 2001 From: Balazs Nadasdi Date: Wed, 26 Jan 2022 11:21:11 +0100 Subject: [PATCH 1/2] fix: uid related issues * e2e path was not updated. * Get tried to use empty name and namespace as a filter, not it does not use those values if they are empty. * UpdateMicroVM required all fields. Related to #291 (issue) Related to #327 (pr) --- core/application/commands.go | 4 +++- infrastructure/containerd/repo.go | 19 +++++++++++++------ test/e2e/e2e_test.go | 18 +++++++++++------- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/core/application/commands.go b/core/application/commands.go index fa0cf927..95c7c8a2 100644 --- a/core/application/commands.go +++ b/core/application/commands.go @@ -113,7 +113,9 @@ func (a *app) DeleteMicroVM(ctx context.Context, uid string) error { } if err := a.ports.EventService.Publish(ctx, defaults.TopicMicroVMEvents, &events.MicroVMSpecUpdated{ - UID: foundMvm.ID.UID(), + ID: foundMvm.ID.Name(), + Namespace: foundMvm.ID.Namespace(), + UID: foundMvm.ID.UID(), }); err != nil { return fmt.Errorf("publishing microvm updated event: %w", err) } diff --git a/infrastructure/containerd/repo.go b/infrastructure/containerd/repo.go index 4be42372..7c5f48ae 100644 --- a/infrastructure/containerd/repo.go +++ b/infrastructure/containerd/repo.go @@ -286,15 +286,22 @@ func (r *containerdRepo) findDigestForSpec(ctx context.Context, ) (*digest.Digest, error) { var digest *digest.Digest - idLabelFilter := labelFilter(NameLabel(), options.Name) - nsFilter := labelFilter(NamespaceLabel(), options.Namespace) - uidLabelFilter := labelFilter(UIDLabel(), options.UID) - versionFilter := labelFilter(VersionLabel(), options.Version) + combinedFilters := []string{} - combinedFilters := []string{idLabelFilter, nsFilter, uidLabelFilter} + if options.Name != "" { + combinedFilters = append(combinedFilters, labelFilter(NameLabel(), options.Name)) + } + + if options.Namespace != "" { + combinedFilters = append(combinedFilters, labelFilter(NamespaceLabel(), options.Namespace)) + } + + if options.UID != "" { + combinedFilters = append(combinedFilters, labelFilter(UIDLabel(), options.UID)) + } if options.Version != "" { - combinedFilters = append(combinedFilters, versionFilter) + combinedFilters = append(combinedFilters, labelFilter(VersionLabel(), options.Version)) } allFilters := strings.Join(combinedFilters, ",") diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 0f96099e..231cfbeb 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -29,7 +29,7 @@ func TestE2E(t *testing.T) { mvmID = "mvm0" secondMvmID = "mvm1" mvmNS = "ns0" - fcPath = "/var/lib/flintlock/vm/%s/%s" + fcPath = "/var/lib/flintlock/vm/%s/%s/%s" mvmPid1 int mvmPid2 int @@ -47,13 +47,15 @@ func TestE2E(t *testing.T) { created := u.CreateMVM(flintlockClient, mvmID, mvmNS) Expect(created.Microvm.Spec.Id).To(Equal(mvmID)) + firstMicroVMPath := fmt.Sprintf(fcPath, mvmNS, mvmID, *created.Microvm.Spec.Uid) + log.Println("TEST STEP: getting (and verifying) existing MicroVM") Eventually(func(g Gomega) error { - g.Expect(fmt.Sprintf(fcPath, mvmNS, mvmID) + "/firecracker.pid").To(BeAnExistingFile()) + g.Expect(firstMicroVMPath + "/firecracker.pid").To(BeAnExistingFile()) // verify that firecracker has started and that a pid has been saved // and that there is actually a running process - mvmPid1 = u.ReadPID(fmt.Sprintf(fcPath, mvmNS, mvmID)) + mvmPid1 = u.ReadPID(firstMicroVMPath) g.Expect(u.PidRunning(mvmPid1)).To(BeTrue()) // get the mVM and check the status @@ -67,13 +69,15 @@ func TestE2E(t *testing.T) { createdSecond := u.CreateMVM(flintlockClient, secondMvmID, mvmNS) Expect(createdSecond.Microvm.Spec.Id).To(Equal(secondMvmID)) + secondMicroVMPath := fmt.Sprintf(fcPath, mvmNS, secondMvmID, *createdSecond.Microvm.Spec.Uid) + log.Println("TEST STEP: listing all MicroVMs") Eventually(func(g Gomega) error { - g.Expect(fmt.Sprintf(fcPath, mvmNS, secondMvmID) + "/firecracker.pid").To(BeAnExistingFile()) + g.Expect(secondMicroVMPath + "/firecracker.pid").To(BeAnExistingFile()) // verify that firecracker has started and that a pid has been saved // and that there is actually a running process for the new mVM - mvmPid2 = u.ReadPID(fmt.Sprintf(fcPath, mvmNS, secondMvmID)) + mvmPid2 = u.ReadPID(secondMicroVMPath) g.Expect(u.PidRunning(mvmPid2)).To(BeTrue()) // get both the mVMs and check the statuses @@ -97,8 +101,8 @@ func TestE2E(t *testing.T) { Eventually(func(g Gomega) error { // verify that the vm state dirs have been removed - g.Expect(fmt.Sprintf(fcPath, mvmNS, mvmID)).ToNot(BeAnExistingFile()) - g.Expect(fmt.Sprintf(fcPath, mvmNS, secondMvmID)).ToNot(BeAnExistingFile()) + g.Expect(firstMicroVMPath).ToNot(BeAnExistingFile()) + g.Expect(secondMicroVMPath).ToNot(BeAnExistingFile()) // verify that the firecracker processes are no longer running g.Expect(u.PidRunning(mvmPid1)).To(BeFalse()) From 9db19d79286716a0a29e578c0dff1caca31a7ac3 Mon Sep 17 00:00:00 2001 From: Balazs Nadasdi Date: Wed, 26 Jan 2022 11:38:11 +0100 Subject: [PATCH 2/2] fix test --- core/application/app_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/application/app_test.go b/core/application/app_test.go index 9d3bfff4..ef5d2e56 100644 --- a/core/application/app_test.go +++ b/core/application/app_test.go @@ -226,7 +226,9 @@ func TestApp_DeleteMicroVM(t *testing.T) { gomock.AssignableToTypeOf(context.Background()), gomock.Eq(defaults.TopicMicroVMEvents), gomock.Eq(&events.MicroVMSpecUpdated{ - UID: testUID, + ID: "id1234", + Namespace: "default", + UID: testUID, }), ) },