Skip to content

Commit 982d429

Browse files
authored
feat: Add list all functionality (#489)
This removes the requirement that a call to GetAllMicroVM must have a namespace argument in the request. We do filtering on the Repo, so it is not harmful to have all params be optional. If a call to GetAllMicroVM is made without namespace or name, then all mvms on the host will be returned. If a call to GetAllMicroVM is made with just name, then all mvms matching that name in any namespace will be returned. If a call to to GetAllMicroVM is made with just namespace, then all mvms under that namespace will be returned. We can consider RBAC implications when we get to that.
1 parent 4285c82 commit 982d429

File tree

3 files changed

+64
-12
lines changed

3 files changed

+64
-12
lines changed

infrastructure/containerd/repo_test.go

+39-8
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,55 @@
11
package containerd_test
22

33
import (
4+
"context"
45
"testing"
56

7+
ctr "github.com/containerd/containerd"
68
. "github.com/onsi/gomega"
79

810
"github.com/weaveworks-liquidmetal/flintlock/core/models"
911
"github.com/weaveworks-liquidmetal/flintlock/core/ports"
1012
"github.com/weaveworks-liquidmetal/flintlock/infrastructure/containerd"
1113
)
1214

15+
const ctrdRepoNS = "flintlock_test_ctr_repo"
16+
1317
func TestMicroVMRepo_Integration(t *testing.T) {
1418
if !runContainerDTests() {
1519
t.Skip("skipping containerd microvm repo integration test")
1620
}
1721

22+
var (
23+
repo ports.MicroVMRepository
24+
ctx context.Context
25+
testVm, testVm2 *models.MicroVM
26+
)
27+
28+
t.Cleanup(func() {
29+
if testVm != nil {
30+
_ = repo.Delete(ctx, testVm)
31+
}
32+
33+
if testVm2 != nil {
34+
_ = repo.Delete(ctx, testVm2)
35+
}
36+
})
37+
1838
RegisterTestingT(t)
1939

20-
client, ctx := testCreateClient(t)
40+
var client *ctr.Client
41+
client, ctx = testCreateClient(t)
2142

22-
repo := containerd.NewMicroVMRepoWithClient(&containerd.Config{
43+
repo = containerd.NewMicroVMRepoWithClient(&containerd.Config{
2344
SnapshotterKernel: testSnapshotter,
2445
SnapshotterVolume: testSnapshotter,
25-
Namespace: testContainerdNS,
46+
Namespace: ctrdRepoNS,
2647
}, client)
2748
exists, err := repo.Exists(ctx, *models.NewVMIDForce(testOwnerName, testOwnerNamespace, testOwnerUID))
2849
Expect(err).NotTo(HaveOccurred())
2950
Expect(exists).To(BeFalse())
3051

31-
testVm := makeSpec(testOwnerName, testOwnerNamespace)
52+
testVm = makeSpec(testOwnerName, testOwnerNamespace, "uid")
3253
savedVM, err := repo.Save(ctx, testVm)
3354
Expect(err).NotTo(HaveOccurred())
3455
Expect(savedVM).NotTo(BeNil())
@@ -40,6 +61,12 @@ func TestMicroVMRepo_Integration(t *testing.T) {
4061
Expect(savedVM).NotTo(BeNil())
4162
Expect(savedVM.Version).To(Equal(3))
4263

64+
testVm2 = makeSpec("bar", "foo", "uid2")
65+
savedVM2, err := repo.Save(ctx, testVm2)
66+
Expect(err).NotTo(HaveOccurred())
67+
Expect(savedVM2).NotTo(BeNil())
68+
Expect(savedVM2.Version).To(Equal(2))
69+
4370
exists, err = repo.Exists(ctx, *models.NewVMIDForce(testOwnerName, testOwnerNamespace, testOwnerUID))
4471
Expect(err).NotTo(HaveOccurred())
4572
Expect(exists).To(BeTrue())
@@ -67,6 +94,10 @@ func TestMicroVMRepo_Integration(t *testing.T) {
6794
Expect(err).NotTo(HaveOccurred())
6895
Expect(len(all)).To(Equal(1))
6996

97+
all2, err := repo.GetAll(ctx, models.ListMicroVMQuery{})
98+
Expect(err).NotTo(HaveOccurred())
99+
Expect(len(all2)).To(Equal(2))
100+
70101
err = repo.Delete(ctx, testVm)
71102
Expect(err).NotTo(HaveOccurred())
72103

@@ -90,12 +121,12 @@ func TestMicroVMRepo_Integration_MultipleSave(t *testing.T) {
90121

91122
client, ctx := testCreateClient(t)
92123

93-
testVm := makeSpec(testOwnerName, testOwnerNamespace)
124+
testVm := makeSpec(testOwnerName, testOwnerNamespace, "uid")
94125

95126
repo := containerd.NewMicroVMRepoWithClient(&containerd.Config{
96127
SnapshotterKernel: testSnapshotter,
97128
SnapshotterVolume: testSnapshotter,
98-
Namespace: testContainerdNS,
129+
Namespace: ctrdRepoNS,
99130
}, client)
100131
savedVM, err := repo.Save(ctx, testVm)
101132
Expect(err).NotTo(HaveOccurred())
@@ -111,8 +142,8 @@ func TestMicroVMRepo_Integration_MultipleSave(t *testing.T) {
111142
Expect(err).NotTo(HaveOccurred())
112143
}
113144

114-
func makeSpec(name, ns string) *models.MicroVM {
115-
vmid, _ := models.NewVMID(name, ns, "uid")
145+
func makeSpec(name, ns, uid string) *models.MicroVM {
146+
vmid, _ := models.NewVMID(name, ns, uid)
116147
return &models.MicroVM{
117148
ID: *vmid,
118149
Version: 1,

infrastructure/grpc/server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func (s *server) ListMicroVMs(ctx context.Context,
147147
) (*mvmv1.ListMicroVMsResponse, error) {
148148
logger := log.GetLogger(ctx)
149149

150-
if req == nil || req.Namespace == "" {
150+
if req == nil {
151151
logger.Error("invalid get microvm request")
152152

153153
//nolint:wrapcheck // don't wrap grpc errors when using the status package

infrastructure/grpc/server_test.go

+24-3
Original file line numberDiff line numberDiff line change
@@ -267,10 +267,31 @@ func TestServer_ListMicroVMs(t *testing.T) {
267267
expect: func(cm *mock.MockMicroVMCommandUseCasesMockRecorder, qm *mock.MockMicroVMQueryUseCasesMockRecorder) {},
268268
},
269269
{
270-
name: "missing namespace should fail with error",
270+
name: "missing namespace should not fail with error",
271271
listReq: &mvm1.ListMicroVMsRequest{Namespace: ""},
272-
expectError: true,
273-
expect: func(cm *mock.MockMicroVMCommandUseCasesMockRecorder, qm *mock.MockMicroVMQueryUseCasesMockRecorder) {},
272+
expectError: false,
273+
expect: func(cm *mock.MockMicroVMCommandUseCasesMockRecorder, qm *mock.MockMicroVMQueryUseCasesMockRecorder) {
274+
qm.GetAllMicroVM(gomock.AssignableToTypeOf(
275+
context.Background()),
276+
gomock.Not(gomock.Eq("")),
277+
).Return(
278+
[]*models.MicroVM{
279+
{
280+
Version: 1,
281+
Status: models.MicroVMStatus{
282+
State: models.CreatedState,
283+
},
284+
},
285+
{
286+
Version: 1,
287+
Status: models.MicroVMStatus{
288+
State: models.CreatedState,
289+
},
290+
},
291+
},
292+
nil,
293+
)
294+
},
274295
},
275296
{
276297
name: "error from usecase should fail with error",

0 commit comments

Comments
 (0)