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

driver: check volume limit #73

Merged
merged 4 commits into from
Sep 5, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 63 additions & 17 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 39 additions & 0 deletions driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
return nil, status.Error(codes.AlreadyExists, "invalid volume capabilities requested. Only SINGLE_NODE_WRITER is supported ('accessModes.ReadWriteOnce' on Kubernetes)")
}

ll.Info("checking volume limit")
if err := d.checkLimit(ctx); err != nil {
return nil, err
}

ll.WithField("volume_req", volumeReq).Info("creating volume")
vol, _, err := d.doClient.Storage.CreateVolume(ctx, volumeReq)
if err != nil {
Expand Down Expand Up @@ -623,6 +628,40 @@ func (d *Driver) waitAction(ctx context.Context, volumeId string, actionId int)
}
}

// checkLimit checks whether the user hit their volume limit to ensure.
func (d *Driver) checkLimit(ctx context.Context) error {
// only one provisioner runs, we can make sure to prevent burst creation
d.readyMu.Lock()
defer d.readyMu.Unlock()

account, _, err := d.doClient.Account.Get(ctx)
if err != nil {
return status.Errorf(codes.Internal,
"couldn't get account information to check volume limit: %s", err.Error())
}

// administrative accounts might have zero length limits, make sure to not check them
if account.VolumeLimit == 0 {
Copy link

Choose a reason for hiding this comment

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

👑

return nil // hail to the king!
}

volumes, _, err := d.doClient.Storage.ListVolumes(ctx, &godo.ListVolumeParams{
Copy link

Choose a reason for hiding this comment

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

🎗 this is listing volumes by region but the received limit is for all regions.
meaning: this is a best-effort check and won't ever hit in case the user has at least 1 volume in a different region.

the proper fix is for us ( DigitalOcean ) to expose limits per region. it is in the works but no ETA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Lucas, good to know. I added a note to the source code so it's aware to the reader as well: 445ec7a

Region: d.region,
})
if err != nil {
return status.Errorf(codes.Internal,
"couldn't get fetch volume list to check volume limit: %s", err.Error())
}

if account.VolumeLimit <= len(volumes) {
return status.Errorf(codes.ResourceExhausted,
"volume limit (%d) has been reached. Current number of volumes: %d. Please contact support.",
account.VolumeLimit, len(volumes))
}

return nil
}

// validateCapabilities validates the requested capabilities. It returns false
// if it doesn't satisfy the currently supported modes of DigitalOcean Block
// Storage
Expand Down
6 changes: 6 additions & 0 deletions driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ func (d *Driver) Run() error {
return resp, err
}

// warn the user, it'll not propagate to the user but at least we see if
// something is wrong in the logs
if err := d.checkLimit(context.Background()); err != nil {
d.log.WithError(err).Warn("CSI plugin will not function correctly, please resolve volume limit")
}

d.srv = grpc.NewServer(grpc.UnaryInterceptor(errHandler))
csi.RegisterIdentityServer(d.srv, d)
csi.RegisterControllerServer(d.srv, d)
Expand Down
13 changes: 13 additions & 0 deletions driver/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,19 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

// return empty response to account, assuming there is no volume limit
if strings.HasPrefix(r.URL.Path, "/v2/account") {
var resp = struct {
Account *godo.Account
}{
Account: &godo.Account{},
}

_ = json.NewEncoder(w).Encode(&resp)
return
}

// rest is /v2/volumes related
switch r.Method {
case "GET":
// A list call
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading