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

GHA: Azure Integration Tests #726

Merged
merged 35 commits into from
Mar 9, 2023
Merged

GHA: Azure Integration Tests #726

merged 35 commits into from
Mar 9, 2023

Conversation

csegarragonz
Copy link
Collaborator

@csegarragonz csegarragonz commented Feb 27, 2023

In this PR I add two workflows that run some tests on a production environment on Azure.

The first workflow runs the demo/hello function on a managed Kuberenetes cluster in AKS. It is only triggered on workflow_dispatch, which means that we need to navigate to the "Actions" tab, click on the workflow name (Azure Integration Tests), and click on the "Run workflow" button. The workflow could be improved in several ways:

  • Run more complex functions (maybe some of the examples?).
  • Be triggered on every tag push to the main branch. (The risk with this one is knowing when the images for the new tag will be ready to be pulled).
  • Be triggered every push to main branch (even though noticeable changes will only happen once we release a new tag).

The second workflow runs the tests on an SGX-enabled Azure VM in hardware mode. It is triggered on workflow_dispatch and when a commit is pushed to the main branch (i.e. when a PR is merged in). This trigger behaviour aims to strike a compromise between not too often triggering yet often enough to catch regressions. Note that this action also provisions a fresh VM and installs Faasm from scratch, what also has helped flushing out installation issues.

To make it easier to access our Azure subscription without poluting the organisation's secrets, these workflows run on a self-hosted runner where we have already ran az login.

NB: Some of the SGX tests in Hardware mode are actually failing. I implement a plethora of fixes in #686, so rather than cherry-picking changes to make the tests pass here, I'd rather fix all SGX issues in #686.

@csegarragonz csegarragonz force-pushed the azure-gha branch 7 times, most recently from f4055e0 to 6565cb2 Compare February 27, 2023 11:59
@csegarragonz csegarragonz changed the title gha: azure integration tests manual action (part 2) GHA: Azure Integration Tests Mar 3, 2023
@csegarragonz csegarragonz force-pushed the azure-gha branch 6 times, most recently from b7b6128 to 7edaf4f Compare March 3, 2023 12:03
@csegarragonz csegarragonz self-assigned this Mar 3, 2023
@csegarragonz csegarragonz marked this pull request as ready for review March 3, 2023 14:07
.env Outdated
FAASM_VERSION=0.9.2
FAASM_CLI_IMAGE=faasm/cli:0.9.2
FAASM_WORKER_IMAGE=faasm/worker:0.9.2
FAASM_VERSION=0.9.3
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tag a new release to fix the errors in k8s files that the integration tests uncovered.

@@ -11,8 +11,51 @@ defaults:
shell: bash
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason this file appears as modified, and not as new, is because in order to test a workflow that is triggered on workflow_dispatch, the workflow needs to already be registered on the default branch.

Thus, I merged in two very simple PRs (#725, #727) to add the skeleton of this file and the next one.

@csegarragonz csegarragonz force-pushed the azure-gha branch 2 times, most recently from 3d685dd to b11687c Compare March 3, 2023 14:17
if: always()
run: ./bin/inv_wrapper.sh cluster.delete --name ${{ env.CLUSTER_NAME }}
working-directory: ${{ github.workspace }}/experiment-base
- name: "Chown all files"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is necessary to ensure that consecutive runs on the same runner can clean the state (i.e. remove all files). Otherwise, some docker-generated files may be root-owned.

./bin/inv_wrapper.sh vm.run-command \
--name ${{ env.VM_NAME }} \
--path "code/faasm" \
--cmd "sudo rm -rf ./clients/cpp/venv && docker compose exec cpp ./bin/inv_wrapper.sh func.local libfake"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We remove the ./venv's to force their re-creation with the first call to ./bin/inv_wrapper.sh, and to avoid possible inconsitencies between the new commits and the docker images.

@csegarragonz
Copy link
Collaborator Author

csegarragonz commented Mar 9, 2023

@Shillaker I agree with you that running on every commit to main may be a bit too much. I am also reluctant to run them on a cron schedule.

For the moment, and while I think about it more, I will merge in this PR with both workflows enabled only on workflow_dispatch, which means that they need to be run manually.

Another option (which I include for reference) is to apply filters on the files modified in the PR, and only trigger the workflow when certain files are modified (./deploy/k8s for the AKS workflow, and ./src/enclave for the SGX workflow).

I am not sure what you mean with a tag to the commit message. Do you mean creating a git tag (e.g. run-integration) and git-tag+push the commits with it when we want to trigger the workflows, and configure the workflows to run on pushes to tags matching the created name? (The same way we trigger the release workflow on tags matching the v*.*.* pattern)

@csegarragonz csegarragonz merged commit 4f05231 into main Mar 9, 2023
@csegarragonz csegarragonz deleted the azure-gha branch March 9, 2023 16:42
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

2 participants