Skip to content

Commit

Permalink
Fix for the bug ResourceQuotaConflictError
Browse files Browse the repository at this point in the history
This commit contains fix for the bug ResourceQuotaConflictError More details about the issue are here kubernetes/kubernetes#67761
  • Loading branch information
yachna authored and tekton-robot committed Aug 31, 2022
1 parent a1ca389 commit 05f28f2
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 4 deletions.
20 changes: 20 additions & 0 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,11 @@ func (c *Reconciler) updateLabelsAndAnnotations(ctx context.Context, tr *v1beta1

func (c *Reconciler) handlePodCreationError(tr *v1beta1.TaskRun, err error) error {
switch {
case isResourceQuotaConflictError(err):
// Requeue if it runs into ResourceQuotaConflictError Error i.e https://github.com/kubernetes/kubernetes/issues/67761
tr.Status.StartTime = nil
tr.Status.MarkResourceOngoing(podconvert.ReasonPending, fmt.Sprint("tried to create pod, but it failed with ResourceQuotaConflictError"))
return controller.NewRequeueAfter(time.Second)
case isExceededResourceQuotaError(err):
// If we are struggling to create the pod, then it hasn't started.
tr.Status.StartTime = nil
Expand Down Expand Up @@ -735,6 +740,7 @@ func (c *Reconciler) createPod(ctx context.Context, ts *v1beta1.TaskSpec, tr *v1
// to fetch it.
podName := pod.Name
pod, err = c.KubeClientSet.CoreV1().Pods(tr.Namespace).Create(ctx, pod, metav1.CreateOptions{})

if err == nil && willOverwritePodSetAffinity(tr) {
if recorder := controller.GetEventRecorder(ctx); recorder != nil {
recorder.Eventf(tr, corev1.EventTypeWarning, "PodAffinityOverwrite", "Pod template affinity is overwritten by affinity assistant for pod %q", pod.Name)
Expand Down Expand Up @@ -897,3 +903,17 @@ func willOverwritePodSetAffinity(taskRun *v1beta1.TaskRun) bool {
}
return taskRun.Annotations[workspace.AnnotationAffinityAssistantName] != "" && podTemplate.Affinity != nil
}

// isResourceQuotaConflictError returns a bool indicating whether the
// k8 error is of kind resourcequotas or not
func isResourceQuotaConflictError(err error) bool {
k8Err, ok := err.(k8serrors.APIStatus)
if !ok {
return false
}
k8ErrStatus := k8Err.Status()
if k8ErrStatus.Reason != metav1.StatusReasonConflict {
return false
}
return k8ErrStatus.Details != nil && k8ErrStatus.Details.Kind == "resourcequotas"
}
20 changes: 16 additions & 4 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2698,6 +2698,12 @@ status:
expectedStatus corev1.ConditionStatus
expectedReason string
}{{
description: "ResourceQuotaConflictError does not fail taskrun",
err: k8sapierrors.NewConflict(k8sruntimeschema.GroupResource{Group: "v1", Resource: "resourcequotas"}, "dummy", errors.New("operation cannot be fulfilled on resourcequotas dummy the object has been modified please apply your changes to the latest version and try again")),
expectedType: apis.ConditionSucceeded,
expectedStatus: corev1.ConditionUnknown,
expectedReason: podconvert.ReasonPending,
}, {
description: "exceeded quota errors are surfaced in taskrun condition but do not fail taskrun",
err: k8sapierrors.NewForbidden(k8sruntimeschema.GroupResource{Group: "foo", Resource: "bar"}, "baz", errors.New("exceeded quota")),
expectedType: apis.ConditionSucceeded,
Expand All @@ -2720,14 +2726,20 @@ status:
t.Run(tc.description, func(t *testing.T) {
c.handlePodCreationError(taskRun, tc.err)
foundCondition := false
reason := ""
var status corev1.ConditionStatus
for _, cond := range taskRun.Status.Conditions {
if cond.Type == tc.expectedType && cond.Status == tc.expectedStatus && cond.Reason == tc.expectedReason {
foundCondition = true
break
if cond.Type == tc.expectedType {
reason = cond.Reason
status = cond.Status
if status == tc.expectedStatus && reason == tc.expectedReason {
foundCondition = true
break
}
}
}
if !foundCondition {
t.Errorf("expected to find condition type %q, status %q and reason %q", tc.expectedType, tc.expectedStatus, tc.expectedReason)
t.Errorf("expected to find condition type %q, status %q and reason %q [Found reason: %q ] [Found status: %q]", tc.expectedType, tc.expectedStatus, tc.expectedReason, reason, status)
}
})
}
Expand Down

0 comments on commit 05f28f2

Please sign in to comment.