Rule ordering

[artemvmin]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Adds priority to drainability rules. This allows custom rules that return a lower priority outcome (e.g. DrainOk) to not get blocked by higher priority outcomes (e.g. SkipDrain).

Special notes for your reviewer:

Parent rebase: #6164

The one complication is that the legacy replica count check is now run after DrainOk rules (i.e. daemonset, safetoevict, and terminal). See drainability/rules.go and the now-combined replicated rule. If this is not okay, I have some alternatives:

1. How far out are we from completely deprecating the legacy logic? Can we remove this logic?
2. Temporarily add duplicate "legacy checks" to the DrainOk rules, so that they are skipped. This is the least ideal, but should be a reasonable last resort.

Does this PR introduce a user-facing change?

NONE

[artemvmin]

/assign x13n

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: artemvmin
Once this PR has been reviewed and has the lgtm label, please ask for approval from x13n. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• cluster-autoscaler/OWNERS

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

[x13n]

I'm not a fan of this approach. It implies rule ordering doesn't matter - only outcome priority does, which is not true (see #6077 for an example why).

Another issue is that it will be impossible to create custom rules blocking drain if there's an existing rule resulting in DrainOk/SkipDrain.

[artemvmin]

I'm not a fan of this approach. It implies rule ordering doesn't matter - only outcome priority does, which is not true (see #6077 for an example why).

Another issue is that it will be impossible to create custom rules blocking drain if there's an existing rule resulting in DrainOk/SkipDrain.

Agreed. Consider this alternate implementation, which makes the framework more flexible, not less: #6196

[x13n]

Thanks. Reviewed the other PR and closing this one.

/close

[k8s-ci-robot]

@x13n: Closed this PR.

In response to this:

Thanks. Reviewed the other PR and closing this one.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.