Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for using different storage classes than bulk #102

Open
shreddedbacon opened this issue Aug 30, 2022 · 9 comments
Open

Support for using different storage classes than bulk #102

shreddedbacon opened this issue Aug 30, 2022 · 9 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@shreddedbacon
Copy link
Member

shreddedbacon commented Aug 30, 2022

In some cases, some projects or environments may want to use a different storage class other than bulk. The AWS EFS CSI seems to have a limitation of 120 access points, so being able to create a new storage class to use could be nice. But Lagoon has no way of allowing this to be changed currently.

We have some commented out block that allowed for changing the storage class, but some point in the past this has been removed
https://github.com/uselagoon/build-deploy-tool/blob/main/legacy/build-deploy-docker-compose.sh#L1282-L1285

Need to think about this from a few viewpoints though, if in the future we decide to enable multiple PVs per service, this new implementation needs to be able to support this easily enough.

I've got some ideas that I will jot down in a follow up comment, but would be nice to get some other input too.

Follow up documentation that calls out the docker-compose label for changing this:
https://docs.lagoon.sh/using-lagoon-the-basics/docker-compose-yml/#persistent-storage

@shreddedbacon shreddedbacon added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 30, 2022
@smlx
Copy link
Member

smlx commented Aug 30, 2022

I think one important aspect of this feature is to avoid k8s leaking through the Lagoon abstraction. So for example don't have bulk as an option, instead have regular which maps to bulk on EKS but possibly something else on another cloud, and then premium which maps to whatever is appropriate for the underlying infrastructure.

@shreddedbacon
Copy link
Member Author

I think one important aspect of this feature is to avoid k8s leaking through the Lagoon abstraction. So for example don't have bulk as an option, instead have regular which maps to bulk on EKS but possibly something else on another cloud, and then premium which maps to whatever is appropriate for the underlying infrastructure.

This could work too, but needs to be configurable by operators of a Lagoon too.

@shreddedbacon
Copy link
Member Author

Basically if an AWS lagoon operation team uses the EFS CSI and hits the 120access point limit on the bulk storage class, they need to be able to easily create a new storage class that can then be added to projects/environments. Whether that is abstracted to regular-2 or something like that doesn't matter, this needs to be easy to do.

@shreddedbacon
Copy link
Member Author

May need to solve part of this in the API too

@tobybellwood
Copy link
Member

tobybellwood commented Aug 30, 2022

Relevant conversations:

@tobybellwood
Copy link
Member

potentially, "bulk" should just be a placeholder, and let the remote-controller allocate it to an available, suitable storageclass, and create a new one if needed (or round-robin?)

@shreddedbacon
Copy link
Member Author

potentially, "bulk" should just be a placeholder, and let the remote-controller allocate it to an available, suitable storageclass, and create a new one if needed (or round-robin?)

I thought about this too, but the logic behind it 🤯

@tobybellwood
Copy link
Member

tobybellwood commented Aug 30, 2022

ok possible solution direction:

  1. convert the existing storageClassName in the PVC charts to be populatable from the values file
  2. Allow setting this storageClass at build time - new builds get a DEFAULT_BULK_STORAGECLASS (or similar, if defined in the build-deploy in the remote - defaulted to bulk for b/c), but existing envs get what they've currently got to avoid them breaking
  3. Potentially allow this variable to be read from the docker-compose file (there is some prior art here)
  4. Create a new StorageClass (e.g. bulk2) and set DEFAULT_BULK_STORAGECLASS=bulk2 in the build-deploy to point new builds at this storageClass

@tobybellwood
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants