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

Modify runcontainer to support running multiple containers simultaneously #129

Closed
wants to merge 1 commit into from

Conversation

nhosoi
Copy link
Contributor

@nhosoi nhosoi commented Jun 21, 2023

This patch adds 3 options:

  • --containers <NUM>: specifies the number of containers. Default to 1.
  • --exclude <TEST PATH>: specifies a test to skip. Default to none. E.g., --exclude tests/tests_default.yml --exclude tests/tests_relp.yml
  • --extra-rpm <RPM>: specifies an additional rpm package to install in the container. Default to none. E.g., --extra-rpm diffutils --extra-rpm sudo

Note:
This is a sample command line to run the logging CI tests on 4 centos-9 containers.

tox -e container-ansible-core-2.15 -- --image-name centos-9 \
--exclude tests_basics_files.yml --exclude tests_ovirt_elasticsearch.yml --exclude tests_relp.yml \
--extra-rpm diffutils --containers 4 tests/tests_*.yml

This command-line excludes tests_basics_files.yml , tests_ovirt_elasticsearch.yml, and tests_relp.yml since these tests fail as they execute selinux role internally. It requires the diffutils rpm package since some tests call diff in it.

Question:
Each role has its own required options for --exclude and --extra-rpm as shown above for the logging role. Where shall I put the information? In each role's README.md or README.md in tox-lsr?

…usly

Add 3 options:
- --containers <NUM>: specifies the number of containers. Default to 1.
- --exclude <TEST PATH>: specifies a test to skip. Default to none. E.g.,
      --exclude tests/tests_default.yml --exclude tests/tests_relp.yml
- --extra-rpm <RPM>: specifies an additional rpm package to install in
      the container. Default to none. E.g.,
      --extra-rpm diffutils --extra-rpm sudo
@richm
Copy link
Contributor

richm commented Jun 21, 2023

This patch adds 3 options:

* `--containers <NUM>`: specifies the number of containers. Default to 1.

* `--exclude <TEST PATH>`: specifies a test to skip. Default to none. E.g., `--exclude tests/tests_default.yml --exclude tests/tests_relp.yml`

* `--extra-rpm <RPM>`: specifies an additional rpm package to install in the container. Default to none.  E.g., `--extra-rpm diffutils --extra-rpm sudo`

Note: This is a sample command line to run the logging CI tests on 4 centos-9 containers.

tox -e container-ansible-core-2.15 -- --image-name centos-9 \
--exclude tests_basics_files.yml --exclude tests_ovirt_elasticsearch.yml --exclude tests_relp.yml \
--extra-rpm diffutils --containers 4 tests/tests_*.yml

This command-line excludes tests_basics_files.yml , tests_ovirt_elasticsearch.yml, and tests_relp.yml since these tests fail as they execute selinux role internally. It requires the diffutils rpm package since some tests call diff in it.

Question: Each role has its own required options for --exclude and --extra-rpm as shown above for the logging role. Where shall I put the information? In each role's README.md or README.md in tox-lsr?

The test should use Ansible tags to exclude tests, like we do for downstream testing. For example, in kernel settings, we cannot run this test in basic smoke test because it will reboot the machine, so we use tag tests::reboot, then the test runner can use ansible-playbook --skip-tags tests::reboot .... I suggest we use a tag tests::uses_selinux for those tests we cannot run in a container because they use selinux. Then if we find tests that cannot run in a container because of some other reason, we can use tests::uses_something. I would rather do this than simply say tests::no_container. I know my wip uses no_container but I now think that isn't a good idea https://github.com/richm/tox-lsr/blob/container-improvements/src/tox_lsr/test_scripts/runcontainer.sh#L13

As far as additional packages to install - this should be done in tests/setup-snapshot.yml - there should be a section in there for test packages e.g. in logging: https://github.com/linux-system-roles/logging/blob/main/tests/setup-snapshot.yml#L16 - just add diffutils to the list of test packages. My wip had support for installing test packages from setup-snapshot.yml - https://github.com/richm/tox-lsr/blob/container-improvements/src/tox_lsr/test_scripts/runcontainer.sh#L221

@@ -115,6 +115,9 @@ refresh_test_container() {
prepkgs="dnf-plugins-core" ;;
*) pkgcmd=dnf; prepkgs="" ;;
esac
for rpm in $EXTRA_RPMS; do
initpkgs="$rpm $initpkgs"
done
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is ok to leave this, but in general, the test framework will install extra packages from tests/setup-snapshot.yml

CONTAINER_COUNT="$1" ;;
--exclude)
shift
EXCLUDES+=("$1") ;;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is ok to leave this, but in general, the test framework will use --skip-tags ... to exclude tests that cannot be run in containers.

@nhosoi
Copy link
Contributor Author

nhosoi commented Jun 26, 2023

Closing this PR in favor of richm#1.

@nhosoi nhosoi closed this Jun 26, 2023
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