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

Frontend charm integration test run bugfix #20

Closed
wants to merge 29 commits into from
Closed

Conversation

mz2
Copy link
Collaborator

@mz2 mz2 commented Jun 7, 2023

Fixes the sole frontend charm integration test scenario.

  • Charm requires juju 3.x for Reasons, although the frontend charm does not explicitly set a requirement for juju 3.x (a known issue apparently that a charm requires a different pylibjuju depending on juju 2.x vs 3.x). I migrated to a fork of the charmed-kubernetes/actions-operator with which I could install more recent versions of juju and microk8s, in strict confinement (Makes juju and microk8s classic confinement optional charmed-kubernetes/actions-operator#57).
  • The containerd template trick for GitHub image access when deploying the charm needed to be done also in the integration test workflow (this workflow now verifies the install prerequisite steps for local development too).
  • Plot twist I realised is that the integration tests actually had not been run ever really on the workflow yet (they had run locally, but not ever really on GitHub correctly thus far).., and actually there was a minor regression caught by this simple test scenario that exists there now): when the frontend charm is deployed on its own, the charm should actually reach a maintenance status (it would error out after some changes that were made in response to the earlier unit tests that didn't test the scenario where the charm is deployed on its own without the backend).
  • Simplifies the relation handling logic: no dependence on _stored, API server hostname and port read from the relation's application data bag (see more at https://juju.is/docs/sdk/integration, in particular the section "Handling integration data from non-relation hooks").

Note: user https://github.com/canonical-hw-certbot was added as a collaborator to the project, and the token set in secrets is theirs (with only packages:read permission set).

@mz2 mz2 marked this pull request as ready for review June 7, 2023 15:33
@@ -4,7 +4,7 @@ on:
branches:
- main
push:
branches: ["main"]
branches: ["main", "integration-test-fix"]
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 suggest that we actually keep this in there because I imagine this will not be the last case where we have to open a branch & PR to fix integration tests issues, and it's awkward having to add a rule each time (or to tag all the time).

@mz2 mz2 requested review from omar-selo and nadzyah June 7, 2023 15:36
…state where relation is not yet connected.

Simplified the charm logic, no _stored state needed (fetching the hostname and port from the relation's application data bag).
@mz2
Copy link
Collaborator Author

mz2 commented Dec 9, 2023

This has been superseded by #54

@mz2 mz2 closed this Dec 9, 2023
@mz2 mz2 deleted the integration-test-fix branch December 9, 2023 22:32
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.

2 participants