Port controller to new pkg/runtime API #165
Conversation
|
ACLs have moved to fluxcd/pkg:
I also think this PR should include test refactoring, replacing Ginko with https://github.com/fluxcd/pkg/blob/main/runtime/testenv/testenv.go |
No, I'm not going to push the boat out further and do that here. I don't think it's a matter of compliance. |
OK -- I would expect that to be its own change. |
Signed-off-by: Michael Bridgen <michael@weave.works>
The helpers for conditions have changed in runtime v0.13.0-rc.2; this commit is near to a mechanical rewrite of the conditions-hamdling code in the controllers. It removes `Set*Readiness` from all api/*/, since it it only a thin shim, and would now need pkg/runtime to be imported. These were exported funcs, but unlikely to be used outside of the controller code, and anyway it's <1.0. meta.DependencyNotReadyReason was removed from pkg/apis/meta, and gets a more specific replacement `ImageRepositoryNotReadyReason` here. One more thing: I went through and checked each MarkFalse (i.e., not ready) to see if it should be a MarkStalled (i.e., add stalled condition, and also not ready). I think for now it is OK to stick with being ready or not ready, and consider what might lead to being stalled later. Signed-off-by: Michael Bridgen <michael@weave.works>
This uses the patch helper from pkg/runtime. The deferred func is a
lot simpler than the example given in
https://pkg.go.dev/github.com/fluxcd/pkg/runtime/patch#Helper
because I do not use extra conditions, or try to infer the status from
the return values.
Signed-off-by: Michael Bridgen <michael@weave.works>
This replaces all the individual patchStatus calls with a single defer. Signed-off-by: Michael Bridgen <michael@weave.works>
Signed-off-by: Michael Bridgen <michael@weave.works>
93cd240 to
0dcdba0
Compare
The contract for .spec.suspend is (I'm told) to not take any actions, including not updating the status. Signed-off-by: Michael Bridgen <michael@weave.works>
0dcdba0 to
650775e
Compare
|
@squaremo can you please create a branch |
Will anything else happen on that branch? |
I'm thinking of:
|
Are those all things that hold up fixing things on main? The last one especially just sounds like a change that could be made equally to main. |
|
I've retargeted it; anyway it will serve as the target of an rc version of the API, so the analogous changes can be made to image-automation-controller. |
There was a problem hiding this comment.
Sorry it took my rather long to properly review this.
Based on the comments about wanting to address certain things in (a) separate PR(s), this seems to aligned with what the new helpers have to offer.
One personal preference I would like to make a note of is that some of the conditions that now result in Ready==False with a descriptive reason may be candidates for a dedicated negative polarity condition. Reason for this is that it may help consumers that e.g. try to build an UI around the API object to make things visible to their end users, as Condition Types are part of the API (and can thus be used to (conditionally) render things), while the reasons are arbitrary.
In any case, off to a great start, and thank you @squaremo 🥇
MarkTrue/...False/...Unknown)patchStatus.spec.suspendRef: fluxcd/flux2#1601