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

[chore] Functional tests v2 #949

Merged
merged 15 commits into from
Oct 10, 2023
Merged

[chore] Functional tests v2 #949

merged 15 commits into from
Oct 10, 2023

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented Oct 5, 2023

Description:
add e2e tests that test cluster receiver metrics and an attempt to test trace auto-instrumentation.

@atoulme atoulme requested review from a team as code owners October 5, 2023 16:07
@atoulme atoulme force-pushed the add_e2e_tests branch 3 times, most recently from 0aac2af to bf72384 Compare October 5, 2023 16:33
Copy link
Contributor

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

I think it's a good start assuming we are going to replace the python tests without losing any coverage instead on maintaining both

e2e_tests/e2e_test.go Outdated Show resolved Hide resolved
e2e_tests/e2e_test.go Outdated Show resolved Hide resolved
@atoulme atoulme force-pushed the add_e2e_tests branch 3 times, most recently from 86b61ed to ed0f61d Compare October 5, 2023 23:29
Makefile Outdated Show resolved Hide resolved
@atoulme atoulme closed this Oct 6, 2023
@atoulme atoulme reopened this Oct 7, 2023
.github/workflows/e2e.yaml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
e2e_tests/e2e_test.go Outdated Show resolved Hide resolved
e2e_tests/e2e_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

Great start! Thank you, @atoulme.

The tests are still failing. PTAL

e2e_tests/testdata/operator_values.yaml Outdated Show resolved Hide resolved
@atoulme atoulme force-pushed the add_e2e_tests branch 2 times, most recently from ab27696 to fa2d800 Compare October 9, 2023 21:01
@atoulme atoulme changed the title [chore] e2e tests [chore] Functional tests v2 Oct 9, 2023
.PHONY: cert-manager
cert-manager: cmctl
# Consider using cmctl to install the cert-manager once install command is not experimental
kubectl apply --validate=false -f https://github.com/jetstack/cert-manager/releases/download/${CERTMANAGER_VERSION}/cert-manager.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

is cert-manager installation for the operator webhook? if so, then can we just enable the cert-manager dependency when installing helm chart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that didn't work. I copied the cert-manager testing code from the operator project instead.

Copy link
Contributor

@jvoravong jvoravong Oct 10, 2023

Choose a reason for hiding this comment

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

Jina's suggestion should theoretically be plausible, I wonder if this a limitation of using kind in a GH workflow. This solution is acceptable for now.

The operator project runs tests in parallel using 1 kind cluster with the kuttl test framework which results in each test case getting a unique k8s namespace to run the test case in. Since you can only have 1 cert-manager per k8s cluster, the operator project runs 'make cert-manager' once when setting up the kind cluster before any tests are run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It really seemed like a weird ordering issue, as in the operator would be failing to get a cert and just hang there. It is probably because of the nature of the deployment, if you install cert-manager at the same time as the operator.

The good news is that we can patch this to use the cert-manager in the helm chart with a smaller delta.

@atoulme atoulme merged commit 8e78f2b into main Oct 10, 2023
24 checks passed
@atoulme atoulme deleted the add_e2e_tests branch October 10, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants