From 2ebe1d4c87484eeec409a35595050d0db82dfa31 Mon Sep 17 00:00:00 2001 From: tylerslaton Date: Tue, 3 Oct 2023 18:56:54 -0400 Subject: [PATCH 1/2] Add VolumeSizeDefault flag Signed-off-by: tylerslaton --- .../01-command-line/acorn_install.md | 5 ++-- pkg/apis/api.acorn.io/v1/types.go | 5 ++-- pkg/config/config.go | 6 ++++ pkg/controller/defaults/defaults.go | 8 ++++-- pkg/controller/defaults/volumesize.go | 28 +++++++++++++++++++ pkg/openapi/generated/openapi_generated.go | 9 +++++- pkg/profiles/default.go | 3 ++ pkg/profiles/production.go | 1 + 8 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 pkg/controller/defaults/volumesize.go diff --git a/docs/docs/100-reference/01-command-line/acorn_install.md b/docs/docs/100-reference/01-command-line/acorn_install.md index 3c101a436..ce50ad05c 100644 --- 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 index ae7dfb833..ac5d3782d 100644 --- 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 index 273877140..29adde533 100644 --- 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/defaults/defaults.go b/pkg/controller/defaults/defaults.go index 40d9f6032..858118533 100644 --- 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 new file mode 100644 index 000000000..1d793cd88 --- /dev/null +++ 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 index 26efbd922..786fabe5d 100644 --- a/pkg/openapi/generated/openapi_generated.go +++ b/pkg/openapi/generated/openapi_generated.go @@ -2196,6 +2196,13 @@ func schema_pkg_apis_apiacornio_v1_Config(ref common.ReferenceCallback) common.O Format: "", }, }, + "volumeSizeDefault": { + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, "networkPolicies": { SchemaProps: spec.SchemaProps{ Type: []string{"boolean"}, @@ -2337,7 +2344,7 @@ func schema_pkg_apis_apiacornio_v1_Config(ref common.ReferenceCallback) common.O }, }, }, - Required: []string{"ingressClassName", "clusterDomains", "letsEncrypt", "letsEncryptEmail", "letsEncryptTOSAgree", "setPodSecurityEnforceProfile", "podSecurityEnforceProfile", "httpEndpointPattern", "internalClusterDomain", "acornDNS", "acornDNSEndpoint", "autoUpgradeInterval", "recordBuilds", "publishBuilders", "builderPerProject", "internalRegistryPrefix", "ignoreUserLabelsAndAnnotations", "allowUserLabels", "allowUserAnnotations", "allowUserMetadataNamespaces", "workloadMemoryDefault", "workloadMemoryMaximum", "useCustomCABundle", "propagateProjectAnnotations", "propagateProjectLabels", "manageVolumeClasses", "networkPolicies", "ingressControllerNamespace", "allowTrafficFromNamespace", "serviceLBAnnotations", "awsIdentityProviderArn", "eventTTL", "features", "certManagerIssuer", "profile", "controllerMemory", "controllerCPU", "apiServerMemory", "apiServerCPU", "buildkitdMemory", "buildkitdCPU", "buildkitdServiceMemory", "buildkitdServiceCPU", "registryMemory", "registryCPU"}, + Required: []string{"ingressClassName", "clusterDomains", "letsEncrypt", "letsEncryptEmail", "letsEncryptTOSAgree", "setPodSecurityEnforceProfile", "podSecurityEnforceProfile", "httpEndpointPattern", "internalClusterDomain", "acornDNS", "acornDNSEndpoint", "autoUpgradeInterval", "recordBuilds", "publishBuilders", "builderPerProject", "internalRegistryPrefix", "ignoreUserLabelsAndAnnotations", "allowUserLabels", "allowUserAnnotations", "allowUserMetadataNamespaces", "workloadMemoryDefault", "workloadMemoryMaximum", "useCustomCABundle", "propagateProjectAnnotations", "propagateProjectLabels", "manageVolumeClasses", "volumeSizeDefault", "networkPolicies", "ingressControllerNamespace", "allowTrafficFromNamespace", "serviceLBAnnotations", "awsIdentityProviderArn", "eventTTL", "features", "certManagerIssuer", "profile", "controllerMemory", "controllerCPU", "apiServerMemory", "apiServerCPU", "buildkitdMemory", "buildkitdCPU", "buildkitdServiceMemory", "buildkitdServiceCPU", "registryMemory", "registryCPU"}, }, }, } diff --git a/pkg/profiles/default.go b/pkg/profiles/default.go index 45b5d2aa4..a44c5fc88 100644 --- 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 index ea5e236a5..e2f156801 100644 --- 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. From 69b228e06e3044cd71620bf291b0a89a0a23d057 Mon Sep 17 00:00:00 2001 From: tylerslaton Date: Wed, 4 Oct 2023 14:29:30 -0400 Subject: [PATCH 2/2] Use appInstance.Status.Defaults.VolumeSize when defaulting pvc size Signed-off-by: tylerslaton --- pkg/config/config.go | 2 +- .../deployspec/filter-user-labels/expected.golden | 3 ++- .../testdata/deployspec/labels/expected.golden | 3 ++- .../testdata/deployspec/no-user-labels/expected.golden | 3 ++- .../testdata/volumes/empty/expected.golden | 3 ++- .../testdata/volumes/no-default-class/expected.golden | 3 ++- pkg/controller/appdefinition/volume.go | 10 +++++++++- 7 files changed, 20 insertions(+), 7 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 29adde533..72a6cc571 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -377,7 +377,7 @@ func merge(oldConfig, newConfig *apiv1.Config) *apiv1.Config { if newConfig.IngressControllerNamespace != nil { mergedConfig.IngressControllerNamespace = newConfig.IngressControllerNamespace } - if newConfig.VolumeSizeDefault == "" { + if newConfig.VolumeSizeDefault != "" { mergedConfig.VolumeSizeDefault = newConfig.VolumeSizeDefault } if newConfig.AWSIdentityProviderARN != nil { diff --git a/pkg/controller/appdefinition/testdata/deployspec/filter-user-labels/expected.golden b/pkg/controller/appdefinition/testdata/deployspec/filter-user-labels/expected.golden index 3a31f6c57..1dcb43f7a 100644 --- 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 index 7dc5c0466..2222408df 100644 --- 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 index c3d59fb99..2a4655821 100644 --- 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 index 09ed04f6c..0c0709c4b 100644 --- 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 index 09ed04f6c..0c0709c4b 100644 --- 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 index 5bbe695b2..369240e46 100644 --- 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) }