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

ci: Add tft plan and workflow #214

Merged
merged 1 commit into from
Jul 25, 2024
Merged

ci: Add tft plan and workflow #214

merged 1 commit into from
Jul 25, 2024

Conversation

spetrosi
Copy link
Collaborator

This change is for running tests in Testing Farm CI. This is a replacement for
BaseOS CI that we are currently using. Running it Testing Farm gives us more
control.

It adds a workflow for running tests, and a plans directory containing a test
plan and a README-plans.md with some info.

Note that this workflow runs from the main branch. This means that changes to
the workflow must be merged to main, then pull requests will be able to run it.
This is because the workflow uses on: issue_comment context, this is a security
measure recommended by GitHub. It saves us from leaking organization secrets.

The functionality is WIP, so await future fixes and updates.

Signed-off-by: Sergei Petrosian [email protected]

@spetrosi spetrosi force-pushed the ci-testingfarm-tests branch 2 times, most recently from fff2842 to 35b995a Compare July 25, 2024 08:13
@tomjelinek
Copy link
Member

plans/README-plans.md states that there are going to be two machines: a controller node and a managed node. Previously, there was just one machine fulfilling both functions. And the single machine didn't have an address usable in the cluster configuration. So the role tests were set to use localhost as a node for the cluster in case ansible_play_hosts_all | length == 1.

If the intention is to separate the controller node and the managed node, this should be addressed in the tests. To be able to do that, the managed node needs to have a domain name which resolves to its IP address, so that these items can be used in the cluster configuration.

__ha_cluster_repos:
- id: rhel-9-for-{{ ansible_architecture }}-highavailability-rpms
name: High Availability
- id: rhel-9-for-{{ ansible_architecture }}-resilientstorage-rpms
Copy link
Member

Choose a reason for hiding this comment

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

Is 'rhel-9' in RHEL 10 config a mistake or a temporary solution? When to switch to rhel-10?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Automation copied vars/RedHat_9.yml into vars/RedHat_10.yml. Can you check if repository for rhel 10 exists?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to 10

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right now, rhel-10-for-x86_64-highavailability-beta-rpms and rhel-10-for-x86_64-resilientstorage-beta-rpms exist. I think the role must identify whether beta version is used by looking into /etc/redhat-release and set repos based on that. I'll do this in a separate PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@richm does it make sense to support beta? Or we can hardcode GA values that will work with RHEL 10 ga?

Copy link
Contributor

Choose a reason for hiding this comment

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

@richm does it make sense to support beta? Or we can hardcode GA values that will work with RHEL 10 ga?

Not sure. Whatever we do, it should work for

  • tox ... -- --image-name rhel-10-beta
  • tox ... -- --image-name rhel-10

and also for downstream testing. Note that in all of these cases, the image/managed node is already configured with the appropriate repos, so we need to ensure that the ha_cluster tasks that enable these repos are skipped.

@spetrosi spetrosi force-pushed the ci-testingfarm-tests branch 2 times, most recently from 20779e3 to 34323fd Compare July 25, 2024 10:16
This change is for running tests in Testing Farm CI. This is a replacement for
BaseOS CI that we are currently using. Running it Testing Farm gives us more
control.

It adds a workflow for running tests, and a plans directory containing a test
plan and a README-plans.md with some info.

Note that this workflow runs from the main branch. This means that changes to
the workflow must be merged to main, then pull requests will be able to run it.
This is because the workflow uses on: issue_comment context, this is a security
measure recommended by GitHub. It saves us from leaking organization secrets.

The functionality is WIP, so await future fixes and updates.

Signed-off-by: Sergei Petrosian <[email protected]>
@spetrosi spetrosi merged commit fd2da6d into main Jul 25, 2024
19 checks passed
@richm
Copy link
Contributor

richm commented Jul 25, 2024

plans/README-plans.md states that there are going to be two machines: a controller node and a managed node. Previously, there was just one machine fulfilling both functions.

In the current (baseos ci) testing, there is a controller node and a managed node. The only difference is that in the testing farm implementation, the test explicitly provisions the controller node. In baseos ci, the controller node is "implicitly provisioned".

However, one of our downstream test scenarios does use a single host for both, and runs with ansible-playbook -c local -i localhost, ... or similar. So the test must run in both scenarios.

And the single machine didn't have an address usable in the cluster configuration. So the role tests were set to use localhost as a node for the cluster in case ansible_play_hosts_all | length == 1.

If the intention is to separate the controller node and the managed node, this should be addressed in the tests.

The test should work in both of the above scenarios.

To be able to do that, the managed node needs to have a domain name which resolves to its IP address, so that these items can be used in the cluster configuration.

Ok, but the test should also run in the -c local scenario too.

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.

3 participants