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

[test_operator] Common default values for container images #2594

Conversation

eduolivares
Copy link
Contributor

test-operator currently supports 4 test frameworks: tempest, tobiko, horizontests and ansibletests. The relevant variables required to download their corresponding container images had to be configured separately, but usually were configured with common values. With this PR, default values for registry, namespace and tag can be configured and they apply to the 4 test frameworks.

Copy link

Thanks for the PR! ❤️
I'm marking it as a draft, once your happy with it merging and the PR is passing CI, click the "Ready for review" button below.

@github-actions github-actions bot marked this pull request as draft December 11, 2024 10:50
@eduolivares eduolivares marked this pull request as ready for review December 11, 2024 10:50
@eduolivares eduolivares requested a review from lpiwowar December 11, 2024 10:57
Copy link
Contributor

@lpiwowar lpiwowar left a comment

Choose a reason for hiding this comment

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

@eduolivares if we want to keep this in the defaults can you update the README.md please?

Otherwise looks good to me!:) 👍

@lpiwowar
Copy link
Contributor

lpiwowar commented Dec 12, 2024

Also, cc @eshulman2

Do you think Ella this will break your PR? -> #2374

I think we should wait with merging this before Ella tells us her opinion.

@eduolivares eduolivares force-pushed the test-operator-unify-container-images branch from fe3aa32 to 9334840 Compare December 12, 2024 11:04
@eduolivares eduolivares force-pushed the test-operator-unify-container-images branch from 9334840 to ed9296a Compare December 12, 2024 11:12
@eduolivares
Copy link
Contributor Author

Also, cc @eshulman2

Do you think Ella this will break your PR? -> #2374

I think we should wait with merging this before Ella tells us her opinion.

I have taken a look at #2374 and wow, it's a big change.
However, I think #2594 won't affect much to it. Because #2374 will resolve any conflicts during task "Overwrite global_vars with stage_vars".
But, yes, let's see what @eshulman2 thinks before approving #2594

Copy link
Contributor

@lpiwowar lpiwowar left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍

Only one small change request in the README as the newly introduced parameters cifmw_test_operator_default_* can not be overridden inside of cifmw_test_operator_stages.

roles/test_operator/README.md Show resolved Hide resolved
test-operator currently supports 4 test frameworks: tempest, tobiko,
horizontests and ansibletests. The relevant variables required to
download their corresponding container images had to be configured
separately, but usually were configured with common values.
With this PR, default values for registry, namespace and tag can be
configured and they apply to the 4 test frameworks.
@eduolivares eduolivares force-pushed the test-operator-unify-container-images branch from 995bf41 to e5b89b8 Compare December 17, 2024 09:16
Copy link
Contributor

@lpiwowar lpiwowar left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍

@lpiwowar
Copy link
Contributor

/lgtm

@pablintino
Copy link
Collaborator

/approve

Copy link
Contributor

openshift-ci bot commented Dec 17, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pablintino

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/e1ced0240524448ab8cda3263320b790

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 14m 52s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 20m 47s
cifmw-crc-podified-edpm-baremetal FAILURE in 1h 31m 26s
cifmw-multinode-tempest FAILURE in 1h 44m 21s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 8m 00s
✔️ cifmw-pod-pre-commit SUCCESS in 7m 50s
✔️ build-push-container-cifmw-client SUCCESS in 37m 48s
✔️ cifmw-molecule-test_operator SUCCESS in 3m 33s

@lpiwowar
Copy link
Contributor

recheck

@openshift-merge-bot openshift-merge-bot bot merged commit 5731e27 into openstack-k8s-operators:main Dec 17, 2024
5 checks passed
@eduolivares eduolivares deleted the test-operator-unify-container-images branch December 18, 2024 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants