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

Add test_functional test #460

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

Conversation

BON4
Copy link
Contributor

@BON4 BON4 commented May 3, 2024

Issue #459

Functional Testing. Recovery testing.

All this cases should be covered:

  • Graceful restart. DB will receive SIGTERM and will shutdown properly . Write data, graceful restart instance.
  • Forceful restart. DB will receive SIGKILL and will shutdown forceful, on start charm/patroni should recover. Write data, forceful restart instance (recovery).
  • Backup-restore case with recovery. Write data, create backup, forceful restart, run recovery, write data, restore, check data consistency.

@BON4 BON4 changed the title Functional Testing. Recovery testing. Add test_functional test May 3, 2024
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.06%. Comparing base (0f2c8c2) to head (6deb1d2).
Report is 20 commits behind head on main.

Current head 6deb1d2 differs from pull request most recent head e7e07d7

Please upload reports for the commit e7e07d7 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #460      +/-   ##
==========================================
- Coverage   80.31%   71.06%   -9.25%     
==========================================
  Files          10       11       +1     
  Lines        2301     2772     +471     
  Branches      376      483     +107     
==========================================
+ Hits         1848     1970     +122     
- Misses        369      704     +335     
- Partials       84       98      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BON4 BON4 marked this pull request as ready for review June 6, 2024 20:16
@BON4
Copy link
Contributor Author

BON4 commented Jun 10, 2024

@delgod @marceloneppel @dragomirp Hey team, can I get some review on this one? Thanks.

logger.info("remove application")
for attempt in Retrying(stop=stop_after_delay(15 * 3), wait=wait_fixed(3), reraise=True):
with attempt:
await ops_test.model.remove_application(APPLICATION_NAME, block_until_done=True)
Copy link
Member

Choose a reason for hiding this comment

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

Testing on Juju 3.5.1, I often faced the following error:

unit-postgresql-test-app-0: 14:50:09 ERROR juju.worker.uniter.operation hook "pgdata-storage-detaching" (via hook dispatching script: dispatch) failed: exit status 1
unit-postgresql-test-app-0: 14:55:11 ERROR unit.postgresql-test-app/0.juju-log Uncaught exception while in charm code:
Traceback (most recent call last):
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/./src/charm.py", line 1645, in <module>
    main(PostgresqlOperatorCharm)
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/venv/ops/main.py", line 544, in main
    manager.run()
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/venv/ops/main.py", line 520, in run
    self._emit()
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/venv/ops/main.py", line 509, in _emit
    _emit_charm_event(self.charm, self.dispatcher.event_name)
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/venv/ops/main.py", line 143, in _emit_charm_event
    event_to_emit.emit(*args, **kwargs)
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/venv/ops/framework.py", line 352, in emit
    framework._emit(event)
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/venv/ops/framework.py", line 851, in _emit
    self._reemit(event_path)
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/venv/ops/framework.py", line 941, in _reemit
    custom_handler(event)
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/./src/charm.py", line 432, in _on_pgdata_storage_detaching
    primary = self._patroni.get_primary(unit_name_pattern=True)
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/./src/charm.py", line 705, in _patroni
    self._peer_members_ips,
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/./src/charm.py", line 735, in _peer_members_ips
    addresses = self.members_ips
  File "/var/lib/juju/agents/unit-postgresql-test-app-0/charm/./src/charm.py", line 756, in members_ips
    return set(json.loads(self._peers.data[self.app].get("members_ips", "[]")))
AttributeError: 'NoneType' object has no attribute 'data'
The test got stuck at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have been testing it for almost a week, and can't reproduce this error for juju 3.5.1 nor for 3.4 and 3.3. Can you elaborate and help me with this one? What version of lxd are you using? Charmcraft, Snapcraft?

Copy link
Member

Choose a reason for hiding this comment

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

Sure.

  • Juju: 3.4.4
  • LXD: 5.21.1-d46c406
  • Charmcraft: 2.7.0
  • Snap: 2.63+24.04

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you test on Juju 3.5.1? Like it was in original issue by you?
I want to know version of LXD, Charmcraft, and Snap on witch this error is reproducible.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. I tested again with Juju 3.5.1. Those are the version I used:

  • Juju: 3.5.1
  • LXD: 5.21.1-d46c406
  • Charmcraft: 2.7.0
  • Snap: 2.63+24.04

I also copied this branch to another PR to test it. The error is the following: https://github.com/canonical/postgresql-operator/actions/runs/9881101326/job/27384074248#step:28:2995

with attempt:
assert await is_postgresql_ready(ops_test, primary_name)

# Get unit hostname and IP.
Copy link
Member

Choose a reason for hiding this comment

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

Is it really retrieving the IP or only the hostname?

with attempt:
assert await is_postgresql_ready(ops_test, primary_name)

# Get unit hostname and IP.
Copy link
Member

Choose a reason for hiding this comment

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

I have the same question from above regarding retrieving the IP.

Too high cost on GitHub-hosted ARM runners

Will try to re-enable on nightly only if costs are low enough, but disabling all for now so that new PR runs do not create further costs
@carlcsaposs-canonical
Copy link
Contributor

@BON4 Due to billing, we needed to disable arm64 integration tests on this repository. main has already been updated, but to ensure that this PR does not accidentally run integration tests on arm64, I cherry-picked a commit from main (that disables arm64 integration tests) and forced push it to your PR branch

Please update your local branch & ensure that future pushes to this PR branch include that commit that disables arm64 integration tests

Please let us know if you have any questions, and if you'd be willing, please let us know when you've updated your local branch

Thank you! Sorry for the inconvenience

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.

4 participants