elastic/integrationsPublic

NotificationsYou must be signed in to change notification settings
Fork545
Star318

Migrate kubernetes module#70

Merged

exekias merged 23 commits intoelastic:masterfrom

ChrsMark:migrate_k8s

Jun 30, 2020

Merged

Migrate kubernetes module#70
exekias merged 23 commits intoelastic:masterfrom
ChrsMark:migrate_k8s

Conversation

Copy link

Member

ChrsMark commentedJun 11, 2020•
edited
Loading

ImplementingKubernetes package migration
Important Notes:

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.
state_* metricsets require the deploymentkube-state-metrics on the k8s cluster in order to retrieve metrics from it.
apiserver,scheduler,proxy andcontrollermanager require access to each component of the k8s cluster accordingly.

Wondering how all these would be possible in combination withagent. 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
adddocs/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

Manual Testing

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

Add automated files

d5e653d

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

Copy link

elasticmachine commentedJun 11, 2020•
edited
Loading

💚 Build Succeeded

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

Add README placeholder

a7c4120

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

mtojek marked this pull request as draft

June 12, 2020 06:51

ChrsMark added2 commits

June 17, 2020 11:24

release beta

a61eea0

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

Add k8s manifests to deploy stack along with Agent

5c19e4c

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

exekias added the Team:Platforms label

Jun 18, 2020

ChrsMark mentioned this pull request

Jun 18, 2020

Add k8s manifests to deploy stack along with Agent#89

Merged

ChrsMark added6 commits

June 24, 2020 12:14

Fix manifests

ca87d38

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

Unify hosts & add overview screenshot

e091568

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

Fix icon name

418132a

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

Add ReadMe.md

be5c10c

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

Add more content in DEADME.md

de8f415

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

final README.md

bfd179a

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

Copy link

MemberAuthor

ChrsMark commentedJun 24, 2020

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

Manual testing is also possible already (usinghttps://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.

Copy link

Contributor

mtojek commentedJun 24, 2020

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

ChrsMark marked this pull request as ready for review

June 25, 2020 07:22

mtojek changed the title~~[WIP] Migrate kubernetes module~~Migrate kubernetes module

Jun 25, 2020

exekias reviewed

Jun 25, 2020

View reviewed changes

Copy link

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

exekiasJun 25, 2020

Choose a reason for hiding this comment

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

Suggested change

	title:Kubernetesapiserver metrics
	title:KubernetesAPI 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

exekiasJun 25, 2020

Choose a reason for hiding this comment

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

Suggested change

	title:Kubernetesapiserver metrics
	description:Collect Kubernetesapiserver metrics
	title:KubernetesAPI Server metrics
	description:Collect KubernetesAPI 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

exekiasJun 25, 2020

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

MemberAuthor

ChrsMarkJun 25, 2020

Choose a reason for hiding this comment

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

Nice catch! Added!

ChrsMark added3 commits

June 25, 2020 14:24

Improve docs

eb74646

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

Add more docs

7e95f8e

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

Update data.jon in READMEs

6491456

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

Copy link

MemberAuthor

ChrsMark commentedJun 25, 2020

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

exekias approved these changes

Jun 25, 2020

View reviewed changes

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

mtojekJun 26, 2020

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 inimport-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

mtojekJun 26, 2020

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

mtojekJun 26, 2020

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

MemberAuthor

ChrsMarkJun 26, 2020

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 themanifest.yml and missed that. 👍

packages/kubernetes/dataset/node/manifest.yml Outdated

		@@ -0,0 +1,44 @@
		title: Kubernetes Node metrics
		release: experimental

Copy link

Contributor

mtojekJun 26, 2020

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

mtojekJun 26, 2020

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

mtojekJun 26, 2020

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

mtojekJun 26, 2020

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

mtojekJun 26, 2020

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

mtojekJun 26, 2020

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

mtojekJun 26, 2020

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).

Copy link

Contributor

mtojek commentedJun 26, 2020•
edited
Loading

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

ChrsMark added4 commits

June 26, 2020 10:42

Merge remote-tracking branch 'upstream/master' into migrate_k8s

be1e57b

Merge remote-tracking branch 'upstream/master' into migrate_k8s

1275bcf

Remove empty package-fields files

ed1d121

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

fix

f23f659

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

ChrsMark added4 commits

June 26, 2020 11:50

Fix release fields

ed071ab

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

Fix version

2b2a72e

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

More review changes

b945c96

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

Fix screenshots

83b9d46

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

Copy link

MemberAuthor

ChrsMark commentedJun 26, 2020

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

ChrsMark requested a review frommtojek

June 26, 2020 09:13

mtojek approved these changes

Jun 26, 2020

View reviewed changes

Copy link

Contributor

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 added2 commits

June 26, 2020 12:41

Remove leftover k8s manifests

217990a

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

Minor descritpion fix

71a97d8

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

Copy link

MemberAuthor

ChrsMark commentedJun 26, 2020•
edited
Loading

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