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(cicd): remove CI pipeline external infra dependencies #2403

Merged
merged 25 commits into from
Jun 5, 2023

Conversation

schoren
Copy link
Collaborator

@schoren schoren commented Apr 18, 2023

This PR focuses on making our pipeline easier to use, a bit faster, and remove dependenies with external resources, mainly cloud kubernetes.

Previously, we deployed the code to a kubernetes instance and created a tunnel via port forwarding. This apprach has a few problems:

  1. Different e2e test parts (web, server, cli) sometimes interfere with each other. Some test rely on an expected app state, and concurrent tests might modify that state. This means we cannot run different e2e in parallel
  2. It's hard to reproduce CI results locally, because envs and states might be difficult to reproduce
  3. We flood our docker hub repo with pr-* images. They need to be deployed somewhere to be accessible to the deployment.

With this new approach, each e2e test starts its own docker-compose instance. They run in parallel without ever interacting with each other.
The pipeline is defined to use bash scripts that live in the repo. This makes it easy to run the exact same tests with the exact same params on the CI environment and locally.

By leveraging docker export/import of images and GitHub Actions artifacts, we don't need to ever push the image to an external repo.

There's also some new documentation about how to run and test the app locally in CONTRIBUTING.md

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

@schoren schoren marked this pull request as ready for review April 18, 2023 19:57
@schoren schoren marked this pull request as draft April 18, 2023 19:57
@schoren schoren force-pushed the speed-up-pr-pipeline branch 2 times, most recently from 6648d2e to d3e2d9f Compare May 4, 2023 13:23
@danielbdias danielbdias mentioned this pull request May 23, 2023
4 tasks
@schoren schoren force-pushed the speed-up-pr-pipeline branch 6 times, most recently from b036096 to f86d71f Compare May 31, 2023 15:38
@schoren schoren marked this pull request as ready for review June 1, 2023 20:39
// wait for pokeshop services
waitForPort("8081")
waitForPort("8082")
time.Sleep(10 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Even with waitForPort for some reason Pokeshop drops the connection for a while, this is why I added time.Sleep again.


**CLI** and **Server** are mainly built from the project root using [GoReleaser](https://goreleaser.com/) to simplify packaging and distribution.

The server is responsible for serving both the API and the web UI. The easiest way to mix those 2 things for distribution is with docker. We rely on GoReleaser again for packaging all those things together.
Copy link
Contributor

Choose a reason for hiding this comment

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

serving both the CLI and the web UI ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it doesn't serve the CLI, it serves the API that the CLI consumes. but it's also the "http server" that delivers the web UI (like index.html, *.js, etc). That's what this means by "serve". Not that it is "at the service of", but that it delivers the web responses for.

Does that make sense? Do you think there's a more clear way to state that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. No, I think it's ok in that way. Thanks for the details.

CONTRIBUTING.md Outdated Show resolved Hide resolved
@xoscar
Copy link
Collaborator

xoscar commented Jun 2, 2023

I love what we did here, as soon as we get v0.11.10 out the door lets merge this!

@schoren schoren merged commit 4e65728 into main Jun 5, 2023
24 checks passed
@schoren schoren deleted the speed-up-pr-pipeline branch June 5, 2023 16:57
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.

None yet

5 participants