From d3feb240c7f0599c2d5dd40241b462f1863d67bc Mon Sep 17 00:00:00 2001 From: Nick Hale <4175918+njhale@users.noreply.github.com> Date: Wed, 14 Feb 2024 21:30:18 -0500 Subject: [PATCH] test: integration for project specified defaults Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com> --- integration/run/run_test.go | 157 ++++++++++--- .../internal.admin.acorn.io/v1/default.go | 54 +++-- .../v1/default_test.go | 208 ++++++++++++++++++ pkg/computeclasses/computeclasses.go | 2 +- pkg/controller/defaults/memory.go | 2 +- .../resolvedofferings/computeclass.go | 4 +- .../resolvedofferings/computeclass_test.go | 4 + .../project-specified-default/existing.yaml | 53 +++++ .../project-specified-default/expected.golden | 71 ++++++ .../project-specified-default/input.yaml | 33 +++ pkg/project/handlers.go | 1 - .../apigroups/acorn/projects/validator.go | 1 - .../acorn/projects/validator_test.go | 80 ++++++- .../apigroups/admin/computeclass/strategy.go | 2 +- 14 files changed, 610 insertions(+), 62 deletions(-) create mode 100644 pkg/apis/internal.admin.acorn.io/v1/default_test.go create mode 100644 pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/existing.yaml create mode 100644 pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/expected.golden create mode 100644 pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/input.yaml diff --git a/integration/run/run_test.go b/integration/run/run_test.go index d16602c90..82370e31d 100644 --- a/integration/run/run_test.go +++ b/integration/run/run_test.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" "testing" + "time" "github.com/acorn-io/baaah/pkg/restconfig" "github.com/acorn-io/baaah/pkg/router" @@ -18,7 +19,7 @@ import ( "github.com/acorn-io/runtime/pkg/client" "github.com/acorn-io/runtime/pkg/config" "github.com/acorn-io/runtime/pkg/imagesource" - kclient "github.com/acorn-io/runtime/pkg/k8sclient" + rkclient "github.com/acorn-io/runtime/pkg/k8sclient" "github.com/acorn-io/runtime/pkg/labels" "github.com/acorn-io/runtime/pkg/run" "github.com/acorn-io/runtime/pkg/scheme" @@ -26,6 +27,7 @@ import ( "github.com/acorn-io/z" "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" storagev1 "k8s.io/api/storage/v1" @@ -33,14 +35,14 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8slabels "k8s.io/apimachinery/pkg/labels" - crClient "sigs.k8s.io/controller-runtime/pkg/client" + kclient "sigs.k8s.io/controller-runtime/pkg/client" ) func TestVolume(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) image, err := c.AcornImageBuild(ctx, "./testdata/volume/Acornfile", &client.AcornImageBuildOptions{ @@ -123,7 +125,7 @@ func TestServiceConsumer(t *testing.T) { ctx := helper.GetCTX(t) c, _ := helper.ClientAndProject(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) image, err := c.AcornImageBuild(ctx, "./testdata/serviceconsumer/Acornfile", &client.AcornImageBuildOptions{ Cwd: "./testdata/serviceconsumer/", @@ -170,7 +172,7 @@ func TestVolumeBadClassInImageBoundToGoodClass(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -231,7 +233,7 @@ func TestVolumeBoundBadClass(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -278,7 +280,7 @@ func TestVolumeClassInactive(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) volumeClass := adminapiv1.ClusterVolumeClass{ @@ -312,7 +314,7 @@ func TestVolumeClassSizeTooSmall(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) volumeClass := adminapiv1.ClusterVolumeClass{ @@ -356,7 +358,7 @@ func TestVolumeClassSizeTooLarge(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) volumeClass := adminapiv1.ClusterVolumeClass{ @@ -400,7 +402,7 @@ func TestVolumeClassRemoved(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -466,7 +468,7 @@ func TestClusterVolumeClass(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kclient := helper.MustReturn(kclient.Default) + kclient := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -529,7 +531,7 @@ func TestClusterVolumeClassValuesInAcornfile(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kclient := helper.MustReturn(kclient.Default) + kclient := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -588,7 +590,7 @@ func TestProjectVolumeClass(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kclient := helper.MustReturn(kclient.Default) + kclient := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -651,7 +653,7 @@ func TestProjectVolumeClassDefaultSizeValidation(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kclient := helper.MustReturn(kclient.Default) + kclient := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -718,7 +720,7 @@ func TestProjectVolumeClassDefaultSizeBadValidation(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kclient := helper.MustReturn(kclient.Default) + kclient := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -765,7 +767,7 @@ func TestProjectVolumeClassValuesInAcornfile(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kclient := helper.MustReturn(kclient.Default) + kclient := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -824,7 +826,7 @@ func TestImageNameAnnotation(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - k8sclient := helper.MustReturn(kclient.Default) + k8sclient := helper.MustReturn(rkclient.Default) c, _ := helper.ClientAndProject(t) image, err := c.AcornImageBuild(helper.GetCTX(t), "./testdata/named/Acornfile", &client.AcornImageBuildOptions{ @@ -889,7 +891,7 @@ func TestDeployParam(t *testing.T) { ctx := helper.GetCTX(t) c, project := helper.ClientAndProject(t) - kclient := helper.MustReturn(kclient.Default) + kclient := helper.MustReturn(rkclient.Default) image, err := c.AcornImageBuild(ctx, "./testdata/params/Acornfile", &client.AcornImageBuildOptions{ Cwd: "./testdata/params", @@ -936,7 +938,7 @@ func TestRequireComputeClass(t *testing.T) { helper.StartController(t) c, _ := helper.ClientAndProject(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) helper.SetRequireComputeClassWithRestore(ctx, t, kc) @@ -1045,7 +1047,7 @@ func TestRequireComputeClass(t *testing.T) { for _, tt := range checks { asClusterComputeClass := adminv1.ClusterComputeClassInstance(tt.computeClass) // Perform the same test cases on both Project and Cluster ComputeClasses - for kind, computeClass := range map[string]crClient.Object{"projectcomputeclass": &tt.computeClass, "clustercomputeclass": &asClusterComputeClass} { + for kind, computeClass := range map[string]kclient.Object{"projectcomputeclass": &tt.computeClass, "clustercomputeclass": &asClusterComputeClass} { testcase := fmt.Sprintf("%v-%v", kind, tt.name) t.Run(testcase, func(t *testing.T) { if !tt.noComputeClass { @@ -1058,7 +1060,7 @@ func TestRequireComputeClass(t *testing.T) { if err := kc.Delete(context.Background(), computeClass); err != nil && !apierrors.IsNotFound(err) { t.Fatal(err) } - err := helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) { + err := helper.EnsureDoesNotExist(ctx, func() (kclient.Object, error) { lookingFor := computeClass err := kc.Get(ctx, router.Key(computeClass.GetNamespace(), computeClass.GetName()), lookingFor) return lookingFor, err @@ -1093,7 +1095,7 @@ func TestRequireComputeClass(t *testing.T) { if err = kc.Delete(context.Background(), app); err != nil && !apierrors.IsNotFound(err) { t.Fatal(err) } - err := helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) { + err := helper.EnsureDoesNotExist(ctx, func() (kclient.Object, error) { lookingFor := app err := kc.Get(ctx, router.Key(app.GetName(), app.GetNamespace()), lookingFor) return lookingFor, err @@ -1122,7 +1124,7 @@ func TestRequireComputeClass(t *testing.T) { func TestUsingComputeClasses(t *testing.T) { helper.StartController(t) c, _ := helper.ClientAndProject(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) ctx := helper.GetCTX(t) @@ -1366,7 +1368,7 @@ func TestUsingComputeClasses(t *testing.T) { for _, tt := range checks { asClusterComputeClass := adminv1.ClusterComputeClassInstance(tt.computeClass) // Perform the same test cases on both Project and Cluster ComputeClasses - for kind, computeClass := range map[string]crClient.Object{"projectcomputeclass": &tt.computeClass, "clustercomputeclass": &asClusterComputeClass} { + for kind, computeClass := range map[string]kclient.Object{"projectcomputeclass": &tt.computeClass, "clustercomputeclass": &asClusterComputeClass} { testcase := fmt.Sprintf("%v-%v", kind, tt.name) t.Run(testcase, func(t *testing.T) { if !tt.noComputeClass { @@ -1379,7 +1381,7 @@ func TestUsingComputeClasses(t *testing.T) { if err := kc.Delete(context.Background(), computeClass); err != nil && !apierrors.IsNotFound(err) { t.Fatal(err) } - err := helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) { + err := helper.EnsureDoesNotExist(ctx, func() (kclient.Object, error) { lookingFor := computeClass err := kc.Get(ctx, router.Key(computeClass.GetNamespace(), computeClass.GetName()), lookingFor) return lookingFor, err @@ -1413,7 +1415,7 @@ func TestUsingComputeClasses(t *testing.T) { if err = kc.Delete(context.Background(), app); err != nil && !apierrors.IsNotFound(err) { t.Fatal(err) } - err := helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) { + err := helper.EnsureDoesNotExist(ctx, func() (kclient.Object, error) { lookingFor := app err := kc.Get(ctx, router.Key(app.GetName(), app.GetNamespace()), lookingFor) return lookingFor, err @@ -1439,6 +1441,91 @@ func TestUsingComputeClasses(t *testing.T) { } } +func TestProjectSpecifiedDefaultComputeClass(t *testing.T) { + helper.StartController(t) + + ctx := helper.GetCTX(t) + c, project := helper.ClientAndProject(t) + kc := helper.MustReturn(rkclient.Default) + + projectComputeClass := adminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "self-specified-default", + Namespace: c.GetNamespace(), + }, + CPUScaler: 0.25, + Default: true, + Memory: adminv1.ComputeClassMemory{ + Min: "1024", + Max: "2Gi", + }, + SupportedRegions: []string{apiv1.LocalRegion}, + } + clusterComputeClass := adminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-specified-default", + }, + CPUScaler: 0.25, + Memory: adminv1.ComputeClassMemory{ + Min: "512", + Max: "1Gi", + }, + SupportedRegions: []string{apiv1.LocalRegion}, + } + + for _, cc := range []kclient.Object{&projectComputeClass, &clusterComputeClass} { + obj := cc + t.Cleanup(func() { + assert.NoError(t, kclient.IgnoreNotFound(kc.Delete(ctx, obj))) + }) + assert.NoError(t, kc.Create(ctx, obj)) + } + + // Set the project's default field and wait until the status field is set + require.EventuallyWithT(t, func(c *assert.CollectT) { + var p v1.ProjectInstance + require.NoError(c, kc.Get(ctx, router.Key("", project.Name), &p)) + p.Spec.DefaultComputeClass = clusterComputeClass.Name + + require.NoError(c, kc.Update(ctx, &p)) + project = &p + }, time.Minute, time.Second, "failed to update project default") + + project = helper.WaitForObject(t, kc.Watch, new(v1.ProjectInstanceList), project, func(obj *v1.ProjectInstance) bool { + return obj.Status.DefaultComputeClass == clusterComputeClass.Name + }) + + cwd := "./testdata/simple" + image, err := c.AcornImageBuild(ctx, cwd+"/Acornfile", &client.AcornImageBuildOptions{ + Cwd: cwd, + }) + require.NoError(t, err) + + appName := "app" + t.Cleanup(func() { + err := kc.Delete(ctx, &v1.AppInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: appName, + Namespace: c.GetNamespace(), + }, + }) + assert.NoError(t, kclient.IgnoreNotFound(err)) + }) + + app, err := c.AppRun(ctx, image.ID, &client.AppRunOptions{Name: appName}) + require.NoError(t, err) + + // Wait for the app to be scheduled and ensure the computeclass matches the project's default + helper.WaitForObject(t, kc.Watch, new(v1.AppInstanceList), apiv1.AppToAppInstance(app), func(obj *v1.AppInstance) bool { + containers, ok := obj.Status.ResolvedOfferings.Containers[""] + if !ok { + return false + } + + return containers.Class == clusterComputeClass.Name + }) +} + func TestJobDelete(t *testing.T) { helper.StartController(t) @@ -1466,7 +1553,7 @@ func TestJobDelete(t *testing.T) { t.Fatal(err) } - _ = helper.EnsureDoesNotExist(ctx, func() (crClient.Object, error) { + _ = helper.EnsureDoesNotExist(ctx, func() (kclient.Object, error) { return c.AppGet(ctx, app.Name) }) } @@ -1494,7 +1581,7 @@ func TestAppWithBadDefaultRegion(t *testing.T) { helper.StartController(t) ctx := helper.GetCTX(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) c, project := helper.ClientAndProject(t) storageClasses := new(storagev1.StorageClassList) @@ -1561,7 +1648,7 @@ func TestCrossProjectNetworkConnection(t *testing.T) { } ctx := helper.GetCTX(t) c, _ := helper.ClientAndProject(t) - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) cfg, err := config.Get(ctx, kc) if err != nil { @@ -1706,7 +1793,7 @@ func TestCrossProjectNetworkConnection(t *testing.T) { } } -func getPodIPFromAppName(ctx context.Context, t *testing.T, kc *crClient.WithWatch, appName, namespace string) string { +func getPodIPFromAppName(ctx context.Context, t *testing.T, kc *kclient.WithWatch, appName, namespace string) string { t.Helper() selector, err := k8slabels.Parse(fmt.Sprintf("%s=%s", labels.AcornAppName, appName)) if err != nil { @@ -1716,7 +1803,7 @@ func getPodIPFromAppName(ctx context.Context, t *testing.T, kc *crClient.WithWat var podList corev1.PodList podIP := "" for podIP == "" { - err = (*kc).List(ctx, &podList, &kclient.ListOptions{ + err = (*kc).List(ctx, &podList, &rkclient.ListOptions{ LabelSelector: selector, Namespace: namespace, }) @@ -1835,7 +1922,7 @@ func TestEnforcedQuota(t *testing.T) { t.Fatal("error while getting rest config:", err) } // Create a project. - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) project := helper.TempProject(t, kc) // Create a client for the project. @@ -1915,7 +2002,7 @@ func TestAutoUpgradeImageValidation(t *testing.T) { if err != nil { t.Fatal("error while getting rest config:", err) } - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) project := helper.TempProject(t, kc) c, err := client.New(restConfig, project.Name, project.Name) @@ -1951,7 +2038,7 @@ func TestAutoUpgradeLocalImage(t *testing.T) { if err != nil { t.Fatal("error while getting rest config:", err) } - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) project := helper.TempProject(t, kc) c, err := client.New(restConfig, project.Name, project.Name) @@ -2003,7 +2090,7 @@ func TestIgnoreResourceRequirements(t *testing.T) { if err != nil { t.Fatal("error while getting rest config:", err) } - kc := helper.MustReturn(kclient.Default) + kc := helper.MustReturn(rkclient.Default) project := helper.TempProject(t, kc) helper.SetIgnoreResourceRequirementsWithRestore(ctx, t, kc) diff --git a/pkg/apis/internal.admin.acorn.io/v1/default.go b/pkg/apis/internal.admin.acorn.io/v1/default.go index 2ac2c5bdb..1a3449999 100644 --- a/pkg/apis/internal.admin.acorn.io/v1/default.go +++ b/pkg/apis/internal.admin.acorn.io/v1/default.go @@ -7,10 +7,11 @@ import ( internalv1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1" "github.com/acorn-io/z" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" ) -func getCurrentClusterComputeClassDefault(ctx context.Context, c client.Client, projectDefault string) (*ClusterComputeClassInstance, error) { +func getCurrentClusterComputeClassDefault(ctx context.Context, c client.Client, projectSpecified string) (*ClusterComputeClassInstance, error) { var clusterComputeClasses ClusterComputeClassInstanceList if err := c.List(ctx, &clusterComputeClasses, &client.ListOptions{}); err != nil { return nil, err @@ -20,7 +21,7 @@ func getCurrentClusterComputeClassDefault(ctx context.Context, c client.Client, return clusterComputeClasses.Items[i].Name < clusterComputeClasses.Items[j].Name }) - var defaultCCC, projectDefaultCCC *ClusterComputeClassInstance + var defaultCCC, projectSpecifiedCCC *ClusterComputeClassInstance for _, clusterComputeClass := range clusterComputeClasses.Items { if clusterComputeClass.Default { if defaultCCC != nil { @@ -33,19 +34,19 @@ func getCurrentClusterComputeClassDefault(ctx context.Context, c client.Client, defaultCCC = z.Pointer(clusterComputeClass) } - if projectDefault != "" && clusterComputeClass.Name == projectDefault { - projectDefaultCCC = z.Pointer(clusterComputeClass) + if projectSpecified != "" && clusterComputeClass.Name == projectSpecified { + projectSpecifiedCCC = z.Pointer(clusterComputeClass) } } - if projectDefaultCCC != nil { - return projectDefaultCCC, nil + if projectSpecifiedCCC != nil { + return projectSpecifiedCCC, nil } return defaultCCC, nil } -func getCurrentProjectComputeClassDefault(ctx context.Context, c client.Client, projectDefault, namespace string) (*ProjectComputeClassInstance, error) { +func getCurrentProjectComputeClassDefault(ctx context.Context, c client.Client, projectSpecified, namespace string) (*ProjectComputeClassInstance, error) { var projectComputeClasses ProjectComputeClassInstanceList if err := c.List(ctx, &projectComputeClasses, &client.ListOptions{Namespace: namespace}); err != nil { return nil, err @@ -55,7 +56,7 @@ func getCurrentProjectComputeClassDefault(ctx context.Context, c client.Client, return projectComputeClasses.Items[i].Name < projectComputeClasses.Items[j].Name }) - var defaultPCC, projectDefaultPCC *ProjectComputeClassInstance + var defaultPCC, projectSpecifiedPCC *ProjectComputeClassInstance for _, projectComputeClass := range projectComputeClasses.Items { if projectComputeClass.Default { if defaultPCC != nil { @@ -68,39 +69,56 @@ func getCurrentProjectComputeClassDefault(ctx context.Context, c client.Client, defaultPCC = z.Pointer(projectComputeClass) } - if projectDefault != "" && projectComputeClass.Name == projectDefault { - projectDefaultPCC = z.Pointer(projectComputeClass) + if projectSpecified != "" && projectComputeClass.Name == projectSpecified { + projectSpecifiedPCC = z.Pointer(projectComputeClass) } } - if projectDefaultPCC != nil { - return projectDefaultPCC, nil + if projectSpecifiedPCC != nil { + return projectSpecifiedPCC, nil } return defaultPCC, nil } -func GetDefaultComputeClass(ctx context.Context, c client.Client, namespace string) (string, error) { +// GetDefaultComputeClassName gets the name of the effective default ComputeClass for a given project namespace. +// The precedence for picking the default ComputeClass is as follows: +// 1. Any ProjectComputeClassInstance (in the project namespace) or ClusterComputeClassInstance that is specified by the +// ProjectInstance's DefaultComputeClass field +// 2. The ProjectComputeClassInstance (in the project namespace) with a Default field set to true +// 3. The ClusterComputeClassInstance with a Default field set to true +// +// If no default ComputeClass is found, an empty string is returned. +func GetDefaultComputeClassName(ctx context.Context, c client.Client, namespace string) (string, error) { var project internalv1.ProjectInstance if err := c.Get(ctx, client.ObjectKey{Name: namespace}, &project); err != nil { return "", fmt.Errorf("failed to get projectinstance to determine default compute class: %w", err) } - projectDefault := project.Status.DefaultComputeClass + projectSpecified := project.Status.DefaultComputeClass - pcc, err := getCurrentProjectComputeClassDefault(ctx, c, projectDefault, namespace) + var defaultComputeClasses []string + pcc, err := getCurrentProjectComputeClassDefault(ctx, c, projectSpecified, namespace) if err != nil { return "", err } if pcc != nil { - return pcc.Name, nil + defaultComputeClasses = append(defaultComputeClasses, pcc.Name) } - ccc, err := getCurrentClusterComputeClassDefault(ctx, c, projectDefault) + ccc, err := getCurrentClusterComputeClassDefault(ctx, c, projectSpecified) if err != nil { return "", err } if ccc != nil { - return ccc.Name, nil + defaultComputeClasses = append(defaultComputeClasses, ccc.Name) + } + + if sets.New(defaultComputeClasses...).Has(projectSpecified) { + return projectSpecified, nil + } + + if len(defaultComputeClasses) > 0 { + return defaultComputeClasses[0], nil } return "", nil diff --git a/pkg/apis/internal.admin.acorn.io/v1/default_test.go b/pkg/apis/internal.admin.acorn.io/v1/default_test.go new file mode 100644 index 000000000..b534ee5e3 --- /dev/null +++ b/pkg/apis/internal.admin.acorn.io/v1/default_test.go @@ -0,0 +1,208 @@ +package v1_test + +import ( + "context" + "testing" + + internalv1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1" + internaladminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1" + "github.com/acorn-io/runtime/pkg/scheme" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestGetDefaultComputeClass(t *testing.T) { + ctx := context.Background() + + type args struct { + namespace string + client kclient.Client + } + type expected struct { + computeClassName string + error bool + } + for _, tt := range []struct { + name string + args args + expected expected + }{ + { + name: "No defaults", + args: args{ + namespace: "pcc-project", + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internalv1.ProjectInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pcc-project", + }, + }, + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-compute-class", + Namespace: "pcc-project", + }, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-compute-class", + }, + }, + ).Build(), + }, + }, + { + name: "Default cluster compute class", + args: args{ + namespace: "pcc-project", + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internalv1.ProjectInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pcc-project", + }, + }, + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-project-compute-class", + Namespace: "pcc-project", + }, + }, + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-compute-class", + Namespace: "pcc-project", + }, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-cluster-compute-class", + }, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-compute-class", + }, + Default: true, + }, + ).Build(), + }, + expected: expected{ + computeClassName: "cluster-compute-class", + }, + }, + { + name: "Project compute classes take precedence over cluster compute classes", + args: args{ + namespace: "pcc-project", + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internalv1.ProjectInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pcc-project", + }, + }, + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-project-compute-class", + Namespace: "pcc-project", + }, + }, + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-compute-class", + Namespace: "pcc-project", + }, + Default: true, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-cluster-compute-class", + }, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-compute-class", + }, + Default: true, + }, + ).Build(), + }, + expected: expected{ + computeClassName: "project-compute-class", + }, + }, + { + name: "Project specified compute class takes precedence over default project compute class", + args: args{ + namespace: "pcc-project", + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internalv1.ProjectInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pcc-project", + }, + Status: internalv1.ProjectInstanceStatus{ + DefaultComputeClass: "project-specified-default", + }, + }, + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "self-specified-default", + Namespace: "pcc-project", + }, + Default: true, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-specified-default", + }, + }, + ).Build(), + }, + expected: expected{ + computeClassName: "project-specified-default", + }, + }, + { + name: "Project specified compute class not found", + args: args{ + namespace: "pcc-project", + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internalv1.ProjectInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pcc-project", + }, + Spec: internalv1.ProjectInstanceSpec{ + DefaultComputeClass: "project-specified-default", + }, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "self-specified-default", + }, + Default: true, + }, + ).Build(), + }, + expected: expected{ + computeClassName: "self-specified-default", + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + kc := tt.args.client + if kc == nil { + kc = fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + } + + actualComputeClassName, err := internaladminv1.GetDefaultComputeClassName(ctx, kc, tt.args.namespace) + if tt.expected.error { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + require.Equal(t, tt.expected.computeClassName, actualComputeClassName) + }) + } +} diff --git a/pkg/computeclasses/computeclasses.go b/pkg/computeclasses/computeclasses.go index a25aef998..990127d1c 100644 --- a/pkg/computeclasses/computeclasses.go +++ b/pkg/computeclasses/computeclasses.go @@ -149,7 +149,7 @@ func GetClassForWorkload(ctx context.Context, c client.Client, computeClasses in ccName := GetComputeClassNameForWorkload(workload, container, computeClasses) if ccName == "" { var err error - ccName, err = internaladminv1.GetDefaultComputeClass(ctx, c, namespace) + ccName, err = internaladminv1.GetDefaultComputeClassName(ctx, c, namespace) if err != nil { return nil, err } diff --git a/pkg/controller/defaults/memory.go b/pkg/controller/defaults/memory.go index e921155e7..7635ff325 100644 --- a/pkg/controller/defaults/memory.go +++ b/pkg/controller/defaults/memory.go @@ -24,7 +24,7 @@ func addDefaultMemory(req router.Request, cfg *apiv1.Config, appInstance *v1.App if value, ok := appInstance.Spec.ComputeClasses[""]; ok { defaultCC = value } else { - defaultCC, err = adminv1.GetDefaultComputeClass(req.Ctx, req.Client, appInstance.Namespace) + defaultCC, err = adminv1.GetDefaultComputeClassName(req.Ctx, req.Client, appInstance.Namespace) if err != nil { return err } diff --git a/pkg/controller/resolvedofferings/computeclass.go b/pkg/controller/resolvedofferings/computeclass.go index 73ef02261..4d9f7c644 100644 --- a/pkg/controller/resolvedofferings/computeclass.go +++ b/pkg/controller/resolvedofferings/computeclass.go @@ -26,7 +26,7 @@ func resolveComputeClasses(req router.Request, cfg *apiv1.Config, appInstance *v if value, ok := appInstance.Spec.ComputeClasses[""]; ok { defaultCC = value } else { - defaultCC, err = adminv1.GetDefaultComputeClass(req.Ctx, req.Client, appInstance.Namespace) + defaultCC, err = adminv1.GetDefaultComputeClassName(req.Ctx, req.Client, appInstance.Namespace) if err != nil { return err } @@ -132,7 +132,7 @@ func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInst // Next, determine the memory request. This is the order of priority: // 1. runtime-level overrides from the user (in app.Spec) // 2. defaults in the acorn image - // 3. defaults from compute class + // 3. defaults from compute class and project status // 4. global default var ( diff --git a/pkg/controller/resolvedofferings/computeclass_test.go b/pkg/controller/resolvedofferings/computeclass_test.go index da94265a2..65d78672c 100644 --- a/pkg/controller/resolvedofferings/computeclass_test.go +++ b/pkg/controller/resolvedofferings/computeclass_test.go @@ -92,3 +92,7 @@ func TestWithAcornfileMemoryAndSpecOverride(t *testing.T) { func TestRemovedContainer(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/computeclass/removed-container", Calculate) } + +func TestProjectSpecifiedDefault(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/computeclass/project-specified-default", Calculate) +} diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/existing.yaml b/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/existing.yaml new file mode 100644 index 000000000..f844329c8 --- /dev/null +++ b/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/existing.yaml @@ -0,0 +1,53 @@ +--- +kind: ProjectComputeClassInstance +apiVersion: internal.admin.acorn.io/v1 +metadata: + name: self-set-default + namespace: app-namespace +description: Simple description for a simple ComputeClass +cpuScaler: 0.25 +default: true +memory: + min: 1Mi # 1Mi + max: 2Mi # 2Mi + default: 1Mi # 1Mi +affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: foo + operator: In + values: + - bar +--- +kind: ClusterComputeClassInstance +apiVersion: internal.admin.acorn.io/v1 +metadata: + name: project-specified-default +description: Simple description for a simple ComputeClass +cpuScaler: 0.25 +memory: + min: 1Mi # 1Mi + max: 2Mi # 2Mi + default: 1Mi # 1Mi +affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: foo + operator: In + values: + - bar +--- +kind: ProjectInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-namespace +spec: {} +status: + defaultComputeClass: project-specified-default + defaultRegion: local + supportedRegions: + - local diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/expected.golden new file mode 100644 index 000000000..a13f1cb6a --- /dev/null +++ b/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/expected.golden @@ -0,0 +1,71 @@ +`apiVersion: internal.acorn.io/v1 +kind: AppInstance +metadata: + creationTimestamp: null + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 +status: + appImage: + buildContext: {} + id: test + imageData: {} + vcs: {} + appSpec: + containers: + oneimage: + build: + context: . + dockerfile: Dockerfile + image: image-name + metrics: {} + ports: + - port: 80 + protocol: http + targetPort: 81 + probes: null + sidecars: + left: + image: foo + metrics: {} + ports: + - port: 90 + protocol: tcp + targetPort: 91 + probes: null + appStatus: {} + columns: {} + conditions: + reason: Success + status: "True" + success: true + type: resolved-offerings + defaults: {} + namespace: app-created-namespace + observedGeneration: 1 + resolvedOfferings: + containers: + "": + class: project-specified-default + cpu: 1 + memory: 1048576 + left: + class: project-specified-default + cpu: 1 + memory: 1048576 + oneimage: + class: project-specified-default + cpu: 1 + memory: 1048576 + region: local + staged: + appImage: + buildContext: {} + imageData: {} + vcs: {} + summary: {} +` diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/input.yaml b/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/input.yaml new file mode 100644 index 000000000..7dca842b8 --- /dev/null +++ b/pkg/controller/resolvedofferings/testdata/computeclass/project-specified-default/input.yaml @@ -0,0 +1,33 @@ +kind: AppInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test + memory: + oneimage: 1048576 # 1Mi +status: + observedGeneration: 1 + namespace: app-created-namespace + appImage: + id: test + appSpec: + containers: + oneimage: + sidecars: + left: + image: "foo" + ports: + - port: 90 + targetPort: 91 + protocol: tcp + ports: + - port: 80 + targetPort: 81 + protocol: http + image: "image-name" + build: + dockerfile: "Dockerfile" + context: "." diff --git a/pkg/project/handlers.go b/pkg/project/handlers.go index 59e5a6a00..1668a8b04 100644 --- a/pkg/project/handlers.go +++ b/pkg/project/handlers.go @@ -35,7 +35,6 @@ func SetDefaultComputeClass(req router.Request, resp router.Response) error { // The compute class does not exist, clear the status field. project.Status.DefaultComputeClass = "" } - // TODO(njhale): Unset the status field if the project does not support the same regions as the compute class? } resp.Objects(req.Object) diff --git a/pkg/server/registry/apigroups/acorn/projects/validator.go b/pkg/server/registry/apigroups/acorn/projects/validator.go index e0b2da2e5..e4e16cf37 100644 --- a/pkg/server/registry/apigroups/acorn/projects/validator.go +++ b/pkg/server/registry/apigroups/acorn/projects/validator.go @@ -42,7 +42,6 @@ func (v *Validator) Validate(ctx context.Context, obj runtime.Object) field.Erro // Some other error occurred while trying to get the compute class, return an internal error. result = append(result, field.InternalError(field.NewPath("spec", "defaultComputeClass"), err)) } - // TODO(njhale): Validate that the compute class shares the project's supported regions? } return result diff --git a/pkg/server/registry/apigroups/acorn/projects/validator_test.go b/pkg/server/registry/apigroups/acorn/projects/validator_test.go index ebacd7b8c..0acb7ec94 100644 --- a/pkg/server/registry/apigroups/acorn/projects/validator_test.go +++ b/pkg/server/registry/apigroups/acorn/projects/validator_test.go @@ -6,6 +6,7 @@ import ( apiv1 "github.com/acorn-io/runtime/pkg/apis/api.acorn.io/v1" v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1" + internaladminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1" "github.com/acorn-io/runtime/pkg/scheme" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -14,11 +15,10 @@ import ( ) func TestProjectCreateValidation(t *testing.T) { - validator := &Validator{} - tests := []struct { name string project apiv1.Project + client kclient.Client wantError bool }{ { @@ -93,10 +93,86 @@ func TestProjectCreateValidation(t *testing.T) { }, }, }, + { + name: "Create project with default computeclass that doesn't exist", + project: apiv1.Project{ + Spec: v1.ProjectInstanceSpec{ + DefaultComputeClass: "compute-class-dne", + }, + }, + wantError: true, + }, + { + name: "Create project with default computeclass that points to a specific clustercomputeclass", + project: apiv1.Project{ + Spec: v1.ProjectInstanceSpec{ + DefaultComputeClass: "cluster-compute-class", + }, + }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-compute-class", + }, + }, + &internaladminv1.ClusterComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-compute-class-2", + }, + }, + ).Build(), + }, + { + name: "Create project with default computeclass that points to a specific projectcomputeclass", + project: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pcc-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultComputeClass: "project-compute-class", + }, + }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-compute-class", + Namespace: "pcc-project", + }, + }, + ).Build(), + }, + { + name: "Create project with default computeclass that points to a projectcomputeclass in a different project", + project: apiv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pcc-project", + }, + Spec: v1.ProjectInstanceSpec{ + DefaultComputeClass: "project-compute-class", + }, + }, + client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects( + &internaladminv1.ProjectComputeClassInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-compute-class", + Namespace: "not-pcc-project", + }, + }, + ).Build(), + wantError: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Set up test validator + validator := Validator{ + Client: tt.client, + } + if validator.Client == nil { + validator.Client = fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + } + if err := validator.Validate(context.Background(), &tt.project); !tt.wantError { if err != nil { t.Fatal(err) diff --git a/pkg/server/registry/apigroups/admin/computeclass/strategy.go b/pkg/server/registry/apigroups/admin/computeclass/strategy.go index 23e1413f4..9e8854b6f 100644 --- a/pkg/server/registry/apigroups/admin/computeclass/strategy.go +++ b/pkg/server/registry/apigroups/admin/computeclass/strategy.go @@ -65,7 +65,7 @@ func (s *Strategy) List(ctx context.Context, namespace string, _ storage.ListOpt for _, ccc := range clusterComputeClasses.Items { if _, ok := projectComputeClassesSeen[ccc.Name]; ok { - // A project volume class with the same name exists, skipping the cluster volume class + // A project compute class with the same name exists, skipping the project compute class continue } if projectDefaultExists {