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

Deduplicate github actions #892

Merged
merged 11 commits into from
Dec 18, 2024
Merged

Deduplicate github actions #892

merged 11 commits into from
Dec 18, 2024

Conversation

petrutlucian94
Copy link
Contributor

@petrutlucian94 petrutlucian94 commented Dec 11, 2024

This PR introduces the following CI workflow improvements:

  • We have multiple github workflows that run e2e tests and share a significant amount of logic. We'll use reusable workflows and composite actions, making the workflows much easier to maintain.
  • Merge the following workflows:
    • integration
    • integration-informing
    • python (we want the lint jobs to pass before running e2e tests)
  • The nightly and "cron" jobs are merged into a single workflow.
    • To easily test cron jobs, we will configure them to run whenever the workflow definition changes.
  • Addresses a few flaky integration tests (test_ingress and test_microk8s_installed).

@petrutlucian94 petrutlucian94 requested a review from a team as a code owner December 11, 2024 13:45
@petrutlucian94 petrutlucian94 marked this pull request as draft December 11, 2024 13:45
@petrutlucian94 petrutlucian94 force-pushed the lpetrut/ci_cleanup branch 27 times, most recently from b08d093 to 4c5c98e Compare December 12, 2024 10:21
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@petrutlucian94 petrutlucian94 marked this pull request as ready for review December 12, 2024 13:36
@petrutlucian94
Copy link
Contributor Author

tests/test_ingress.py::test_ingress is constantly failing on Moonray, need to figure out why.

We have multiple github actions that run e2e tests and share a
significant amount of logic.

We'll add reusable actions, making the workflows much easier to
maintain.
As part of the test cleanup, we're removing the k8s snap, ensuring
that its services and mounts go away.

One of the tests installs microk8s, which interferes with the k8s
snap cleanup assertions.

We'll fix this flaky test by removing the microk8s snap.
get_external_service_ip returns an empty string, however
the test asserts that the ip is not None and proceeds with the
curl:

  2024-12-12 11:28:46 DEBUG Execute command ['curl', '', '-H', 'Host: foo.bar.com']
  in instance k8s-integration-530bc4-37

We'll update the assertion and catch empty strings as well.

At the same time, we'll increase the timeouts to reduce test
flakiness.
The nightly job is also a cron job that executes daily, so it
makes sense to merge those two workflows.
The moonray job is failing, however we only have logs from the
"default" and "kube-system" namespaces.

This change will collect logs from all k8s namespaces.
We'll need to apply the strict/moonray patches not only when
building the snap, but also when running the tests.
test_containerd_path_cleanup_on_failed_init holds an open port
and expects the bootstrap to fail, however that won't be the case
when using the lxd harness.

We'll skip this test for now.
Copy link
Member

@berkayoz berkayoz left a comment

Choose a reason for hiding this comment

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

Did a first pass, great work

tests/integration/tests/test_ingress.py Show resolved Hide resolved
k8s/scripts/inspect.sh Outdated Show resolved Hide resolved
.github/workflows/nightly-test.yaml Show resolved Hide resolved
.github/workflows/nightly-test.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

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

Nice work @petrutlucian94, I love the move of shared bits into actions!
Did a first pass and left some suggestions.

.github/actions/download-k8s-snap/action.yaml Show resolved Hide resolved
.github/actions/download-k8s-snap/action.yaml Outdated Show resolved Hide resolved
.github/workflows/build-snap.yaml Outdated Show resolved Hide resolved
.github/workflows/build-snap.yaml Outdated Show resolved Hide resolved
.github/workflows/build-snap.yaml Outdated Show resolved Hide resolved
.github/workflows/e2e-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/e2e-tests.yaml Show resolved Hide resolved
.github/workflows/nightly-test.yaml Show resolved Hide resolved
.github/workflows/nightly-test.yaml Outdated Show resolved Hide resolved
tests/integration/tests/test_cleanup.py Outdated Show resolved Hide resolved
@louiseschmidtgen
Copy link
Contributor

Could you please also update the integration test in k8s-dqlite to use your new actions? https://github.com/canonical/k8s-dqlite/blob/master/.github/workflows/k8s-snap-integration.yaml

@petrutlucian94
Copy link
Contributor Author

petrutlucian94 commented Dec 17, 2024

Could you please also update the integration test in k8s-dqlite to use your new actions? https://github.com/canonical/k8s-dqlite/blob/master/.github/workflows/k8s-snap-integration.yaml

Sure, we can handle the k8s-dqlite actions after this PR merges. Let's add a separate card though.

Copy link
Contributor

@bschimke95 bschimke95 left a comment

Choose a reason for hiding this comment

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

Great work @petrutlucian94
Did a first pass

.github/actions/download-k8s-snap/action.yaml Show resolved Hide resolved
.github/actions/download-k8s-snap/action.yaml Outdated Show resolved Hide resolved
.github/actions/download-k8s-snap/action.yaml Outdated Show resolved Hide resolved
.github/actions/install-lxd/action.yaml Outdated Show resolved Hide resolved
.github/actions/install-lxd/action.yaml Show resolved Hide resolved
.github/workflows/nightly-test.yaml Show resolved Hide resolved
.github/workflows/security-scan.yaml Outdated Show resolved Hide resolved
.github/workflows/security-scan.yaml Outdated Show resolved Hide resolved
.github/workflows/security-scan.yaml Show resolved Hide resolved
k8s/scripts/inspect.sh Outdated Show resolved Hide resolved
* cover 1.32 as part of the nightly tests
* get go version from go.mod
* update step names
* add some TODOs
* make lxd channel configurable
* bump ubuntu versions
* add get-e2e-tags dependencies
@petrutlucian94
Copy link
Contributor Author

Follow-up items that will be handled in subsequent PRs:

Copy link
Contributor

@bschimke95 bschimke95 left a comment

Choose a reason for hiding this comment

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

LGTM

@petrutlucian94 petrutlucian94 merged commit 5c3e5e0 into main Dec 18, 2024
30 checks passed
@petrutlucian94 petrutlucian94 deleted the lpetrut/ci_cleanup branch December 18, 2024 09:41
@petrutlucian94 petrutlucian94 mentioned this pull request Jan 8, 2025
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