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 ceph osd tests #93

Merged

Conversation

n-pochet
Copy link
Contributor

@n-pochet n-pochet commented Aug 8, 2018

The goal is to create functional tests that matches the Amulet tests
that are currently used for testing.

  • Add low-level ceph-osd test class to gather the ceph-osd's specific
    tests (processes, services, ...)
  • Add utilities that matches the ones in charm-helpers
  • Add unit tests

@n-pochet n-pochet force-pushed the feature/add-ceph-osd-tests branch 2 times, most recently from fbd2e89 to 8652927 Compare August 9, 2018 08:55
Copy link
Collaborator

@fnordahl fnordahl left a comment

Choose a reason for hiding this comment

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

A lot of great work in progress here! Added a few comments/questions

@@ -151,3 +153,208 @@ def get_yaml_config(config_file):
# the pwd.
logging.info('Using config %s' % (config_file))
return yaml.load(open(config_file, 'r').read())


def get_process_id_list(unit_name, process_name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a question: Have you seen

async def async_block_until_service_status(unit_name, services, target_status,
example usage here
model.block_until_service_status(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_process_id_list will just output the list of PID(s) for a specific process.
But thanks for pointing me to that function. It will help me to simplify test_services.


Verify the expected services are running on the service units.
"""
services = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What you in effect are doing here is cinder and glance charm tests in the ceph charm test.

This is one of the Amuletisms we want to avoid in Zaza. What we could do instead is pull in relevant config or test snippets in the tests.yaml that in effect would validate that the other services are working as they should.

An example for the keystone charm which by pulling in a glance config function in effect validates that the glance services are running, that it successfully related itself to keystone etc etc:
https://review.openstack.org/#/c/589832/2/tests/tests.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I will move that away.
I was indeed just copy/pasting the amulet tests.
Thanks!

@n-pochet n-pochet force-pushed the feature/add-ceph-osd-tests branch 27 times, most recently from 4f1b8db to 8bff8c3 Compare August 16, 2018 08:52
@n-pochet n-pochet force-pushed the feature/add-ceph-osd-tests branch 3 times, most recently from 3628bd1 to 8ac818f Compare September 21, 2018 19:34
sleep_time = 30
retry_count = 30
file_mtime = None
time.sleep(sleep_time)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@n-pochet this should now be unnecessary. When you added the config_change contextmanager your test code will not be executed until after the config has been applied and all units are idle and thus the initial sleep is no longer needed. Please remove it.

retry_sleep_time = 10
with self.config_change(set_default, set_alternate):
with tempfile.TemporaryDirectory() as tempdir:
while tries <= retry_count and not file_mtime:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any situation where ceph-osd unit will appear idle and ready before the crypt setup has finished after the config change? If not I believe the retry logic should be removed too.

n-pochet added a commit to n-pochet/charm-ceph-osd that referenced this pull request Oct 9, 2018
Depends-On: openstack-charmers/zaza#93
Change-Id: I9a19960fdb239eb5a8d421f135285e89b8405267
The goal is to create functional tests that matches the Amulet tests
that are currently used for testing.

* Add low-level ceph-osd test class to gather the ceph-osd's specific
tests (processes, services, ...)
* Add utilities that matches the ones in charm-helpers
* Add unit tests
* Add functional test
* Add helper functions
* Add unit tests
Patch `zaza.utilities.generic.model.run_on_unit`
The name first_unit was removed and changed to lead_unit.
The output of the `ceph osd lspools` can be:
* 1 glance,2rbd, or
* 1 glance
  2 rbd
If the unit is in `ready` state, it should also have configured
everything correctly. There's therefore no need for a retry logic.
@fnordahl fnordahl merged commit b399694 into openstack-charmers:master Oct 10, 2018
openstack-gerrit pushed a commit to openstack/charm-ceph-osd that referenced this pull request Oct 10, 2018
Depends-On: openstack-charmers/zaza#93
Change-Id: I9a19960fdb239eb5a8d421f135285e89b8405267
coreycb pushed a commit to coreycb/zaza that referenced this pull request Oct 17, 2023
No origin for designate-bind series upgrade
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.

None yet

2 participants