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

Garden doesn't respect changing the StorageClass on the sync deployment #1120

Closed
jmeickle opened this issue Aug 22, 2019 · 1 comment · Fixed by #1125
Closed

Garden doesn't respect changing the StorageClass on the sync deployment #1120

jmeickle opened this issue Aug 22, 2019 · 1 comment · Fixed by #1125
Assignees
Labels
Milestone

Comments

@jmeickle
Copy link

Bug

Current Behavior

We've been having issues with Garden's NFS provisioner hanging or dying, requiring frequent redeploys. In general, we don't want to manage our own NFS-based service, even if it's via a packaged chart, so we investigated replacing this with Amazon EFS.

I set up the EFS provisioner and storageclass (outside of Garden), and then set it on providers[].storage.sync.storageClass. The size was respected, but the storageClass remained as the NFS provisioner. Additionally, the Garden NFS provisioner was still deployed.

Expected behavior

Setting storage.sync.storageClass on results in the created sync PVC actually using that storage class.

Workaround

Manually create a new PVC (e.g. garden-build-sync-efs) that is identical except for using the EFS storageclass. Then hand-edit the garden-build-sync and garden-docker-daemon Deployments to use the new PVC. This will trigger a restart of the containers to mount the new EFS-backed PVC.

This does work, as I was able to use the remote sync plus in-cluster Docker builder. However, this will result in every Garden command warning that the provider configuration is "out of date".

Suggested solution(s)

I'd suggest that the garden-system-nfs storageclass and the NFS provisioner deployment shouldn't be created if the user is providing their own StorageClass, since that means they either don't need it or know what they're doing.

Your environment

[eronarn@ip-192-168-10-243 qf]$ garden version && kubectl version && docker version
0.10.6
Client Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.3", GitCommit:"5e53fd6bc17c0dec8434817e69b04a25d8ae0ff0", GitTreeState:"clean", BuildDate:"2019-06-06T01:44:30Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.2", GitCommit:"f6278300bebbb750328ac16ee6dd3aa7d3549568", GitTreeState:"clean", BuildDate:"2019-08-05T09:15:22Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"linux/amd64"}
Client: Docker Engine - Community
 Version:           19.03.1
 API version:       1.40
 Go version:        go1.12.5
 Git commit:        74b1e89
 Built:             Thu Jul 25 21:18:17 2019
 OS/Arch:           darwin/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          19.03.1
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.12.5
  Git commit:       74b1e89
  Built:            Thu Jul 25 21:17:52 2019
  OS/Arch:          linux/amd64
  Experimental:     true
 containerd:
  Version:          v1.2.6
  GitCommit:        894b81a4b802e4eb2a91d1ce216b8817763c29fb
 runc:
  Version:          1.0.0-rc8
  GitCommit:        425e105d5a03fabd737a126ad93d62a9eeede87f
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683
@edvald
Copy link
Collaborator

edvald commented Aug 22, 2019

Thanks @Eronarn! I agree, your suggested solution seems like the way to go. One thing to consider, is that the storageClass needs to support ReadWriteMany access. I gather the EFS PV provider does support that, so that should work fine. We'll make sure to document that along the way.

@edvald edvald added this to the 0.10.7 milestone Aug 22, 2019
@edvald edvald added the bug label Aug 22, 2019
@edvald edvald self-assigned this Aug 22, 2019
edvald added a commit that referenced this issue Aug 23, 2019
Previously, the `storage.sync.storageClass` parameter was silently
ignored. We now respect the parameter, and skip installing the NFS
provisioner if a storageClass is specified for the volume.

Fixes #1120
edvald added a commit that referenced this issue Aug 24, 2019
Previously, the `storage.sync.storageClass` parameter was silently
ignored. We now respect the parameter, and skip installing the NFS
provisioner if a storageClass is specified for the volume.

Fixes #1120
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants