Skip to content

Commit

Permalink
Fix retry for non-swallowed errors
Browse files Browse the repository at this point in the history
Signed-off-by: Connor Catlett <[email protected]>
  • Loading branch information
ConnorJC3 committed Jan 19, 2024
1 parent 80d1adf commit c92dde9
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
8 changes: 4 additions & 4 deletions pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func newNodeService(driverOptions *DriverOptions) nodeService {

// Remove taint from node to indicate driver startup success
// This is done at the last possible moment to prevent race conditions or false positive removals
go removeTaintInBackground(cloud.DefaultKubernetesAPIClient)
go removeTaintInBackground(cloud.DefaultKubernetesAPIClient, removeNotReadyTaint)

return nodeService{
metadata: metadata,
Expand Down Expand Up @@ -854,12 +854,12 @@ type JSONPatch struct {
}

// removeTaintInBackground is a goroutine that retries removeNotReadyTaint with exponential backoff
func removeTaintInBackground(k8sClient cloud.KubernetesAPIClient) {
func removeTaintInBackground(k8sClient cloud.KubernetesAPIClient, removalFunc func(cloud.KubernetesAPIClient) error) {
backoffErr := wait.ExponentialBackoff(taintRemovalBackoff, func() (bool, error) {
err := removeNotReadyTaint(k8sClient)
err := removalFunc(k8sClient)
if err != nil {
klog.ErrorS(err, "Unexpected failure when attempting to remove node taint(s)")
return false, err
return false, nil
}
return true, nil
})
Expand Down
16 changes: 16 additions & 0 deletions pkg/driver/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/golang/mock/gomock"
"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud"
"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/internal"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -2405,6 +2406,21 @@ func TestRemoveNotReadyTaint(t *testing.T) {
}
}

func TestRemoveTaintInBackground(t *testing.T) {
mockRemovalCount := 0
mockRemovalFunc := func(_ cloud.KubernetesAPIClient) error {
mockRemovalCount += 1
if mockRemovalCount == 3 {
return nil
} else {
return fmt.Errorf("Taint removal failed!")
}
}

removeTaintInBackground(nil, mockRemovalFunc)
assert.Equal(t, mockRemovalCount, 3)
}

func getNodeMock(mockCtl *gomock.Controller, nodeName string, returnNode *corev1.Node, returnError error) (kubernetes.Interface, *MockNodeInterface) {
mockClient := NewMockKubernetesClient(mockCtl)
mockCoreV1 := NewMockCoreV1Interface(mockCtl)
Expand Down

0 comments on commit c92dde9

Please sign in to comment.