common
common copied to clipboard
kubeflow
[feature] Add reconciler.v1 package along with controller.v1
The common repo offers a controller.v1 package which is designed for low-level controller mode in kubebuilder. As we are working on a unified controller, it seems a reconciler.v1 package will help future developers to code in high-level reconciler mode.
I would like to take a try if developers think such a reconciler.v1 package will be helpful.
There will be some code overlapping between the controller and reconciler packages, which we can extract into an individual package so that these code can be re-used.
I think, we should move away from low level controller mode so that code is easy to maintain and new devs will find it easy. However, difficulty is to ensure stability of the code base when doing the porting work.
Agreed. We should only expose modules to developers when needed. Otherwise it might introduce additional maintenance efforts such as backwards compatibility and versioning of the new module.
I see. So let me prepare the reconciler.v1 package first. We can further discuss how to deal with the controller mode code.
Current library provides reconcile logics. It's kind of neutral and we did some refactor last year to make it work with both low level controller (tf, mxnet) or high level reconciler (xgboost). It we plan to move to reconciler.v1. Let's add more details what need to do in reconciler.v1. Does it bring locking to specific framework like kubebuilder?
@zw0610 Are there tasks left? We need to determine whether to use controller.v1 or reconciler.v1 when we merge kubeflow/common to kubeflow/training-operator.
/cc @gaocegege @johnugeorge @terrytangyuan
ref: https://github.com/kubeflow/training-operator/issues/1714
no. we shall keep controller.v1 in the merge given the minimal changes to the code.
On Wed, Jan 18, 2023 at 04:48 Yuki Iwai @.***> wrote:
@zw0610 https://github.com/zw0610 Are there tasks left? We need to determine whether to use controller.v1 or reconciler.v1 when we merge kubeflow/common to kubeflow/training-operator.
/cc @gaocegege https://github.com/gaocegege @johnugeorge https://github.com/johnugeorge @terrytangyuan https://github.com/terrytangyuan
— Reply to this email directly, view it on GitHub https://github.com/kubeflow/common/issues/140#issuecomment-1386032196, or unsubscribe https://github.com/notifications/unsubscribe-auth/AK7V6IV6AFLMZJQHTW33XR3WS4ATFANCNFSM4762ZUYQ . You are receiving this because you were mentioned.Message ID: @.***>
no. we shall keep controller.v1 in the merge given the minimal changes to the code.
Makes sense. Let's switch to reconciler.v1 after we merge kubeflow/common to kubeflow/training-operator.