Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrate kubernetes module #70

Merged
merged 23 commits into from
Jun 30, 2020
Merged

Migrate kubernetes module #70

merged 23 commits into from
Jun 30, 2020

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Jun 11, 2020 •
edited
Loading

Implementing Kubernetes package migration
Important Notes:

  1. container, system, node, pod, volume metricsets require access to the Kubelet API. Usually these are enabled when Metricbeat is deployed as Daemonset on k8s nodes.
  2. state_* metricsets require the deployment kube-state-metrics on the k8s cluster in order to retrieve metrics from it.
  3. apiserver, scheduler, proxy and controllermanager require access to each component of the k8s cluster accordingly.

Wondering how all these would be possible in combination with agent. Do we need to define how agent will run in k8s (deployment, daemonset) in order to make it work? @exekias do you have anything in mind about this?

Docs: https://www.elastic.co/guide/en/beats/metricbeat/master/metricbeat-module-kubernetes.html

Migration Steps:

  • docker-compose -f snapshot.yml -f local.yml up --force-recreate
  • PACKAGES=kubernetes mage ImportBeats
  • icon check
  • Review titles and descriptions in manifest files
  • add docs/README.md
  • screenshots check
  • check spelling
  • Review fields file and exported fields in docs
  • Metricbeat: add missing configuration options
  • Define all variable properties
  • use stream.dataset field instead of event.dataset

Screenshots

Screenshot 2020-06-26 at 13 01 04
Screenshot 2020-06-26 at 12 59 05
Screenshot 2020-06-26 at 12 57 41
Screenshot 2020-06-26 at 12 58 45

Manual Testing

  1. Using the testing env introduced at Add k8s manifests to deploy stack along with Agent #89 we spin up the Stack and Agent on a k8s cluster.
  2. For the 2 different configs (see Add k8s manifests to deploy stack along with Agent #89) enable the Package once for node scope Agent and once for cluster scope.
  3. Verify that all datasets are shipping metrics and Dashboards are being populated.

@ChrsMark
Add automated files
d5e653d 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@elasticmachine
Copy link

elasticmachine commented Jun 11, 2020 •
edited
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Branch indexing]

  • Start Time: 2020-06-29T17:25:26.637+0000

  • Duration: 4 min 49 sec

@ChrsMark
Add README placeholder
a7c4120 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@mtojek mtojek marked this pull request as draft June 12, 2020 06:51
@ChrsMark
release beta
a61eea0 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add k8s manifests to deploy stack along with Agent
5c19e4c 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark mentioned this pull request Jun 18, 2020
Merged
@ChrsMark
Fix manifests
ca87d38 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Unify hosts & add overview screenshot
e091568 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix icon name
418132a 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add ReadMe.md
be5c10c 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add more content in DEADME.md
de8f415 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
final README.md
bfd179a 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

@exekias @mtojek this one seems to be completed in terms of "package migration". Would love your feedback here.

Manual testing is also possible already (using https://github.com/elastic/integrations-dev/issues/58) in order to verify that data are being collected and Dashboards work ok.

We still need to verify what will happen with Agent's side regarding docs and manifests around the Deployment strategies on k8s.

@mtojek
Copy link
Contributor

mtojek commented Jun 24, 2020

I think you can convert it to ordinary pull-request. Thanks for the contribution.

@ChrsMark ChrsMark marked this pull request as ready for review June 25, 2020 07:22
@mtojek mtojek changed the title [WIP] Migrate kubernetes module Migrate kubernetes module Jun 25, 2020
exekias
exekias reviewed Jun 25, 2020
View reviewed changes
Copy link

@exekias exekias left a comment

Choose a reason for hiding this comment

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

Didn't test it, but it looks good. I think we need to improve docs, left a comment on that regard

packages/kubernetes/dataset/apiserver/manifest.yml Outdated
@@ -0,0 +1,38 @@
title: Kubernetes apiserver metrics
Copy link

Choose a reason for hiding this comment

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

Suggested change
title: Kubernetes apiserver metrics
title: Kubernetes API Server metrics

packages/kubernetes/dataset/apiserver/manifest.yml Outdated
Comment on lines 37 to 38
title: Kubernetes apiserver metrics
description: Collect Kubernetes apiserver metrics
Copy link

Choose a reason for hiding this comment

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

Suggested change
title: Kubernetes apiserver metrics
description: Collect Kubernetes apiserver metrics
title: Kubernetes API Server metrics
description: Collect Kubernetes API Server metrics

dev/import-beats-resources/kubernetes/docs/README.md

This integration is used to collect metrics from
[Kubernetes clusters](https://kubernetes.io/).

Copy link

Choose a reason for hiding this comment

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

I'm missing many of the explanations that we have here: https://www.elastic.co/guide/en/beats/metricbeat/current/metricbeat-module-kubernetes.html

It would be nice to provide good understanding on the variety of datasets that we have and how they need to be configured

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch! Added!

@ChrsMark
Improve docs
eb74646 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add more docs
7e95f8e 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Update data.jon in READMEs
6491456 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

Updated the Docs and changed the sample json events with new ones according to the new indexing approach.

mtojek
mtojek reviewed Jun 26, 2020
View reviewed changes
packages/kubernetes/dataset/apiserver/fields/package-fields.yml Outdated
@@ -0,0 +1,2 @@
- name: kubernetes
type: group
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that according to latest changes in import-beat, pushed by @kaiyan-sheng , this empty file is not required.

packages/kubernetes/dataset/container/fields/package-fields.yml Outdated
@@ -0,0 +1,2 @@
- name: kubernetes
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

packages/kubernetes/dataset/event/agent/stream/stream.yml.hbs
@@ -0,0 +1 @@
metricsets: ["event"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need any configuration options? There is even no "period" defined hence wondering.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added the configuration manually in the manifest.yml and missed that. 👍

packages/kubernetes/dataset/node/manifest.yml Outdated
@@ -0,0 +1,44 @@
title: Kubernetes Node metrics
release: experimental
Copy link
Contributor

Choose a reason for hiding this comment

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

Please adjusts all release fields. We will release all new packages as "beta".

packages/kubernetes/dataset/state_resourcequota/manifest.yml Outdated
show_user: true
default:
- kube-state-metrics:8080
title: Kubernetes state_resourcequota metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can update the title to a pleasant, human-readable form (no underscore).

packages/kubernetes/manifest.yml Outdated
format_version: 1.0.0
name: kubernetes
title: Kubernetes
version: 0.0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

version: 0.1.0

packages/kubernetes/manifest.yml Outdated
requirement:
kibana:
versions: '>=7.3.0 <8.0.0'
elasticsearch: {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This field (elasticsearch) can be removed.

packages/kubernetes/manifest.yml Outdated
categories:
- metrics
release: beta
removable: true
Copy link
Contributor

Choose a reason for hiding this comment

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

This field too.

packages/kubernetes/manifest.yml Outdated
description: Collect metrics from Kubernetes
inputs:
- type: kubernetes/metrics
title: Collect Kubernetes API Server, Container, Controller Manager, Event, Node, Pod, Proxy, Scheduler, state_container, state_cronjob, state_deployment, state_node, state_persistentvolume, state_persistentvolumeclaim, state_pod, state_replicaset, state_resourcequota, state_service, state_statefulset, state_storageclass, System and Volume metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you can shorten this one :D

simply - Collect Kubernetes metrics?

packages/kubernetes/manifest.yml Outdated
inputs:
- type: kubernetes/metrics
title: Collect Kubernetes API Server, Container, Controller Manager, Event, Node, Pod, Proxy, Scheduler, state_container, state_cronjob, state_deployment, state_node, state_persistentvolume, state_persistentvolumeclaim, state_pod, state_replicaset, state_resourcequota, state_service, state_statefulset, state_storageclass, System and Volume metrics
description: Collecting API Server, Container, Controller Manager, Event, Node, Pod, Proxy, Scheduler, state_container, state_cronjob, state_deployment, state_node, state_persistentvolume, state_persistentvolumeclaim, state_pod, state_replicaset, state_resourcequota, state_service, state_statefulset, state_storageclass, System and Volume metrics from Kubernetes
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that all captions, descriptions, etc. should present names which are human readable (no underscores).

@mtojek
Copy link
Contributor

mtojek commented Jun 26, 2020 •
edited
Loading

Just noticed on screenshots, please improve case sensitivity on screenshots.

@ChrsMark
Fix release fields
ed071ab 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix version
2b2a72e 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
More review changes
b945c96 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix screenshots
83b9d46 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

Thanks for the review @mtojek! Comments should be addressed now.

@ChrsMark ChrsMark requested a review from mtojek June 26, 2020 09:13
mtojek
mtojek approved these changes Jun 26, 2020
View reviewed changes
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Good job, Christos!

@ChrsMark
Remove leftover k8s manifests
217990a 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Minor descritpion fix
71a97d8 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

ChrsMark commented Jun 26, 2020 •
edited
Loading

@exekias @andresrc this integration is completed. Tested manually using testing/environments/kubernetes. Let me know if we can merge/ship it already or we want to wait for a Running Agent on k8s part on Agent's side.

@mtojek
Copy link
Contributor

mtojek commented Jun 30, 2020

@ChrsMark

I think you have to merge this PR, otherwise you'll have to rebase against the master branch, as there are some breaking changes coming: #132

@exekias
Copy link

exekias commented Jun 30, 2020

++ Let's get this in. I think it would be nice to follow up with docs on the Agent side on how to deploy in Kubernetes

Merging this on behalf of @ChrsMark, as he is off

@exekias exekias merged commit eca38ee into elastic:master Jun 30, 2020
@ChrsMark ChrsMark mentioned this pull request Jul 8, 2020
Merged
This was referenced Jul 28, 2020
Merged
Merged
@tetianakravchenko tetianakravchenko mentioned this pull request Nov 15, 2021
Merged
4 tasks
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
@ChrsMark
Migrate kubernetes module (elastic#70)
0213ec7 
* Add automated files

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Add README placeholder

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* release beta

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Add k8s manifests to deploy stack along with Agent

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Fix manifests

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Unify hosts & add overview screenshot

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Fix icon name

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Add ReadMe.md

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Add more content in DEADME.md

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* final README.md

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Improve docs

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Add more docs

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Update data.jon in READMEs

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Remove empty package-fields files

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* fix

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Fix release fields

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Fix version

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* More review changes

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Fix screenshots

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Remove leftover k8s manifests

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

* Minor descritpion fix

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtojek mtojek mtojek approved these changes

@exekias exekias exekias approved these changes

Assignees
No one assigned
Labels
Integration:kubernetes Kubernetes New Integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ChrsMark @elasticmachine @mtojek @exekias @andrewkroh