add args for NodeResourcesFit plugin #88842

angao · 2020-03-05T10:54:45Z

What type of PR is this?

/kind feature

What this PR does / why we need it:
In our scenario, we use custom extended resources in the container, such as example.com/rc1, example.com/rc2, where rc1 and rc2 are dynamic, so we can't through NodeResourcesFit plugin ignoredResources args settings because it only can set up a fixed set of resource name. But we want to be able to set some prefixes for extension resources that can skip the NodeResourcesFit plugin check and check-in our custom plugin.

Which issue(s) this PR fixes:

Fixes #88887

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Users can configure a resource prefix to ignore a group of resources.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2020-03-05T10:54:53Z

Hi @angao. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

angao · 2020-03-05T10:56:09Z

/sig scheduling

angao · 2020-03-05T10:57:42Z

/assign @bsalamat @yujuhong

alculquicondor · 2020-03-05T16:03:24Z

/hold this needs further discussion

Could you open an issue to describe your usecase in more detail?

angao · 2020-03-10T03:51:28Z

If this pr is acceptable, maybe ExtenderConfig can also be extended.

alculquicondor · 2020-03-10T14:15:58Z

/ok-to-test

One API at a time :) Also, the extender API needs a new version before we modify it.

pkg/scheduler/framework/plugins/noderesources/fit.go

alculquicondor · 2020-03-10T14:23:27Z

pkg/scheduler/framework/plugins/noderesources/fit.go

 }

 // FitArgs holds the args that are used to configure the plugin.
 type FitArgs struct {
 	// IgnoredResources is the list of resources that NodeResources fit filter
 	// should ignore.
 	IgnoredResources []string `json:"ignoredResources,omitempty"`
+	// IgnoredResourceGroups defines the list of resource prefix that NodeResources fit filter should ignore.
+	IgnoredResourceGroups []string `json:"ignoredResourceGroups,omitempty"`


Let's add some format validation for ignoredResourceGroups. Maybe there is some validation for label keys that you could reuse.

@alculquicondor can you clarify how this struct is used and why it has json annotations? is this serialized as a user-visible API or does it deserialize API input from a user?

Yes, it is deserialized from component config. Please see #87784

Ok... spreading API-oriented code out into non-API packages makes it very easy to make breaking changes without realizing it. Anything that serializes or deserializes content from a config file or API request should be in an API-specific package and should be versioned.

Agreed. Do you prefer that #87784 gets solved before this PR?

pkg/scheduler/framework/plugins/noderesources/fit_test.go

alculquicondor · 2020-06-09T13:46:44Z

@liggitt anything to add?

pkg/scheduler/framework/plugins/noderesources/fit.go

liggitt · 2020-06-09T14:01:13Z

pkg/scheduler/framework/plugins/noderesources/fit.go

 		ignoredExtendedResources = sets.NewString()
 	}

+	if ignoredResourceGroups == nil {
+		ignoredResourceGroups = sets.NewString()
+	}


can drop these and avoid allocating these on every call... Len() and Has() can be run on nil string sets

…nded resources

angao · 2020-06-19T02:12:33Z

/test pull-kubernetes-kubemark-e2e-gce-big

angao · 2020-06-29T06:24:43Z

Any other questions?

alculquicondor · 2020-06-29T14:56:40Z

ping @tallclair @liggitt

liggitt · 2020-06-29T17:45:46Z

/approve

for API portions

k8s-ci-robot · 2020-06-29T17:46:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, angao, liggitt

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:

~~pkg/kubelet/OWNERS~~ [liggitt]
~~pkg/scheduler/OWNERS~~ [alculquicondor,liggitt]
~~staging/src/k8s.io/kube-scheduler/config/OWNERS~~ [liggitt]

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

angao · 2020-07-07T02:12:48Z

Is it possible to merge? @alculquicondor

alculquicondor · 2020-07-07T13:55:11Z

/hold cancel
/lgtm

fejta-bot · 2020-07-07T16:48:53Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-07-07T21:00:47Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

angao · 2020-07-08T02:09:15Z

/retest

angao · 2020-07-08T07:01:10Z

/test pull-kubernetes-node-e2e

angao · 2020-07-08T07:01:17Z

/test pull-kubernetes-e2e-gce

angao · 2020-07-08T07:01:44Z

/test pull-kubernetes-verify

fejta-bot · 2020-07-08T13:27:48Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

liggitt · 2020-07-08T20:30:31Z

/retest

fejta-bot · 2020-07-09T02:03:05Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-07-09T11:09:19Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot requested review from liu-cong and tallclair March 5, 2020 10:57

k8s-ci-robot assigned bsalamat and yujuhong Mar 5, 2020

angao changed the title ~~add args for noderesources plugin~~ add args for NodeResourcesFit plugin Mar 5, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 5, 2020

angao mentioned this pull request Mar 6, 2020

add another arg for NodeResourcesFit plugin #88887

Closed

angao force-pushed the fit-arg branch from a5c324c to d8966f1 Compare March 10, 2020 02:45

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 10, 2020

alculquicondor reviewed Mar 10, 2020

View reviewed changes

liggitt reviewed Jun 9, 2020

View reviewed changes

pkg/scheduler/framework/plugins/noderesources/fit.go Show resolved Hide resolved

liggitt reviewed Jun 9, 2020

View reviewed changes

angao force-pushed the fit-arg branch from 748458d to 50c0618 Compare June 10, 2020 02:52

add arg for noderesourcesfit plugin to support ignore a group of exte…

b21b298

…nded resources

angao force-pushed the fit-arg branch from 50c0618 to b21b298 Compare June 10, 2020 02:53

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 29, 2020

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 7, 2020

liggitt added this to the v1.19 milestone Jul 8, 2020

k8s-ci-robot merged commit 70e09f2 into kubernetes:master Jul 9, 2020

github-actions bot mentioned this pull request Jul 14, 2020

Week Ending July 12, 2020 dev-obs/actus#193

Open

liggitt moved this from Assigned to API review completed, 1.19 in API Reviews Jul 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add args for NodeResourcesFit plugin #88842

add args for NodeResourcesFit plugin #88842

angao commented Mar 5, 2020 •

edited

Loading

k8s-ci-robot commented Mar 5, 2020

angao commented Mar 5, 2020

angao commented Mar 5, 2020

alculquicondor commented Mar 5, 2020

angao commented Mar 10, 2020

alculquicondor commented Mar 10, 2020

alculquicondor Mar 10, 2020

liggitt Mar 24, 2020

alculquicondor Mar 24, 2020

liggitt Mar 24, 2020

alculquicondor Mar 25, 2020

alculquicondor commented Jun 9, 2020

liggitt Jun 9, 2020 •

edited

Loading

angao Jun 10, 2020

angao commented Jun 19, 2020

angao commented Jun 29, 2020

alculquicondor commented Jun 29, 2020

liggitt commented Jun 29, 2020

k8s-ci-robot commented Jun 29, 2020

angao commented Jul 7, 2020

alculquicondor commented Jul 7, 2020

fejta-bot commented Jul 7, 2020

fejta-bot commented Jul 7, 2020

angao commented Jul 8, 2020

angao commented Jul 8, 2020

angao commented Jul 8, 2020

angao commented Jul 8, 2020

fejta-bot commented Jul 8, 2020

liggitt commented Jul 8, 2020

fejta-bot commented Jul 9, 2020

fejta-bot commented Jul 9, 2020

add args for NodeResourcesFit plugin #88842

add args for NodeResourcesFit plugin #88842

Conversation

angao commented Mar 5, 2020 • edited Loading

k8s-ci-robot commented Mar 5, 2020

angao commented Mar 5, 2020

angao commented Mar 5, 2020

alculquicondor commented Mar 5, 2020

angao commented Mar 10, 2020

alculquicondor commented Mar 10, 2020

alculquicondor Mar 10, 2020

Choose a reason for hiding this comment

liggitt Mar 24, 2020

Choose a reason for hiding this comment

alculquicondor Mar 24, 2020

Choose a reason for hiding this comment

liggitt Mar 24, 2020

Choose a reason for hiding this comment

alculquicondor Mar 25, 2020

Choose a reason for hiding this comment

alculquicondor commented Jun 9, 2020

liggitt Jun 9, 2020 • edited Loading

Choose a reason for hiding this comment

angao Jun 10, 2020

Choose a reason for hiding this comment

angao commented Jun 19, 2020

angao commented Jun 29, 2020

alculquicondor commented Jun 29, 2020

liggitt commented Jun 29, 2020

k8s-ci-robot commented Jun 29, 2020

angao commented Jul 7, 2020

alculquicondor commented Jul 7, 2020

fejta-bot commented Jul 7, 2020

fejta-bot commented Jul 7, 2020

angao commented Jul 8, 2020

angao commented Jul 8, 2020

angao commented Jul 8, 2020

angao commented Jul 8, 2020

fejta-bot commented Jul 8, 2020

liggitt commented Jul 8, 2020

fejta-bot commented Jul 9, 2020

fejta-bot commented Jul 9, 2020

angao commented Mar 5, 2020 •

edited

Loading

liggitt Jun 9, 2020 •

edited

Loading