I think it would be good to explicitly clarify what the critical path context actually means. The current description does not help much.

@psss sorry I cannot add more as I added,

+ * `bodhi_update_push_stable_critpath` decision context is used for gating RPM builds in critical path in Bodhi updates before getting to the stable repository.

Asking for guidance from @adamwill or @kevin

Ah actually:

https://fedoraproject.org/wiki/Critical_path_package?rd=Critical_path_packages

So is the proposed gating.yaml format expected to be used only for critical path components or for all? From looking at the config (and without further explanation) I'd say/expect such config enables extra check for critical path components.

@psss tried to add more context

Bodhi queries on the _critpath version of the context if it's querying for an update containing a critical path package. It queries on the 'regular' version of the context if it's querying for an update that does not contain a critical path package.

We enforce this policy in both cases because Fedora CI runs these tests on both critpath and non-critpath packages, and we want to gate both kinds of updates on these tests.

The reason the different contexts exist is that openQA does not test all updates. It only tests updates containing critical path packages (and some other packages based on an allowlist in the scheduler, but that's not relevant here). So we can't just have a policy for openQA's tests that is applied to all updates, because the openQA tests aren't run for all updates. We needed to be able to specify a policy that is only applied to critpath updates.

This was the approach I chose to solve that problem. For reference, the Bodhi PR was https://github.com/fedora-infra/bodhi/pull/4180 and the infra greenwave change was in https://pagure.io/fedora-infra/ansible/c/bb513f59 .

@psss still more context needed?

@adamwill, thanks for the detailed explanation. Just to confirm I understand the change correctly: All critical packages which have tests and gating enabled in dist git now have to update their gating.yaml to contain this new bodhi_update_push_testing_critpath decision context as well. Otherwise results of the Fedora CI testing would not be included in the greenwave decision, right? For packages, which are not on the critical path, the single decision context is just fine?

Thinking a bit more about the approach: If there is a similar change in the future, does it mean that many packages will (again) have to update their individual gating.yaml configs? What if bodhi did the additional check instead? In this way only bodhi would have to be modified to check for both bodhi_update_push_testing and bodhi_update_push_testing_critpath and component owners would not have to change anything?

For the config itself, just a nitpick: The product_versions key is using the multiline list syntax. What about using the same for the decision_contexts as well to make it consistent and a bit more readable?

 --- !Policy
 product_versions:
   - fedora-*
 decision_contexts:
   - bodhi_update_push_testing
   - bodhi_update_push_testing_critpath
 subject_type: koji_build
 rules:
   - !PassingTestCaseRule {test_case_name: fedora-ci.koji-build.tier0.functional}

@lholecek CCing for awareness of the conversation

@psss that's actually a great point. I think you're correct, and we didn't consider that in deciding on the approach for openQA critpath gating. I'll see if it would be practical to query on two decision contexts, I'm not sure if that's an 'expected' approach for greenwave.

Thanks for considering this. I'd say that even two separate calls to greenwave for each of the above mentioned decision contexts (if greenwave does not support both at once) would be much better than having to update hundreds of git repositories.

After the bodhi fix there should be no need to update individual gating.yaml configs with additional decision contexts, thus closing.