kubernetes-sigs / kubebuilder-declarative-pattern

A toolkit for building declarative operators with kubebuilder
Apache License 2.0
254 stars 84 forks source link

Better status functions #272

Closed justinsb closed 1 year ago

justinsb commented 1 year ago

We pass in a function to query the current state of objects, this allows us to cache the results (if using SSA) and allows for cluster targeting.

atoato88 commented 1 year ago

/lgtm Sorry for my delay reviewing.

atoato88 commented 1 year ago

Oh, this PR needs rebase..

@justinsb Could you rebase?

yuwenma commented 1 year ago

/LGTM

Two questions and the PR looks great. I'm very looking forward to use this feature in my operator!

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, yuwenma

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: - ~~[OWNERS](https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/blob/master/OWNERS)~~ [justinsb] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
yuwenma commented 1 year ago

/lgtm

justinsb commented 1 year ago

/retest

justinsb commented 1 year ago

/retest

Going to roll the dice one more time, because I think this is the uncertain ordering between watch & apply, so we're closer to fixing it if this goes in, but if that is true I would expect the tests to pass about half the time.

yuwenma commented 1 year ago

For watch/update conflict, would it help if we bump the test manager's cancelling period https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/pull/272/files#diff-62091d2b32c6b6c94aaed7ea3055732f56d47b34c5f66f31079a61a3908c3689R141 , assuming the probability of happening a second status update (after watch) increases?

justinsb commented 1 year ago

/retest

yuwenma commented 1 year ago

/lgtm