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 for e2e test setup #2479

Merged
merged 4 commits into from
Jan 8, 2024

Conversation

swiatekm
Copy link
Contributor

@swiatekm swiatekm commented Dec 23, 2023

Description:
Right now, we have a very ad-hoc and inconsistent way of doing setup for e2e tests:

  • The operator image is set via kustomize
  • target allocator and opamp bridge images are set by doing a text replace over the test manifests
  • Feature gates for the operator are set by redeploying the operator in kuttl commands, in the test definition itself

This change introduces a single mechanism for e2e test setup. All the per-testsuite setup is done via kustomize, primarily through json patches on the operator args - both images and feature gates can be set this way.

As a result, we have:

  • All test setup for our E2E tests is done via the prepare-e2e make target, or its flavors
  • It is now straightforward to run different test suites with specific feature flags enabled or disabled
  • Tests themselves do no setup, and external consumers like the Helm Chart can easily do their own
  • We get rid of the hacky script for text-replacing images in manifests
  • All the local changes are done to the config/manager/kustomization.yaml file, which makes them easy to control

Testing:
Went through a bunch of typical dev workflows manually.

@swiatekm swiatekm added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 23, 2023
@swiatekm swiatekm force-pushed the test/e2e-images branch 5 times, most recently from 3049f3f to cec256f Compare December 23, 2023 15:21
Comment on lines -31 to +35
- --enable-leader-election
- "--metrics-addr=127.0.0.1:8080"
- "--enable-leader-election"
- "--zap-log-level=info"
- "--zap-time-encoding=rfc3339nano"
- "--feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx"
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 moved this so I could patch these args in the manager kustomization.yaml, as opposed to the one in config/default. I don't see any reason for it not to be here.

@swiatekm swiatekm marked this pull request as ready for review December 27, 2023 16:45
@swiatekm swiatekm requested a review from a team December 27, 2023 16:45
- e2e-pdb
- e2e-opampbridge
- e2e-prometheuscr
- e2e-multi-instrumentation
include:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very slick. I like it :)

@jaronoff97
Copy link
Contributor

This LGTM. The operator helm chart also runs these E2E tests. Do we need to make any changes there?

@swiatekm
Copy link
Contributor Author

This LGTM. The operator helm chart also runs these E2E tests. Do we need to make any changes there?

Test run by the Helm Chart should work as they are. This will let us simplify their setup, which can happen in a later change.

@pavolloffay pavolloffay merged commit 443abb6 into open-telemetry:main Jan 8, 2024
27 checks passed
@swiatekm swiatekm deleted the test/e2e-images branch January 8, 2024 14:16
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* Use kustomize for setting target allocator image in E2E tests

* Use kustomize for setting opamp bridge image in E2E tests

* Use kustomize for setting feature gates in E2E tests

* Move operator flags to manager manifest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants