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

Introduce a structured integration tests approach #4745

Open
pmalek opened this issue Sep 27, 2023 · 2 comments
Open

Introduce a structured integration tests approach #4745

pmalek opened this issue Sep 27, 2023 · 2 comments

Comments

@pmalek
Copy link
Member

pmalek commented Sep 27, 2023

Problem statement

With time, KIC has accrued a considerable amount of tests of which the integration tests take the most of the CI time required for a PR to pass its checks. Those tests consistently take above 30 minutes to run.

Moreover, the tests are not well structured or not well defined. It's hard to know what particular feature is tested where and what each failure means.

Manifests used in test code are not easily translatable to manifests which could be applied by users hence it's hard to track what's tested and what's used by the users.

Proposed solution

This issue tries to propose an alternative approach to structure the tests. It might also be used for other types of tests like e.g. e2e tests but for now this will focus solely on integration tests to maximize the impact on tests runtime and developer experience.

The approach proposed in here might be broken down into smaller issues and should be considered to be done incrementally as rewriting the whole testing harness be an overwhelming endeavour.

The approach is as follows:

  1. Use a similar framework as it's used by gateway-api's conformance tests

    Meaning, define a structure similar to gateway-api's suite.ConformanceTestSuite which will hold fields like:

    • name
    • description
    • clients
    • the actual test closure which will run to run test's assertions
    • manifest files to be deployed for the test

    This way tests should have easy to follow descriptions on what they are testing and manifests attached to them (listed in the "suite struct") which could be directly applied to a cluster outside of tests.

  2. Split tests that we currently have into 2 buckets (maybe more if need be):

    • Those that can share a controller manager instance and Kong Gateway. Those tests will each be run in their own namespace (as it's currently being done)

      NOTE: This might not be the desired isolation level for tests as it will require developers to judge if a test can break other tests running in parallel. If that turns out to be the case then we might consider introducing (as a replacement for this bucket or as a separate one) a third bucket for tests relying on e.g. --watch-namespace flag and running separate managers and sharing Kong Gateway instance utilizing Kong Workspaces(note: this is an EE feature).

    • Those that cannot share controller manager instance and Kong Gateway. Those tests will require their own manager being started and Kong Gateway deployed (via helm/ktf). Those are going to be predominantly tests that test broken configuration or which require locking a global resource like UDP proxy port.

    The former will be placed in one test directory as it's done in the gateway-api conformance tests, while the latter will have their own separate directory - possibly shared - but using a separate setup (and teardown) logic which will run separate manager instance and deploy a separate kong instance.

  3. Migrate tests that are currently defined in test/integration one by one to be run as part of the new framework.

  4. The above process should be done iteratively to not cause conflicts arising due to long development time (which is expected for this) and to prevent massive review headaches.

Prior art, issues

Related: #4099. Hasn't picked up much steam though.

@pmalek pmalek added this to the Fixit 2023 milestone Sep 27, 2023
@czeslavo
Copy link
Contributor

I agree we could benefit from having a well-defined structure that every test has to follow 👍

This issue made me wonder whether we could completely replace our integration tests with time and resource-wise cheaper envtests. That could give us the possibility to run integration tests without an actual Kind cluster. Food for thought:

  • Manager in envtest could configure Kong deployed in a docker container (could be spawned with testcontainers, we could trick Gateway Discovery to work as expected by injecting EndpointSlices pointing to the container - this could be a detail hidden in setup). Also, as suggested in the issue, we could have two buckets of tests: one for the ones that can share a single Kong instance and, a second for ones that need a separate one (manager instance would be always separate as that’s cheap in envtests).
  • For sample Services used in Ingresses, GW API Routes, etc. (httpbin, go-echo), we could have them shared between all tests as they’re stateless (also deployed with testcontainers in setup). We could inject EndpointSlices so that Kong’s upstreams are populated with correct IPs pointing to respective containers so we can make assertions in tests by calling Kong. Having them shared would give us speedup because no Kubernetes resources would have to be provisioned by every test case separately.

@pmalek
Copy link
Member Author

pmalek commented Sep 28, 2023

Manager in envtest could configure Kong deployed in a docker container (could be spawned with testcontainers, we could trick Gateway Discovery to work as expected by injecting EndpointSlices pointing to the container - this could be a detail hidden in setup)

I like this idea 👍 One thing to think about is whether we could use this approach for all tests (I'm not sure yet but I doubt) or just a part of them. For instance in places where we'd test resource status conditions etc controllers require particular things to happen like

  • a Service having an IP
  • Deployment/Pod being ready
  • EndpointSlice existing
  • etc. etc.

There might be more and/or more complex conditions for controllers to continue reconciliation of resources into Kong Gateway configuration. I'm not saying this is a blocker but this is something to keep in mind that we'd need to be responsible for mocking this (or rather doing this manually) in code, similarly as we do it in unit tests (like we go in the operator's *Reconcile tests).

For sample Services used in Ingresses, GW API Routes, etc. (httpbin, go-echo), we could have them shared between all tests as they’re stateless

That's true and that's a valid point. As an implementation detail I'd say that I'd prefer us to pursue (at some point) using something like https://github.com/kubernetes-sigs/e2e-framework to


Just as I expect, catching the whole idea into a Github issue but not be enough and a design doc of sorts would be helpful 😅

@lahabana lahabana removed this from the Fixit 2023 milestone Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants