fix: don't default null replicas/readyReplicas in Readiness checks

[sureshmelvinsigera]

isReplicaSetReady and isReplicationControllerReady silently treated a null spec.replicas as 1 and null status.readyReplicas as 0, which can misreport readiness. isStatefulSetReady already had this exact issue fixed by a maintainer in a prior review - this applies the same fix to the two remaining copy-pasted methods, plus regression tests.

See bc40df9 ("Addressed comment") for the original StatefulSet fix this mirrors.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sureshmelvinsigera
Once this PR has been reviewed and has the lgtm label, please assign brendandburns for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[sureshmelvinsigera]

Fixed two of the regression tests (nullReadyReplicas cases). They used replicas=3, which passed under both old and new code and didn't actually catch the bug. Changed to replicas=0, which is the case where the old default (readyReplicas=null becomes 0) would wrongly report ready.

[brendandburns]

Some of this is philosophical, I guess. Is a replica set with zero replicas "ready" or not?

Can you check what client-go does in this case? That's probably as close to a spec as there exists for this question.

[sureshmelvinsigera]

Checked client-go's WaitForReadyReplicaSet - it does *(rs.Spec.Replicas) == rs.Status.ReadyReplicas, so a 0-replica RS is ready there (0 == 0).

But: ReadyReplicas is omitempty on a plain int32 in the real API, so it's omitted from JSON whenever it's 0, not just when unset. We can't tell those apart - both show up as null. So the previous fix was too strict: a genuinely zero-replica RS would show null and we'd say "not ready," disagreeing with client-go.

Pushed an update: null spec.replicas still returns false (client-go doesn't guard against that either, so we're just safer there), but null readyReplicas is now treated as 0, matching what it actually means on the wire. Added tests for both cases.