Skip to content

Frontend charm integration test run bugfix - #20

Closed
mz2 wants to merge 29 commits into
mainfrom
integration-test-fix
Closed

Frontend charm integration test run bugfix#20
mz2 wants to merge 29 commits into
mainfrom
integration-test-fix

Conversation

@mz2

@mz2 mz2 commented Jun 7, 2023

Copy link
Copy Markdown
Contributor

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
- main
push:
branches: ["main"]
branches: ["main", "integration-test-fix"]

Copy link
Copy Markdown
Contributor 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 nadzyah and omar-selo 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
mz2 force-pushed the integration-test-fix branch from 7d4789d to 0a30cee Compare June 7, 2023 20:01
Comment thread .github/workflows/frontend_charm_integration_tests.yml Outdated
@mz2

mz2 commented Dec 9, 2023

Copy link
Copy Markdown
Contributor Author

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