Integration test for EDOT agents with operator, plus skeleton for collector #16
Conversation
|
@gizas @jackshirazi @rogercoll who can be a good reviewer to this PR? |
| bash test/operator/wait_for_pod_start.sh cert-manager cert-manager- 1/1 3 | ||
| helm repo add open-telemetry https://open-telemetry.github.io/opentelemetry-helm-charts | ||
| helm repo update | ||
| helm install opentelemetry-operator --namespace opentelemetry-operator-system open-telemetry/opentelemetry-operator --create-namespace --set manager.collectorImage.repository="docker.elastic.co/beats/elastic-agent:8.15.0-SNAPSHOT",manager.extraArgs={"--enable-go-instrumentation=true"} |
There was a problem hiding this comment.
I think you would need 8.16.0-SNAPSHOT image for this to work. This is because with previous versions you would need to override the container's command in order to launch the otel collector instead of the elastic-agent binary; elastic/elastic-agent#5248
In addition to that, the ELASTIC_AGENT_OTEL environment variable need to be defined too. Example https://github.com/elastic/opentelemetry/pull/11/files#diff-99b053f068c4e75b4029bd9dca1f76667178c4c2174db8313605c0a440e0fc37R22
There was a problem hiding this comment.
It's a skeleton implementation. I'm expecting this to use exactly the instructions we provide to users, rather than this skeleton. Let's do that in a different PR, as it needs syncing the installation with the readme instructions so not straightforward
There was a problem hiding this comment.
nit:
helm repo add open-telemetry https://open-telemetry.github.io/opentelemetry-helm-charts --force update
(and delete line 43
There was a problem hiding this comment.
If that's the preference, update it in https://github.com/elastic/opentelemetry/blob/main/docs/onboarding/8_16/operator/README.md because the upcoming version of this will use exactly what is in there
|
|
||
| - name: Install Operator Skeleton | ||
| run: | | ||
| kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.15.3/cert-manager.yaml |
There was a problem hiding this comment.
This is needed only for apm tests right?
There was a problem hiding this comment.
WDYT shall we pin the versions of certmanager and azure/setup-helm@v4 or use the latest? In order those tests to catch any possible issues for the users as well?
There was a problem hiding this comment.
This is needed for the operator install unless the helm chart does that for you
There was a problem hiding this comment.
If we use the Onboarding steps, this won't be needed as cert-manager is disabled.
|
A small readme on how to run those locally maybe? Sth like a set of commands to be useful for everybody to kick this off might be really useful |
| bash test/operator/wait_for_pod_start.sh cert-manager cert-manager- 1/1 3 | ||
| helm repo add open-telemetry https://open-telemetry.github.io/opentelemetry-helm-charts | ||
| helm repo update | ||
| helm install opentelemetry-operator --namespace opentelemetry-operator-system open-telemetry/opentelemetry-operator --create-namespace --set manager.collectorImage.repository="docker.elastic.co/beats/elastic-agent:8.15.0-SNAPSHOT",manager.extraArgs={"--enable-go-instrumentation=true"} |
There was a problem hiding this comment.
The opentelemetry-operator-system namespace should be created as well in prereq step is not it?
There was a problem hiding this comment.
as above, will use the actual installation procedure we specify in the readme. in another PR
|
Any open issues with this PR bearing in mind the next steps already outlined in #13 ? If not can someone approve please? |
There was a problem hiding this comment.
Template lgtm, let's use the actual Onboarding command in a follow-up PR if possible. In addition, it would be great to have some documentation in test/operator/README.md.
I am also wondering if this repository, is the best for these tests. But we can move this discussion async.
The integration test needs further work, to be outlined in an issue, but this should work now for 3 out of 5 agents