Create k8s asset collector #38

MichaelKatsoulis · 2023-02-24T15:38:34Z

This PR creates a k8s asset input which collects the Kubernetes Resources from a cluster and publishes them.

Related issue: #1

ruflin · 2023-02-27T07:07:06Z

Are there plans to support asset_types like discussed in #20 ? The reason I bring this up because k8s will have so many different asset types, I wonder if all should be collected on the same period. But could also well be a premature optimisation, so don't hold back on this one.

MichaelKatsoulis · 2023-02-28T10:29:19Z

Are there plans to support asset_types like discussed in #20 ?

Thanks for bringing this to my attention. I will raise this issue in our upcoming catchup with the rest of the team.
Assets_types is a very good idea for a configuration option and the different period for each type is valid.
Nodes for example do no change that often as a pod or container, so collection period can be different.

ChrsMark · 2023-03-08T04:16:18Z

input/assets_k8s/assets_k8s.go

+}
+
+func collectK8sPods(ctx context.Context, log *logp.Logger, client kubernetes.Interface, publisher stateless.Publisher) error {
+	pods, err := client.CoreV1().Pods("").List(context.TODO(), metav1.ListOptions{})


Hey @MichaelKatsoulis! Keep in mind, this kind of querying might not scale so better to use a "Watcher" approach and on ticker's tick just return what is cached. We have hit this issue at elastic/beats#33307 (and elastic/elastic-agent-autodiscover#31).

Maybe in this way you can also re-use the elastic-agent-autodiscovery library for some parts.

This is a first implementation of the k8s asset collector as part of the Topology initiative. Regarding the watcher approach, I am not sure that these queries would need to happen at very short periods as their main purpose is to gather topology data.
But maybe watchers is the better tested solution and we already use it in autodiscovery. But regarding your second comment as well, the inpurunner may be run by agent in the future but it shouldn't be confused with the kubernetes integration. Its goal is different.

this sounds interesting, let's also discuss this in the context of the other asset inputs

Watchers' usage example: https://github.com/ChrsMark/k8sdiscovery

ChrsMark · 2023-03-08T04:20:48Z

input/assets_k8s/assets_k8s.go

+func collectK8sNodes(ctx context.Context, log *logp.Logger, client kubernetes.Interface, publisher stateless.Publisher) error {
+
+	// collect the nodes using the client
+	nodes, err := client.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{})


Sth related to my other comment: have we considered what would be the result of querying the k8s API multiple times from multiple places? I'm not sure how this input runner would be integrated with Elastic Agent, but Elastic Agent is already querying the k8s API from a couple (++) of places so I wonder if this is sth really efficient or if we should consider re-using already existing information.

For example the kubernetes provider knows at any given time what are the Pods or Nodes running. So why to perform the very same process from another different place?

input/assets/internal/publish.go

tommyers-elastic · 2023-03-21T11:33:18Z

hey @MichaelKatsoulis, is this ready for review? let's get it out of draft and merged so we can work on tightening up the k8s<->CSP asset associations. any outstanding issues that can't be quickly resolved, let's just open issues for and address in smaller PRs.

ChrsMark

As a PoC looks good. I left some minor cleanup suggestions.

Moving forward however would require to deal with the 2 issues I already mentioned:

Use Informers/caching instead of direct API calls (Create k8s asset collector #38 (comment))
Figure out if information can be re-used since Elastic Agent is already collecting such information (Create k8s asset collector #38 (comment))

I would suggest filing a follow up issue for these to not forget.

deploy/inputrunner-kubernetes-manifest.yml

input/assets/k8s/assets_k8s.go

ChrsMark · 2023-03-21T23:38:30Z

input/assets/k8s/assets_k8s.go

+	return &assetsK8s{cfg}, nil
+}
+
+type config struct {


I would move structs definitions to the top.

input/assets/k8s/assets_k8s.go

MichaelKatsoulis · 2023-03-22T08:41:15Z

Use Informers/caching instead of direct API calls (#38 (comment))

Yes this will come in follow up PR

Figure out if information can be re-used since Elastic Agent is already collecting such information (#38 (comment))

I am not sure I follow. This input is not run by agent for now. So how can I use information collected by agent?

ChrsMark · 2023-03-22T09:03:15Z

Figure out if information can be re-used since Elastic Agent is already collecting such information (#38 (comment))

I am not sure I follow. This input is not run by agent for now. So how can I use information collected by agent?

Well this is a high level architectural comment mostly.
I'm not fully aware of how these inputs would be running but if those will be running along with Agent then we will have the same data being collected from 2 different places.
If these inputs are completely independent of Agent then we cannot do a lot to re-use the information.

MichaelKatsoulis · 2023-03-22T09:53:58Z

Well this is a high level architectural comment mostly. I'm not fully aware of how these inputs would be running but if those will be running along with Agent then we will have the same data being collected from 2 different places. If these inputs are completely independent of Agent then we cannot do a lot to re-use the information.

I keep your comment as it is a valid concern. When everything is more clear on how things will run together, we need to revisit such things.

ChrsMark

lgtm

MichaelKatsoulis marked this pull request as draft February 24, 2023 15:38

MichaelKatsoulis force-pushed the asset_k8s-input branch from 2a0112b to b70206f Compare March 1, 2023 15:33

ChrsMark reviewed Mar 8, 2023

View reviewed changes

MichaelKatsoulis added 5 commits March 8, 2023 16:30

Add a sample k8s asset input

f205b40

Add pod asset collector

f9714eb

Make use of assetTypes and add a test

8453c0d

Add Dockerfile and inputrunner manifest

173686a

Rebase with updated structure

fb98440

MichaelKatsoulis force-pushed the asset_k8s-input branch from bc46e87 to fb98440 Compare March 8, 2023 14:41

MichaelKatsoulis added 2 commits March 21, 2023 10:56

Merge branch 'main' into asset_k8s-input

d357511

Rebase publish and update tests

e5a6445

dmathieu reviewed Mar 21, 2023

View reviewed changes

input/assets/internal/publish.go Outdated Show resolved Hide resolved

MichaelKatsoulis marked this pull request as ready for review March 21, 2023 11:50

Review updates

7ea73a1

MichaelKatsoulis requested review from dmathieu and ChrsMark March 21, 2023 14:30

ChrsMark reviewed Mar 21, 2023

View reviewed changes

MichaelKatsoulis added 2 commits March 22, 2023 11:51

Remove not needed mounts from inputrunner manifest

d5a930c

Review updates. Update pods parents. Should match the ean of node

b6108dc

Update variable name nodeEan

d1d1531

MichaelKatsoulis requested a review from ChrsMark March 22, 2023 09:58

Fix linter errors

5c04b34

ChrsMark approved these changes Mar 22, 2023

View reviewed changes

MichaelKatsoulis mentioned this pull request Mar 22, 2023

Use watchers for k8s asset collections #101

Closed

MichaelKatsoulis added this pull request to the merge queue Mar 23, 2023

MichaelKatsoulis merged commit 361c461 into main Mar 23, 2023

dmathieu deleted the asset_k8s-input branch March 23, 2023 09:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create k8s asset collector #38

Create k8s asset collector #38

MichaelKatsoulis commented Feb 24, 2023

ruflin commented Feb 27, 2023

MichaelKatsoulis commented Feb 28, 2023

ChrsMark Mar 8, 2023

MichaelKatsoulis Mar 8, 2023 •

edited

Loading

tommyers-elastic Mar 9, 2023

ChrsMark Mar 22, 2023

MichaelKatsoulis Mar 22, 2023

ChrsMark Mar 8, 2023

tommyers-elastic commented Mar 21, 2023

ChrsMark left a comment

ChrsMark Mar 21, 2023

MichaelKatsoulis commented Mar 22, 2023

ChrsMark commented Mar 22, 2023

MichaelKatsoulis commented Mar 22, 2023

ChrsMark left a comment

Create k8s asset collector #38

Create k8s asset collector #38

Conversation

MichaelKatsoulis commented Feb 24, 2023

ruflin commented Feb 27, 2023

MichaelKatsoulis commented Feb 28, 2023

ChrsMark Mar 8, 2023

Choose a reason for hiding this comment

MichaelKatsoulis Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

tommyers-elastic Mar 9, 2023

Choose a reason for hiding this comment

ChrsMark Mar 22, 2023

Choose a reason for hiding this comment

MichaelKatsoulis Mar 22, 2023

Choose a reason for hiding this comment

ChrsMark Mar 8, 2023

Choose a reason for hiding this comment

tommyers-elastic commented Mar 21, 2023

ChrsMark left a comment

Choose a reason for hiding this comment

ChrsMark Mar 21, 2023

Choose a reason for hiding this comment

MichaelKatsoulis commented Mar 22, 2023

ChrsMark commented Mar 22, 2023

MichaelKatsoulis commented Mar 22, 2023

ChrsMark left a comment

Choose a reason for hiding this comment

MichaelKatsoulis Mar 8, 2023 •

edited

Loading