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 de6ea67fb..8de67c590 100644 --- a/pkg/controller/appdefinition/testdata/deployspec/filter-user-labels/expected.golden +++ b/pkg/controller/appdefinition/testdata/deployspec/filter-user-labels/expected.golden @@ -415,8 +415,7 @@ status: status: "True" success: true type: defined - defaults: - volumeSize: 10G + defaults: {} 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 95f114d47..f5da0fbdf 100644 --- a/pkg/controller/appdefinition/testdata/deployspec/labels/expected.golden +++ b/pkg/controller/appdefinition/testdata/deployspec/labels/expected.golden @@ -423,8 +423,7 @@ status: status: "True" success: true type: defined - defaults: - volumeSize: 10G + defaults: {} 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 b58c38fdd..30f9740a7 100644 --- a/pkg/controller/appdefinition/testdata/deployspec/no-user-labels/expected.golden +++ b/pkg/controller/appdefinition/testdata/deployspec/no-user-labels/expected.golden @@ -297,8 +297,7 @@ status: status: "True" success: true type: defined - defaults: - volumeSize: 10G + defaults: {} 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 571ae3c9b..aec68b6f4 100644 --- a/pkg/controller/appdefinition/testdata/volumes/empty/expected.golden +++ b/pkg/controller/appdefinition/testdata/volumes/empty/expected.golden @@ -175,8 +175,7 @@ status: status: "True" success: true type: defined - defaults: - volumeSize: 10G + defaults: {} 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 571ae3c9b..aec68b6f4 100644 --- a/pkg/controller/appdefinition/testdata/volumes/no-default-class/expected.golden +++ b/pkg/controller/appdefinition/testdata/volumes/no-default-class/expected.golden @@ -175,8 +175,7 @@ status: status: "True" success: true type: defined - defaults: - volumeSize: 10G + defaults: {} namespace: app-created-namespace staged: appImage: diff --git a/pkg/controller/appdefinition/volume.go b/pkg/controller/appdefinition/volume.go index c62da0925..e585abe0c 100644 --- a/pkg/controller/appdefinition/volume.go +++ b/pkg/controller/appdefinition/volume.go @@ -17,9 +17,9 @@ import ( "github.com/acorn-io/runtime/pkg/volume" "github.com/acorn-io/z" name2 "github.com/rancher/wrangler/pkg/name" - "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" klabels "k8s.io/apimachinery/pkg/labels" kclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -196,14 +196,16 @@ func toPVCs(req router.Request, appInstance *v1.AppInstance) (result []kclient.O pvc.Spec.VolumeName = pvName if volumeRequest.Size == "" { - // 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 + // If the volumeRequest does not have a size set, then we need to determine what default to use. If status.Defaults.Volumes has this + // volume set, then we use that. Otherwise, we use v1 package level default size. + defaultSize := *v1.DefaultSize + if defaultVolume, hasDefaultSet := appInstance.Status.Defaults.Volumes[vol]; hasDefaultSet { + defaultSize, err = resource.ParseQuantity(string(defaultVolume.Size)) + if err != nil { + return nil, fmt.Errorf("failed to parse default volume size %q for volume %q: %w", defaultVolume.Size, vol, err) + } } - pvc.Spec.Resources.Requests[corev1.ResourceStorage] = *appInstance.Status.Defaults.VolumeSize + pvc.Spec.Resources.Requests[corev1.ResourceStorage] = defaultSize } 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 index 858118533..0e97e3fc8 100644 --- a/pkg/controller/defaults/defaults.go +++ b/pkg/controller/defaults/defaults.go @@ -30,11 +30,10 @@ func Calculate(req router.Request, resp router.Response) (err error) { } }() - // 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 - } + // addVolumeClassDefaults should run everytime as the function itself will not overwrite any existing + // defaults. Effectively, this means that volume defaults only get set if they have not been set before. + if err = addVolumeClassDefaults(req.Ctx, req.Client, appInstance); err != nil { + return err } if appInstance.Generation != appInstance.Status.ObservedGeneration { @@ -56,10 +55,6 @@ func calculate(req router.Request, appInstance *internalv1.AppInstance) error { return err } - if err = addVolumeClassDefaults(req.Ctx, req.Client, appInstance); err != nil { - return err - } - if err = addDefaultMemory(req, cfg, appInstance); err != nil { return err } diff --git a/pkg/controller/defaults/testdata/memory/all-set-overwrite/expected.golden b/pkg/controller/defaults/testdata/memory/all-set-overwrite/expected.golden index a332e3ddd..094fafdc3 100644 --- a/pkg/controller/defaults/testdata/memory/all-set-overwrite/expected.golden +++ b/pkg/controller/defaults/testdata/memory/all-set-overwrite/expected.golden @@ -51,7 +51,6 @@ status: left: 0 oneimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/memory/all-set/expected.golden b/pkg/controller/defaults/testdata/memory/all-set/expected.golden index 90f759476..67d1a40a5 100644 --- a/pkg/controller/defaults/testdata/memory/all-set/expected.golden +++ b/pkg/controller/defaults/testdata/memory/all-set/expected.golden @@ -50,7 +50,6 @@ status: left: 0 oneimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/memory/container/expected.golden b/pkg/controller/defaults/testdata/memory/container/expected.golden index fb0623c02..d2b5b4bd7 100644 --- a/pkg/controller/defaults/testdata/memory/container/expected.golden +++ b/pkg/controller/defaults/testdata/memory/container/expected.golden @@ -50,7 +50,6 @@ status: left: 0 oneimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/memory/job/expected.golden b/pkg/controller/defaults/testdata/memory/job/expected.golden index 88b4e2d77..6d4a068f2 100644 --- a/pkg/controller/defaults/testdata/memory/job/expected.golden +++ b/pkg/controller/defaults/testdata/memory/job/expected.golden @@ -50,7 +50,6 @@ status: left: 0 oneimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/memory/overwrite-acornfile-memory/expected.golden b/pkg/controller/defaults/testdata/memory/overwrite-acornfile-memory/expected.golden index f519f1778..7006c74d2 100644 --- a/pkg/controller/defaults/testdata/memory/overwrite-acornfile-memory/expected.golden +++ b/pkg/controller/defaults/testdata/memory/overwrite-acornfile-memory/expected.golden @@ -51,7 +51,6 @@ status: left: 0 oneimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/memory/same-generation/expected.golden b/pkg/controller/defaults/testdata/memory/same-generation/expected.golden index e12f76429..7ea75f112 100644 --- a/pkg/controller/defaults/testdata/memory/same-generation/expected.golden +++ b/pkg/controller/defaults/testdata/memory/same-generation/expected.golden @@ -49,7 +49,6 @@ status: "": 0 left: 0 oneimage: 0 - volumeSize: 10G namespace: app-created-namespace staged: appImage: diff --git a/pkg/controller/defaults/testdata/memory/sidecar/expected.golden b/pkg/controller/defaults/testdata/memory/sidecar/expected.golden index 4f7b83e25..78ef621ea 100644 --- a/pkg/controller/defaults/testdata/memory/sidecar/expected.golden +++ b/pkg/controller/defaults/testdata/memory/sidecar/expected.golden @@ -50,7 +50,6 @@ status: left: 0 oneimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/memory/two-ccc-defaults-should-error/expected.yaml b/pkg/controller/defaults/testdata/memory/two-ccc-defaults-should-error/expected.yaml index e3878aae9..9699fbeac 100644 --- a/pkg/controller/defaults/testdata/memory/two-ccc-defaults-should-error/expected.yaml +++ b/pkg/controller/defaults/testdata/memory/two-ccc-defaults-should-error/expected.yaml @@ -11,7 +11,6 @@ spec: status: defaults: region: local - volumeSize: 10G observedGeneration: 1 namespace: app-created-namespace appImage: diff --git a/pkg/controller/defaults/testdata/memory/two-containers/expected.golden b/pkg/controller/defaults/testdata/memory/two-containers/expected.golden index 2c638eb71..34a0213d4 100644 --- a/pkg/controller/defaults/testdata/memory/two-containers/expected.golden +++ b/pkg/controller/defaults/testdata/memory/two-containers/expected.golden @@ -52,7 +52,6 @@ status: oneimage: 0 twoimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/memory/two-pcc-defaults-should-error/expected.yaml b/pkg/controller/defaults/testdata/memory/two-pcc-defaults-should-error/expected.yaml index e3878aae9..9699fbeac 100644 --- a/pkg/controller/defaults/testdata/memory/two-pcc-defaults-should-error/expected.yaml +++ b/pkg/controller/defaults/testdata/memory/two-pcc-defaults-should-error/expected.yaml @@ -11,7 +11,6 @@ spec: status: defaults: region: local - volumeSize: 10G observedGeneration: 1 namespace: app-created-namespace appImage: diff --git a/pkg/controller/defaults/testdata/memory/with-acornfile-memory/expected.golden b/pkg/controller/defaults/testdata/memory/with-acornfile-memory/expected.golden index a0010b4eb..e23679fdb 100644 --- a/pkg/controller/defaults/testdata/memory/with-acornfile-memory/expected.golden +++ b/pkg/controller/defaults/testdata/memory/with-acornfile-memory/expected.golden @@ -49,7 +49,6 @@ status: left: 0 oneimage: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/region/default/expected.golden b/pkg/controller/defaults/testdata/region/default/expected.golden index da9814e21..ff24b6397 100644 --- a/pkg/controller/defaults/testdata/region/default/expected.golden +++ b/pkg/controller/defaults/testdata/region/default/expected.golden @@ -31,7 +31,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/region/project-default-status/expected.golden b/pkg/controller/defaults/testdata/region/project-default-status/expected.golden index da9814e21..ff24b6397 100644 --- a/pkg/controller/defaults/testdata/region/project-default-status/expected.golden +++ b/pkg/controller/defaults/testdata/region/project-default-status/expected.golden @@ -31,7 +31,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/region/region-on-spec/expected.golden b/pkg/controller/defaults/testdata/region/region-on-spec/expected.golden index 27b037d51..fe4b17aaf 100644 --- a/pkg/controller/defaults/testdata/region/region-on-spec/expected.golden +++ b/pkg/controller/defaults/testdata/region/region-on-spec/expected.golden @@ -31,7 +31,6 @@ status: memory: "": 0 container-name: 0 - volumeSize: 10G namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/volumeclass/cluster-and-project-class-same-name/expected.golden b/pkg/controller/defaults/testdata/volumeclass/cluster-and-project-class-same-name/expected.golden index e311fba42..17231acd8 100644 --- a/pkg/controller/defaults/testdata/volumeclass/cluster-and-project-class-same-name/expected.golden +++ b/pkg/controller/defaults/testdata/volumeclass/cluster-and-project-class-same-name/expected.golden @@ -38,7 +38,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G volumes: foo: accessModes: diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-same-gen/expected.golden b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-same-gen/expected.golden index 67f2c63f2..0d0f1f3a2 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-same-gen/expected.golden +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-same-gen/expected.golden @@ -30,13 +30,14 @@ status: appStatus: {} columns: {} conditions: + - error: true + message: 'cannot establish defaults because two defaults volume classes exist: + test-volume-class and test-volume-class-1' observedGeneration: 1 - reason: Success - status: "True" - success: true + reason: Error + status: "False" type: defaults - defaults: - volumeSize: 10G + defaults: {} namespace: app-created-namespace observedGeneration: 1 staged: diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/existing.yaml b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/existing.yaml new file mode 100644 index 000000000..d5b958611 --- /dev/null +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/existing.yaml @@ -0,0 +1,24 @@ +--- +kind: ProjectVolumeClassInstance +apiVersion: internal.admin.acorn.io/v1 +metadata: + name: test-volume-class + namespace: app-namespace +description: Just a simple test volume class +default: true +storageClassName: test-storage-class +size: + min: 1Gi + max: 10Gi + default: 2Gi +allowedAccessModes: ["readWriteOnce"] +--- +kind: ProjectInstance +apiVersion: internal.acorn.io/v1 +metadata: + name: app-namespace +spec: {} +status: + defaultRegion: local + supportedRegions: + - local diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/expected.golden b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/expected.golden new file mode 100644 index 000000000..775c3f3f6 --- /dev/null +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/expected.golden @@ -0,0 +1,54 @@ +`apiVersion: internal.acorn.io/v1 +kind: AppInstance +metadata: + creationTimestamp: null + generation: 2 + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test +status: + appImage: + buildContext: {} + id: test + imageData: {} + vcs: {} + appSpec: + containers: + container-name: + dirs: + /var/tmp: + secret: {} + volume: foo + image: image-name + metrics: {} + probes: null + volumes: + foo: {} + appStatus: {} + columns: {} + conditions: + observedGeneration: 2 + reason: Success + status: "True" + success: true + type: defaults + defaults: + memory: + "": 0 + container-name: 0 + region: local + volumes: + foo: + class: test-volume-class + size: 4Gi + namespace: app-created-namespace + observedGeneration: 1 + staged: + appImage: + buildContext: {} + imageData: {} + vcs: {} + summary: {} +` diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/input.yaml b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/input.yaml new file mode 100644 index 000000000..69c2aab84 --- /dev/null +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-defaults-set/input.yaml @@ -0,0 +1,26 @@ +kind: AppInstance +apiVersion: internal.acorn.io/v1 +metadata: + generation: 2 + name: app-name + namespace: app-namespace + uid: 1234567890abcdef +spec: + image: test +status: + observedGeneration: 1 + namespace: app-created-namespace + appImage: + id: test + appSpec: + containers: + container-name: + image: "image-name" + dirs: + "/var/tmp": + volume: foo + defaults: + volumes: + foo: + class: test-volume-class + size: 4Gi diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-cluster-default/expected.golden b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-cluster-default/expected.golden index 5617fffa8..ced454b76 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-cluster-default/expected.golden +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-cluster-default/expected.golden @@ -37,7 +37,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G volumes: foo: accessModes: diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults-with-bind/expected.golden b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults-with-bind/expected.golden index 7049500a3..f604b7258 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults-with-bind/expected.golden +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults-with-bind/expected.golden @@ -41,7 +41,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G volumes: foo: accessModes: diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults/expected.golden b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults/expected.golden index ee32303a1..45fd223b6 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults/expected.golden +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-defaults/expected.golden @@ -38,7 +38,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G volumes: foo: accessModes: diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-project-default/expected.golden b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-project-default/expected.golden index 98e410aad..9a87fb84b 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-project-default/expected.golden +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-project-default/expected.golden @@ -37,7 +37,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G volumes: foo: accessModes: diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-size/expected.golden b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-size/expected.golden index 2a46db555..9062fe05c 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-size/expected.golden +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-fill-size/expected.golden @@ -40,7 +40,6 @@ status: "": 0 container-name: 0 region: local - volumeSize: 10G volumes: foo: size: 2Gi diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-two-cluster-defaults/expected.yaml b/pkg/controller/defaults/testdata/volumeclass/volume-class-two-cluster-defaults/expected.yaml index 14ba80fa7..d8b5915a0 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-two-cluster-defaults/expected.yaml +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-two-cluster-defaults/expected.yaml @@ -7,9 +7,6 @@ metadata: spec: image: test status: - defaults: - region: local - volumeSize: 10G observedGeneration: 1 namespace: app-created-namespace appImage: diff --git a/pkg/controller/defaults/testdata/volumeclass/volume-class-two-project-defaults/expected.yaml b/pkg/controller/defaults/testdata/volumeclass/volume-class-two-project-defaults/expected.yaml index 14ba80fa7..d8b5915a0 100644 --- a/pkg/controller/defaults/testdata/volumeclass/volume-class-two-project-defaults/expected.yaml +++ b/pkg/controller/defaults/testdata/volumeclass/volume-class-two-project-defaults/expected.yaml @@ -7,9 +7,6 @@ metadata: spec: image: test status: - defaults: - region: local - volumeSize: 10G observedGeneration: 1 namespace: app-created-namespace appImage: diff --git a/pkg/controller/defaults/volumeclass.go b/pkg/controller/defaults/volumeclass.go index 3539229ef..b841d81b1 100644 --- a/pkg/controller/defaults/volumeclass.go +++ b/pkg/controller/defaults/volumeclass.go @@ -6,6 +6,7 @@ import ( "github.com/acorn-io/baaah/pkg/typed" v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1" + "github.com/acorn-io/runtime/pkg/config" "github.com/acorn-io/runtime/pkg/volume" kclient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -36,17 +37,39 @@ func addVolumeClassDefaults(ctx context.Context, c kclient.Client, app *v1.AppIn }) for name, vol := range app.Status.AppSpec.Volumes { - var volDefaults v1.VolumeDefault + volDefaults := app.Status.Defaults.Volumes[name] vol = volume.CopyVolumeDefaults(vol, volumeBindings[name], volDefaults) - if vol.Class == "" && defaultVolumeClass != nil { - volDefaults.Class = defaultVolumeClass.Name - vol.Class = volDefaults.Class - } - if vol.Size == "" { - volDefaults.Size = volumeClasses[vol.Class].Size.Default + + // This is a bit of a hack as we're migrating away from the VolumeSize field. Essentially, + // we want to ensure that app.Status.Volumes[name] always has a size set. If the VolumeSize + // field has been set in the past, we want to mirgrate that over to be set on app.Status.Volumes[name]. + // There is another edge case where the Size field was set by a VolumeClass's default size. In this + // case we want to leave the Size field alone. + if app.Status.Defaults.VolumeSize != nil && volDefaults.Size == "" { + volDefaults.Size = v1.Quantity(app.Status.Defaults.VolumeSize.String()) } - if len(vol.AccessModes) == 0 { - volDefaults.AccessModes = volumeClasses[vol.Class].AllowedAccessModes + + // If the Volume already has defaults, skip these steps. We don't want to overwrite + // default class or access modes for volumes as it can lead to unexpected behavior when + // volume classes are updated. + if _, alreadySet := app.Status.Defaults.Volumes[name]; !alreadySet { + if vol.Class == "" && defaultVolumeClass != nil { + volDefaults.Class = defaultVolumeClass.Name + vol.Class = volDefaults.Class + } + if len(vol.AccessModes) == 0 { + volDefaults.AccessModes = volumeClasses[vol.Class].AllowedAccessModes + } + if vol.Size == "" { + volDefaults.Size = volumeClasses[vol.Class].Size.Default + if volDefaults.Size == "" { + defaultSize, err := getDefaultVolumeSize(ctx, c) + if err != nil { + return err + } + volDefaults.Size = defaultSize + } + } } app.Status.Defaults.Volumes[name] = volDefaults @@ -54,3 +77,19 @@ func addVolumeClassDefaults(ctx context.Context, c kclient.Client, app *v1.AppIn return nil } + +func getDefaultVolumeSize(ctx context.Context, c kclient.Client) (v1.Quantity, error) { + cfg, err := config.Get(ctx, c) + if err != nil { + return "", err + } + + // If the default volume size is set in the config, use that. Otherwise use the + // package level default in v1. + defaultVolumeSize := v1.DefaultSizeQuantity + if cfg.VolumeSizeDefault != "" { + defaultVolumeSize = v1.Quantity(cfg.VolumeSizeDefault) + } + + return defaultVolumeSize, nil +} diff --git a/pkg/controller/defaults/volumeclass_test.go b/pkg/controller/defaults/volumeclass_test.go index b0d030e31..b0a97b143 100644 --- a/pkg/controller/defaults/volumeclass_test.go +++ b/pkg/controller/defaults/volumeclass_test.go @@ -13,6 +13,10 @@ func TestCalculateSameGeneration(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/volumeclass/volume-class-defaults-same-gen", Calculate) } +func TestVolumeClassDefaultsSet(t *testing.T) { + tester.DefaultTest(t, scheme.Scheme, "testdata/volumeclass/volume-class-defaults-set", Calculate) +} + func TestFillVolumeClassDefaults(t *testing.T) { tester.DefaultTest(t, scheme.Scheme, "testdata/volumeclass/volume-class-fill-defaults", Calculate) } diff --git a/pkg/controller/defaults/volumesize.go b/pkg/controller/defaults/volumesize.go deleted file mode 100644 index 1d793cd88..000000000 --- a/pkg/controller/defaults/volumesize.go +++ /dev/null @@ -1,28 +0,0 @@ -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/controller/quota/quota.go b/pkg/controller/quota/quota.go index 58f146350..11834ed8a 100644 --- a/pkg/controller/quota/quota.go +++ b/pkg/controller/quota/quota.go @@ -205,12 +205,8 @@ func addStorage(req router.Request, appInstance *v1.AppInstance, quotaRequest *a // defaultVolumeSize determines the default size of the specified volume. If the volume has a default size set // on the status.Defaults.Volumes, it uses that. Otherwise, it uses the default size set on the status.Defaults.VolumeSize. func defaultVolumeSize(appInstance *v1.AppInstance, name string) resource.Quantity { - // Figure out what the default volume size should be. If the status has a default volume size set, use that. Otherwise, - // use the default size set in the v1 package. + // Use the v1.DefaultSize if the appInstance doesn't have a default size set on the status. result := *v1.DefaultSize // Safe to dereference because it is statically set in the v1 package. - if appInstance.Status.Defaults.VolumeSize != nil { - result = *appInstance.Status.Defaults.VolumeSize - } // If the volume has a default size set on status.Defaults.Volumes, use that. if defaultVolume, set := appInstance.Status.Defaults.Volumes[name]; set { diff --git a/pkg/controller/quota/testdata/status-default-volume-size/input.yaml b/pkg/controller/quota/testdata/status-default-volume-size/input.yaml index 6f8db5aff..6b83f6d5b 100644 --- a/pkg/controller/quota/testdata/status-default-volume-size/input.yaml +++ b/pkg/controller/quota/testdata/status-default-volume-size/input.yaml @@ -40,4 +40,6 @@ status: accessModes: - readWriteOnce defaults: - volumeSize: 1Gi + volumes: + volume-name: + size: 1Gi