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 support for host-agent integration tests #400

Conversation

NilanjanDaw
Copy link
Contributor

@NilanjanDaw NilanjanDaw commented Feb 25, 2022

What this PR does / why we need it:
Add support functions required for integration tests to be added for the host-agent. The integration tests will reuse functions from the E2E tests with some refactors, which are also included here.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Partial #399

Additional information
Handle different network configurations for integration and E2E tests
The E2E tests use kind for deploying the management cluster and then apply cluster-template in workload cluster. In the case of the integration tests, we wanted to use envtest instead, just like the controller integration tests.

The docker container creation helper methods were written with E2E tests in mind, so by default, they were creating containers in the kind network. However, in our case, the management cluster runs within the host network.

One workaround was to bind the api-server started by envtest to 0.0.0.0 so as to enable access from within Docker containers. However, the cert generated for the api-server by default is valid for localhost, and not for other addresses that we are binding to. One solution would have been to use controller-runtime helpers for generating certificates for the other addresses. However, we feel that would add unnecessary complexity to the tests.

Instead, we refactor the createDockerContainer() to dynamically attach the Docker container to the host network for envtest based clusters and to the kind network for E2E tests. This ensures that the management cluster and the hosts are in the same network.

Handle redundant host-agent compilation
The SetupByoDockerHost() was structured to always build the host-agent binary before setting up the docker containers. However, for tests involving multiple containers, this leads to the host-agent being compiled multiple times. To remove these redundant compilations, the agent build process has been moved to the BeforeEach() function of each test and a reference for the path to the compiled binary is passed to the setupByoDockerHost() function

Separate Byoh docker create and exec
The SetupByoDockerHost() has further been refactored to separate the docker creation and execution part, this would allow us to reuse the same docker container for multiple integration test cases

@NilanjanDaw NilanjanDaw self-assigned this Feb 25, 2022
@NilanjanDaw NilanjanDaw changed the title Add support functions for host-agent integration tests Add support for host-agent integration tests Feb 25, 2022
agent/host_agent_suite_test.go Outdated Show resolved Hide resolved
agent/host_agent_test.go Show resolved Hide resolved
test/e2e/e2e_docker_helper.go Outdated Show resolved Hide resolved
NilanjanDaw added 7 commits February 26, 2022 18:14
Move the agent build process to before each instead of
performing it ad-hoc during container setup. This should
save some time by removing redundant agent compilations
test/e2e/docker_helper.go Show resolved Hide resolved
test/e2e/docker_helper.go Outdated Show resolved Hide resolved
Copy link
Contributor

@anusha94 anusha94 left a comment

Choose a reason for hiding this comment

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

This looks good. We can do more refactoring when adding new tests. Since all the checks are passing, and we are unblocked on refactored docker helper methods, let's get this merged and then iterate!

Copy link
Contributor

@dharmjit dharmjit left a comment

Choose a reason for hiding this comment

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

changes looks good to me.

@NilanjanDaw NilanjanDaw merged commit 00b281d into vmware-tanzu:main Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants