Scheduler: remove direct dependency for k8s.io/kubernetes/pkg/util/node #97818
Conversation
7f40c32 to
415723d
Compare
There was a problem hiding this comment.
/cc @ahg-g @ingvagabund
would like to get back to working on these this year :)
|
/lgtm for the scheduler |
| @@ -0,0 +1,57 @@ | |||
| /* | |||
There was a problem hiding this comment.
k8s.io/component-helpers/node directory respects the sig name. Though, I'd like to avoid using util keyword in the path. Can you use k8s.io/component-helpers/node/failuredomain or similar (e.g. topology), that will name a group of semantically close helpers together?
There was a problem hiding this comment.
+1 to not using util as the pkg name, topology sounds reasonable to me.
There was a problem hiding this comment.
Forgot to change k8s.io/component-helpers/node/util
There was a problem hiding this comment.
Ah yeah, forgot to actually run make update. Done
There was a problem hiding this comment.
vendor/modules.txt still has k8s.io/component-helpers/node/util listed
415723d to
fad7a1c
Compare
b882245 to
7b5b527
Compare
|
Bumping this, I believe I addressed the feedback that was already requested |
|
Someone from sig node should sign off. (and add an OWNERS file for component-helpers/node?) |
|
@lavalamp this does add an OWNERS for component-helpers/node (https://github.com/kubernetes/kubernetes/pull/97818/files#diff-1fb4d3172057ae242a64e8442ec2ab6c0a4aaebbed96464ece57cb642829710d) :) Agree that they should sign off on this |
|
For the owners file: For someone from node to look at this: |
|
/cc @sjenning |
|
Needs rebase |
7b5b527 to
905b349
Compare
|
Rebased |
905b349 to
b31eeae
Compare
staging/src/k8s.io/component-helpers/node/topology/helpers_test.go
Outdated
Show resolved
Hide resolved
b31eeae to
af04508
Compare
|
/lgtm for scheduler |
|
/lgtm for sig-node |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damemi, derekwaynecarr, lavalamp The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
|
@damemi: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
|
/retest |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
This replaces #90040, part of the ongoing effort to reduce scheduler dependencies on in-tree k8s code by gradually moving helpers to a staging repo
Which issue(s) this PR fixes:
Ref #89930
/sig scheduling