diff --git a/cmd/lambda/run.go b/cmd/lambda/run.go index b66ea1e..4ff80c4 100644 --- a/cmd/lambda/run.go +++ b/cmd/lambda/run.go @@ -120,16 +120,9 @@ func generateLambdaOptions(cmd *cli.Command) (*corepb.RunAndWaitOptions, error) return nil, fmt.Errorf("parse storage: %w", err) } - storage := resourcetypes.RawParams{ - "storage-request": storageRequest, - "storage-limit": storageLimit, - "volumes-request": cmd.StringSlice("volume-request"), - "volumes-limit": cmd.StringSlice("volume"), - } - resources, err := utils.EncodeResources(cmd, resourcetypes.Resources{ utils.ResourceCPUMem: cpumem, - utils.ResourceStorage: utils.CompactParams(storage), + utils.ResourceStorage: utils.StorageParams(storageRequest, storageLimit, cmd.StringSlice("volume-request"), cmd.StringSlice("volume")), }) if err != nil { return nil, err diff --git a/cmd/lambda/run_test.go b/cmd/lambda/run_test.go index ec02f98..49ec8b5 100644 --- a/cmd/lambda/run_test.go +++ b/cmd/lambda/run_test.go @@ -58,6 +58,7 @@ func TestGenerateLambdaOptions(t *testing.T) { tests := []struct { name string args []string + wantStorage bool wantStorageReq int64 wantStorageLim int64 wantVolumesReq []string @@ -72,7 +73,8 @@ func TestGenerateLambdaOptions(t *testing.T) { wantMemoryLimit: 512 * 1024 * 1024, }, { - name: "storage and volumes", + name: "storage and volumes", + wantStorage: true, args: []string{ "lambda", "--storage-request", "1G", @@ -95,9 +97,8 @@ func TestGenerateLambdaOptions(t *testing.T) { opts := runLambdaCommand(t, tt.args) raw, ok := opts.DeployOptions.Resources[utils.ResourceStorage] - wantStorage := tt.wantStorageReq != 0 || tt.wantStorageLim != 0 || tt.wantVolumesReq != nil || tt.wantVolumesLim != nil - if ok != wantStorage { - t.Fatalf("storage entry present=%v, want %v", ok, wantStorage) + if ok != tt.wantStorage { + t.Fatalf("storage entry present=%v, want %v", ok, tt.wantStorage) } if ok { storage := decodeParams(t, raw) diff --git a/cmd/pod/capacity.go b/cmd/pod/capacity.go index b6d3c1f..bd252ca 100644 --- a/cmd/pod/capacity.go +++ b/cmd/pod/capacity.go @@ -92,10 +92,7 @@ func capacityResources(cmd *cli.Command) (map[string][]byte, error) { } return utils.EncodeResources(cmd, resourcetypes.Resources{ - utils.ResourceCPUMem: cpumem, - utils.ResourceStorage: utils.CompactParams(resourcetypes.RawParams{ - "storage-request": storage, - "storage-limit": storage, - }), + utils.ResourceCPUMem: cpumem, + utils.ResourceStorage: utils.StorageParams(storage, storage, nil, nil), }) } diff --git a/cmd/utils/resource.go b/cmd/utils/resource.go index 3174d1f..5e695e5 100644 --- a/cmd/utils/resource.go +++ b/cmd/utils/resource.go @@ -15,23 +15,22 @@ const ( FlagExtraResources = "extra-resources" ) -// CompactParams drops zero values so an all-default plugin entry defers to --extra-resources. -func CompactParams(params resourcetypes.RawParams) resourcetypes.RawParams { - compact := make(resourcetypes.RawParams, len(params)) - for key, value := range params { - switch v := value.(type) { - case int64: - if v == 0 { - continue - } - case []string: - if len(v) == 0 { - continue - } - } - compact[key] = value +// StorageParams builds the storage plugin request; zero values stay out, so an untouched entry defers to --extra-resources. +func StorageParams(storageRequest, storageLimit int64, volumesRequest, volumesLimit []string) resourcetypes.RawParams { + params := resourcetypes.RawParams{} + if storageRequest != 0 { + params["storage-request"] = storageRequest + } + if storageLimit != 0 { + params["storage-limit"] = storageLimit + } + if len(volumesRequest) != 0 { + params["volumes-request"] = volumesRequest + } + if len(volumesLimit) != 0 { + params["volumes-limit"] = volumesLimit } - return compact + return params } // EncodeResources encodes plugin params for the core rpc; --extra-resources fills only the plugins not already present. diff --git a/cmd/utils/resource_test.go b/cmd/utils/resource_test.go index b804252..daad500 100644 --- a/cmd/utils/resource_test.go +++ b/cmd/utils/resource_test.go @@ -85,27 +85,16 @@ func TestEncodeResources(t *testing.T) { } } -func TestCompactParamsLetsExtraResourcesFillStorage(t *testing.T) { - storage := resourcetypes.RawParams{ - "storage-request": int64(0), - "storage-limit": int64(0), - "volumes-request": []string(nil), - "volumes-limit": []string{}, - } - if got := CompactParams(storage); len(got) != 0 { - t.Errorf("got %v, want every zero value dropped", got) +func TestStorageParamsStaysSparse(t *testing.T) { + if got := StorageParams(0, 0, nil, []string{}); len(got) != 0 { + t.Errorf("got %v, want an untouched request empty so --extra-resources can fill it", got) } - kept := resourcetypes.RawParams{ - "storage-request": int64(0), - "storage-limit": int64(1073741824), - "volumes-limit": []string{"/data0:1G"}, - } want := resourcetypes.RawParams{ "storage-limit": int64(1073741824), "volumes-limit": []string{"/data0:1G"}, } - if got := CompactParams(kept); !maps.EqualFunc(got, want, func(a, b any) bool { + if got := StorageParams(0, 1073741824, nil, []string{"/data0:1G"}); !maps.EqualFunc(got, want, func(a, b any) bool { aj, _ := json.Marshal(a) bj, _ := json.Marshal(b) return string(aj) == string(bj) @@ -114,7 +103,7 @@ func TestCompactParamsLetsExtraResourcesFillStorage(t *testing.T) { } cmd := commandWithExtraResources(t, `{"resource-storage":{"storage":2147483648}}`) - encoded, err := EncodeResources(cmd, resourcetypes.Resources{"resource-storage": CompactParams(storage)}) + encoded, err := EncodeResources(cmd, resourcetypes.Resources{"resource-storage": StorageParams(0, 0, nil, nil)}) if err != nil { t.Fatal(err) } diff --git a/cmd/workload/deploy.go b/cmd/workload/deploy.go index 75ce992..066870f 100644 --- a/cmd/workload/deploy.go +++ b/cmd/workload/deploy.go @@ -145,16 +145,9 @@ func generateDeployOptions(ctx context.Context, cmd *cli.Command) (*corepb.Deplo if cmd.Bool("cpu-bind") { cpumem["cpu-bind"] = true } - storage := resourcetypes.RawParams{ - flagStorageRequest: storageRequest, - flagStorageLimit: storageLimit, - flagVolumesRequest: specs.VolumesRequest, - flagVolumesLimit: specs.Volumes, - } - resources, err := utils.EncodeResources(cmd, resourcetypes.Resources{ utils.ResourceCPUMem: cpumem, - utils.ResourceStorage: utils.CompactParams(storage), + utils.ResourceStorage: utils.StorageParams(storageRequest, storageLimit, specs.VolumesRequest, specs.Volumes), }) if err != nil { return nil, err diff --git a/cmd/workload/realloc.go b/cmd/workload/realloc.go index b6c55a0..a1f07bd 100644 --- a/cmd/workload/realloc.go +++ b/cmd/workload/realloc.go @@ -86,13 +86,6 @@ func generateReallocOptions(cmd *cli.Command) (*corepb.ReallocOptions, error) { flagMemoryRequest: memoryRequest, flagMemoryLimit: memoryLimit, } - storage := resourcetypes.RawParams{ - flagStorageRequest: storageRequest, - flagStorageLimit: storageLimit, - flagVolumesRequest: volumesRequest, - flagVolumesLimit: volumesLimit, - } - switch { case bindCPU: cpumem["cpu-bind"] = true @@ -102,7 +95,7 @@ func generateReallocOptions(cmd *cli.Command) (*corepb.ReallocOptions, error) { resources, err := utils.EncodeResources(cmd, resourcetypes.Resources{ utils.ResourceCPUMem: cpumem, - utils.ResourceStorage: utils.CompactParams(storage), + utils.ResourceStorage: utils.StorageParams(storageRequest, storageLimit, volumesRequest, volumesLimit), }) if err != nil { return nil, err