search

kubeflow / common

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

Use PriorityClassInterface instead of lister #155

Closed Jeffwan closed 3 years ago

Jeffwan commented 3 years ago

Fix this issue in v0.3 branch

https://github.com/kubeflow/tf-operator/issues/1382

The major problem is we didn't initial PriorityClassLister after common migration. When it tries to create podGroup and calculate resources, it's panic.

I've tested with tf-operator master. v1.2.0 should be fixed same way

/cc @kubeflow/wg-training-leads

google-oss-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please ask for approval from jeffwan after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubeflow/common/blob/release-0.3/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
Jeffwan commented 3 years ago

kubeflow/common master needs this change as well. Since file structure has been changed a little bit. Let's check in the codes later

Jeffwan commented 3 years ago

I didn't notice Ce already filed https://github.com/kubeflow/tf-operator/pull/1384. I will close this one.

Jeffwan commented 3 years ago

@gaocegege is there a way we can make this consistent between 1.2-branch and master? Seem 1.2 uses lister and master has to use Interface

Jeffwan commented 3 years ago

Let me create a new informer from clientset and we can stick to lister here.

gaocegege commented 3 years ago

Can you please open an issue t keep track of it?