Never retry on conflicts - Quota exceeded

[ezeriver94]

Problem description

When creating an Openstack resource, one of the potential errors we can get is "Quota exceeded", which is returned alongisde 409 HTTP code. Based on the behavior of all controllers, when 409 is returned we treat it as terminal, thus never reconciling again until the generation changes

	// We should require the spec to be updated before retrying a create which returned a conflict
	if orcerrors.IsConflict(err) {
		err = orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "invalid configuration creating resource: "+err.Error(), err)
	}

Quota errors are not solved by changing something in the resource spec, so I believe ORC should retry on this specific error type; perhaps only checking for IsConflict is not enough and ORC should be more thorough when deciding what to do based on OS errors.

ORC version

v2.3.0

Additional information

No response

Relevant log output

[eshulman2]

Thanks for the report! This is a valid observation - quota exceeded is indeed a transient condition that can resolve without spec changes.

My analysis:

Looking at this, special-casing quota errors seems problematic to me because the same argument applies to many 409 Conflict errors:

• Duplicate name → resolved by deleting the conflicting resource
• Port in use → resolved by detaching it
• Resource in wrong state → resolved by waiting

From what I can tell, special-casing just quota would be inconsistent. It looks to me like the options are:

1. Exponential backoff for all 409s
2. Keep terminal for all 409s (current behavior)

Honestly, if I were implementing this from scratch, I'd probably go with exponential backoff for all 409s. However, as things stand now, changing this seems like a major behavioral change that would require further consensus from the team before
moving forward.

Potential solution:

There's a #668 being proposed that adds a configurable resyncPeriod option. If implemented, you'd be able to set:

spec:
managedOptions:
resyncPeriod: 30m # Retry terminal errors every 30 minutes

This would provide automatic recovery for all terminal errors (including quota 409s) at a user-controlled interval.

Current workaround:

To trigger an immediate reconciliation, modify an annotation on the resource:
kubectl annotate reconcile-trigger=$(date +%s) --overwrite

This are at least my 2 cents on the issue but lets wait for others to reply and see if we can get consensus.

[ezeriver94]

HI @eshulman2, thanks for the quick and detailed response.
I agree that trying to identify each potential error that would possibly be fixed externally could be an overkill (and prone to errors).
Based on your options, I would say that considering 409 as non-terminal should be a good solution (at least from a user's perspective). Do you know which 409 errors can be solely solved by changing the spec?
In my opinion, terminal errors from K8s controllers should only be used based on criteria that just looks at the spec of a resource and determines it is really wrong, and not to delegate that to an external source (in this case an Openstack service). Basically, the behavior should be similar as with validating webhooks.
Related to #668, it sounds to me like a workaround that could be avoided by just using exponential backoff in case any external action could fix the reconciliation. I don't think reconciling more than we need is that bad, even more if we use exponential backoff to not overload the API server. Perhaps this is my opinion based on my specific use cases where I don't have so many resources being managed by K-ORC.
Finally, related to your current workaround, I will give it a try, although I don't think it will work as ORC uses generation vs observedGeneration, and generation only changes when something in the spec is modified (this excludes labels, annotations and other metadata fields). Maybe I'm wrong but I believe I have tried labeling the resource without success.

func shouldReconcile(obj orcv1alpha1.ObjectWithConditions) bool {
	progressing := meta.FindStatusCondition(obj.GetConditions(), orcv1alpha1.ConditionProgressing)
	if progressing == nil {
		return true
	}

	if progressing.Status == metav1.ConditionTrue {
		return true
	}

	return progressing.ObservedGeneration != obj.GetGeneration()
}