Scheduler: remove direct import to /pkg/volume/util #90007
Conversation
|
/assign @ahg-g |
|
/priority backlog |
|
/cc @mikedanese Mike, we have a dependency on VolumeBinder (a volume resources-related cache), so this PR isn't going to cut the dependency on the volume pkg |
sorry, tagged the wrong Mike :) /cc @damemi |
b0c0378 to
652597f
Compare
652597f to
bf206c8
Compare
|
@ahg-g I'm not very familiar with the volumebinding, so this may be one that gives us some more trouble trying to clean up? |
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
|
/remove-lifecycle stale |
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
|
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
bf206c8 to
cc05374
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gavinfish The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-kubernetes-bazel-test |
| @@ -29,6 +31,38 @@ import ( | |||
| "k8s.io/kubernetes/pkg/scheduler/framework" | |||
| ) | |||
|
|
|||
| // NOTE: constants in this file are copied from pkg/volume/util package to avoid external scheduler dependency | |||
There was a problem hiding this comment.
An important part of this is to avoid code duplication, so we don't just want to copy constants/functions we want to actually move them and update all callers to refer to the new location.
So the code in this PR may be a better fit to move to component-helpers. Updating the references in other places will also force us to get review from other sigs, which is good (I think sig-storage would be most relevant to this)
There was a problem hiding this comment.
Yes, indeed I want to go further here. After some investigation, I think it is time for us to entirely remove some dependencies here. Ref #96852.
|
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
|
@fejta-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Scheduler: remove direct import to /pkg/volume/util
Which issue(s) this PR fixes:
Ref #89930
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: