fix(controller): Tolerate resourcequota conflict errors. Fixes argopr…

…oj#3217
alexec · Aug 10, 2020 · aa4a591 · aa4a591
1 parent 5eda8b8
commit aa4a591
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 3 deletions.
diff --git a/util/retry/retry.go b/util/retry/retry.go
@@ -24,12 +24,18 @@ var DefaultRetry = wait.Backoff{
 func IsRetryableKubeAPIError(err error) bool {
 	// get original error if it was wrapped
 	err = argoerrs.Cause(err)
-	if apierr.IsNotFound(err) || apierr.IsForbidden(err) || apierr.IsTooManyRequests(err) || apierr.IsInvalid(err) || apierr.IsMethodNotSupported(err) {
+	if apierr.IsNotFound(err) || apierr.IsForbidden(err) || apierr.IsTooManyRequests(err) || IsResourceQuotaConflictErr(err) || apierr.IsInvalid(err) || apierr.IsMethodNotSupported(err) {
 		return false
 	}
 	return true
 }
 
+// It is possible to create a pod and it be prevented by a conflict updating the resource quota,
+// this func identifies those errors and allows us to retry muck like `Forbidden` or `TooManyRequests` errors.
+func IsResourceQuotaConflictErr(err error) bool {
+	return apierr.IsConflict(err) && strings.Contains(err.Error(), "Operation cannot be fulfilled on resourcequota")
+}
+
 // IsRetryableNetworkError returns whether or not the error is a retryable network error
 func IsRetryableNetworkError(err error) bool {
 	if err == nil {

diff --git a/util/retry/retry_test.go b/util/retry/retry_test.go
@@ -0,0 +1,14 @@
+package retry
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	apierr "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+func TestIsResourceQuotaConflictErr(t *testing.T) {
+	assert.False(t, IsResourceQuotaConflictErr(apierr.NewConflict(schema.GroupResource{}, "", nil)))
+	assert.True(t, IsResourceQuotaConflictErr(apierr.NewConflict(schema.GroupResource{"v1", "resourcequotas"}, "", nil)))
+}
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
@@ -2144,7 +2144,7 @@ func (woc *wfOperationCtx) executeScript(nodeName string, templateScope string,
 }
 
 func (woc *wfOperationCtx) checkForbiddenErrorAndResubmitAllowed(err error, nodeName string, tmpl *wfv1.Template) (*wfv1.NodeStatus, error) {
-	if (apierr.IsForbidden(err) || apierr.IsTooManyRequests(err)) && isResubmitAllowed(tmpl) {
+	if (apierr.IsForbidden(err) || apierr.IsTooManyRequests(err) || retry.IsResourceQuotaConflictErr(err)) && isResubmitAllowed(tmpl) {
 		// Our error was most likely caused by a lack of resources. If pod resubmission is allowed, keep the node pending
 		woc.requeue(0)
 		return woc.markNodePending(nodeName, err), nil

diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go
@@ -20,6 +20,7 @@ import (
 	"github.com/argoproj/argo/errors"
 	"github.com/argoproj/argo/pkg/apis/workflow"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo/util/retry"
 	"github.com/argoproj/argo/workflow/common"
 	"github.com/argoproj/argo/workflow/util"
 )
@@ -335,7 +336,7 @@ func (woc *wfOperationCtx) createWorkflowPod(nodeName string, mainCtr apiv1.Cont
 			woc.log.Infof("Skipped pod %s (%s) creation: already exists", nodeName, nodeID)
 			return created, nil
 		}
-		if apierr.IsForbidden(err) || apierr.IsTooManyRequests(err) {
+		if apierr.IsForbidden(err) || apierr.IsTooManyRequests(err) || retry.IsResourceQuotaConflictErr(err) {
 			return nil, err
 		}
 		woc.log.Infof("Failed to create pod %s (%s): %v", nodeName, nodeID, err)