Skip to content

Remove Volume.size_in_mb from api; improve pkg/queue tests #718

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

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

Remove Volume.size_in_mb from api; improve pkg/queue tests #718

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
5bba80b
api: remove `Volume.size_in_mb`
aryan9600 May 25, 2023
f163021
remove t.Fatal calls from non test goroutines
aryan9600 May 25, 2023
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
5 changes: 0 additions & 5 deletions api/services/microvm/v1alpha1/microvms.swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,11 +496,6 @@
"partitionId": {
"type": "string",
"description": "PartitionID is the uuid of the boot partition."
},
"sizeInMb": {
"type": "integer",
"format": "int32",
"description": "Size is the size to resize this volume to."
}
},
"description": "Volume represents the configuration for a volume to be attached to a microvm."
Expand Down
174 changes: 81 additions & 93 deletions api/types/microvm.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions api/types/microvm.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,6 @@ message Volume {
VolumeSource source = 4;
// PartitionID is the uuid of the boot partition.
optional string partition_id = 5;
// Size is the size to resize this volume to.
Copy link
Contributor

@Skarlso Skarlso May 30, 2023

Choose a reason for hiding this comment

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

I know it's an optional field, but shouldn't we be deprecating this first?

Copy link
Contributor Author

@aryan9600 aryan9600 Jun 9, 2023

Choose a reason for hiding this comment

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

cc: @richardcase

Copy link
Member

@richardcase richardcase Jun 20, 2023

Choose a reason for hiding this comment

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

We'd need to add reserved 6 and probably reserved "size_in_mb" to the definition of Volume.

optional int32 size_in_mb = 6;
// TODO: add rate limiting
}

Expand Down
2 changes: 1 addition & 1 deletion buf.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ deps:
- remote: buf.build
owner: googleapis
repository: googleapis
commit: 463926e7ee924d46ad0a726e1cf4eacd
commit: cc916c31859748a68fd229a3c8d7a2e8
- remote: buf.build
owner: grpc-ecosystem
repository: grpc-gateway
Expand Down
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ require (
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/subcommands v1.0.1 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
Expand All @@ -119,6 +120,7 @@ require (
github.com/pierrec/lz4 v2.6.1+incompatible // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pkg/xattr v0.4.9 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
github.com/prometheus/common v0.42.0 // indirect
github.com/prometheus/procfs v0.9.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ github.com/google/pprof v0.0.0-20201218002935-b9804c9f04c2/go.mod h1:kpwsk12EmLe
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 h1:yAJXTCF9TqKcTiHJAE8dj7HMvPfh66eeA2JYW7eFpSE=
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
github.com/google/subcommands v1.0.1 h1:/eqq+otEXm5vhfBrbREPCSVQbvofip6kIz+mX5TUH7k=
github.com/google/subcommands v1.0.1/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3yTrtFlrHVk=
github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
Expand Down
5 changes: 0 additions & 5 deletions infrastructure/grpc/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ func convertVolumeToModel(volume *types.Volume) *models.Volume {
convertedVol.PartitionID = *volume.PartitionId
}

if volume.SizeInMb != nil {
convertedVol.Size = *volume.SizeInMb
}

if volume.Source != nil {
if volume.Source.ContainerSource != nil {
convertedVol.Source.Container = &models.ContainerVolumeSource{
Expand Down Expand Up @@ -198,7 +194,6 @@ func convertModelToVolumne(modelVolume *models.Volume) *types.Volume {
Id: modelVolume.ID,
IsReadOnly: modelVolume.IsReadOnly,
PartitionId: &modelVolume.PartitionID,
SizeInMb: &modelVolume.Size,
}

if modelVolume.Source.Container != nil {
Expand Down
2 changes: 0 additions & 2 deletions infrastructure/grpc/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ func createTestCreateRequest(id, namespace string) *mvm1.CreateMicroVMRequest {
filename := "kernel"
mac := "AA:FF:00:00:00:01"
containerSource := "docker.io/library/ubuntu:groovy"
rootVolSize := int32(20000)

return &mvm1.CreateMicroVMRequest{
Microvm: &types.MicroVMSpec{
Expand Down Expand Up @@ -489,7 +488,6 @@ func createTestCreateRequest(id, namespace string) *mvm1.CreateMicroVMRequest {
Source: &types.VolumeSource{
ContainerSource: &containerSource,
},
SizeInMb: &rootVolSize,
},
},
}
Expand Down
22 changes: 20 additions & 2 deletions pkg/queue/queue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ func TestSimpleSyncQueue_Basic(t *testing.T) {
numConsumers := 5
consumersWG := sync.WaitGroup{}
consumersWG.Add(numConsumers)
var fatal bool
for i := 0; i < numConsumers; i++ {
if fatal {
break
}
go func(i int) {
defer consumersWG.Done()
for {
Expand All @@ -47,14 +51,19 @@ func TestSimpleSyncQueue_Basic(t *testing.T) {
return
}
if !strings.HasPrefix(item.(string), "ns1/vm") {
t.Fatal("received item from queue after shutdown")
fatal = true
return
}
atomic.AddInt32(&countConsumed, 1)
time.Sleep(3 * time.Millisecond)
}
}(i)
}

if fatal {
t.Fatal("received item from queue after shutdown")
}

producersWG.Wait()
t.Log("shutting queue down")
q.Shutdown()
Expand Down Expand Up @@ -82,7 +91,11 @@ func TestSimpleSyncQueue_Duplicate(t *testing.T) {
numConsumers := 5
consumersWG := sync.WaitGroup{}
consumersWG.Add(numConsumers)
var fatal bool
for i := 0; i < numConsumers; i++ {
if fatal {
break
}
go func(i int) {
defer consumersWG.Done()
for {
Expand All @@ -91,14 +104,19 @@ func TestSimpleSyncQueue_Duplicate(t *testing.T) {
return
}
if !strings.HasPrefix(item.(string), "ns1/vm") {
t.Fatal("received item from queue after shutdown")
fatal = true
return
}
atomic.AddInt32(&countConsumed, 1)
time.Sleep(3 * time.Millisecond)
}
}(i)
}

if fatal {
t.Fatal("received item from queue after shutdown")
}

t.Log("shutting queue down")
q.Shutdown()
t.Log("enqueing message after shutdown")
Expand Down
3 changes: 1 addition & 2 deletions userdocs/docs/grpc/types/proto.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ Volume represents the configuration for a volume to be attached to a microvm.
| is_read_only | [bool](#bool) | | IsReadOnly specifies that the volume is to be mounted readonly. |
| mount_point | [string](#string) | optional | MountPoint allows you to optionally specify a mount point for the volume. This only applied to additional volumes and it will use cloud-init to mount the volumes. |
| source | [VolumeSource](#flintlock.types.VolumeSource) | | Source is where the volume will be sourced from. |
| partition_id | [string](#string) | optional | PartitionID is the uuid of the boot partition. |
| size_in_mb | [int32](#int32) | optional | Size is the size to resize this volume to.
| partition_id | [string](#string) | optional | PartitionID is the uuid of the boot partition.

TODO: add rate limiting |

Expand Down