Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

what we can do to make resourceconsist much more easier for users #34

Open
WeichengWang1 opened this issue Feb 20, 2024 · 0 comments
Open
Assignees

Comments

@WeichengWang1
Copy link
Collaborator

WeichengWang1 commented Feb 20, 2024

a few todos:

diff --git a/pkg/frame/controller/resourceconsist_controller.go b/pkg/frame/controller/resourceconsist_controller.go
index 57538cf..70ede67 100644
--- a/pkg/frame/controller/resourceconsist_controller.go
+++ b/pkg/frame/controller/resourceconsist_controller.go
@@ -104,6 +104,7 @@ func watch(c controller.Controller, mgr manager.Manager, adapter ReconcileAdapte
                return err
        }
 
+       // todo, this can be optional
        return c.Watch(employeeSource, employeeEventHandler, employeePredicateFuncs)
 }
 
diff --git a/pkg/frame/controller/types.go b/pkg/frame/controller/types.go
index d678a52..8e093a1 100644
--- a/pkg/frame/controller/types.go
+++ b/pkg/frame/controller/types.go
@@ -89,19 +89,26 @@ type ReconcileRequeueOptions interface {
        EmployeeSyncRequeueInterval() time.Duration
 }
 
+// todo separate employer and employee
 // ReconcileAdapter is the interface that customized controllers should implement.
 type ReconcileAdapter interface {
+       // todo make it default, no need to implement a method
        GetControllerName() string
 
+       // todo make it optional, since no need to be implemented if not follow podopslifecycle
        // GetSelectedEmployeeNames returns employees' names selected by employer
-       // note: in multi cluster case, if adapters deployed in fed and employees are under local, the format of employeeName
-       // should be "employeeName" + "#" + "clusterName"
+       // note: in multi cluster case, if adapters following PodOpsLifecycle deployed in fed and employees are under local,
+       // the format of employeeName should be "employeeName" + "#" + "clusterName"
        GetSelectedEmployeeNames(ctx context.Context, employer client.Object) ([]string, error)
 
+       // todo 命名修改,这里其实是变更employer、employee相关的资源,而不是变更employer employee
+       // todo discussion, shall we pass an interface between these methods, so that adapters can pass something
+
        // GetExpectedEmployer and GetCurrentEmployer return expect/current status of employer from related backend provider
        GetExpectedEmployer(ctx context.Context, employer client.Object) ([]IEmployer, error)
        GetCurrentEmployer(ctx context.Context, employer client.Object) ([]IEmployer, error)
 
+       // todo discussion: do we need separate Create/Update/Delete
        // CreateEmployer/UpdateEmployer/DeleteEmployer handles creation/update/deletion of resources related to employer on related backend provider
        CreateEmployer(ctx context.Context, employer client.Object, toCreates []IEmployer) ([]IEmployer, []IEmployer, error)
        UpdateEmployer(ctx context.Context, employer client.Object, toUpdates []IEmployer) ([]IEmployer, []IEmployer, error)
@WeichengWang1 WeichengWang1 self-assigned this Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant