Fully Replace Testing Farm Packit Integration with ATEX integration

[ggbecker]

Description:

• Fully Replace Testing Farm Packit Integration with ATEX integration
• This makes use of the newly introduce upstream plan in the contest repository: add /plans/upstream to be used by CaC/content CI RHSecurityCompliance/contest#516

Rationale:

• All the tests executed with packit integration are now part of the ATEX integration whenever it makes sense to port.

Review Hints:

• These tests won't run on this pull request as this workflow is set with the workflow_run tag:

on:
  workflow_run:
    workflows: ["ATEX - Build Content"]
    types:
      - completed

Because of token potential exposures.

TODO LIST

• Make https://github.com/ComplianceAsCode/content/blob/master/tests/run_tests_testingfarm.py#L124 return fail if any of the tests executed failed.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[github-actions]

ATEX Test Results

Test artifacts have been submitted to Testing Farm.

Results: View Test Results
Workflow Run: View Workflow Details

This comment was automatically generated by the ATEX workflow.

[comps]

I don't think you can have a section like this in YAML - it's not just a template, it's a real section that can be referenced via YAML features later, but it still needs to be a valid section.

So I would just copy/paste the job, trigger and fmf_path to the remaining two sections and drop this "template". We will probably deal with rpmbuild-ctest-fedora and fedora-cis in later PRs, moving/changing them further.

Otherwise LGTM, 10 remotes (run_tests_testingfarm.py argument default) per CentOS Stream version should be fine.

[ggbecker]

something like this?

downstream_package_name: scap-security-guide
upstream_package_name: scap-security-guide
specfile_path: scap-security-guide.spec

actions:
  get-current-version:
  - bash utils/version.sh

srpm_build_deps:
  - bash

jobs:
- job: copr_build
  trigger: pull_request
  targets:
  - fedora-all-x86_64

- job: copr_build
  trigger: commit
  branch: "gh-readonly-queue/.*"
  targets:
  - fedora-all-x86_64

- job: tests
  trigger: pull_request
  fmf_path: tests/tmt
  identifier: /rpmbuild-ctest-fedora
  tmt_plan: /plans/contest/rpmbuild-ctest-fedora$
  targets:
    fedora-all: {}

- job: tests
  trigger: pull_request
  fmf_path: tests/tmt
  identifier: fedora-cis
  tmt_plan: /plans/fedora-cis$
  targets:
    fedora-all: {}

[ggbecker]

btw the previous format proposed in this PR seemed to work as it was a valid packit config and the tests ran correctly. I'm not sure if you are talking about some other step not seen here. But I basically followed the previous structure of the file with the templating part.

[comps]

Hmm, then maybe Packit has added some check like "if the config block doesn't have identifier, ignore it", allowing templating.

But, otherwise, the original code that started with - &test-static-checks was a valid test (or rather a series of tests, everything under /static-checks), it wasn't just a template.

My point is that YAML allows to template sections (using "anchors" / "aliases") like this:

vars:
   service1:
       config: &service_config
           env: prod
           retries: 3
           version: 4.8
   service2:
       config: *service_config
   service3:
       config: *service_config

where all services receive the same config, but service1 is still a valid service, you cannot create a service_template that defines config but otherwise does nothing, and then use it for service1, service2 and service3.

So the original /static-checks code worked, because it was a valid test specification aside from being used as a template later on, but you then removed its identifier / tmt_plan / etc., resulting in an invalid test specification.

...
At least that's what I thought before I looked at the raw file and saw I misread the diff. You're right in that the code would work, because it merges the template-ish definition with /rpmbuild-ctest-fedora:

- &test-static-checks
  job: tests
  trigger: pull_request
  fmf_path: tests/tmt
  identifier: /rpmbuild-ctest-fedora
  tmt_plan: /plans/contest/rpmbuild-ctest-fedora$
  targets:
    fedora-all: {}

- <<: *test-static-checks
  identifier: fedora-cis
  tmt_plan: /plans/fedora-cis$

I guess I got confused by the &test-static-checks because the section doesn't have anything to do with /static-checks anymore, so the alias name doesn't make sense. Sorry.

[comps]

something like this?

Either would work, I just misunderstood your original code, sorry.

If you decide to leave it as-is, please at least rename the anchor, so it doesn't say "static checks".

[ggbecker]

okay, I will rename the section

[ggbecker]

Done by amending the last commit. 8ed1f08

[comps]

One more thing I'm not sure - this might impact Packit RPM building as well. I'm not sure if we want to do that as the building provides at least some level of RPM-buildability-checking.

Maybe just removing these from job: tests targets below is sufficient?

[ggbecker]

Maybe just removing these from job: tests targets below is sufficient?

I'm not sure if I understand this part. I guess if we just revert this deletion:

  - centos-stream-8-x86_64
  - centos-stream-9-x86_64
  - centos-stream-10-x86_64

we should have the build for centos stream as well.

[ggbecker]

let me try that

[comps]

I'm not sure if I understand this part. I guess if we just revert this deletion:

That's what I meant - that the existing changes already removed the centos-stream-* targets from the job: tests block, so re-adding the job: copr_build targets back (as you did) should be sufficient. 🙂

[comps]

Otherwise the PR LGTM, thanks!

[ggbecker]

As soon as we merge this we need to flip the switches on the "Required" checks list in the protected branches, to remove the packit jobs that won't exist anymore, and add the Gating / ATEX - Test and Upload Results.

[ggbecker]

I think this can wait a bit longer until we manage to make the ATEX script to exit non-zero when there is at least one failing test. Otherwise we won't be able to gate the PR properly, it would always report pass even if a test ended with failed status.

[ggbecker]

This seems to be working well when reporting the failures, see: ggbecker#46

The check reported fail when the testing farm ended with some of the tests ended with fail

At the same time, there seems to be some failing tests that were unexpected. We will need to investigate this first.