Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

[feature] Add reconciler.v1 package along with controller.v1 #140

Open
zw0610 opened this issue Jul 7, 2021 · 8 comments
Open

[feature] Add reconciler.v1 package along with controller.v1 #140

zw0610 opened this issue Jul 7, 2021 · 8 comments

Comments

@zw0610
Copy link
Member

zw0610 commented Jul 7, 2021

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.

@zw0610
Copy link
Member Author

zw0610 commented Jul 7, 2021

/cc @terrytangyuan @Jeffwan

@johnugeorge
Copy link
Member

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.

@terrytangyuan
Copy link
Member

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.

@zw0610
Copy link
Member Author

zw0610 commented Jul 8, 2021

I see. So let me prepare the reconciler.v1 package first. We can further discuss how to deal with the controller mode code.

@Jeffwan
Copy link
Member

Jeffwan commented Jul 8, 2021

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?

@tenzen-y
Copy link
Member

tenzen-y commented Jan 17, 2023

@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: kubeflow/training-operator#1714

@zw0610
Copy link
Member Author

zw0610 commented Jan 17, 2023 via email

@tenzen-y
Copy link
Member

tenzen-y commented Jan 17, 2023

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants