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.

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

Add kubernetes composable inputs provider #21480

Merged
merged 6 commits into from
Oct 6, 2020

Conversation

exekias
Copy link
Contributor

@exekias exekias commented Oct 2, 2020

What does this PR do?

Add kubernetes dynamic provider for Pods, this will allow to launch input configurations for discovered Pods in a Kubernetes cluster. Example config:

providers.kubernetes:
  kube_config: /Users/exekias/.kube/config
  node: minikube

inputs:
  - type: logfile
    streams:
      - paths: /var/log/containers/*${kubernetes.container.id}.log

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

Closes #19271

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 2, 2020
@exekias exekias added the Team:Platforms Label for the Integrations - Platforms team label Oct 2, 2020
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 2, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 2, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #21480 updated]

  • Start Time: 2020-10-06T14:25:07.403+0000

  • Duration: 37 min 22 sec

Test stats 🧪

Test Results
Failed 0
Passed 1386
Skipped 4
Total 1390


p.logger.Debugf("Initializing a new Kubernetes watcher using node: %v", p.config.Node)

watcher, err := kubernetes.NewWatcher(client, &kubernetes.Pod{}, kubernetes.WatchOptions{
Copy link
Member

Choose a reason for hiding this comment

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

Do we have in mind how/if we will handle different resources other than Pods? Like Services and Nodes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, I'm only adding Pods in this PR as a first step. I will open an issue to discuss more resources

@exekias exekias marked this pull request as ready for review October 5, 2020 21:02
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

client, err := kubernetes.GetKubernetesClient(p.config.KubeConfig)
if err != nil {
// info only; return nil (do nothing)
p.logger.Infof("Kubernetes provider skipped, unable to connect: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Being that this will occur on most hosts not in Kubernetes, should this be Debugf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, I added a message for the cases where this succeeds

composable.Providers.AddDynamicProvider("kubernetes", DynamicProviderBuilder)
}

type kubernertesContainerData struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

This struct seems to be unused.

// Emit all containers in the pod
p.emitContainers(pod, pod.Spec.Containers, pod.Status.ContainerStatuses)

// TODO deal with init containers stopping after initialization
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the TODO mean here? Does this mean that AddOrUpdate will be called for InitContainers but the Remove will never be called? That seems like it will leak then in the provider, if that is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, right now Remove will be called when the Pod stops, so inputs won't leak.

The thing is that these containers may stop before the Pod itself, we should be able to detect this and stop them earlier.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest-management (Team:Ingest Management)

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

This is looking good, I like how providers can be implemented in the agent.

Is there a place to add the documentation?

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Looks good. Seems like a good start.

// Emit all containers in the pod
p.emitContainers(pod, pod.Spec.Containers, pod.Status.ContainerStatuses)

// TODO deal with init containers stopping after initialization
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay.

@ph
Copy link
Contributor

ph commented Oct 6, 2020

@exekias looks good what are the remaining steps/next steps for this?

@exekias
Copy link
Contributor Author

exekias commented Oct 6, 2020

Thanks for the reviews! @ph my plan was to get this in for 7.10 and open some follow up issues to discuss next steps. Does that sound good to you?

@ph
Copy link
Contributor

ph commented Oct 6, 2020

I am ok to merge it as is and do followup after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Team:Platforms Label for the Integrations - Platforms team v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor autodiscover to make it reusable in the Agent
6 participants