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 agent fleet enrolment k8s manifest #26566

Conversation

MichaelKatsoulis
Copy link
Contributor

@MichaelKatsoulis MichaelKatsoulis commented Jun 29, 2021

What does this PR do?

This PR provides the proposed manifests to start agent as Daemonset and automatically enrol the pods to fleet server.

Why is it important?

Users need a proposed way to deploy agent pods as daemonset in their cluster and automatically enrol them to the feet server. Manually enroling agents through Kibana UI is not needed anymore.

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.

How to test this PR locally

Spin up an elastic stack with fleet server. Then configure properly elastic-agent-managed-kubernetes.yaml with the
right values for FLEET_URL, KIBANA_HOST, KIBANA_FLEET_USERNAME, KIBANA_FLEET_PASSWORD and apply the manifest in a kubernetes cluster.
Notice in Kibana UI-->Management-->Fleet-->Agents section the new agents automatically enroling to fleet.

Related issues

@MichaelKatsoulis MichaelKatsoulis added the Team:Integrations Label for the Integrations team label Jun 29, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 29, 2021
@cla-checker-service
Copy link

cla-checker-service bot commented Jun 29, 2021

💚 CLA has been signed

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 29, 2021
@mtojek mtojek self-requested a review June 29, 2021 13:01
@@ -0,0 +1,80 @@
apiVersion: apps/v1
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. This might be my ignorance, but why do we keep them in two directories (deploy/kubernetes/elastic-agent-managed vs deploy/kubernetes) ?
    I'm asking as most likely I'm interested in replacing the https://github.com/elastic/elastic-package/blob/master/internal/install/static_kubernetes_elastic_agent_yml.go with something more sophisticated and as small as possible.

  2. Is it only an example or is it tested by CI somehow?

Copy link
Contributor Author

@MichaelKatsoulis MichaelKatsoulis Jun 29, 2021

Choose a reason for hiding this comment

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

  1. This is a good question. In deploy/kubernetes/elastic-agent-managed the manifests are splited by kind while in the deploy/kubernetes it is just one manifest for all. I followed the pattern of the rest. Maybe this gives users more flexibility on how to deploy the manifests and which to deploy. @ChrsMark do you know if there is another reason?

  2. I have tested it locally with the snapshot image version and elastic stack in elastic cloud.

Copy link
Member

Choose a reason for hiding this comment

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

  1. Under deploy/kubernetes/elastic-agent-managed the manifests are split per kind as @MichaelKatsoulis explained. Then deploy/kubernetes/elastic-agent-managed.yml is the final complete manifest based on them if you run make all (see Makefile in same directory). To be honest I don't really know the reason for this but we keep doing like this so far.
  2. CI only tests that these manifests are valid as far as I know. See lint stage at

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 29, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #26566 updated

  • Start Time: 2021-07-01T12:35:31.944+0000

  • Duration: 67 min 40 sec

  • Commit: 08e9149

Trends 🧪

Image of Build Times

❕ Flaky test report

No test was executed to be analysed.

@MichaelKatsoulis MichaelKatsoulis force-pushed the add-agent-fleet-enrolment-k8s-manifest branch from 3d637d7 to 8ede699 Compare June 29, 2021 13:38
Signed-off-by: MichaelKatsoulis <michaelkatsoulis88@gmail.com>
Signed-off-by: MichaelKatsoulis <michaelkatsoulis88@gmail.com>
@ChrsMark
Copy link
Member

ChrsMark commented Jul 1, 2021

@MichaelKatsoulis after this one is merged, could you please take care of the docs too? Now that we have the updated manifests we need to add docs back that were removed with elastic/observability-docs#799.

Copy link
Member

@ChrsMark ChrsMark 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 overall. I just left some comments/questions.

@@ -1,4 +1,4 @@
ALL=filebeat metricbeat auditbeat heartbeat elastic-agent-standalone
ALL=filebeat metricbeat auditbeat heartbeat elastic-agent-standalone elastic-agent-managed
Copy link
Member

Choose a reason for hiding this comment

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

@ruflin do you think naming is ok here? Should we call it just elastic-agent maybe? Or elastic-agent-fleet? elastic-agent-managed is fine for me too.

Copy link
Member

Choose a reason for hiding this comment

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

No strong preference. managed looks fine as we have also standalone.

env:
- name: FLEET_ENROLL
value: "1"
- name: FLEET_INSECURE
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried about this, FLEET_INSECURE option. Do we have a way to go with the secure approach? @mtojek any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the fleet server is deployed in elastic cloud or in a cloud with trusted CA then FLEET_INSECURE can be false which is default value.
If it is deployed in another cluster with tls enabled and self signed certificates then FLEET_CA has to be used and the CA needs to be mounted.
If tls is not enabled (a fine option for private cloud) then FLEET_INSECURE has to be set to 1.
In our case as it is a proposed manifest I believe we should leave the default value and add a comment on top of the variable of its purpose.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for clarifying, I think we are ok for now and we can iterate on it later if we see requests for that.

value: "1"
- name: FLEET_INSECURE
value: "1"
- name: FLEET_URL
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a description in a comment about this var and maybe a sample value (in the comment)? I expect users having difficulties in recognising what value they should put here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Description added. The weird part on the url is that the port is not implied by the protocol and it must be added in the end.

@ChrsMark ChrsMark added the backport-v7.15.0 Automated backport with mergify label Jul 1, 2021
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

lgtm.
if we figure out the naming thing you are good to go.

@MichaelKatsoulis
Copy link
Contributor Author

It seems that we are good to go with this PR.
@ruflin do you have any preference regarding the naming (#26566 (comment)) or can we merge it?

@MichaelKatsoulis MichaelKatsoulis added review in progress Pull request is currently in progress. and removed in progress Pull request is currently in progress. labels Jul 6, 2021
@MichaelKatsoulis MichaelKatsoulis merged commit caecb83 into elastic:master Jul 7, 2021
mergify bot pushed a commit that referenced this pull request Jul 7, 2021
* Add manifests for agent automatic enrolment to fleet

Signed-off-by: MichaelKatsoulis <michaelkatsoulis88@gmail.com>
(cherry picked from commit caecb83)
MichaelKatsoulis added a commit that referenced this pull request Jul 7, 2021
* Add manifests for agent automatic enrolment to fleet

Signed-off-by: MichaelKatsoulis <michaelkatsoulis88@gmail.com>
(cherry picked from commit caecb83)

Co-authored-by: Michael Katsoulis <michaelkatsoulis88@gmail.com>
v1v added a commit to v1v/beats that referenced this pull request Jul 8, 2021
* upstream/master: (430 commits)
  CI: increase timeout (elastic#26764)
  Heartbeat: add datastream fields to synthetics (elastic#26774)
  Osquerybeat: Change the query timeout from 3 secs to 60 secs (elastic#26775)
  Remove experimental warning for inputs with variables. (elastic#26762)
  Add latest k8s versions in testing (elastic#26729)
  change type of max_bytes to ByteType (elastic#26699)
  [Elastic Agent] Fix broken enrollment command (elastic#26749)
  Update agent managed manifest to include enrolment token variable (elastic#26756)
  Filebeat: Ensure module pipelines compatibility with previous versions of Elasticsearch (elastic#26737)
  Forward port changelog for 7.13.3 (elastic#26731) to master (elastic#26754)
  Upgrade PyYAML dependency used for tests (elastic#26746)
  Add agent fleet enrolment k8s manifest (elastic#26566)
  CI: retry the step only (elastic#26736)
  Osquerybeat: Fix the configuration poll interval setting (elastic#26739)
  [Filebeat] Replace copy_from with templated value (elastic#26631)
  Reduce the verbosity of the debug log for monitoring (elastic#26583)
  Add instructions on testing metricbeat kubernetes module (elastic#26643)
  Revert "[CI] fight the flakiness with some retry option in the CI only for the Pull Requests (elastic#26617)" (elastic#26704)
  Packaging: linux/armv7 is not supported (elastic#26706)
  Cyberarkpas: Link to official docs on how to setup TLS (elastic#26614)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.15.0 Automated backport with mergify enhancement review Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Elastic Agent] Provide k8s manifest to deploy agent on k8s with automatic enrolment to fleet.
5 participants