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

Integration tests for COS integration for OpenStack #277

Merged

Conversation

cbartz
Copy link
Collaborator

@cbartz cbartz commented May 6, 2024

Overview

Follow-up to #266 to provide integration tests running on the openstack cloud.

An integration test related to COS and repo policy compliance will be provided in a follow-up PR.

The OpenStack tests are not yet included in the CI, as there is another PR working on getting Openstack to work on the CI: #269

This PR does also refactor the helper modules. Due to parallel development on the openstack integration tests, the other integration tests have not been tested to see if they still work and this will need to be investigated in a follow-up.

The openstack applicable tests are marked now with an openstack pytest mark.

This PR also fixes/adds:

  • Private ssh keys have now correct permission and are owned by ubuntu user (to avoid ssh permission warning)
  • RunnerStop metric is also issued on abnormal termination
  • More context on pull metrics file errors (add a warning log that the metrics file size could not be determined)

Rationale

Add integration tests to ensure proper functioning.

Juju Events Changes

n/a

Module Changes

A separate package has been introduced and the helper modules have been placed there. There is a module for LXD and Openstack that contains cloud specific code. An interface InstanceHelper has been defined which is implemented for both clouds. It is required to handle cloud specific helper code as run_in_instance' or ensure_one_runner' or `get_runner_name'.

The test_charm_metrics_{success,failure} tests have been adapted and the openstack applicable tests are marked with an openstack mark.

Beyond that:

  • openstack_manager: Now writes ssh key with correct permission and ubuntu user
  • openstack_userdata.j2: Fix case where write_post_metrics function is not called on non-zero exit code of previous command

Library Changes

Checklist

cbartz added 30 commits April 25, 2024 17:29
ValidationError inherits from ValueError
…ack-repo-policy-ISD-1825

# Conflicts:
#	src/openstack_cloud/openstack_manager.py
@cbartz cbartz changed the title [WIP] Integration tests for COS integration for OpenStack Integration tests for COS integration for OpenStack May 7, 2024
@cbartz cbartz marked this pull request as ready for review May 7, 2024 21:01
@cbartz cbartz requested a review from a team as a code owner May 7, 2024 21:01
Copy link
Collaborator

@yhaliaw yhaliaw left a comment

Choose a reason for hiding this comment

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

Some comments.

.github/workflows/e2e_test_run.yaml Show resolved Hide resolved
templates/openstack-userdata.sh.j2 Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
Copy link
Collaborator

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

Reviewed until helpers/lxd.py, cool stuffs!

.github/workflows/e2e_test_run.yaml Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/conftest.py Show resolved Hide resolved
tests/integration/helpers/common.py Outdated Show resolved Hide resolved
tests/integration/helpers/lxd.py Show resolved Hide resolved
tests/integration/helpers/lxd.py Show resolved Hide resolved
tests/integration/helpers/lxd.py Outdated Show resolved Hide resolved
tests/integration/helpers/lxd.py Outdated Show resolved Hide resolved
@yhaliaw
Copy link
Collaborator

yhaliaw commented May 8, 2024

@yanksyoon I think you can merge this, when you approve it.

Copy link
Collaborator

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the changes!

@yanksyoon yanksyoon merged commit 01f9f9b into feat/openstack-integration May 9, 2024
3 of 9 checks passed
@yanksyoon yanksyoon deleted the feat/integration-test-cos-ISD-1824 branch May 9, 2024 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants