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

feat: jaas integration tests #561

Merged

Conversation

kian99
Copy link
Contributor

@kian99 kian99 commented Aug 30, 2024

Description

This PR adds an integration test workflow to test the provider against a JIMM controller.

The full list of changes include:

  • Add workflow to start JAAS and run tests.
  • Add logic to skip tests that are not appropriate for JAAS.
  • Fix offer tests that used a fixed "admin" user in assertions.

Fixes: CSS-6347

Type of change

  • Change in tests (one or several tests have been changed)

Additional notes

The Github action at canonical/jimm/.github/action/test-server is ready but because the canonical/jimm repo is not public, it is not accessible. The repo will become public very early next week. Marking this PR as a draft until then.

@kian99
Copy link
Contributor Author

kian99 commented Aug 30, 2024

@hmlanigan This PR and #552 are ready for review, the only blocker on the JAAS integration tests is that we are waiting on the JIMM repo to be made public which should be done on Monday.

@kian99 kian99 force-pushed the CSS-6347-jaas-integration-tests branch 2 times, most recently from 98b621f to 5681284 Compare September 3, 2024 11:12
@kian99 kian99 marked this pull request as ready for review September 3, 2024 11:12
@kian99 kian99 force-pushed the CSS-6347-jaas-integration-tests branch 4 times, most recently from f024791 to 52cab76 Compare September 3, 2024 15:08
Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

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

I'd like to see the job run successfully before approving. I'll need help setting up the secret for this action in GitHub.

.github/workflows/test_integration_jaas.yaml Show resolved Hide resolved
jimm-version: v3.1.8
juju-channel: 3/stable
ghcr-pat: ${{ secrets.GHCR_JAAS_PAT }}
# TODO(Kian): Verify whether this is needed.
Copy link
Member

Choose a reason for hiding this comment

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

question: where is the version of lxd being used configured?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's currently a fixed version of LXD in the JIMM-setup action with "5.19/stable"

.github/workflows/test_integration_jaas.yaml Outdated Show resolved Hide resolved
@kian99 kian99 force-pushed the CSS-6347-jaas-integration-tests branch 2 times, most recently from 9d4a09b to 6eee13c Compare September 5, 2024 06:51
- Add workflow to start JAAS and run tests.
- Add logic to skip tests that are not appropriate for JAAS.
@kian99 kian99 force-pushed the CSS-6347-jaas-integration-tests branch from 6eee13c to 32ad8f0 Compare September 5, 2024 07:38
@kian99 kian99 requested a review from hmlanigan September 5, 2024 08:21
@kian99
Copy link
Contributor Author

kian99 commented Sep 5, 2024

@hmlanigan tests are now passing. Two notes,

  1. The issue pulling from the Github container registry was solved by using the GITHUB_TOKEN and adding the packages: read permission AND in the JIMM repo, the image was set to "internal", that has now been changed to "public".
  2. I noticed once the tests started working, in the first run, all tests passed except for one with an error "model not found", seems something went wrong in a flakey way. Tough to say if it was Juju or JAAS that caused the issue but I'll keep an eye on these scenarios.

Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

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

Nice.

@hmlanigan
Copy link
Member

/merge

1 similar comment
@hmlanigan
Copy link
Member

/merge

@jujubot jujubot merged commit 19e0845 into juju:jaas-resources Sep 5, 2024
29 of 30 checks passed
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.

3 participants