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 eck-apm-server Helm Chart #7533

Merged
merged 10 commits into from
Feb 9, 2024
Merged

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Feb 5, 2024

closes #7489

What is this change?

This adds an ECK APM Server Helm Chart for deploying APM Server instances using the ECK Operator.

Testing

Simple install using basic.yaml and version 8.12.0 works fine:

❯ helm upgrade -i eck-stack . -n elastic --create-namespace -f examples/apm-server/basic.yaml --set eck-elasticsearch.version=8.12.0 --set eck-kibana.version=8.12.0 --set eck-apm-server.version=8.12.0
Release "eck-stack" has been upgraded. Happy Helming!
NAME: eck-stack
LAST DEPLOYED: Tue Feb  6 14:15:34 2024
NAMESPACE: elastic
STATUS: deployed
REVISION: 2
TEST SUITE: None
NOTES:
Elasticsearch ECK-Stack 0.10.0-SNAPSHOT has been deployed successfully!

To see status of all resources, run

kubectl get elastic -n elastic -l "app.kubernetes.io/instance"=eck-stack

More information on the Elastic ECK Operator, and its Helm chart can be found
within our documentation.

https://www.elastic.co/guide/en/cloud-on-k8s/current/index.html

Everything becomes healthy

❯ kubectl get pod -n elastic
NAME                                                   READY   STATUS    RESTARTS   AGE
eck-stack-eck-apm-server-apm-server-6447b96464-ktpmg   1/1     Running   0          65s
elasticsearch-es-default-0                             1/1     Running   0          68s
elasticsearch-es-default-1                             1/1     Running   0          68s
elasticsearch-es-default-2                             1/1     Running   0          68s
kibana-kb-85b4b56ffc-77gng                             1/1     Running   0          65s

All Components are green

❯ kubectl get elastic -n elastic -l "app.kubernetes.io/instance"=eck-stack
NAME                                                    HEALTH   NODES   VERSION   AGE
apmserver.apm.k8s.elastic.co/eck-stack-eck-apm-server   green    1       8.12.0    11m

NAME                                                       HEALTH   NODES   VERSION   PHASE   AGE
elasticsearch.elasticsearch.k8s.elastic.co/elasticsearch   green    3       8.12.0    Ready   11m

NAME                                  HEALTH   NODES   VERSION   AGE
kibana.kibana.k8s.elastic.co/kibana   green    1       8.12.0    11m

Missing ES instance check works:

❯ helm upgrade -i eck-stack . -n elastic --create-namespace -f examples/apm-server/basic.yaml --set eck-elasticsearch.version=8.12.0 --set eck-kibana.version=8.12.0 --set eck-apm-server.version=8.12.0
Error: UPGRADE FAILED: execution error at (eck-stack/charts/eck-apm-server/templates/apmserver.yaml:35:43): A reference to an Elasticsearch instance is required

What isn't working?

  • APM+Kibana integration
  • APM+Fleet+Kibana testing

Todo

  • extended testing
  • documentation

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono added >enhancement Enhancement of existing functionality :helm-charts labels Feb 5, 2024
Update example configurations.
Add basic example.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono marked this pull request as ready for review February 6, 2024 20:23
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

Haven't tested it yet. Just read through the code. Looks good to me just a few small things.

deploy/eck-stack/README.md Outdated Show resolved Hide resolved
deploy/eck-stack/charts/eck-apm-server/Chart.yaml Outdated Show resolved Hide resolved
deploy/eck-stack/charts/eck-apm-server/templates/NOTES.txt Outdated Show resolved Hide resolved
deploy/eck-stack/charts/eck-apm-server/templates/NOTES.txt Outdated Show resolved Hide resolved
Comment on lines +29 to +31
xpack.fleet.packages:
- name: apm
version: latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still not mentioned in the APM docs (I know we added it as part of #6063) but I wonder why they don't mention it.

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'll craft something and get our docs updated, since it seems to be required.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was more thinking of the docs maintained by the APM team. But an update to our docs would be good too. Can be a separate PR from this one though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pebrc Mind double-checking the wording added in 764038d?

I will be added an issue concerning a larger update to the documentation concerning Kibana being a requirement, at least since 8.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#7541 created.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Taking a look now.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono requested a review from pebrc February 9, 2024 14:29
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

Chart LGTM, only the doc thingy needs to go into another section IMO

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono requested a review from pebrc February 9, 2024 16:51
@naemono naemono merged commit e0de4bf into elastic:main Feb 9, 2024
5 checks passed
@naemono naemono deleted the 7490/apm-helm-chart branch February 9, 2024 20:36
@thbkrkr thbkrkr changed the title ECK APM Server Helm Chart Add eck-apm-server Helm Chart Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality :helm-charts v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Helm][APM] add APM Server Helm chart
2 participants