[scheduler cleanup phase 1]:Move scheduler cache interface and implementation to pkg/scheduler/in… #68968

wgliang · 2018-09-22T00:33:15Z

…ternal/cache

What this PR does / why we need it:

The scheduler’s cache implementation is not meant to be exported as a golang library outside of the scheduler. It should be moved from k8s.io/kubernetes/pkg/scheduler/cache to k8s.io/kubernetes/pkg/scheduler/internal/cache.

There are some types in the current cache package, namely NodeInfo, which should remain where they are. And all named as node package.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #68961

Special notes for your reviewer:

Release note:

Move scheduler cache interface and implementation to pkg/scheduler/internal/cache.

wgliang · 2018-09-25T02:34:30Z

/kind cleanup

wgliang · 2018-10-03T05:36:41Z

/retest

misterikkit · 2018-10-04T00:47:04Z

pkg/scheduler/cache/node_info.go

+	return n.generation
+}
+
+// SetGeneration sets the generation on this node.


This needs a comment that it is only to be used for testing. I think we can get rid of this in a future cleanup.

misterikkit

/lgtm

misterikkit · 2018-10-04T00:58:11Z

over to @bsalamat or @k82cn to approve.

k8s-ci-robot · 2018-10-04T01:08:29Z

@wgliang: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-local-e2e-containerized	c442316e668ed150aa80eff8ba8c413627ce3c46	link	`/test pull-kubernetes-local-e2e-containerized`

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.

wgliang · 2018-10-04T02:08:30Z

/retest

wgliang · 2018-10-04T04:28:14Z

/ping @bsalamat @k82cn for approve

k82cn · 2018-10-04T08:36:33Z

/approve

soltysh · 2018-10-04T17:01:48Z

pkg/kubectl/.import-restrictions

@@ -122,6 +122,7 @@
                "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities/util",
                "k8s.io/kubernetes/pkg/scheduler/api",
                "k8s.io/kubernetes/pkg/scheduler/cache",
+                "k8s.io/kubernetes/pkg/scheduler/internal/cache",


Why is this needed?

I think it's due to a transitive dependency. Obviously that package cannot be imported by kubectl, but this line was needed to make the tooling happy.

/cc @lavalamp

Yeah, import boss doesn't understand about internal packages.

Since we can't do better ...
/approve

edit: doesn't seem to work from here :-)
edit-edit: Actually it does

Huang-Wei · 2018-10-04T18:15:57Z

I think leaving the release note as NONE would be more appropriate as it's internal code refactoring.

misterikkit · 2018-10-04T21:27:02Z

I think leaving the release note as NONE would be more appropriate as it's internal code refactoring.

It's okay to put NONE for internal cleanup most of the time, but it doesn't hurt to to have it either. Humans will review release notes before they go out, and decide whether this should be included.

timothysc

/approve

wgliang · 2018-10-06T12:41:47Z

It’s awful and there is a conflict. I will rebase immediately.

…ternal/cache

wgliang · 2018-10-06T12:53:36Z

Just rebse.
Need your last lgtm lable.
/ping @misterikkit

timothysc · 2018-10-06T13:21:21Z

/lgtm

k8s-ci-robot · 2018-10-06T13:21:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse, k82cn, misterikkit, timothysc, wgliang

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:

~~cmd/kubeadm/OWNERS~~ [timothysc]
~~pkg/kubectl/OWNERS~~ [apelisse]
~~pkg/scheduler/OWNERS~~ [k82cn,timothysc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from ravisantoshgudimetla and resouer September 22, 2018 00:33

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 22, 2018

wgliang changed the title ~~Move scheduler cache interface and implementation to pkg/scheduler/in…~~ [WIP]Move scheduler cache interface and implementation to pkg/scheduler/in… Sep 22, 2018

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 22, 2018

wgliang force-pushed the scheduler2/cleanup-cache branch 3 times, most recently from dc3ee68 to 41f2d4d Compare September 25, 2018 00:36

wgliang changed the title ~~[WIP]Move scheduler cache interface and implementation to pkg/scheduler/in…~~ Move scheduler cache interface and implementation to pkg/scheduler/in… Sep 25, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 25, 2018

wgliang changed the title ~~Move scheduler cache interface and implementation to pkg/scheduler/in…~~ [scheduler cleanup phase 1]:Move scheduler cache interface and implementation to pkg/scheduler/in… Sep 25, 2018

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Sep 25, 2018

wgliang force-pushed the scheduler2/cleanup-cache branch 3 times, most recently from aa5f872 to bc109ed Compare September 25, 2018 14:57

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 25, 2018

wgliang force-pushed the scheduler2/cleanup-cache branch from bc109ed to b3567b3 Compare September 25, 2018 15:03

shubheksha mentioned this pull request Sep 25, 2018

Simplify kube-scheduler's run() method #69037

Merged

wgliang force-pushed the scheduler2/cleanup-cache branch 3 times, most recently from 3046456 to 10b9742 Compare September 26, 2018 00:06

k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Oct 3, 2018

wgliang force-pushed the scheduler2/cleanup-cache branch 2 times, most recently from 2b85221 to 137d081 Compare October 4, 2018 00:41

misterikkit reviewed Oct 4, 2018

View reviewed changes

wgliang force-pushed the scheduler2/cleanup-cache branch from 137d081 to 6cc8320 Compare October 4, 2018 00:53

misterikkit approved these changes Oct 4, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2018

soltysh reviewed Oct 4, 2018

View reviewed changes

k8s-ci-robot requested a review from lavalamp October 4, 2018 17:28

timothysc approved these changes Oct 6, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 6, 2018

Move scheduler cache interface and implementation to pkg/scheduler/in…

187e2e0

…ternal/cache

wgliang force-pushed the scheduler2/cleanup-cache branch from 6cc8320 to 187e2e0 Compare October 6, 2018 12:51

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2018

k8s-ci-robot assigned timothysc Oct 6, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2018

k8s-ci-robot merged commit c63462a into kubernetes:master Oct 6, 2018

misterikkit mentioned this pull request Jan 2, 2019

[scheduler cleanup phase 2]: Rename pkg/scheduler/util to pkg/scheduler/internal/podinfo #71879

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[scheduler cleanup phase 1]:Move scheduler cache interface and implementation to pkg/scheduler/in… #68968

[scheduler cleanup phase 1]:Move scheduler cache interface and implementation to pkg/scheduler/in… #68968

wgliang commented Sep 22, 2018 •

edited

Loading

wgliang commented Sep 25, 2018

wgliang commented Oct 3, 2018

misterikkit Oct 4, 2018

misterikkit left a comment

misterikkit commented Oct 4, 2018

k8s-ci-robot commented Oct 4, 2018 •

edited

Loading

wgliang commented Oct 4, 2018

wgliang commented Oct 4, 2018

k82cn commented Oct 4, 2018

soltysh Oct 4, 2018

misterikkit Oct 4, 2018

lavalamp Oct 4, 2018

soltysh Oct 5, 2018

apelisse Oct 5, 2018 •

edited

Loading

Huang-Wei commented Oct 4, 2018

misterikkit commented Oct 4, 2018

timothysc left a comment

wgliang commented Oct 6, 2018

wgliang commented Oct 6, 2018

timothysc commented Oct 6, 2018

k8s-ci-robot commented Oct 6, 2018

[scheduler cleanup phase 1]:Move scheduler cache interface and implementation to pkg/scheduler/in… #68968

[scheduler cleanup phase 1]:Move scheduler cache interface and implementation to pkg/scheduler/in… #68968

Conversation

wgliang commented Sep 22, 2018 • edited Loading

wgliang commented Sep 25, 2018

wgliang commented Oct 3, 2018

misterikkit Oct 4, 2018

Choose a reason for hiding this comment

misterikkit left a comment

Choose a reason for hiding this comment

misterikkit commented Oct 4, 2018

k8s-ci-robot commented Oct 4, 2018 • edited Loading

wgliang commented Oct 4, 2018

wgliang commented Oct 4, 2018

k82cn commented Oct 4, 2018

soltysh Oct 4, 2018

Choose a reason for hiding this comment

misterikkit Oct 4, 2018

Choose a reason for hiding this comment

lavalamp Oct 4, 2018

Choose a reason for hiding this comment

soltysh Oct 5, 2018

Choose a reason for hiding this comment

apelisse Oct 5, 2018 • edited Loading

Choose a reason for hiding this comment

Huang-Wei commented Oct 4, 2018

misterikkit commented Oct 4, 2018

timothysc left a comment

Choose a reason for hiding this comment

wgliang commented Oct 6, 2018

wgliang commented Oct 6, 2018

timothysc commented Oct 6, 2018

k8s-ci-robot commented Oct 6, 2018

wgliang commented Sep 22, 2018 •

edited

Loading

k8s-ci-robot commented Oct 4, 2018 •

edited

Loading

apelisse Oct 5, 2018 •

edited

Loading