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

Use Kustomize to build elastic-agent manifests for both managed and standalone more #2104

Merged
merged 17 commits into from
Jan 17, 2023

Conversation

gsantoro
Copy link
Contributor

@gsantoro gsantoro commented Jan 16, 2023

What does this PR do?

Use Kustomize to simplify building manifest for elastic-agent both for managed and standalone mode.

Why is it important?

This PR reduces the amount of boiler plate code needed to build manifests for elastic-agent. Some enhancements:

  • we can also install kube-state-metrics just by adding an extra resource github.com/kubernetes/kube-state-metrics?ref=main to common/kustomization.yml
  • most of the resources (aka roles, role-binding, service accounts, cluster-role, cluster-role binding) are exactly the same for both standalone and managed mode. This reduces massively the amount of duplication.
  • some resources (like config maps and extra roles and role bindings) are only necessary in standalone mode. There is no need to duplicates them in both mode
  • it makes possible to customise environment variables without changing the manifests . Default values for those environment variables are provided in .env files under dev-tools/kubernetes/base//.env (added to git) while custom values are under dev-tools/kubernetes/overlays//.env (those should not be under git. that's why they are added to .gitignore).
  • namespace references are only present in kustomization.yml files. It is easier to deploy all resources to a separate namespace
  • overlays can be used to customize the docker image tag of the elastic-agent without changing the manifest parts
  • Common resources have generic names so that you can use prefixes specific to the mode you want to deploy (eg. elastic-agent-standalone- or elastic-agent-managed). This enables to deploy standalone and managed agents side by side.
  • comments in manifest parts are removed in the final manifest
  • yaml code is pretty formatted when the final manifest is generated

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/fragments using the changelog tool

Author's Checklist

  • You can deploy elastic-agent in both standalone and managed modes

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@gsantoro gsantoro added enhancement New feature or request draft labels Jan 16, 2023
@gsantoro gsantoro requested a review from a team January 16, 2023 12:06
@gsantoro gsantoro self-assigned this Jan 16, 2023
@gsantoro gsantoro requested a review from a team as a code owner January 16, 2023 12:06
@gsantoro gsantoro requested review from aleksmaus and michalpristas and removed request for a team January 16, 2023 12:06
@mergify
Copy link
Contributor

mergify bot commented Jan 16, 2023

This pull request does not have a backport label. Could you fix it @gsantoro? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 16, 2023

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-17T16:28:02.524+0000

  • Duration: 18 min 18 sec

Test stats 🧪

Test Results
Failed 0
Passed 4825
Skipped 13
Total 4838

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

requests:
cpu: 100m
memory: 200Mi
volumeMounts:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think following part can also be splitted to multilple overalys.

We want to have needed mounts for observability and extra ones that imposed from security

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mind elaborate a bit more on the topic. I vaguely recollect some suggestions from the security team but I wouldn't know how to change this part of the manifest

@@ -0,0 +1,3 @@
ES_USERNAME=elastic
ES_PASSWORD=changeme
ES_HOST=https://elasticsearch:9200
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to put namespace as variable 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.

I don't think you can use a variable from an .env file in the kustomization.yml file (which is where I set the namespace for all the resources)

@@ -0,0 +1,3 @@
ES_USERNAME=elastic
ES_PASSWORD=changeme
ES_HOST=https://elasticsearch:9200
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have as boolean on/off options the ability to install kube state metrics here?
By default to be on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. This env file is only used to create a configmap and I don't think we can use an env variable from a configmap to decide if we want to add a manifest based on that. We could use an overlay with an extra resource to install or not the kube-state-metrics

Copy link
Contributor

Choose a reason for hiding this comment

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

But then we will need to edit the kustomize.yaml.

Maybe we can think that we can provide diffrent kustomzation files ?
eg kustomize-ksmenabled, kustomize-ksmdisabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I believe we have to go that way. Different overlays for different behaviours

Copy link
Contributor

@gizas gizas Jan 16, 2023

Choose a reason for hiding this comment

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

1st Option:
A tool worth looking:
https://carvel.dev/ytt/
So we would like to provide some basic YAML templating and then kustomize installation.

2nd Option:
Maybe our new MAKEFILE that we can run and produce specific kustomize folders and run as oneliner command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I heard about ytt but it seemed way too complicated compared to kustomize. Should we create another issue with the list of requirements before diving into ytt?

@gizas
Copy link
Contributor

gizas commented Jan 16, 2023

Summarising some small comments from relevant slack commnication:

For first iteration of this effort:

  • Only default elastic-agent managed and standalone scenarios to be implemented. Any extra scenarios to be defined in extra stories
  • Include installation of KSM as part of the resources file. We would like ideally the UX to be that with one command to install all needed resources
  • Namespace and toggle option of KSM installation to be provided
  • Correlation between our manifest files with specific KSM by default. I guess this also be an env variable which can be overrided if user wants on runtime

Extra stories:

@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 16, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 98.305% (58/59) 👍
Files 69.268% (142/205) 👍
Classes 69.231% (270/390) 👍
Methods 54.047% (828/1532) 👍
Lines 39.229% (9001/22945) 👎 -0.031
Conditionals 100.0% (0/0) 💚

…c-agent labels, namespace and other changes from the various kustomizations.yaml
@gsantoro gsantoro merged commit b1b5a43 into elastic:main Jan 17, 2023
@gsantoro gsantoro deleted the feature/1668_kustomize branch January 17, 2023 18:16
- configMapRef:
# Fleet Server URL to enroll the Elastic Agent into
# FLEET_URL can be found in Kibana, go to Management > Fleet > Settings
# - 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.

Just out of curiosity, why all these settings are commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they are mostly left there for documentation. We should address them (one way or another) in the next update about on this topic.

Copy link
Member

Choose a reason for hiding this comment

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

But when those vars should be uncommented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created this PR to fix this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Kustomize.io to build elastic-agent manifests for k8s
4 participants