Warn user to disable default node selector when using daemonsets #8125

tnozicka · 2018-03-14T17:57:05Z

Stems from https://bugzilla.redhat.com/show_bug.cgi?id=1501514#c9

Permissions were disabled here: openshift/origin#18971

This is a part of solving 3.9 blocker, so we need to backport it in time.

/cc @mfojtik

tnozicka · 2018-03-14T17:58:08Z

/cherrypick enterprise-3.9

openshift-cherrypick-robot · 2018-03-14T17:58:09Z

@tnozicka: once the present PR merges, I will cherry-pick it on top of enterprise-3.9 in a new PR and assign it to you.

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.

tnozicka · 2018-03-14T18:44:06Z

/cc @openshift/team-documentation

tnozicka · 2018-03-15T10:13:01Z

This is 3.9 blocker, needs to go asap.

/cc @vikram-redhat @adellape @bmcelvee @ahardin-rh @gaurav-nelson

vikram-redhat · 2018-03-15T10:26:25Z

@tnozicka: https://github.com/openshift/openshift-docs/blob/master/contributing_to_docs/create_or_edit_content.adoc#submit-pr-to-merge-your-work.

Does this apply to dedicated and online as well?

vikram-redhat · 2018-03-15T10:27:05Z

@tnozicka please squash commits as well: https://github.com/openshift/openshift-docs/blob/master/contributing_to_docs/create_or_edit_content.adoc#add-changes-to-an-existing-pr-if-required

vikram-redhat · 2018-03-15T10:28:59Z

dev_guide/daemonsets.adoc

+    '{"metadata": {"annotations": {"openshift.io/node-selector": ""}}}'
+----
+
+xref:../admin_guide/manage_projects.adoc#using-node-selectors[Learn more about node selectors.]


managing_projects.adoc

good catch; one would say that CI should scream though

vikram-redhat · 2018-03-15T10:30:02Z

@tnozicka I have one comment. Assigning it to @mburke5678 to have a proper look when he is in for a follow up.

vikram-redhat · 2018-03-15T10:54:59Z

@mburke5678 note that @tnozicka has already started the CP process by using the robot. If it fails, you will need to resolve and fix on command line. If it succeeds, remember to merge the CP.

tnozicka · 2018-03-15T13:12:12Z

@vikram-redhat thanks for the quick reply!

Does this apply to dedicated and online as well?

Well, it kinda does apply to online but only for it's admins since users can't create daemonsets.

@tnozicka please squash commits as

I don't think squashing unrelated commits is a good idea as you will make cherry-picking or reverting harder, but I can do that if you want me to.

mburke5678 · 2018-03-15T13:48:51Z

dev_guide/daemonsets.adoc

@@ -19,8 +19,36 @@ A daemonset can be used to run replicas of a pod on specific or all nodes in an
 Use daemonsets to create shared storage, run a logging pod on every node in
 your cluster, or deploy a monitoring agent on every node.

+For security reasons and by their nature creating DaemonSets is restricted to cluster administrators.
+


@tnozicka Do we need the and by their nature?
For security reasons, only cluster administrators can create daemonsets.

I'll rephrase

mburke5678 · 2018-03-15T14:18:44Z

dev_guide/daemonsets.adoc

+[IMPORTANT]
+====
+Before you start using daemonset you need to make sure to disable project default node selector in your namespace.
+


Before you start using daemonsets ensure that you disable the default project-wide xref:../admin_guide/managing_projects.adoc#using-node-selectors[node selector] in your namespace.

s/daemonset/DaemonSets

I'd prefer it to be this way but the rest of the doc uses all lowercase.

@tnozicka I agree. The term is used both ways throughout the full docs set. But, we should be consistent in this topic at least.

maybe we can get rid of you need to make sure so the sentence becomes Before you start using daemonsets, disable the default project-wide... as per @mburke5678 's comment.

mburke5678 · 2018-03-15T14:19:09Z

dev_guide/daemonsets.adoc

+----
+
+xref:../admin_guide/managing_projects.adoc#using-node-selectors[Learn more about node selectors.]
+


If you use the node selector link above, don't need it here.

If you are pointing out those commands to disable it there is no such patch in the link. There is one for setting the selector, not disabling it.

mburke5678 · 2018-03-15T14:19:41Z

dev_guide/daemonsets.adoc

+
+xref:../admin_guide/managing_projects.adoc#using-node-selectors[Learn more about node selectors.]
+
+Daemonset scheduling is incompatible with project default node selector. Nor you usually want the default node selector


lowercase to honor rest of the doc, plus D in the start of a sentence

mburke5678 · 2018-03-15T14:25:28Z

@tnozicka A couple small comments. Let me know when you are ready and we can merge.

enj · 2018-03-15T16:51:29Z

@tnozicka we probably need to link between this and #8126

tnozicka · 2018-03-15T19:51:03Z

updated as @enj sugested

-For security reasons and by their nature creating DaemonSets is restricted to cluster administrators.
+For security reasons, only cluster administrators can create daemonsets.
+(xref:../admin_guide/manage_rbac.adoc#admin-guide-granting-users-daemonset-permissions[Granting Users Daemonset Permissions.])

gaurav-nelson · 2018-03-15T22:59:35Z

dev_guide/daemonsets.adoc

+====
+Before you start using daemonset you need to make sure to disable project default node selector in your namespace.
+
+If you are creating a new project make sure to overwrite the default node selector using


As with previous case, maybe If you are creating a new project, overwrite the...

gaurav-nelson · 2018-03-15T23:00:15Z

dev_guide/daemonsets.adoc

+`oc adm new-project --node-selector=""`.
+
+To disable the default node selector on existing project set namespace annotation
+`openshift.io/node-selector` to empty string:


s/to empty string/to an empty string

vikram-redhat · 2018-03-15T23:31:00Z

I don't think squashing unrelated commits is a good idea as you will make cherry-picking or reverting harder, but I can do that if you want me to.

Yes please.

vikram-redhat · 2018-03-15T23:31:24Z

@geliu2016 would you be the right person to review this from the QE side?

gaurav-nelson · 2018-03-16T00:36:21Z

dev_guide/daemonsets.adoc

 For more information on daemonsets, see the link:http://kubernetes.io/docs/admin/daemons/[Kubernetes documentation].

+[IMPORTANT]


We can reword it like this:

shows what goes wrong first

and then how to deal with it.

[IMPORTANT] ==== Daemonset scheduling is incompatible with project's default node selector. If you fail to disable it, the daemonset gets restricted by merging with the default node selector. This results in frequent pod recreates on the nodes that got unselected by the merged node selector, which in turn puts unwanted load on the cluster. Therefore, * Before you start using daemonsets, disable the default project-wide xref:../admin_guide/managing_projects.adoc#using-node-selectors[node selector] in your namespace, by setting the namespace annotation `openshift.io/node-selector` to an empty string: + ---- # oc patch namespace myproject -p \ '{"metadata": {"annotations": {"openshift.io/node-selector": ""}}}' ---- * If you are creating a new project, overwrite the default node selector using `oc adm new-project --node-selector=""`. ====

@vikram-redhat LGTM, thx

vikram-redhat · 2018-03-19T03:19:28Z

@mburke5678 - we have QE approval as well. Not sure if it requires peer review. Commits need squashing before merging.

tnozicka · 2018-03-19T14:19:13Z

comments addressed; commits squasched per request

openshift-cherrypick-robot · 2018-03-19T15:31:02Z

@tnozicka: new pull request created: #8197

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.

mburke5678 · 2018-04-10T21:24:04Z

@tnozicka @geliu2016
Should this go into 3.7 also? Or any other OpenShift versions?
Please see #8674

enj · 2018-05-04T12:51:55Z

@tnozicka please work with @kalexand-rh to get these docs updated to v3.9.26-1.

https://bugzilla.redhat.com/show_bug.cgi?id=1536304#c19

Per David Eads in https://bugzilla.redhat.com/show_bug.cgi?id=1555363#c5:

> https://github.com/openshift/ose/pull/1205 merged, so the controller doesn't
> create pods it knows will be rejected.  "fixing" the default role to
> disallow the creation of a daemonset was a bug.

The change is merged as of v3.9.26-1

This readds the deamonset permission as it is safe for normal users to have.  Thus this is the expected behavior.

openshift-ci-robot requested a review from mfojtik March 14, 2018 17:57

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

tnozicka requested a review from bfallonf March 14, 2018 17:58

tnozicka requested a review from Miciah March 15, 2018 10:01

tnozicka changed the title ~~Warn user to disable default node selector when using daemonset~~ Warn user to disable default node selector when using daemonsets Mar 15, 2018

openshift-ci-robot requested review from adellape, ahardin-rh, bmcelvee, gaurav-nelson and vikram-redhat March 15, 2018 10:13

vikram-redhat added the branch/enterprise-3.9 label Mar 15, 2018

vikram-redhat added this to the Future Release milestone Mar 15, 2018

vikram-redhat reviewed Mar 15, 2018

View reviewed changes

vikram-redhat assigned mburke5678 Mar 15, 2018

tnozicka added the branch/dedicated label Mar 15, 2018

tnozicka force-pushed the daemonsets-disable-default-node-selector branch from 66d965d to 160befe Compare March 15, 2018 13:14

mburke5678 reviewed Mar 15, 2018

View reviewed changes

tnozicka force-pushed the daemonsets-disable-default-node-selector branch from 160befe to 656965e Compare March 15, 2018 19:50

gaurav-nelson reviewed Mar 15, 2018

View reviewed changes

gaurav-nelson reviewed Mar 16, 2018

View reviewed changes

tnozicka force-pushed the daemonsets-disable-default-node-selector branch from 656965e to 97308db Compare March 19, 2018 14:13

Disable default project node selector when using daemonsets

1a7b10c

tnozicka force-pushed the daemonsets-disable-default-node-selector branch from 97308db to 1a7b10c Compare March 19, 2018 14:16

mburke5678 merged commit 68da66c into openshift:master Mar 19, 2018

openshift-cherrypick-robot mentioned this pull request Mar 19, 2018

[enterprise-3.9] Warn user to disable default node selector when using daemonsets #8197

Merged

tnozicka deleted the daemonsets-disable-default-node-selector branch March 19, 2018 16:37

vikram-redhat modified the milestones: Future Release, OCP 3.9 GA Mar 28, 2018

		----

		xref:../admin_guide/managing_projects.adoc#using-node-selectors[Learn more about node selectors.]


		xref:../admin_guide/managing_projects.adoc#using-node-selectors[Learn more about node selectors.]

		Daemonset scheduling is incompatible with project default node selector. Nor you usually want the default node selector

		For more information on daemonsets, see the link:http://kubernetes.io/docs/admin/daemons/[Kubernetes documentation].

		[IMPORTANT]

Warn user to disable default node selector when using daemonsets #8125

Warn user to disable default node selector when using daemonsets #8125

Conversation

tnozicka commented Mar 14, 2018

tnozicka commented Mar 14, 2018

openshift-cherrypick-robot commented Mar 14, 2018

tnozicka commented Mar 14, 2018

tnozicka commented Mar 15, 2018

vikram-redhat commented Mar 15, 2018

vikram-redhat commented Mar 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vikram-redhat commented Mar 15, 2018

vikram-redhat commented Mar 15, 2018

tnozicka commented Mar 15, 2018

mburke5678 Mar 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mburke5678 Mar 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaurav-nelson Mar 15, 2018 • edited Loading

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

mburke5678 commented Mar 15, 2018

enj commented Mar 15, 2018

tnozicka commented Mar 15, 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

vikram-redhat commented Mar 15, 2018

vikram-redhat commented Mar 15, 2018

gaurav-nelson Mar 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vikram-redhat commented Mar 19, 2018

tnozicka commented Mar 19, 2018

openshift-cherrypick-robot commented Mar 19, 2018

mburke5678 commented Apr 10, 2018 • edited Loading

enj commented May 4, 2018

mburke5678 Mar 15, 2018 •

edited

Loading

mburke5678 Mar 15, 2018 •

edited

Loading

gaurav-nelson Mar 15, 2018 •

edited

Loading

gaurav-nelson Mar 16, 2018 •

edited

Loading

mburke5678 commented Apr 10, 2018 •

edited

Loading