add apm-server helm chart #324

pbecotte · 2019-10-13T03:52:27Z

Chart version not bumped (the versions are all bumped and released at the same time)
README.md updated with any new values or changes
Updated template tests in ${CHART}/tests/*.py
Updated integration tests in ${CHART}/examples/*/test/goss.yaml

Take a stab at adding the apm-server helm chart. Not sure about an open source image, or if there's a good way to flesh out the goss tests. I am NOT an apm-server expert, any suggestions are welcome. I have this running on my cluster currently.

elasticmachine · 2019-10-13T03:52:28Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

jmlrt · 2019-10-14T08:45:45Z

related to #68

jmlrt · 2019-10-17T12:20:46Z

Hi @pbecotte,
Thank you for this great PR!
Adding yourself Goss tests and Python tests is really nice 👍.
The first think I see is that httpPort is missing in your values.yml.
We'll do a full review soon.

apm-server/examples/default/test/goss.yaml

apm-server/examples/oss/test/goss.yaml

apm-server/examples/oss/values.yaml

apm-server/examples/security/test/goss.yaml

apm-server/templates/hpa.yaml

apm-server/Chart.yaml

apm-server/examples/oss/values.yaml

apm-server/tests/apmserver_test.py

apm-server/examples/security/values.yaml

apm-server/values.yaml

pbecotte · 2019-11-08T02:24:46Z

Addressed your comments- not sure how I had so many bugs in there- the tests passed at some point, but apparently I didn't run them before pushing? Not sure what would be a good test for apm server- maybe there is a health check url we should send a request to?

jmlrt

Thanks for your changes.

I don't think we have a proper healthcheck endpoint yet in APM server, however testing that the displayed version is the proper one with localhost:8200?pretty is a good start.

Outside of that, still one SERVER_HOST not defined and one elasticsearchHoststo remove and I need to re-run the full tests.

Side note, when I will approve this PR, I'll request another review from Elastic team, but we'll also need to discuss internally about the right time to merge it and release it on Helm repo (TLDR, we already lack man-power dedicated to these helm charts and are struggling to follow issues/pr on existing charts so adding another chart currently does also means more issues/pr so we need to ensure we can "scale" support).

However this is a great PR and I would really like to be able to merge it soon 👍

apm-server/examples/oss/values.yaml

jmlrt · 2019-11-12T23:21:34Z

jenkins test this please

jmlrt · 2020-01-13T17:14:05Z

Hi @pbecotte,

Better late than never, we are thinking about merging this PR before the end of the month and release APM server chart in alpha for 7.6.0.

I think we still had a few comments in #324 (review) and certainly a few others to come in the next days.

Are you still interested to work on it?

pbecotte · 2020-01-13T20:20:16Z

Yeah, didn't want to get too far out in front of it, but would be happy to go through and update it.

jmlrt

I compared the chart to our current charts and updated reviews with the change we made between the time you created the PR and now. Lots of change required but half of them is helpers prefix :)

In addition, you could add an entry to the global README.md.

Thanks for your work on this chart 👍

apm-server/Chart.yaml

apm-server/README.md

jmlrt · 2020-01-16T11:21:01Z

apm-server/README.md

+| `readinessProbe`         | Parameters to pass to [readiness probe](https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-probes/) checks for values such as timeouts and thresholds.                                                                                   | `failureThreshold: 3`<br>`initialDelaySeconds: 10`<br>`periodSeconds: 10`<br>`successThreshold: 3`<br>`timeoutSeconds: 5` |
+| `resources`              | Allows you to set the [resources](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/) for the `DaemonSet`                                                                                                                                | `requests.cpu: 100m`<br>`requests.memory: 100Mi`<br>`limits.cpu: 1000m`<br>`limits.memory: 200Mi`                         |
+| `serviceAccount`         | Custom [serviceAccount](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) that APM Server will use during execution. By default will use the service account created by this chart.                                                        | `""`                                                                                                                      |
+| `secretMounts`           | Allows you easily mount a secret as a file inside the `DaemonSet`. Useful for mounting certificates and other secrets. See [values.yaml](./values.yaml) for an example                                                                                                      | `[]`                                                                                                                      |


Suggested change

| `secretMounts` | Allows you easily mount a secret as a file inside the `DaemonSet`. Useful for mounting certificates and other secrets. See [values.yaml](./values.yaml) for an example | `[]` |

| `secretMounts` | Allows you easily mount a secret as a file inside the `Deployment`. Useful for mounting certificates and other secrets. See [values.yaml](./values.yaml) for an example | `[]` |

apm-server/templates/hpa.yaml

apm-server/templates/ingress.yaml

apm-server/templates/service.yaml

apm-server/templates/serviceaccount.yaml

jmlrt

Still a few things to change, mostly in the README (replace relative urls, change parameters description mentioning daemonsets...).

apm-server/examples/6.x/test/goss.yaml

README.md

jmlrt · 2020-01-20T06:20:33Z

apm-server/README.md

+* The default APM Server configuration file for this chart is configured to use an 
+Elasticsearch endpoint as configured by the rest of these helm charts. This can
+easily be overridden in the config value apmConfig.apm-server.yml
+


When I mentionned a note, it was a bullet point like in other README (sorry for not being enough explicit).

jmlrt · 2020-01-20T06:23:57Z

jenkins test this please

jmlrt · 2020-01-24T10:34:49Z

jenkins test this please

jmlrt · 2020-01-24T12:16:01Z

jenkins test this please

jmlrt

LGTM! There is still a few non-blocking comments and a strange issue with goss test but let's merge and I'll manage the rest in a following PR.

Many thanks for your work @pbecotte

jmlrt added the feature label Oct 17, 2019

jmlrt suggested changes Oct 30, 2019

View reviewed changes

pbecotte force-pushed the apm-server branch from 9941993 to f21dba0 Compare November 8, 2019 02:52

jmlrt suggested changes Nov 12, 2019

View reviewed changes

apm-server/examples/oss/values.yaml Outdated Show resolved Hide resolved

pbecotte force-pushed the apm-server branch from f21dba0 to 31ad414 Compare January 14, 2020 02:36

jmlrt suggested changes Jan 16, 2020

View reviewed changes

pbecotte force-pushed the apm-server branch from 31ad414 to 959fb82 Compare January 17, 2020 13:30

add apm-server helm chart

5a477ac

pbecotte force-pushed the apm-server branch from 959fb82 to 5a477ac Compare January 17, 2020 21:57

jmlrt suggested changes Jan 20, 2020

View reviewed changes

jmlrt mentioned this pull request Jan 21, 2020

Helm charts for APM #68

Closed

address the last few (hopefully) comments

14518ad

jmlrt mentioned this pull request Jan 24, 2020

[apm-server] add ci tests for apm-server chart #459

Merged

jmlrt mentioned this pull request Jan 24, 2020

Apm server tests #460

Closed

jmlrt approved these changes Jan 24, 2020

View reviewed changes

jmlrt merged commit 6aaabcd into elastic:master Jan 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add apm-server helm chart #324

add apm-server helm chart #324

pbecotte commented Oct 13, 2019 •

edited

Loading

elasticmachine commented Oct 13, 2019

jmlrt commented Oct 14, 2019

jmlrt commented Oct 17, 2019

pbecotte commented Nov 8, 2019

jmlrt left a comment

jmlrt commented Nov 12, 2019

jmlrt commented Jan 13, 2020

pbecotte commented Jan 13, 2020

jmlrt left a comment •

edited

Loading

jmlrt Jan 16, 2020

jmlrt left a comment

jmlrt Jan 20, 2020

jmlrt commented Jan 20, 2020

jmlrt commented Jan 24, 2020

jmlrt commented Jan 24, 2020

jmlrt left a comment

	\| `secretMounts` \| Allows you easily mount a secret as a file inside the `DaemonSet`. Useful for mounting certificates and other secrets. See [values.yaml](./values.yaml) for an example \| `[]` \|
	\| `secretMounts` \| Allows you easily mount a secret as a file inside the `Deployment`. Useful for mounting certificates and other secrets. See [values.yaml](./values.yaml) for an example \| `[]` \|

add apm-server helm chart #324

add apm-server helm chart #324

Conversation

pbecotte commented Oct 13, 2019 • edited Loading

elasticmachine commented Oct 13, 2019

jmlrt commented Oct 14, 2019

jmlrt commented Oct 17, 2019

pbecotte commented Nov 8, 2019

jmlrt left a comment

Choose a reason for hiding this comment

jmlrt commented Nov 12, 2019

jmlrt commented Jan 13, 2020

pbecotte commented Jan 13, 2020

jmlrt left a comment • edited Loading

Choose a reason for hiding this comment

jmlrt Jan 16, 2020

Choose a reason for hiding this comment

jmlrt left a comment

Choose a reason for hiding this comment

jmlrt Jan 20, 2020

Choose a reason for hiding this comment

jmlrt commented Jan 20, 2020

jmlrt commented Jan 24, 2020

jmlrt commented Jan 24, 2020

jmlrt left a comment

Choose a reason for hiding this comment

pbecotte commented Oct 13, 2019 •

edited

Loading

jmlrt left a comment •

edited

Loading