diff --git a/internal/cmd/cluster/create_test.go b/internal/cmd/cluster/create_test.go index 80233a1..83b44cc 100644 --- a/internal/cmd/cluster/create_test.go +++ b/internal/cmd/cluster/create_test.go @@ -3,6 +3,7 @@ package cluster_test import ( "context" "encoding/json" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -437,6 +438,202 @@ func TestCreateCluster_PackageByResourcesNoMatch(t *testing.T) { assert.Contains(t, err.Error(), "no package found") } +func TestCreateCluster_PackageByResourcesNoMatchSuggestsClosest(t *testing.T) { + env := testutil.NewTestEnv(t) + + pkg := func(name, cpu, ram, disk string) *bookingv1.Package { + return &bookingv1.Package{ + Id: "pkg-" + name, + Name: name, + ResourceConfiguration: &bookingv1.ResourceConfiguration{ + Cpu: cpu, + Ram: ram, + Disk: disk, + }, + } + } + env.BookingServer.ListPackagesCalls.Returns(&bookingv1.ListPackagesResponse{ + Items: []*bookingv1.Package{ + pkg("large", "4000m", "16GiB", "128Gi"), + pkg("tiny", "500m", "1GiB", "8GiB"), + pkg("medium", "2000m", "8GiB", "64GiB"), + pkg("small", "1000m", "4GiB", "32GiB"), + }, + }, nil) + + _, _, err := testutil.Exec(t, env, + "cluster", "create", + "--name", "my-cluster", + "--cloud-provider", "aws", + "--cloud-region", "us-east-1", + "--cpu", "2", + "--ram", "4GiB", + ) + require.Error(t, err) + assert.Equal(t, `no package found matching cpu="2" ram="4GiB" +next biggest package: medium (cpu=2 ram=8GiB) +closest available packages: + NAME CPU RAM DISK + medium 2 8GiB 64GiB + small 1 4GiB 32GiB + large 4 16GiB 128GiB + tiny 500m 1GiB 8GiB +use 'package list' to see all available packages`, err.Error()) + assert.Equal(t, 0, env.Server.CreateClusterCalls.Count()) +} + +func TestCreateCluster_PackageByResourcesNoMatchSuggestsNextBiggestFirst(t *testing.T) { + env := testutil.NewTestEnv(t) + + pkg := func(name, cpu, ram string) *bookingv1.Package { + return &bookingv1.Package{ + Id: "pkg-" + name, + Name: name, + ResourceConfiguration: &bookingv1.ResourceConfiguration{Cpu: cpu, Ram: ram, Disk: "16Gi"}, + } + } + env.BookingServer.ListPackagesCalls.Returns(&bookingv1.ListPackagesResponse{ + Items: []*bookingv1.Package{ + pkg("gpx1", "500m", "2Gi"), + pkg("mx1", "500m", "4Gi"), + pkg("mx2", "1000m", "8Gi"), + pkg("mx3", "2000m", "16Gi"), + pkg("gpx4", "4000m", "16Gi"), + }, + }, nil) + + _, _, err := testutil.Exec(t, env, + "cluster", "create", + "--name", "my-cluster", + "--cloud-provider", "aws", + "--cloud-region", "eu-central-1", + "--cpu", "0.5", + "--ram", "12Gi", + ) + require.Error(t, err) + assert.Equal(t, `no package found matching cpu="500m" ram="12GiB" +next biggest package: mx3 (cpu=2 ram=16GiB) +closest available packages: + NAME CPU RAM DISK + mx3 2 16GiB 16GiB + mx1 500m 4GiB 16GiB + mx2 1 8GiB 16GiB + gpx1 500m 2GiB 16GiB + gpx4 4 16GiB 16GiB +use 'package list' to see all available packages`, err.Error()) +} + +func TestCreateCluster_PackageByResourcesNoMatchNothingBigEnough(t *testing.T) { + env := testutil.NewTestEnv(t) + + pkg := func(name, cpu, ram string) *bookingv1.Package { + return &bookingv1.Package{ + Id: "pkg-" + name, + Name: name, + ResourceConfiguration: &bookingv1.ResourceConfiguration{Cpu: cpu, Ram: ram, Disk: "32Gi"}, + } + } + env.BookingServer.ListPackagesCalls.Returns(&bookingv1.ListPackagesResponse{ + Items: []*bookingv1.Package{ + pkg("small", "1000m", "4Gi"), + pkg("large", "4000m", "16Gi"), + pkg("medium", "2000m", "8Gi"), + }, + }, nil) + + _, _, err := testutil.Exec(t, env, + "cluster", "create", + "--name", "my-cluster", + "--cloud-provider", "aws", + "--cloud-region", "us-east-1", + "--cpu", "8", + "--ram", "64Gi", + ) + require.Error(t, err) + assert.Equal(t, `no package found matching cpu="8" ram="64GiB" +closest available packages: + NAME CPU RAM DISK + large 4 16GiB 32GiB + medium 2 8GiB 32GiB + small 1 4GiB 32GiB +use 'package list' to see all available packages`, err.Error()) +} + +func TestCreateCluster_PackageByResourcesNoMatchWithGPU(t *testing.T) { + env := testutil.NewTestEnv(t) + + pkg := func(name, cpu, ram, gpu string) *bookingv1.Package { + return &bookingv1.Package{ + Id: "pkg-" + name, + Name: name, + ResourceConfiguration: &bookingv1.ResourceConfiguration{Cpu: cpu, Ram: ram, Gpu: &gpu, Disk: "128Gi"}, + } + } + env.BookingServer.ListPackagesCalls.Returns(&bookingv1.ListPackagesResponse{ + Items: []*bookingv1.Package{ + pkg("ax4", "4000m", "16Gi", "1000m"), + pkg("ax5", "8000m", "32Gi", "1000m"), + pkg("ax-big", "8000m", "64Gi", "2000m"), + }, + }, nil) + + _, _, err := testutil.Exec(t, env, + "cluster", "create", + "--name", "my-cluster", + "--cloud-provider", "aws", + "--cloud-region", "us-east-1", + "--cpu", "4", + "--ram", "32Gi", + "--gpu", "2", + ) + require.Error(t, err) + assert.Equal(t, `no package found matching cpu="4" ram="32GiB" gpu="2" +next biggest package: ax-big (cpu=8 ram=64GiB gpu=2) +closest available packages: + NAME CPU RAM GPU DISK + ax-big 8 64GiB 2 128GiB + ax4 4 16GiB 1 128GiB + ax5 8 32GiB 1 128GiB +use 'package list' to see all available packages`, err.Error()) + + req, ok := env.BookingServer.ListPackagesCalls.Last() + require.True(t, ok) + assert.True(t, req.GetGpu()) +} + +func TestCreateCluster_PackageByResourcesNoMatchLimitsSuggestions(t *testing.T) { + env := testutil.NewTestEnv(t) + + var items []*bookingv1.Package + for i := 1; i <= 8; i++ { + items = append(items, &bookingv1.Package{ + Id: fmt.Sprintf("pkg-%d", i), + Name: fmt.Sprintf("pkg-%d", i), + ResourceConfiguration: &bookingv1.ResourceConfiguration{Cpu: fmt.Sprintf("%d", i), Ram: fmt.Sprintf("%dGiB", 4*i)}, + }) + } + + env.BookingServer.ListPackagesCalls.Returns(&bookingv1.ListPackagesResponse{Items: items}, nil) + + _, _, err := testutil.Exec(t, env, + "cluster", "create", + "--name", "my-cluster", + "--cloud-provider", "aws", + "--cloud-region", "us-east-1", + "--cpu", "3", + "--ram", "4GiB", + ) + require.Error(t, err) + msg := err.Error() + for _, name := range []string{"pkg-1 ", "pkg-2 ", "pkg-3 ", "pkg-4 ", "pkg-5 "} { + assert.Contains(t, msg, name) + } + + for _, name := range []string{"pkg-6 ", "pkg-7 ", "pkg-8 "} { + assert.NotContains(t, msg, name) + } +} + func TestCreateCluster_PackageByResourcesAmbiguous(t *testing.T) { env := testutil.NewTestEnv(t) diff --git a/internal/cmd/cluster/scale_test.go b/internal/cmd/cluster/scale_test.go index 619cf2f..7ba1097 100644 --- a/internal/cmd/cluster/scale_test.go +++ b/internal/cmd/cluster/scale_test.go @@ -427,6 +427,28 @@ func TestScale_PackageNotFound(t *testing.T) { assert.Contains(t, err.Error(), "no package found") } +func TestScale_PackageNotFoundSuggestsClosest(t *testing.T) { + env := testutil.NewTestEnv(t) + currentPkg := newPkg(pkgID1, "1000m", "4GiB", "50GiB") + env.Server.GetClusterCalls.Returns(&clusterv1.GetClusterResponse{Cluster: baseCluster()}, nil) + env.BookingServer.GetPackageCalls.Returns(&bookingv1.GetPackageResponse{Package: currentPkg}, nil) + env.BookingServer.ListPackagesCalls.Returns(&bookingv1.ListPackagesResponse{Items: []*bookingv1.Package{ + newPkg("pkg-big", "4000m", "16GiB", "100GiB"), + newPkg("pkg-near", "2000m", "8GiB", "80GiB"), + }}, nil) + + _, _, err := testutil.Exec(t, env, "cluster", "scale", "cluster-123", "--cpu", "3", "--force") + require.Error(t, err) + assert.Equal(t, `no package found matching cpu="3" ram="4GiB" +next biggest package: pkg-big (cpu=4 ram=16GiB) +closest available packages: + NAME CPU RAM DISK + pkg-big 4 16GiB 100GiB + pkg-near 2 8GiB 80GiB +use 'package list' to see all available packages`, err.Error()) + assert.Equal(t, 0, env.Server.UpdateClusterCalls.Count()) +} + func TestScale_AmbiguousPackageMatch(t *testing.T) { env := testutil.NewTestEnv(t) currentPkg := newPkg(pkgID1, "1000m", "4GiB", "50GiB") diff --git a/internal/cmd/clusterutil/resolve.go b/internal/cmd/clusterutil/resolve.go index 072f03c..e8eea0e 100644 --- a/internal/cmd/clusterutil/resolve.go +++ b/internal/cmd/clusterutil/resolve.go @@ -1,9 +1,14 @@ package clusterutil import ( + "cmp" "context" + "errors" "fmt" + "math" + "slices" "strings" + "text/tabwriter" "github.com/spf13/cobra" @@ -228,7 +233,12 @@ func ResolvePackageByResources( case 1: return matches[0], nil case 0: - return nil, fmt.Errorf("no package found matching %s; use 'package list' to see available packages", desc) + suggestions := closestPackages(q, resp.GetItems(), maxPackageSuggestions) + if len(suggestions) == 0 { + return nil, fmt.Errorf("no package found matching %s; use 'package list' to see available packages", desc) + } + + return nil, noPackageFoundError(desc, suggestions, q.GPU != 0) default: names := make([]string, len(matches)) for i, p := range matches { @@ -239,6 +249,136 @@ func ResolvePackageByResources( } } +// maxPackageSuggestions is how many packages a "no package found" error suggests. +const maxPackageSuggestions = 5 + +// packageSuggestion is a package together with its parsed resources and its +// distance from the requested resources. +type packageSuggestion struct { + pkg *bookingv1.Package + cpu resource.Millicores + ram resource.ByteQuantity + gpu resource.Millicores + distance float64 + // fits is true if the package has at least the requested amount of every resource. + fits bool +} + +// closestPackages returns up to limit packages ordered by how far their +// resources are from the requested ones. The distance sums, over every +// requested dimension, the absolute log2 ratio between package and request, so +// doubling or halving a dimension counts the same and a package that matches +// one dimension exactly ranks above one that is off in all of them. +// The next biggest package, the closest one that has at least the requested +// amount of every resource, always comes first. +func closestPackages(q PackageResourceQuery, pkgs []*bookingv1.Package, limit int) []packageSuggestion { + var suggestions []packageSuggestion + for _, p := range pkgs { + rc := p.GetResourceConfiguration() + cpu, _ := resource.ParseMillicores(rc.GetCpu()) + ram, _ := resource.ParseByteQuantity(rc.GetRam()) + gpu, _ := resource.ParseMillicores(rc.GetGpu()) + + var distance float64 + missing := false + fits := true + for _, d := range []struct{ want, have int64 }{ + {int64(q.CPU), int64(cpu)}, + {int64(q.RAM), int64(ram)}, + {int64(q.GPU), int64(gpu)}, + } { + if d.want == 0 { + continue + } + + if d.have <= 0 { + missing = true + break + } + + distance += math.Abs(math.Log2(float64(d.have) / float64(d.want))) + if d.have < d.want { + fits = false + } + } + + if missing { + continue + } + + suggestions = append(suggestions, packageSuggestion{pkg: p, cpu: cpu, ram: ram, gpu: gpu, distance: distance, fits: fits}) + } + + slices.SortStableFunc(suggestions, func(a, b packageSuggestion) int { + return cmp.Or( + cmp.Compare(a.distance, b.distance), + cmp.Compare(a.cpu, b.cpu), + cmp.Compare(a.ram, b.ram), + cmp.Compare(a.gpu, b.gpu), + ) + }) + + if i := slices.IndexFunc(suggestions, func(sg packageSuggestion) bool { return sg.fits }); i > 0 { + next := suggestions[i] + copy(suggestions[1:i+1], suggestions[:i]) + suggestions[0] = next + } + + if len(suggestions) > limit { + suggestions = suggestions[:limit] + } + + return suggestions +} + +// noPackageFoundError builds the error for a resource request that no package +// matches. It names the next biggest package, if there is one, and lists the +// suggestions as an indented table, one package per line. +func noPackageFoundError(desc string, suggestions []packageSuggestion, includeGPU bool) error { + var sb strings.Builder + fmt.Fprintf(&sb, "no package found matching %s\n", desc) + + if next := suggestions[0]; next.fits { + fmt.Fprintf(&sb, "next biggest package: %s (cpu=%s ram=%s", next.pkg.GetName(), next.cpu.String(), next.ram.String()) + if includeGPU { + fmt.Fprintf(&sb, " gpu=%s", next.gpu.String()) + } + + sb.WriteString(")\n") + } + + sb.WriteString("closest available packages:\n") + + w := tabwriter.NewWriter(&sb, 0, 0, 2, ' ', 0) + + header := " NAME\tCPU\tRAM" + if includeGPU { + header += "\tGPU" + } + + fmt.Fprintln(w, header+"\tDISK") + + for _, sg := range suggestions { + line := fmt.Sprintf(" %s\t%s\t%s", sg.pkg.GetName(), sg.cpu.String(), sg.ram.String()) + if includeGPU { + line += "\t" + sg.gpu.String() + } + + disk := sg.pkg.GetResourceConfiguration().GetDisk() + if parsed, err := resource.ParseByteQuantity(disk); err == nil { + disk = parsed.String() + } + + fmt.Fprintln(w, line+"\t"+disk) + } + + _ = w.Flush() + + sb.WriteString("use 'package list' to see all available packages") + + return errors.New(sb.String()) +} + // CalculateAdditionalDisk returns the additional disk (in GiB) needed beyond // what the package includes. Returns 0 if the requested disk is not larger // than the package's included disk.