Error is logged even if result of `status.Preflight` doesn't have one

[tomasaschan]

What happened:

I implemented a custom declarative.Status like this:

type myStatus {}

func (myStatus) Preflight(ctx context.Context, o declarative.DeclarativeObject) error {
    // omitted: some precondition is not fulfilled
    // want to requeue with backoff and try again after some other controller has had a chance to make the precondition true
    return declarative.ErrorResult{
        Result: result.Result{Requeue: true},
        // note: Err: nil implied here
    }
}

func NewMyStatus() declarative.Status {
    return declarative.StatusBuilder{
        Preflight: myStatus{},
    }
}

When my controller reconciles with this, it logs "preflight check failed, not reconciling" with a stack trace that doesn't point to anywhere in my own code. That's annoying and a little confusing.

What you expected to happen: reconciliation is requeued without any errors logged.

How to reproduce it (as minimally and precisely as possible):

Create a declarative controller with a status implementation like the above, reconcile an instance and observe logs.

Anything else we need to know?:

This seems related to, but is different from #232. I imagine if my status also had a Reconcile function, it would be confusing that it gets called even though the preflight check failed (signalled by returning a non-nil result, even if that result has a nil Err). That, in turn, happens because this deferred function doesn't check for the preflight result. (It seems it once did check, but it's unclear to me why that was removed...)

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[tomasaschan]

/remove-lifecycle stale

I've encountered this again recently, and have a mind on hacking on a better implementation here.