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

GitHub workflow for validating terraform deploy + upgrading charms #81

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

mz2
Copy link
Collaborator

@mz2 mz2 commented Dec 12, 2023

  • Introduces a GitHub workflow for validating that terraform deploy results in a successful deploy on a fresh microk8s cloud.
    • The new CI checks deploys the plan for the application into a fresh new local microk8s cloud, i.e. it deploys a version of the application presently available on charmhub
    • It then refreshes locally built backend and frontend charms into the running cluster, leading to the upgrade hooks in the charm to be tested as well
  • Fixes some dreadful mutable state driven logical errors in the charm: upon observing the newly introduced CI check fail (1/3 of the application units would launch, with the non-leaders left waiting for the db relation) I proceeded to fix that.
  • Removes the metadata.yml and moves all the charm metadata to the modern charmcraft.yml only format.
  • Adds an online state check (see https://github.com/canonical/pebble#layer-specification) to the backend charm.
  • Updates charm libraries.
  • Limits pointless running of frontend and backend tests on commits that don't change them.

@@ -1,8 +1,34 @@
name: test-observer-api
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 migrated to the new one-yaml format.

class UnitReadinessFault:
"""A ReadinessFault describes the reason for a unit not to be ready."""

def __init__(self, reason: str, parent=None):
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 experienced a Python noob issue here that I worked around by ignoring type hinting the parent here: I tried adding a type hint parent=UnitReadinessFault | None here but VS Code with mypy running didn't appreciate that. I also tried stringifying it, which made VS Code happy but caused a runtime issue with "str" | None being taken at charm runtime to be the type.

if not self.unit.is_leader():
raise SystemExit(0)
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 moved away from this needless SystemExit(0) business everywhere I saw.

@mz2 mz2 changed the title GitHub workflow for validating terraform deploy GitHub workflow for validating terraform deploy and charm upgrading Dec 12, 2023
@mz2 mz2 changed the title GitHub workflow for validating terraform deploy and charm upgrading GitHub workflow for validating terraform deploy + upgrading charms Dec 12, 2023
@mz2 mz2 marked this pull request as ready for review December 12, 2023 16:42
@mz2 mz2 requested a review from jocave December 12, 2023 18:05
"python.analysis.extraPaths": [
"./backend/charm/lib"
],
"cmake.sourceDirectory": "/home/mz2/Developer/test_observer/frontend/linux"
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you sure about including this path to settings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this was intentional since VS Code looked like it was now doing relative path updates. However, what I committed was the wrong state of the file 🤦

self._update_layer_and_restart(None)

def _on_database_relation_broken(self, event):
self.unit.status = WaitingStatus("Waiting for database relation")
self.unit.status = WaitingStatus("Waiting for database relation after breaking relation")
raise SystemExit(0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you use sys.exit(0) instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Neither should be done, it should just return.

backend/charm/src/charm.py Outdated Show resolved Hide resolved
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