Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Allow volumes with sizes less than 10G in Acornfile (#1232) #1235

Merged
merged 1 commit into from
Feb 21, 2023

Conversation

thedadams
Copy link
Contributor

If a volume is defined in an Acornfile with a size less than 10G, then the size would be overwritten by the parser to be 10G. This was because it mistakenly thought there was a volume binding with size 10G. After this change, volumes defined in an Acornfile with size less than 10G will not be accidentally overwritten.

Issue: #1232

Checklist

  • The title of this PR would make a good line in Acorn's Release Note's Changelog
  • The title of this PR ends with a link to the main issue being address in paranthesis, like: This is a title (#1216). Here's an example
  • All relevant issues are referenced in the PR description. NOTE: don't use GitHub keyworkds that auto-close issues
  • Commits follow contributing guidance
  • Automated tests added to cover the changes. If tests couldn't be added, an explanation is provided in the Verification and Testing section
  • Changes to user-facing functionality, API, CLI, and upgrade impacts are clearly called out in PR description

If a volume is defined in an Acornfile with a size less than 10G, then
the size would be overwritten by the parser to be 10G. This was because
it mistakenly thought there was a volume binding with size 10G. After
this change, volumes defined in an Acornfile with size less than 10G
will not be accidentally overwritten.

Signed-off-by: Donnie Adams <donnie@acorn.io>
Copy link
Contributor

@jacobdonenfeld jacobdonenfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, small comments and questions.

One thing I couldn't find when walking the code is adequate usage of MinSize/MinSizeQuantity. Is there feedback to the user when defining or updating a volume attempting to shrink it, or when below the minimum value? Should there be a check to ensure its above the minimum size somewhere in volume.go:toPVCs? I see it referenced once.

pkg/apis/internal.acorn.io/v1/unmarshal.go Show resolved Hide resolved
pkg/apis/internal.acorn.io/v1/unmarshal.go Show resolved Hide resolved
@thedadams thedadams merged commit bd27105 into acorn-io:main Feb 21, 2023
@thedadams thedadams deleted the gh-1232 branch February 21, 2023 13:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants