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.

Sign up for GitHub

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

Jump to bottom

[DPE-4669] Test: Scale to zero units #509

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

[DPE-4669] Test: Scale to zero units #509

wants to merge 3 commits into from

Conversation

BalabaDmitri
Copy link
Contributor

Issue #508

Solution

Test coverage of the following cases:

  1. Scaling with and without storage.
  2. Whether writes are increasing.
  3. The shutdown of units after scale to zero.
  4. Scaling up to 1 unit uses PVC with the other cluster's storage.
  5. Remove the unit with another cluster's storage.
  6. Restore own hostpath in PVC.
  7. Scaling up to 1 and checking test data.
  8. Scaling up to 3 and checking test data.

Implementation

  1. Processing behaviour when using another cluster storage.

@BalabaDmitri BalabaDmitri force-pushed the scaling_to_zero branch 2 times, most recently from cb77a05 to 95ed5b5 Compare June 13, 2024 16:18
@lucasgameiroborges
Copy link
Member

Hi @BalabaDmitri !
Could you please fix the linting issues so we can see the integration test running on CI? Thanks!

marceloneppel
marceloneppel requested changes Jun 14, 2024
View reviewed changes
Copy link
Member

@marceloneppel marceloneppel left a comment

Choose a reason for hiding this comment

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

Great work, @BalabaDmitri!

tests/integration/ha_tests/helpers.py
connection_string: Database connection string
"""
with psycopg2.connect(connection_string) as connection:
connection.autocommit = True
Copy link
Member

Choose a reason for hiding this comment

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

I believe this is not needed when you have a read-only statement only.

tests/integration/ha_tests/test_self_healing.py
Comment on lines +493 to +494
logger.info("scaling to one unit")
await scale_application(ops_test, app, 1)
Copy link
Member

Choose a reason for hiding this comment

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

This is a great addition to the tests (I mean scaling from 0 to 1 unit and later from 1 to 3 units to ensure it works after this sequence of operations).

What I see missing is the previously checked situation, which scales from 0 to 3 units.

src/charm.py
Comment on lines +829 to +830
if self._patroni.cluster_system_id_mismatch(unit_name=self.unit.name):
self.unit.status = BlockedStatus(THIRD_PARTY_STORAGE_MESSAGE)
Copy link
Member

Choose a reason for hiding this comment

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

Question: this blocked status can only be resolved by removing the unit and fixing the storage.

If so, I think we can return directly without calling event.defer().

@taurus-forever taurus-forever mentioned this pull request Jun 17, 2024
Open
@lucasgameiroborges
Copy link
Member

This PR seems stuck for a while, is it still under development? @BalabaDmitri

lucasgameiroborges
lucasgameiroborges reviewed Oct 31, 2024
View reviewed changes
Copy link
Member

@lucasgameiroborges lucasgameiroborges left a comment

Choose a reason for hiding this comment

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

Should we close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucasgameiroborges lucasgameiroborges lucasgameiroborges left review comments

@marceloneppel marceloneppel marceloneppel requested changes

@dragomirp dragomirp Awaiting requested review from dragomirp

@taurus-forever taurus-forever Awaiting requested review from taurus-forever

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BalabaDmitri @lucasgameiroborges @marceloneppel