Update K8s docs to include OpenShift config info. #8300

dedemorton · 2018-09-13T00:01:12Z

Closes #8253

Changes:

Added OpenShift config info.
Edited to make more concise and remove passive voice, comma splices, run-on sentences, etc.

Please respond to the comments that start with //REVIEWERS:

jsoriano

Thanks for adding OpenShift instructions, I think this will be useful for many people.

jsoriano · 2018-09-14T08:51:08Z

metricbeat/docs/running-on-kubernetes.asciidoc

+the manifest file and enable the container to run as privileged.
+
+// REVIEWERS: I wanted to make it clearer as to where this info needs to go. Is
+// this correct? 


Looks correct to me, maybe we could put here the whole kubernetes.yml, so it is more clear how it is expected to be at the end.
Also I think it'd be useful to add it too, commented out, to the deployment files.

jsoriano · 2018-09-14T08:58:42Z

metricbeat/docs/running-on-kubernetes.asciidoc

+If `kube-state-metrics` is not already running, deploy it now.
+
+// REVIEWERS: Should we show the command for deploying kube-state-metrics? Looks
+// like it's kubectl apply -f kubernetes. Is that correct?


Yes, it is just this command, but they need to download the files. We might link to their deployment documentation (https://github.com/kubernetes/kube-state-metrics#kubernetes-deployment)

mikeh-elastic · 2018-09-14T17:31:09Z

I've verified an additional step in #8253 for daemonset scheduling with the defaultNodeSelector.

Tested on latest 3.10 okd release for both sets of filebeat and metricbeat changes by downloading the manifest and making only those changes.

dedemorton · 2018-09-15T03:01:10Z

deploy/kubernetes/metricbeat-kubernetes.yaml

@@ -77,6 +77,11 @@ data:
      period: 10s
      host: ${NODE_NAME}
      hosts: ["localhost:10255"]
+      # If using Red Hat OpenShift uncomment this:
+      #hosts: ["https://${HOSTNAME}:10250"]


@jsoriano Do user also need to remove line 79, or will line 81 overwrite the previous setting if users forget to remove 79?

I think that actually this is overwritten, but not sure if this is always the case. It'd be better to mention that previous hosts entry should be removed.

dedemorton · 2018-09-17T23:57:22Z

@mikeh-elastic @jsoriano @exekias Is this ready to go? I need at least one person to give final approval.

exekias

Leaving some comments related to the build. This is looking good, we will need to test it before merging

exekias · 2018-09-19T08:43:47Z

deploy/kubernetes/filebeat-kubernetes.yaml

@@ -89,6 +89,8 @@ spec:
          value:
        securityContext:
          runAsUser: 0
+          # If using Red Hat OpenShift uncomment this:
+          #privileged: true


this file is autogenerated, that's why tests are failing. You will need to update https://github.com/elastic/beats/blob/master/deploy/kubernetes/filebeat/filebeat-daemonset.yaml and run make

@exekias Can we add comments to the generated file so that folks (like me) will know this in the future?

I'm divided here, as we offer this file to download, users normally download it and review it before applying. Would it be ok if they see this comment? It could be misleading

@exekias That's a good point, and I agree. It would be confusing to users, so let's not add the comment. That's why we have the tests, right.

exekias · 2018-09-19T08:44:32Z

deploy/kubernetes/metricbeat-kubernetes.yaml

+      #hosts: ["https://${HOSTNAME}:10250"]
+      #bearer_token_file: /var/run/secrets/kubernetes.io/serviceaccount/token
+      #ssl.certificate_authorities:
+        #- /var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt      


same thing here: https://github.com/elastic/beats/blob/master/deploy/kubernetes/metricbeat/metricbeat-daemonset-configmap.yaml

dedemorton · 2018-09-19T23:04:46Z

@exekias Updated the files. Mike did some testing, but it wouldn't hurt for someone to test the updated manifest files. Does someone in dev have the right environment set up to do that?

exekias · 2018-10-02T15:09:03Z

I just tested this and it works flawlessly for Filebeat, there is a permissions issue in Metricbeat, we need to add this to https://github.com/elastic/beats/blob/master/deploy/kubernetes/metricbeat/metricbeat-role.yaml:

- apiGroups:
  - ""
  resources:
  - nodes/stats
  verbs:
  - get

and then run make script to update metricbeat-kubernetes.yml

exekias

Thank you so mach for your effort @mikeh-elastic & @dedemorton! adding backport label targeting 6.5

* Update K8s docs to include OpenShift config info. * Add changes from the review * Add another fix from review * Update correct yaml files and run make update * Update permissions

* Update K8s docs to include OpenShift config info. (#8300) * Update K8s docs to include OpenShift config info. * Add changes from the review * Add another fix from review * Update correct yaml files and run make update * Update permissions * Remove lsbeat (#8456) * Removes extraneous heading from the PostgreSQL module docs (#8530) * fixing typo in title * Remove extraneous heading from the PostgreSQL module docs * Fix docker command for loading dashboards (#8531)

Update K8s docs to include OpenShift config info.

a23fa86

dedemorton added docs review needs_backport PR is waiting to be backported to other branches. labels Sep 13, 2018

dedemorton requested review from exekias, mikeh-elastic and jsoriano September 13, 2018 00:01

jsoriano reviewed Sep 14, 2018

View reviewed changes

Add changes from the review

26a9f93

dedemorton force-pushed the issue#8253 branch from d061daa to 26a9f93 Compare September 15, 2018 02:59

dedemorton commented Sep 15, 2018

View reviewed changes

Add another fix from review

52b621a

dedemorton force-pushed the issue#8253 branch from 875572d to 52b621a Compare September 17, 2018 23:58

jsoriano approved these changes Sep 18, 2018

View reviewed changes

exekias suggested changes Sep 19, 2018

View reviewed changes

Update correct yaml files and run make update

f904075

Update permissions

c8aedf0

exekias approved these changes Oct 2, 2018

View reviewed changes

exekias added the v6.5.0 label Oct 3, 2018

dedemorton merged commit 3487b6c into elastic:master Oct 5, 2018

dedemorton mentioned this pull request Oct 19, 2018

Backport to 6.x: Multiple doc PRs (#8300 #8456 #8530 #8531) #8651

Merged

dedemorton removed the needs_backport PR is waiting to be backported to other branches. label Oct 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update K8s docs to include OpenShift config info. #8300

Update K8s docs to include OpenShift config info. #8300

dedemorton commented Sep 13, 2018

jsoriano left a comment

jsoriano Sep 14, 2018

jsoriano Sep 14, 2018

mikeh-elastic commented Sep 14, 2018

dedemorton Sep 15, 2018

jsoriano Sep 17, 2018

dedemorton commented Sep 17, 2018

exekias left a comment

exekias Sep 19, 2018

dedemorton Sep 19, 2018

exekias Sep 20, 2018

dedemorton Sep 20, 2018

exekias Sep 19, 2018

dedemorton commented Sep 19, 2018

exekias commented Oct 2, 2018 •

edited

Loading

exekias left a comment

Update K8s docs to include OpenShift config info. #8300

Update K8s docs to include OpenShift config info. #8300

Conversation

dedemorton commented Sep 13, 2018

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikeh-elastic commented Sep 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dedemorton commented Sep 17, 2018

exekias 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

dedemorton commented Sep 19, 2018

exekias commented Oct 2, 2018 • edited Loading

exekias left a comment

Choose a reason for hiding this comment

exekias commented Oct 2, 2018 •

edited

Loading