Skip to content

Commit

Permalink
increased backoff for attach/detach
Browse files Browse the repository at this point in the history
reflect.StructField requires atleast go 1.17 or above

Signed-off-by: Niclas Schad <niclas.schad@stackit.de>

add exponential backoff for volume readiness (RPC: CreateVolume) (#7)

Signed-off-by: Niclas Schad <niclas.schad@stackit.de>
  • Loading branch information
einfachnuralex authored and Niclas Schad committed Dec 1, 2023
1 parent c04bc58 commit 68853da
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 10 deletions.
10 changes: 10 additions & 0 deletions .snyk
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Snyk (https://snyk.io) policy file, patches or ignores known vulnerabilities.
# ignores vulnerabilities until expiry date; change duration by modifying expiry date
ignore:
SNYK-GOLANG-GITHUBCOMEMICKLEIGORESTFUL-2435653:
- '*':
reason: We do not use CORS
expires: 2022-06-12T09:26:24.310Z
created: 2022-05-13T09:26:24.313Z
version: v1.25.0
patch: {}
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,18 @@ TAR_FILE ?= rootfs.tar
GOOS ?= $(shell go env GOOS)
GOPROXY ?= $(shell go env GOPROXY)
VERSION ?= $(shell git describe --dirty --tags --match='v*')
VERSION ?= $(shell git describe --exact-match > /dev/null || \
git describe --tags --always --abbrev=6)
ALPINE_ARCH :=
DEBIAN_ARCH :=
QEMUARCH :=
QEMUVERSION := "v4.2.0-4"
GOARCH :=
GOFLAGS :=
TAGS :=
LDFLAGS := "-w -s -X 'k8s.io/component-base/version.gitVersion=$(VERSION)' -X 'k8s.io/cloud-provider-openstack/pkg/version.Version=$(VERSION)'"
GOX_LDFLAGS := $(shell echo "$(LDFLAGS) -extldflags \"-static\"")
REGISTRY ?= registry.k8s.io/provider-os
REGISTRY ?= reg.infra.ske.eu01.stackit.cloud/stackitcloud
IMAGE_OS ?= linux
IMAGE_NAMES ?= openstack-cloud-controller-manager \
cinder-csi-plugin \
Expand Down
20 changes: 18 additions & 2 deletions pkg/csi/cinder/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package cinder

import (
"fmt"
"k8s.io/apimachinery/pkg/util/wait"
"strconv"
"time"

"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/gophercloud/gophercloud/openstack/blockstorage/v3/snapshots"
Expand Down Expand Up @@ -94,12 +96,16 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
if volSizeGB != volumes[0].Size {
return nil, status.Error(codes.AlreadyExists, "Volume Already exists with same name and different capacity")
}

if volumes[0].Status != openstack.VolumeAvailableStatus {
return nil, status.Error(codes.Internal, fmt.Sprintf("Volume %s is not in available state", volumes[0].ID))
}

klog.V(4).Infof("Volume %s already exists in Availability Zone: %s of size %d GiB", volumes[0].ID, volumes[0].AvailabilityZone, volumes[0].Size)
return getCreateVolumeResponse(&volumes[0], ignoreVolumeAZ, req.GetAccessibilityRequirements()), nil
} else if len(volumes) > 1 {
klog.V(3).Infof("found multiple existing volumes with selected name (%s) during create", volName)
return nil, status.Error(codes.Internal, "Multiple volumes reported by Cinder with same name")

}

// Volume Create
Expand Down Expand Up @@ -137,11 +143,21 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
}

vol, err := cloud.CreateVolume(volName, volSizeGB, volType, volAvailability, snapshotID, sourcevolID, &properties)

if err != nil {
klog.Errorf("Failed to CreateVolume: %v", err)
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume failed with error %v", err))
}

targetStatus := []string{openstack.VolumeAvailableStatus}
err = cloud.WaitVolumeTargetStatusWithCustomBackoff(vol.ID, targetStatus,
&wait.Backoff{
Duration: 20 * time.Second,
Steps: 5,
Factor: 1.28,
})
if err != nil {
klog.Errorf("Failed to WaitVolumeTargetStatus of volume %s: %v", vol.ID, err)
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume Volume %s failed getting available in time: %v", vol.ID, err))
}

klog.V(4).Infof("CreateVolume: Successfully created volume %s in Availability Zone: %s of size %d GiB", vol.ID, vol.AvailabilityZone, vol.Size)
Expand Down
2 changes: 2 additions & 0 deletions pkg/csi/cinder/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package openstack

import (
"fmt"
"k8s.io/apimachinery/pkg/util/wait"
"os"

"github.com/gophercloud/gophercloud"
Expand Down Expand Up @@ -49,6 +50,7 @@ type IOpenStack interface {
DetachVolume(instanceID, volumeID string) error
WaitDiskDetached(instanceID string, volumeID string) error
WaitVolumeTargetStatus(volumeID string, tStatus []string) error
WaitVolumeTargetStatusWithCustomBackoff(volumeID string, tStatus []string, backoff *wait.Backoff) error
GetAttachmentDiskPath(instanceID, volumeID string) (string, error)
GetVolume(volumeID string) (*volumes.Volume, error)
GetVolumesByName(name string) ([]volumes.Volume, error)
Expand Down
47 changes: 40 additions & 7 deletions pkg/csi/cinder/openstack/openstack_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ const (
operationFinishInitDelay = 1 * time.Second
operationFinishFactor = 1.1
operationFinishSteps = 10
diskAttachInitDelay = 1 * time.Second
diskAttachFactor = 1.2
diskAttachSteps = 15
diskDetachInitDelay = 1 * time.Second
diskDetachFactor = 1.2
diskDetachSteps = 13
diskAttachInitDelay = 7 * time.Second
diskAttachFactor = 1.4
diskAttachSteps = 10
diskDetachInitDelay = 7 * time.Second
diskDetachFactor = 1.4
diskDetachSteps = 10
volumeDescription = "Created by OpenStack Cinder CSI driver"
)

Expand Down Expand Up @@ -197,6 +197,12 @@ func (os *OpenStack) AttachVolume(instanceID, volumeID string) (string, error) {
return "", fmt.Errorf("failed to attach %s volume to %s compute: %v", volumeID, instanceID, err)
}

//redundant waitDiskAttached, workaround for raise condition in backend
err = os.WaitDiskAttached(instanceID, volumeID)
if err != nil {
return "", err
}

return volume.ID, nil
}

Expand All @@ -219,7 +225,7 @@ func (os *OpenStack) WaitDiskAttached(instanceID string, volumeID string) error
})

if err == wait.ErrWaitTimeout {
err = fmt.Errorf("Volume %q failed to be attached within the alloted time", volumeID)
err = fmt.Errorf("Volume %q failed to be attached within the allowed time", volumeID)
}

return err
Expand Down Expand Up @@ -258,6 +264,33 @@ func (os *OpenStack) WaitVolumeTargetStatus(volumeID string, tStatus []string) e
return waitErr
}

//WaitVolumeTargetStatusWithCustomBackoff waits for volume to be in target state with custom backoff
func (os *OpenStack) WaitVolumeTargetStatusWithCustomBackoff(volumeID string, tStatus []string, backoff *wait.Backoff) error {
waitErr := wait.ExponentialBackoff(*backoff, func() (bool, error) {
vol, err := os.GetVolume(volumeID)
if err != nil {
return false, err
}
for _, t := range tStatus {
if vol.Status == t {
return true, nil
}
}
for _, eState := range volumeErrorStates {
if vol.Status == eState {
return false, fmt.Errorf("Volume is in Error State : %s", vol.Status)
}
}
return false, nil
})

if waitErr == wait.ErrWaitTimeout {
waitErr = fmt.Errorf("Timeout on waiting for volume %s status to be in %v", volumeID, tStatus)
}

return waitErr
}

// DetachVolume detaches given cinder volume from the compute
func (os *OpenStack) DetachVolume(instanceID, volumeID string) error {
volume, err := os.GetVolume(volumeID)
Expand Down

0 comments on commit 68853da

Please sign in to comment.