Update Scheduler Docs #10742

mburke5678 · 2018-07-10T19:27:22Z

From BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1596472
1. _Wrong statements or examples that I can tell:
Predicates [3] are boolean functions, they return true/false. The example is wrong, it does not make sense to have a weigh in a predicate. Both ServiceAffinity and LabelsPresence Configurable Predicates examples are wrong.

Configurable priorities [4] examples are wrong.
They apply only to one label. So label must be in singular "label", not "labels"

Weight can be any positive value. It does not need to range between 0 and 10. Actually, in [5] you can see a default weight of 10000 in a function._

2. Do we want to add:
Service-Anti-Affinity based on custom labels is not recommended to achieve equal distribution of pods across multiple zones... it's highly recommended to use failure-domain.beta.kubernetes.io/region and failure-domain.beta.kubernetes.io/zone node label
https://access.redhat.com/solutions/3432401

mburke5678 · 2018-07-10T19:42:09Z

@aveshagarwal @ravisantoshgudimetla I worked you on this topic some time back via #7313

Can you PTAL at admin_guide/scheduling/scheduler.adoc to address Jorge's concerns in https://bugzilla.redhat.com/show_bug.cgi?id=1596472

mburke5678 · 2018-07-16T19:49:34Z

@aveshagarwal @ravisantoshgudimetla PTAL at my changes and recommend any other changes?

Jorge Luis Tudela Gonzalez de Riancho
The scheduler doc topic [1] contains some bad examples and inaccurate statements.
...someone from Engineering should deeply review this.

https://bugzilla.redhat.com/show_bug.cgi?id=1596472

mburke5678 · 2018-07-19T15:06:28Z

@aveshagarwal @ravisantoshgudimetla PTAL at my changes and recommend any other changes?

Jorge Luis Tudela Gonzalez de Riancho
The scheduler doc topic [1] contains some bad examples and inaccurate statements.
...someone from Engineering should deeply review this.

https://bugzilla.redhat.com/show_bug.cgi?id=1596472

aveshagarwal · 2018-07-24T15:04:58Z

@mburke5678 Could you remove weight from the following stanza: https://github.com/openshift/openshift-docs/pull/10742/files#diff-c881c052264120bfc3a506b42de92c15R481

         "name":"RackPreferred",
         "weight" : "1"
         "argument":{
            "labelsPresence":{
               "labels":[
                  "rack"
            "labelsPresence:"{
                "labels:"[
                - "region"
                presence: true

aveshagarwal · 2018-07-24T15:09:22Z

@mburke5678 also the above part is incorrectly formatted.

aveshagarwal · 2018-07-24T15:13:45Z

admin_guide/scheduling/scheduler.adoc

@@ -634,7 +629,7 @@ concentration of pods.
   ]
 ----
 <1> Specify a name for the priority.
-<2> Specify a weight from 1 (bad fit) to 10 (best fit).
+<2> Specify a weight. Weight can be any positive value.


it could be negative too.

aveshagarwal · 2018-07-24T15:16:28Z

admin_guide/scheduling/scheduler.adoc

@@ -657,22 +652,20 @@ regardless of its value.
 "predicates":[


should be priorities.

aveshagarwal · 2018-07-24T15:17:57Z

admin_guide/scheduling/scheduler.adoc

-                  "<label>" <3>
-                presence: true/false <4>
+                  "<label>" <2>
+                presence: true/false <3>


the above part seems completely incorrect. We are talking about priorities and label preference but the above shows predicate and label presence. The above should be fixed.

@aveshagarwal I changed these parameters back to how they were in previous version, before the changes to the scheduling topic were made.
Did you see @jtudelag comments in the BZ: _scheduling based on custom labels does not work... Most of the examples then do not apply.... I am not sure what needs to be changed. It looks like the examples at the bottom have been there for quite some time.

@mburke5678, I guess what @aveshagarwal means is that first of all, this is a priority function, not a predicate. Then it is "LabelPreference", not "LabelsPresence".

Also, as a function, It always should have a weight. It is still correct in OCP 3.5 examples:
https://docs.openshift.com/container-platform/3.5/admin_guide/scheduler.html#configurable-priority-functions

It was wrongly modifies in 3.6, probably a copy paste (as the file was moved in the doc hierarchy) shit happens ;)
https://docs.openshift.com/container-platform/3.6/admin_guide/scheduling/scheduler.html#configurable-priority-functions

So I guess a backport of all these changes should be done to 3.6 ,3.7 and 3.9.

Main question is if this is still honoured by the scheduler in OCP. Also, the explanation of what the LabelPreference function does should be re-written, as now it is just a copy paste of the LabelPresence predicate. @aveshagarwal should answer these questions, I dont have enought knowledge to do it ;(.

So, now I have Configurable Priority: LabelPreference, with weight, and label singlular/
Configurable Predicate: LabelsPresence, no weight and labels plural. Also has presence: true/false.
Need a definition for LabelPreference. They are slightly different
LabelPreference prefers nodes that have a particular label defined or not, regardless of its value.
LabelsPresence checks whether a particular node has a certain label defined or not, regardless of its value.

@mburke5678, I guess what @aveshagarwal means is that first of all, this is a priority function, not a predicate. Then it is "LabelPreference", not "LabelsPresence".

Also, as a function, It always should have a weight. It is still correct in OCP 3.5 examples:
https://docs.openshift.com/container-platform/3.5/admin_guide/scheduler.html#configurable-priority-functions

+1

Main question is if this is still honoured by the scheduler in OCP.

If I understand your questions correctly so yes LabelPreference and LabelPresence still work.

Also, the explanation of what the LabelPreference function does should be re-written, as now it is just a copy paste of the LabelPresence predicate. @aveshagarwal should answer these questions, I dont have enought knowledge to do it ;(.

+1

So, now I have Configurable Priority: LabelPreference, with weight, and label singlular/

Correct. LabelPreference also has presense: true/false

Configurable Predicate: LabelsPresence, no weight and labels plural. Also has presence: true/false.

correct.

Need a definition for LabelPreference. They are slightly different
LabelPreference prefers nodes that have a particular label defined or not, regardless of its value.
LabelsPresence checks whether a particular node has a certain label defined or not, regardless of its value.

From the code for LabelPreference:

// LabelPreference holds the parameters that are used to configure the corresponding priority function type LabelPreference struct { // Used to identify node "groups" Label string `json:"label"` // This is a boolean flag // If true, higher priority is given to nodes that have the label // If false, higher priority is given to nodes that do not have the label Presence bool `json:"presence"` }

From the code for LabelPresence :

// LabelsPresence holds the parameters that are used to configure the corresponding predicate in scheduler policy configuration. type LabelsPresence struct { // The list of labels that identify node "groups" // All of the labels should be either present (or absent) for the node to be considered a fit for hosting the pod Labels []string `json:"labels"` // The boolean flag that indicates whether the labels should be present or absent from the node Presence bool `json:"presence"` }

@aveshagarwal
"If true, higher priority is given to nodes that have the label"

What is "true" in this case? The presence of the specified label on a node?

**_LabelPreference_** give priority based on the specified label. If the label is present on a node, that node is given priority. If no label is specified, priority is given to nodes that do not have a label.

What is "true" in this case? The presence of the specified label on a node?

Yes.

presence should be quoted

openshift-docs-preview-bot · 2018-07-24T20:19:42Z

The preview will be availble shortly at:

admin_guide/scheduling/scheduler.adoc: https://BZ-1596472--ocpdocs.netlify.com/openshift-enterprise/(head%20detached%20at%20fetch_head)/admin_guide/scheduling/scheduler.html

mburke5678 · 2018-07-26T14:45:25Z

@wjiangjay PTAL

jtudelag · 2018-07-25T19:05:40Z

admin_guide/scheduling/scheduler.adoc

@@ -241,6 +241,12 @@ you require.
 # master-restart api master-restart controllers
 ----

+[NOTE]
+====
+There are cases where using configuring scheduler policy to use custom labels does not spread pod as expected. 


@mburke5678 I'm not a native speaker, but "using configuring scheduler policy" does not seem grammarly correct.

Would be good to mention that is only about ServiceAntiAffinity specifically.

@jtudelag You are correct. I saw that, too; but hadn't pushed the change yet. Thank you.
@aveshagarwal I reworded the note to mention ServiceAntiAffinity and moved it to that section.
https://github.com/openshift/openshift-docs/pull/10742/files#diff-c881c052264120bfc3a506b42de92c15R648

@mburke5678 lgtm.

@mburke5678 lgtm too ;).

ghost · 2018-08-01T02:27:55Z

admin_guide/scheduling/scheduler.adoc

+            "labelsPreference":{
+               "label":[
+                  "label":[  <3>
+                  "rack"


Two label here?

ghost · 2018-08-01T02:49:31Z

admin_guide/scheduling/scheduler.adoc

               ]
            }
         }
      }
   ],
 ----
 <1> Specify a name for the predicate.
-<2> Specify a weight from 1 (bad fit) to 10 (best fit).
-<3> Specify a label for matching.
+<2> Specify a label for matching.
 For example:


For example: should be started with newline

ghost · 2018-08-01T02:50:36Z

Others LGTM

mburke5678 · 2018-08-01T14:35:47Z

@avesh
Can you take a quick look at the changes I made from @wjiangjay comments above?
Line 659 presence: true/false <3> Presence should have be quoted
SB
presence: "true/false"

Line 667 Two labels here?
Should be:

"labelsPreference":{
     "labels":[
           "<label>"
  ]

mburke5678 · 2018-08-01T14:39:04Z

@openshift/team-documentation PTAL

kalexand-rh

Couple of picks, suggestions, and questions.

kalexand-rh · 2018-08-01T14:46:20Z

admin_guide/scheduling/scheduler.adoc

               ]
            }
         }
      }
   ],
 ----
 <1> Specify a name for the predicate.
-<2> Specify a weight from 1 (bad fit) to 10 (best fit).
-<3> Specify a label for matching.
+<2> Specify a label for matching.


s/for matching/to match ?

kalexand-rh · 2018-08-01T14:47:48Z

admin_guide/scheduling/scheduler.adoc

-defined or not, regardless of its value. Matching by label can be useful, for
-example, where nodes have their physical location or status defined by labels.
+**_LabelsPresence_** checks whether a particular node has a specific label. The labels create node _groups_ that the
+`LabelPreference` priority uses. Matching by label can be useful, for example, where nodes have their physical location or status defined by labels.


s/`LabelPreference`/`labelPreference` ?

This one is correct (I think). Passed dev and QE reviews.

kalexand-rh · 2018-08-01T14:47:49Z

admin_guide/scheduling/scheduler.adoc

-**_LabelsPresence_** checks whether a particular node has a certain label
-defined or not, regardless of its value. Matching by label can be useful, for
-example, where nodes have their physical location or status defined by labels.
+**_LabelsPresence_** checks whether a particular node has a specific label. The labels create node _groups_ that the


s/**_LabelsPresence_**\The `labelsPresence` parameter

kalexand-rh · 2018-08-01T14:48:30Z

admin_guide/scheduling/scheduler.adoc

-<2> Specify a weight from 1 (bad fit) to 10 (best fit).
-<3> Specify a label for matching.
-<4> Specify whether the labels are required.
+<2> Specify a label for matching.


s/for matching/to match

kalexand-rh · 2018-08-01T14:50:48Z

admin_guide/scheduling/scheduler.adoc

-                  "<label>" <3>
-                presence: true/false <4>
+                  "<label>" <2>
+                presence: "true/false" <3>


I'd s/"true/false"/"true" and specify the valid parameters in annotation 3.

kalexand-rh · 2018-08-01T14:51:50Z

admin_guide/scheduling/scheduler.adoc

 ----
+<1> Specify a name for the priority.
+<2> Specify a weight. Weight can be any non-zero positive value.
+<3> Specify a label for matching.


s/for matching/to match

kalexand-rh · 2018-08-01T14:52:39Z

admin_guide/scheduling/scheduler.adoc

-regardless of its value.
+[NOTE]
+====
+There are situations when using `ServiceAntiAffinity` based on custom labels does not spread pod as expected. 


s/There are situations when/In some situations,

kalexand-rh · 2018-08-01T14:53:03Z

admin_guide/scheduling/scheduler.adoc

+See link:https://access.redhat.com/solutions/3432401[this Red Hat Solution]. 
+====
+
+**_LabelPreference_** gives priority based on the specified label. 


s/**_LabelPreference_**\The `labelPreference` parameter

kalexand-rh · 2018-08-01T14:53:48Z

admin_guide/scheduling/scheduler.adoc

 ----
+<1> Specify a name for the priority.
+<2> Specify a weight. Weight can be any non-zero positive value.


s/Weight can be/Enter

kalexand-rh · 2018-08-01T14:53:58Z

admin_guide/scheduling/scheduler.adoc

               ]
            }
         }
      }
   ],
 ----
 <1> Specify a name for the priority.
-<2> Specify a weight from 1 (bad fit) to 10 (best fit).
+<2> Specify a weight. Weight can be any non-zero positive value.


s/Weight can be/Enter

aveshagarwal · 2018-08-01T15:19:56Z

@mburke5678 lgtm.

aveshagarwal · 2018-08-01T15:32:18Z

admin_guide/scheduling/scheduler.adoc

-**_LabelsPresence_** checks whether a particular node has a certain label
-defined or not, regardless of its value. Matching by label can be useful, for
-example, where nodes have their physical location or status defined by labels.
+The `abelsPresence` parameter checks whether a particular node has a specific label. The labels create node _groups_ that the


typo: abelsPresence (L) is missing

mburke5678 · 2018-08-01T15:45:57Z

/cherrypick enterprise-3.6

openshift-cherrypick-robot · 2018-08-01T15:46:08Z

@mburke5678: new pull request created: #11277

In response to this:

/cherrypick enterprise-3.6

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.

mburke5678 · 2018-08-01T15:49:28Z

/cherrypick enterprise-3.7

openshift-cherrypick-robot · 2018-08-01T15:55:43Z

@mburke5678: new pull request created: #11278

In response to this:

/cherrypick enterprise-3.7

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.

mburke5678 · 2018-08-01T15:56:18Z

/cherrypick enterprise-3.9

mburke5678 · 2018-08-01T15:56:25Z

/cherrypick enterprise-3.10

openshift-cherrypick-robot · 2018-08-01T15:56:26Z

@mburke5678: new pull request created: #11279

In response to this:

/cherrypick enterprise-3.9

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.

openshift-cherrypick-robot · 2018-08-01T15:56:34Z

@mburke5678: new pull request created: #11280

In response to this:

/cherrypick enterprise-3.10

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.

mburke5678 · 2018-08-01T15:57:19Z

/cherrypick enterprise-3.11

openshift-cherrypick-robot · 2018-08-01T15:57:27Z

@mburke5678: new pull request created: #11281

In response to this:

/cherrypick enterprise-3.11

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.

ghost · 2018-08-02T03:29:18Z

admin_guide/scheduling/scheduler.adoc

-                  "<label>" <3>
-                presence: true/false <4>
+                  "<label>" <2>
+                presence: "true" <3>


here, the structure should be:

"predicates":[ { "name":"<name>", (1) "argument":{ "labelsPresence":{ "labels":[ "<label>" (2) ], "presence": true (3) } } } ],

Made this change in a follow-up PR: #11335

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 10, 2018

aveshagarwal reviewed Jul 24, 2018

View reviewed changes

admin_guide/scheduling/scheduler.adoc

@@ -657,22 +652,20 @@ regardless of its value.

"predicates":[

Copy link

aveshagarwal Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be priorities.

aveshagarwal reviewed Jul 24, 2018

View reviewed changes

aveshagarwal mentioned this pull request Jul 25, 2018

Scheduler configurable precidates section is not accurate #9637

Closed

mburke5678 mentioned this pull request Jul 25, 2018

Scheduler configurable precidates section is not accurate #9598

Closed

jtudelag approved these changes Jul 26, 2018

View reviewed changes

ghost reviewed Aug 1, 2018

View reviewed changes

admin_guide/scheduling/scheduler.adoc Outdated

"labelsPreference":{

"label":[

"label":[ <3>

"rack"

Copy link

ghost Aug 1, 2018 •

edited by ghost

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two label here?

ghost reviewed Aug 1, 2018

View reviewed changes

mburke5678 added branch/enterprise-3.6 branch/enterprise-3.7 branch/enterprise-3.9 branch/enterprise-3.10 branch/enterprise-3.11 labels Aug 1, 2018

mburke5678 added this to the Next Release milestone Aug 1, 2018

kalexand-rh reviewed Aug 1, 2018

View reviewed changes

kalexand-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Aug 1, 2018

mburke5678 force-pushed the BZ-1596472 branch from cab0fdb to 56a65db Compare August 1, 2018 15:28

aveshagarwal reviewed Aug 1, 2018

View reviewed changes

Update Scheduler Docs

441d4fd

mburke5678 force-pushed the BZ-1596472 branch from 56a65db to 441d4fd Compare August 1, 2018 15:35

mburke5678 merged commit f494843 into openshift:master Aug 1, 2018

mburke5678 deleted the BZ-1596472 branch August 1, 2018 15:45

openshift-cherrypick-robot mentioned this pull request Aug 1, 2018

[enterprise-3.6] Update Scheduler Docs #11277

Merged

openshift-cherrypick-robot mentioned this pull request Aug 1, 2018

[enterprise-3.7] Update Scheduler Docs #11278

Merged

openshift-cherrypick-robot mentioned this pull request Aug 1, 2018

[enterprise-3.9] Update Scheduler Docs #11279

Merged

openshift-cherrypick-robot mentioned this pull request Aug 1, 2018

[enterprise-3.10] Update Scheduler Docs #11280

Merged

openshift-cherrypick-robot mentioned this pull request Aug 1, 2018

[enterprise-3.11] Update Scheduler Docs #11281

Merged

ghost reviewed Aug 2, 2018

View reviewed changes

mburke5678 mentioned this pull request Aug 2, 2018

Update Scheduler Docs Follow-up #11335

Merged

		@@ -657,22 +652,20 @@ regardless of its value.
		"predicates":[

Update Scheduler Docs #10742

Update Scheduler Docs #10742

Conversation

mburke5678 commented Jul 10, 2018 • edited

mburke5678 commented Jul 10, 2018 • edited

mburke5678 commented Jul 16, 2018

mburke5678 commented Jul 19, 2018

aveshagarwal commented Jul 24, 2018

aveshagarwal commented Jul 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mburke5678 Jul 24, 2018 • edited

Choose a reason for hiding this comment

jtudelag Jul 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-docs-preview-bot commented Jul 24, 2018

mburke5678 commented Jul 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Aug 1, 2018 • edited by ghost

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Aug 1, 2018

mburke5678 commented Aug 1, 2018

mburke5678 commented Aug 1, 2018

kalexand-rh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aveshagarwal commented Aug 1, 2018

Choose a reason for hiding this comment

mburke5678 commented Aug 1, 2018

openshift-cherrypick-robot commented Aug 1, 2018

mburke5678 commented Aug 1, 2018

openshift-cherrypick-robot commented Aug 1, 2018

mburke5678 commented Aug 1, 2018

mburke5678 commented Aug 1, 2018

openshift-cherrypick-robot commented Aug 1, 2018

openshift-cherrypick-robot commented Aug 1, 2018

mburke5678 commented Aug 1, 2018

openshift-cherrypick-robot commented Aug 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mburke5678 commented Jul 10, 2018 •

edited

mburke5678 commented Jul 10, 2018 •

edited

mburke5678 Jul 24, 2018 •

edited

jtudelag Jul 24, 2018 •

edited

ghost Aug 1, 2018 •

edited by ghost