apps: retry when scaler hits the conflict

[mfojtik]

Fixes: #19270

This is weird, seems like the new genericScaler does not handle the conflicts? Probably need more investigation.

//cc @tnozicka
//cc @DirectXMan12

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mfojtik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/apps/OWNERS [mfojtik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tnozicka]

I am fine with this hot fix, while we look for what went wrong and better way to fix it

[mfojtik]

@tnozicka lets wait for the tests to prove this is fixing the problem, I'm still curious why the new genericScaler is not handing the conflicts (if that is per-design)... Also wonder if upstream deployments scaler is dealing with conflicts in the caller now (not that deployers acts as kubectl client here, so I guess this should be handled somewhere in kubectl scale code?)

[mfojtik]

/cc @liggitt

[deads2k]

it was bugged. kubernetes/kubernetes#61790 comes to mind.

[soltysh]

That only fixes the result, not the cause. Do we want the scalers to handle conflicts? Personally, the current approach is fine, it's the callers duty to handle those.

[soltysh]

This is another reason we should not use kubectl scaler but rather directly implement that functionality.

[tnozicka]

the genericScaler should already handle retries on conflict

[tnozicka]

conflicts are handled here:

origin/vendor/k8s.io/kubernetes/pkg/kubectl/scale.go

Lines 548 to 549 in bdd4e1b

	cond := ScaleCondition(s, preconditions, namespace, resourceName, newSize, nil)
	if err := wait.PollImmediate(retry.Interval, retry.Timeout, cond); err != nil {

and here:

origin/vendor/k8s.io/kubernetes/pkg/kubectl/scale.go

Lines 132 to 136 in bdd4e1b

	case ScaleError:
	// Retry only on update conflicts.
	if e.FailureType == ScaleUpdateConflictFailure {
	return false, nil
	}

[mfojtik]

Closing in favor of #19275