Merge pull request #2215 from tylerslaton/default-volume-size

tylerslaton · web-flow · commit 1205e2c201f1 · 2023-10-06T16:32:49.000-04:00
Add new --volume-size-default install flag
diff --git a/docs/docs/100-reference/01-command-line/acorn_install.md b/docs/docs/100-reference/01-command-line/acorn_install.md
@@ -71,8 +71,9 @@ acorn install
       --set-pod-security-enforce-profile                Set the PodSecurity profile on created namespaces (default true)
       --skip-checks                                     Bypass installation checks
       --use-custom-ca-bundle                            Use CA bundle for admin supplied secret for all acorn control plane components. Defaults to false.
-  -m, --workload-memory-default string                  Set the default memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and "." and "_" seperators (default 0)
-      --workload-memory-maximum string                  Set the maximum memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and "." and "_" seperators (default 0)
+      --volume-size-default string                      Set the default size for acorn volumes. Accepts storage suffixes (K, M, G, Ki, Mi, Gi, etc) and "." and "_" separators (default 0)
+  -m, --workload-memory-default string                  Set the default memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and "." and "_" separators (default 0)
+      --workload-memory-maximum string                  Set the maximum memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and "." and "_" separators (default 0)
 ```
 
 ### Options inherited from parent commands
diff --git a/pkg/apis/api.acorn.io/v1/types.go b/pkg/apis/api.acorn.io/v1/types.go
@@ -540,12 +540,13 @@ type Config struct {
 	AllowUserLabels                []string        `json:"allowUserLabels" name:"allow-user-label" usage:"Allow these labels to propagate to dependent objects, no effect if --ignore-user-labels-and-annotations not true"`
 	AllowUserAnnotations           []string        `json:"allowUserAnnotations" name:"allow-user-annotation" usage:"Allow these annotations to propagate to dependent objects, no effect if --ignore-user-labels-and-annotations not true"`
 	AllowUserMetadataNamespaces    []string        `json:"allowUserMetadataNamespaces" name:"allow-user-metadata-namespace" usage:"Allow these namespaces to propagate labels and annotations to dependent objects, no effect if --ignore-user-labels-and-annotations not true"`
-	WorkloadMemoryDefault          *int64          `json:"workloadMemoryDefault" name:"workload-memory-default" quantity:"true" usage:"Set the default memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and \".\" and \"_\" seperators (default 0)" short:"m"`
-	WorkloadMemoryMaximum          *int64          `json:"workloadMemoryMaximum" name:"workload-memory-maximum" quantity:"true" usage:"Set the maximum memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and \".\" and \"_\" seperators (default 0)"`
+	WorkloadMemoryDefault          *int64          `json:"workloadMemoryDefault" name:"workload-memory-default" quantity:"true" usage:"Set the default memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and \".\" and \"_\" separators (default 0)" short:"m"`
+	WorkloadMemoryMaximum          *int64          `json:"workloadMemoryMaximum" name:"workload-memory-maximum" quantity:"true" usage:"Set the maximum memory for acorn workloads. Accepts binary suffixes (Ki, Mi, Gi, etc) and \".\" and \"_\" separators (default 0)"`
 	UseCustomCABundle              *bool           `json:"useCustomCABundle" name:"use-custom-ca-bundle" usage:"Use CA bundle for admin supplied secret for all acorn control plane components. Defaults to false."`
 	PropagateProjectAnnotations    []string        `json:"propagateProjectAnnotations" name:"propagate-project-annotation" usage:"The list of keys of annotations to propagate from acorn project to app namespaces"`
 	PropagateProjectLabels         []string        `json:"propagateProjectLabels" name:"propagate-project-label" usage:"The list of keys of labels to propagate from acorn project to app namespaces"`
 	ManageVolumeClasses            *bool           `json:"manageVolumeClasses" name:"manage-volume-classes" usage:"Manually manage volume classes rather than sync with storage classes, setting to 'true' will delete Acorn-created volume classes"`
+	VolumeSizeDefault              string          `json:"volumeSizeDefault" name:"volume-size-default" usage:"Set the default size for acorn volumes. Accepts storage suffixes (K, M, G, Ki, Mi, Gi, etc) and \".\" and \"_\" separators (default 0)"`
 	NetworkPolicies                *bool           `json:"networkPolicies" name:"network-policies" usage:"Create Kubernetes NetworkPolicies which block cross-project network traffic (default false)"`
 	IngressControllerNamespace     *string         `json:"ingressControllerNamespace" name:"ingress-controller-namespace" usage:"The namespace where the ingress controller runs - used to secure published HTTP ports with NetworkPolicies."`
 	AllowTrafficFromNamespace      []string        `json:"allowTrafficFromNamespace" name:"allow-traffic-from-namespace" usage:"Namespaces that are allowed to send network traffic to all Acorn apps"`
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -107,6 +107,9 @@ func complete(ctx context.Context, c *apiv1.Config, getter kclient.Reader, inclu
 	if c.IngressControllerNamespace == nil {
 		c.IngressControllerNamespace = profile.IngressControllerNamespace
 	}
+	if c.VolumeSizeDefault == "" {
+		c.VolumeSizeDefault = profile.VolumeSizeDefault
+	}
 	if c.AWSIdentityProviderARN == nil {
 		c.AWSIdentityProviderARN = profile.AWSIdentityProviderARN
 	}
@@ -374,6 +377,9 @@ func merge(oldConfig, newConfig *apiv1.Config) *apiv1.Config {
 	if newConfig.IngressControllerNamespace != nil {
 		mergedConfig.IngressControllerNamespace = newConfig.IngressControllerNamespace
 	}
+	if newConfig.VolumeSizeDefault != "" {
+		mergedConfig.VolumeSizeDefault = newConfig.VolumeSizeDefault
+	}
 	if newConfig.AWSIdentityProviderARN != nil {
 		mergedConfig.AWSIdentityProviderARN = newConfig.AWSIdentityProviderARN
 	}
diff --git a/pkg/controller/appdefinition/testdata/deployspec/filter-user-labels/expected.golden b/pkg/controller/appdefinition/testdata/deployspec/filter-user-labels/expected.golden
@@ -407,7 +407,8 @@ status:
     status: "True"
     success: true
     type: defined
-  defaults: {}
+  defaults:
+    volumeSize: 10G
   namespace: app-created-namespace
   staged:
     appImage:
diff --git a/pkg/controller/appdefinition/testdata/deployspec/labels/expected.golden b/pkg/controller/appdefinition/testdata/deployspec/labels/expected.golden
@@ -415,7 +415,8 @@ status:
     status: "True"
     success: true
     type: defined
-  defaults: {}
+  defaults:
+    volumeSize: 10G
   namespace: app-created-namespace
   staged:
     appImage:
diff --git a/pkg/controller/appdefinition/testdata/deployspec/no-user-labels/expected.golden b/pkg/controller/appdefinition/testdata/deployspec/no-user-labels/expected.golden
@@ -285,7 +285,8 @@ status:
     status: "True"
     success: true
     type: defined
-  defaults: {}
+  defaults:
+    volumeSize: 10G
   namespace: app-created-namespace
   staged:
     appImage:
diff --git a/pkg/controller/appdefinition/testdata/volumes/empty/expected.golden b/pkg/controller/appdefinition/testdata/volumes/empty/expected.golden
@@ -166,7 +166,8 @@ status:
     status: "True"
     success: true
     type: defined
-  defaults: {}
+  defaults:
+    volumeSize: 10G
   namespace: app-created-namespace
   staged:
     appImage:
diff --git a/pkg/controller/appdefinition/testdata/volumes/no-default-class/expected.golden b/pkg/controller/appdefinition/testdata/volumes/no-default-class/expected.golden
@@ -166,7 +166,8 @@ status:
     status: "True"
     success: true
     type: defined
-  defaults: {}
+  defaults:
+    volumeSize: 10G
   namespace: app-created-namespace
   staged:
     appImage:
diff --git a/pkg/controller/appdefinition/volume.go b/pkg/controller/appdefinition/volume.go
@@ -16,6 +16,7 @@ import (
 	"github.com/acorn-io/runtime/pkg/secrets"
 	"github.com/acorn-io/runtime/pkg/volume"
 	name2 "github.com/rancher/wrangler/pkg/name"
+	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -200,7 +201,14 @@ func toPVCs(req router.Request, appInstance *v1.AppInstance) (result []kclient.O
 			pvc.Spec.VolumeName = pvName
 
 			if volumeRequest.Size == "" {
-				pvc.Spec.Resources.Requests[corev1.ResourceStorage] = *v1.DefaultSize
+				// This shouldn't happen because we set a default in the status. However
+				// if this situation does occur, we'll just use the default size for runtime
+				// and log it at the debug level. As an example, our unit tests hit this case.
+				if appInstance.Status.Defaults.VolumeSize == nil {
+					logrus.Debugf("no default volume size found in status, using static default size of %v", v1.DefaultSize)
+					appInstance.Status.Defaults.VolumeSize = v1.DefaultSize
+				}
+				pvc.Spec.Resources.Requests[corev1.ResourceStorage] = *appInstance.Status.Defaults.VolumeSize
 			} else {
 				pvc.Spec.Resources.Requests[corev1.ResourceStorage] = *v1.MustParseResourceQuantity(volumeRequest.Size)
 			}
diff --git a/pkg/controller/defaults/defaults.go b/pkg/controller/defaults/defaults.go
@@ -5,7 +5,6 @@ import (
 	internalv1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
 	"github.com/acorn-io/runtime/pkg/condition"
 	"github.com/acorn-io/runtime/pkg/config"
-	"github.com/acorn-io/z"
 )
 
 // Calculate is a handler that sets the defaults for an AppInstance to its status if
@@ -31,8 +30,11 @@ func Calculate(req router.Request, resp router.Response) (err error) {
 		}
 	}()
 
-	if appInstance.Status.Defaults.VolumeSize == nil {
-		appInstance.Status.Defaults.VolumeSize = z.Pointer(internalv1.DefaultSize.DeepCopy())
+	// Only set the default volume size if it hasn't been set yet.
+	if appInstance.Status.Defaults.Volumes == nil {
+		if err := addDefaultVolumeSize(req.Ctx, req.Client, appInstance); err != nil {
+			return err
+		}
 	}
 
 	if appInstance.Generation != appInstance.Status.ObservedGeneration {
diff --git a/pkg/controller/defaults/volumesize.go b/pkg/controller/defaults/volumesize.go
@@ -0,0 +1,28 @@
+package defaults
+
+import (
+	"context"
+
+	v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
+	"github.com/acorn-io/runtime/pkg/config"
+	"github.com/acorn-io/z"
+	"k8s.io/apimachinery/pkg/api/resource"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func addDefaultVolumeSize(ctx context.Context, c client.Client, appInstance *v1.AppInstance) error {
+	cfg, err := config.Get(ctx, c)
+	if err != nil {
+		return err
+	}
+
+	defaultVolumeSize := z.Pointer(v1.DefaultSize.DeepCopy())
+
+	// If the default volume size is set in the config, use that instead.
+	if cfgVolumeSize, err := resource.ParseQuantity(cfg.VolumeSizeDefault); err == nil && cfg.VolumeSizeDefault != "" {
+		defaultVolumeSize = &cfgVolumeSize
+	}
+
+	appInstance.Status.Defaults.VolumeSize = defaultVolumeSize
+	return nil
+}
diff --git a/pkg/openapi/generated/openapi_generated.go b/pkg/openapi/generated/openapi_generated.go
diff --git a/pkg/profiles/default.go b/pkg/profiles/default.go
@@ -29,6 +29,8 @@ var (
 		FeatureImageAllowRules:         false,
 		FeatureImageRoleAuthorizations: false,
 	}
+
+	DefaultVolumeSize = "10G"
 )
 
 func defaultProfile() apiv1.Config {
@@ -51,6 +53,7 @@ func defaultProfile() apiv1.Config {
 		LetsEncryptEmail:               "",
 		LetsEncryptTOSAgree:            new(bool),
 		ManageVolumeClasses:            new(bool),
+		VolumeSizeDefault:              DefaultVolumeSize,
 		NetworkPolicies:                new(bool),
 		PodSecurityEnforceProfile:      "baseline",
 		Profile:                        new(string),
diff --git a/pkg/profiles/production.go b/pkg/profiles/production.go
@@ -19,6 +19,7 @@ func productionProfile() apiv1.Config {
 	conf.ManageVolumeClasses = z.Pointer(true)
 	conf.NetworkPolicies = z.Pointer(true)
 	conf.PublishBuilders = z.Pointer(true)
+	conf.VolumeSizeDefault = "1Gi"
 
 	// These values are based on internal testing and usage
 	// statistics. They are not based on any formal benchmarking.

Original file line number	Diff line number	Diff line change
`@@ -107,6 +107,9 @@ func complete(ctx context.Context, c *apiv1.Config, getter kclient.Reader, inclu`
`107`	`107`	`if c.IngressControllerNamespace == nil {`
`108`	`108`	`c.IngressControllerNamespace = profile.IngressControllerNamespace`
`109`	`109`	`}`
	`110`	`+ if c.VolumeSizeDefault == "" {`
	`111`	`+ c.VolumeSizeDefault = profile.VolumeSizeDefault`
	`112`	`+ }`
`110`	`113`	`if c.AWSIdentityProviderARN == nil {`
`111`	`114`	`c.AWSIdentityProviderARN = profile.AWSIdentityProviderARN`
`112`	`115`	`}`
`@@ -374,6 +377,9 @@ func merge(oldConfig, newConfig apiv1.Config) apiv1.Config {`
`374`	`377`	`if newConfig.IngressControllerNamespace != nil {`
`375`	`378`	`mergedConfig.IngressControllerNamespace = newConfig.IngressControllerNamespace`
`376`	`379`	`}`
	`380`	`+ if newConfig.VolumeSizeDefault != "" {`
	`381`	`+ mergedConfig.VolumeSizeDefault = newConfig.VolumeSizeDefault`
	`382`	`+ }`
`377`	`383`	`if newConfig.AWSIdentityProviderARN != nil {`
`378`	`384`	`mergedConfig.AWSIdentityProviderARN = newConfig.AWSIdentityProviderARN`
`379`	`385`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,6 @@ import (`
`5`	`5`	`internalv1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"`
`6`	`6`	`"github.com/acorn-io/runtime/pkg/condition"`
`7`	`7`	`"github.com/acorn-io/runtime/pkg/config"`
`8`		`- "github.com/acorn-io/z"`
`9`	`8`	`)`
`10`	`9`
`11`	`10`	`// Calculate is a handler that sets the defaults for an AppInstance to its status if`
`@@ -31,8 +30,11 @@ func Calculate(req router.Request, resp router.Response) (err error) {`
`31`	`30`	`}`
`32`	`31`	`}()`
`33`	`32`
`34`		`- if appInstance.Status.Defaults.VolumeSize == nil {`
`35`		`- appInstance.Status.Defaults.VolumeSize = z.Pointer(internalv1.DefaultSize.DeepCopy())`
	`33`	`+ // Only set the default volume size if it hasn't been set yet.`
	`34`	`+ if appInstance.Status.Defaults.Volumes == nil {`
	`35`	`+ if err := addDefaultVolumeSize(req.Ctx, req.Client, appInstance); err != nil {`
	`36`	`+ return err`
	`37`	`+ }`
`36`	`38`	`}`
`37`	`39`
`38`	`40`	`if appInstance.Generation != appInstance.Status.ObservedGeneration {`