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: GHA to smoke test against last released exemplar #2983

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

safeer
Copy link
Contributor

@safeer safeer commented Oct 3, 2024

This adds a GitHub Action that smoke tests branches against the exemplar in the latest tagged release.

It will catch regressions that may affect users on FTL upgrades, but it doesn't specifically test the no-downtime upgrade path that users will go through. That's upcoming; in order to do it, the exemplar will first need to be run in FTL's long living cluster.

@safeer safeer added the run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue label Oct 3, 2024
This was referenced Oct 3, 2024
@safeer safeer force-pushed the saf/smoketest_gha branch 23 times, most recently from 07c135d to 5725b01 Compare October 7, 2024 22:17
@safeer safeer changed the title chore: WIP smoke test upgrade path chore: GHA to smoke test against last released exemplar Oct 7, 2024
@safeer safeer marked this pull request as ready for review October 7, 2024 22:24
@safeer safeer requested review from alecthomas and a team as code owners October 7, 2024 22:24
cache: true
- name: Build Cache
uses: ./.github/actions/build-cache
- name: Docker Compose
Copy link
Collaborator

Choose a reason for hiding this comment

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

Everything is handled by kube, we should not need docker compose or initdb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, yeah gonna do some cleanup here

@@ -0,0 +1,95 @@
on:
pull_request:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we actually want this to be on push, as we are not using merge groups any more.

jobs:
smoke-test-upgrade-path:
name: Smoke Test Upgrade Path
# if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || contains( github.event.pull_request.labels.*.name, 'run-all')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to uncomment this before merging.

@@ -375,3 +375,57 @@ jobs:
else
echo "Integration tests passed"
fi
smoke-test-released-exemplar:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be removed from ci.yaml, as it is duplicated in smoketest.yaml, which also runs on PRs?

fetch-tags: true
- name: Get last tagged release
run: |
latest_release=$(git tag --sort=-v:refname | grep -v v1 | head -n 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

At some point (not now) we are going to need to think about testing older releases. The 'latest release' is often just a few commits ago.

- name: Replace the tagged release smoketest with the current smoketest
run: |
set -euo pipefail
echo "Replacing tagged release smoketest with current smoketest"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This approach will break if the smoke test adds new features that are in the new version but not the old version, as you are testing the new code again old FTL.

This is fine for now but I think but will need some thought going forward.

run: |
set -euo pipefail
echo "Deploying the tagged release to the cluster"
cd deployment && just deploy-version ${{ env.LATEST_VERSION }} && just wait-for-kube && cd ..
Copy link
Collaborator

Choose a reason for hiding this comment

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

wait-for-kube is already in the harness, if you leave it off here it will speed up the test.

@safeer
Copy link
Contributor Author

safeer commented Oct 16, 2024

Ingress is working, but looks like PubSub is also borked in the kube environment. Working through that.

@stuartwdouglas thanks for the review! Definitely addressing these.. I should've marked this as draft again 😂

@safeer safeer marked this pull request as draft October 16, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants