Reuse restored volumes on CreateVolume retries and check the maximum size before restoring

pkg/cloud/capabilities.go

@@ -0,0 +1,41 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package cloud
+
+import (
+	"context"
+
+	"k8s.io/klog/v2"
+)
+
+func (c *client) GetMaxCustomDiskSizeGB(ctx context.Context) (int64, error) {
+	logger := klog.FromContext(ctx)
+	logger.V(2).Info("CloudStack API call", "command", "ListCapabilities")
+
+	resp, err := c.Configuration.ListCapabilities(c.Configuration.NewListCapabilitiesParams())
+	if err != nil {
+		return 0, err
+	}
+	if resp.Capabilities == nil {
+		return 0, nil
+	}
+
+	return resp.Capabilities.Customdiskofferingmaxsize, nil
+}

pkg/cloud/cloud.go

@@ -46,6 +46,10 @@ type Interface interface {
 	DetachVolume(ctx context.Context, volumeID string) error
 	ExpandVolume(ctx context.Context, volumeID string, newSizeInGB int64) error
 
+	// GetMaxCustomDiskSizeGB returns CloudStack's maximum size for volumes with
+	// a custom disk offering (0 if CloudStack reports no limit).
+	GetMaxCustomDiskSizeGB(ctx context.Context) (int64, error)
+
 	CreateVolumeFromSnapshot(ctx context.Context, zoneID, name, projectID, snapshotID string, sizeInGB int64) (*Volume, error)
 	GetSnapshotByID(ctx context.Context, snapshotID string) (*Snapshot, error)
 	GetSnapshotByName(ctx context.Context, name string) (*Snapshot, error)
@@ -69,6 +73,9 @@ type Volume struct {
 
 	VirtualMachineID string
 	DeviceID         string
+
+	// State is the CloudStack volume state, e.g. Allocated, Ready, Destroy.
+	State string
 }
 
 // Snapshot represents a CloudStack snapshot.
@@ -83,6 +90,9 @@ type Snapshot struct {
 
 	VolumeID  string
 	CreatedAt string
+
+	// State is the CloudStack snapshot state, e.g. Creating, BackingUp, BackedUp.
+	State string
 }
 
 // VM represents a CloudStack Virtual Machine.

pkg/cloud/fake/fake.go

@@ -158,6 +158,10 @@ func (f *fakeConnector) ExpandVolume(_ context.Context, volumeID string, newSize
 	return cloud.ErrNotFound
 }
 
+func (f *fakeConnector) GetMaxCustomDiskSizeGB(_ context.Context) (int64, error) {
+	return 0, nil
+}
+
 func (f *fakeConnector) CreateVolumeFromSnapshot(_ context.Context, zoneID, name, _, _ string, sizeInGB int64) (*cloud.Volume, error) {
 	vol := &cloud.Volume{
 		ID:             "fake-vol-from-snap-" + name,

pkg/cloud/snapshots.go

@@ -21,7 +21,6 @@ package cloud
 
 import (
 	"context"
-	"strings"
 
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -46,10 +45,13 @@ func (c *client) GetSnapshotByID(ctx context.Context, snapshotID string) (*Snaps
 	return &Snapshot{
 		ID:        snapshot.Id,
 		Name:      snapshot.Name,
+		State:     snapshot.State,
+		Size:      snapshot.Virtualsize,
 		DomainID:  snapshot.Domainid,
 		ProjectID: snapshot.Projectid,
 		ZoneID:    snapshot.Zoneid,
 		VolumeID:  snapshot.Volumeid,
+		CreatedAt: snapshot.Created,
 	}, nil
 }
 
@@ -70,6 +72,7 @@ func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Sn
 	return &Snapshot{
 		ID:        snapshot.Id,
 		Name:      snapshot.Name,
+		State:     snapshot.State,
 		Size:      snapshot.Virtualsize,
 		DomainID:  snapshot.Domainid,
 		ProjectID: snapshot.Projectid,
@@ -82,11 +85,10 @@ func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Sn
 func (c *client) DeleteSnapshot(_ context.Context, snapshotID string) error {
 	p := c.Snapshot.NewDeleteSnapshotParams(snapshotID)
 	_, err := c.Snapshot.DeleteSnapshot(p)
-	if err != nil && strings.Contains(err.Error(), "4350") {
-		// CloudStack error InvalidParameterValueException
-		return ErrNotFound
-	}
 
+	// Errors are returned as they are. CloudStack reports an already deleted
+	// snapshot with several different messages, so the caller checks whether
+	// the snapshot still exists instead of matching error text.
 	return err
 }
 
@@ -107,6 +109,8 @@ func (c *client) GetSnapshotByName(ctx context.Context, name string) (*Snapshot,
 	return &Snapshot{
 		ID:        snapshot.Id,
 		Name:      snapshot.Name,
+		State:     snapshot.State,
+		Size:      snapshot.Virtualsize,
 		DomainID:  snapshot.Domainid,
 		ProjectID: snapshot.Projectid,
 		ZoneID:    snapshot.Zoneid,
@@ -148,6 +152,7 @@ func (c *client) ListSnapshots(ctx context.Context, volumeID, snapshotID string)
 		s := &Snapshot{
 			ID:        snapshot.Id,
 			Name:      snapshot.Name,
+			State:     snapshot.State,
 			Size:      snapshot.Virtualsize,
 			DomainID:  snapshot.Domainid,
 			ProjectID: snapshot.Projectid,

pkg/cloud/volumes.go

@@ -23,7 +23,6 @@ import (
 	"context"
 	"fmt"
 	"strconv"
-	"strings"
 
 	"github.com/apache/cloudstack-go/v2/cloudstack"
 	"k8s.io/klog/v2"
@@ -42,6 +41,7 @@ func mapVolume(vol *cloudstack.Volume) *Volume {
 		ZoneID:           vol.Zoneid,
 		VirtualMachineID: vol.Virtualmachineid,
 		DeviceID:         strconv.FormatInt(vol.Deviceid, 10),
+		State:            vol.State,
 	}
 }
 
@@ -113,11 +113,11 @@ func (c *client) DeleteVolume(ctx context.Context, id string) error {
 		"id": id,
 	})
 	_, err := c.Volume.DeleteVolume(p)
-	if err != nil && strings.Contains(err.Error(), "4350") {
-		// CloudStack error InvalidParameterValueException
-		return ErrNotFound
-	}
 
+	// Errors are returned as they are. CloudStack uses the same error code for
+	// "no such volume" and for refusals such as "volume is attached", so the
+	// caller checks whether the volume still exists before treating a failed
+	// delete as done.
 	return err
 }