search

kubeflow / common

Common APIs and libraries shared by other Kubeflow operator repositories.
Apache License 2.0
51 stars 73 forks source link

Missing methods in reconciler.v1 #159

Open zw0610 opened 3 years ago

zw0610 commented 3 years ago

In https://github.com/kubeflow/common/pull/141, methods implemented as panic("implement me!") that are aimed to let developers overriding is removed as the pull request is merged. However, when creating a demo reconciler or even run the test file pkg/reconciler.v1/common/service_test.go, it raises error:

# github.com/kubeflow/common/pkg/reconciler.v1/common
pkg/reconciler.v1/common/reconciler.go:60:24: cannot use jobInter (type *KubeflowJobReconciler) as type JobInterface in assignment:
    *KubeflowJobReconciler does not implement JobInterface (missing ExtractJobStatus method)
pkg/reconciler.v1/common/reconciler.go:66:24: cannot use jobInter (type *KubeflowJobReconciler) as type JobInterface in assignment:
    *KubeflowJobReconciler does not implement JobInterface (missing ExtractJobStatus method)
pkg/reconciler.v1/common/reconciler.go:73:34: cannot use jobInter (type *KubeflowJobReconciler) as type JobInterface in assignment:
    *KubeflowJobReconciler does not implement JobInterface (missing ExtractJobStatus method)
FAIL    command-line-arguments [build failed]
FAIL

Simply comment out the DefaultKubeflowReconciler method in pkg/reconciler.v1/common/reconciler.go does not resolve the problem as new error appears:

# github.com/kubeflow/common/test_job/reconciler.v1/test_job
test_job/reconciler.v1/test_job/test_job_reconciler.go:54:24: cannot use jobInter (type *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler) as type "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface in assignment:
    *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler does not implement "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface (missing ExtractJobStatus method)
test_job/reconciler.v1/test_job/test_job_reconciler.go:60:24: cannot use jobInter (type *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler) as type "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface in assignment:
    *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler does not implement "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface (missing ExtractJobStatus method)
test_job/reconciler.v1/test_job/test_job_reconciler.go:67:34: cannot use jobInter (type *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler) as type "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface in assignment:
    *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler does not implement "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface (missing ExtractJobStatus method)
FAIL    command-line-arguments [build failed]
FAIL

While the error message shows the ExtractJobStatus method is missing, there are other methods still needs to be implemented.

There seems two solutions to this problem:

  1. comment out DefaultKubeflowReconciler method in pkg/reconciler.v1/common/reconciler.go and let user to implement these methods in a lower reconciler level (JobReconciler) level instead of the top level
  2. add these methods back in pkg/reconciler.v1/common with panic("implement me!") so developers only need to override these methods at the top level.
Jeffwan commented 2 years ago

I think first option means we can not provide high level reconciler for users? If that's true, I prefer second option and probably give clear documents and let user know this should be overrided