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

Implement kubernetes agent cluster scope leader election #19731

Closed
exekias opened this issue Jul 8, 2020 · 18 comments · Fixed by #20512 or #20601
Closed

Implement kubernetes agent cluster scope leader election #19731

exekias opened this issue Jul 8, 2020 · 18 comments · Fixed by #20512 or #20601
Assignees
Labels
autodiscovery discuss Issue needs further discussion. enhancement ext-goal External goal of an iteration Team:Platforms Label for the Integrations - Platforms team

Comments

@exekias
Copy link
Contributor

exekias commented Jul 8, 2020

Right now, when deploying Metricbeat on Kubernetes we deploy two different resources:

  • A Daemonset, used to deploy an agent per node in the cluster, each agent will monitor the host and all it's running workloads.
  • A Deployment, to deploy an additional agent to monitor at the cluster scope, currently used for kube-state-metrics and Kubernetes events.

We could simplify this by removing the need for a Deployment in favour of some type of leader election within the DaemonSet. In a nutshell, we need a way to flag one of the agents in the Daemonset as leader, making it in charge of both host and cluster scopes.

While leader election is challenging, API Server provides a great locking mechanism that is present in K8S, and we can leverage in many ways. So we don't really need things like agents talking to each other.

We should discuss our options here, one possibility is doing something like this:

  • Use a secret/configmap to claim leadership, agents should try to create it. If it exists and it's not expired (through some defined TTL), there is an active leader.
  • Make the Kubernetes autodiscover provider handle this, and pass a leader field as part of autodiscover events. This way cluster scope metricsets can be configured based on that.
@exekias exekias added enhancement discuss Issue needs further discussion. autodiscovery Team:Platforms Label for the Integrations - Platforms team labels Jul 8, 2020
@elasticmachine
Copy link
Collaborator

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

@jsoriano
Copy link
Member

jsoriano commented Jul 8, 2020

go-client includes a leader election implementation https://godoc.org/k8s.io/client-go/tools/leaderelection

@mostlyjason
Copy link

@ph @ruflin FYI

@ChrsMark
Copy link
Member

+1 for using leader election package @jsoriano proposed.
Something similar Kubernetes core implements for autoscaler: https://github.com/kubernetes/autoscaler/blob/a63c7abbe47773be4eec49b13210a3cd9c4a719f/cluster-autoscaler/main.go#L421

I drafted an experimental code snippet and seemed to work, so the implementation should be quite straight forward on leader election level. However we need to figure out at which point we will integrate this change.

One possible solution could be to add this on Module level. In this, we will give the option to the users to define an extra flag to define that this module configuration should be applied only on leader elected nodes. For instance:

- module: kubernetes
      metricsets:
        - state_node
        - state_deployment
        - state_replicaset
        - state_pod
        - state_container
        - state_cronjob
        - state_resourcequota
        # Uncomment this to get k8s events:
        - event
      period: 10s
      on_leader_election: true
      host: ${NODE_NAME}

@exekias @jsoriano any thoughts on this?

@exekias
Copy link
Contributor Author

exekias commented Jul 23, 2020

Love to see this is viable! Thank you for digging in.

I was thinking this could make more sense in autodiscover? This may go beyond the scope of the kubernetes module, as I may have some other inputs I want to launch as a singleton for the whole cluster. (ie, running certain uptime monitors, a global gateway for logs, etc)

If we add a leader field to autodiscover events, we could leverage it to decide when to launch certain configs, including these.

I'm not fully onboard with this either, would like to hear other opinions and use cases

@jsoriano
Copy link
Member

jsoriano commented Jul 23, 2020

I was also thinking that it could make sense in autodiscover, it could be a new provider with a config like this:

metricbeat.autodiscover:
  providers:
    - type: kubernetes_leader
      lock_id: somename
      configmap: someconfigmap

Or this:

metricbeat.autodiscover:
  providers:
    - type: kubernetes_leader
      lock_id: somename
      config:
      - module: kubernetes
        metricsets: ...

The configuration could be in a configmap, so all the leader candidates would have the same configuration, though maybe this is not so important if they are deployed with a Deployment.

If a provider gets the lock, it generates an event to start the configuration, if it loses it, it stops it.

And this could be used with other modules with cluster scope.

@exekias
Copy link
Contributor Author

exekias commented Jul 23, 2020

Do you foresee any situation where you may want to use Pods/Nodes/Service autodiscover in combination with leader election? If that's the case, perhaps the leader flag should be just present in the kubernetes provider (too)

@jsoriano
Copy link
Member

Do you foresee any situation where you may want to use Pods/Nodes/Service autodiscover in combination with leader election?

Umm, yeah, we might want to enable it only if some pods or services exist, and using their metadata.

If that's the case, perhaps the leader flag should be just present in the kubernetes provider (too)

Where would you put this flag? at the template level so we can have different leaders?

metricbeat.autodiscover:
  providers:
    - type: kubernetes
      scope: cluster
      templates:
        - leader: mb-kubernetes
          config:
            - module: kubernetes
              ...
        - leader: mb-kafka
          condition:
            contains:
              kubernetes.labels.app: "kafka"
          config:
            - module: kafka
              hosts: "${data.host}:${data.port}"
        - condition:
            contains:
              kubernetes.labels.app: "redis"
          config:
            - module: redis
              hosts: "${data.host}:${data.port}"

Or at the provider level? and multiple providers are needed if multiple leaderships are wanted?

metricbeat.autodiscover:
  providers:
    - type: kubernetes
      scope: cluster
      leader: mb-kubernetes
      resource: service
      templates:
        - leader: mb-kubernetes
          config:
            - module: kubernetes
              ...
    - type: kubernetes
      scope: cluster
      resource: service
      leader: mb-kafka
      templates:
        - leader: mb-kafka
          condition:
            contains:
              kubernetes.labels.app: "kafka"
          config:
            - module: kafka
              hosts: "${data.host}:${data.port}"
    - type: kubernetes
      templates:
        - condition:
            contains:
              kubernetes.labels.app: "redis"
          config:
            - module: redis
              hosts: "${data.host}:${data.port}"

At the provider level it looks better as the provider keeps state, and we probably don't need multiple leaderships on the same instance.

@masci
Copy link

masci commented Jul 23, 2020

++ for making this an autodiscover feature as several integrations other than kubernetes will benefit from it.

After reading the comments I'm wondering, should this feature be called leader election at all? As a user, if I deploy 3 metricbeats and I see "leader election" I suppose one of them will be somehow special, but it seems we'll have a different abstraction layer here: with the leader being elected among the providers and not the beats, we may potentially have several leaders around at the same time and for a single beat instance. Would something like this be more intuitive?

    metricbeat.autodiscover:
      providers:
        - type: kubernetes
          scope: cluster
          unique: true  # defaults to false if omitted
          templates:
             - config:
                 - module: redis
                   period: 10s
                   hosts: ["${data.host}:6379"]

Note I also omitted the configmap name / leader id as I think it'd be better if we could hide it as an implementation detail. This would save the users the extra cognitive effort and us the burden of validating user input.

@jsoriano
Copy link
Member

unique: true also sounds good to me and would be enough for most of the cases, it could use something as the beat name as default identifier.
We could still need another optional setting to specify an identifier in case we want to support multiple providers definitions or multiple beats with different configurations, but we can also keep it for a future enhancement if we see requests for that.

@ChrsMark
Copy link
Member

ChrsMark commented Jul 27, 2020

Do you foresee any situation where you may want to use Pods/Nodes/Service autodiscover in combination with leader election?

Umm, yeah, we might want to enable it only if some pods or services exist, and using their metadata.

This would fit in a case where we want to automatically identify kube-state-metrics and start state_* metricsets for instance. In this case we would need 2 different templates though for the same leader since we would need a condition in order to start state_* while for instance events metricset would not need any condition to be matched.

+1 for @masci 's version (plus having hardcoded the beat name as default identifier):

    metricbeat.autodiscover:
      providers:
        - type: kubernetes
          scope: cluster
          unique: true  # defaults to false if omitted
          templates:
             - config:
                 - module: redis
                   period: 10s
                   hosts: ["${data.host}:6379"]
        - type: kubernetes
          include_annotations: ["prometheus.io.scrape"]
          templates:
          ...

So every provider that is "tagged" with unique: true will only enable its configs when take the leadership.

wdyt @exekias ?

@ChrsMark
Copy link
Member

PoC for further discussion: #20281

@exekias
Copy link
Contributor Author

exekias commented Jul 28, 2020

sounds good to me folks! +1 on Jaime's comment to have another parameter to specify the identifier, with a good default

@exekias
Copy link
Contributor Author

exekias commented Jul 29, 2020

Something we need to figure out is what happens to metadata. When doing unique: true we may want two different thigns:

  • Do autodiscover at a cluster scope, to monitor some services in the cluster
  • Run some module/input only from a single place, as a singleton for the cluster

For the later part, how will the UX be? by doing this with autodiscover we may be adding unwanted metadata to the input, as autodiscover may understand it's launching the input for certain Pod, right?

@andresrc andresrc added the ext-goal External goal of an iteration label Jul 31, 2020
@ChrsMark
Copy link
Member

ChrsMark commented Jul 31, 2020

For the later part, how will the UX be? by doing this with autodiscover we may be adding unwanted metadata to the input, as autodiscover may understand it's launching the input for certain Pod, right?

I think that's right in principle. However we don't ship any meta with this kind of leadership events (see https://github.com/elastic/beats/pull/20281/files#diff-3b0c1598273f3d8e5f8fc4c5f134ad17R201). I have in mind mostly the case where we want to launch cluster wide metricsets so I might miss some parts of your concerns.

@ChrsMark
Copy link
Member

ChrsMark commented Aug 6, 2020

Pinging @david-kow who might be also interested into this new feature.

@andresrc
Copy link
Contributor

Reopening to check we are done with documentation / recommendations / migration path.

@andresrc andresrc reopened this Aug 17, 2020
@zube zube bot added [zube]: Inbox and removed [zube]: Done labels Aug 17, 2020
@masci
Copy link

masci commented Aug 17, 2020

Good catch, we've some conversations running about docs/recommendations here #20601

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autodiscovery discuss Issue needs further discussion. enhancement ext-goal External goal of an iteration Team:Platforms Label for the Integrations - Platforms team
Projects
None yet
7 participants