#74 Resolve Fedora relevance_key policy hacks
Closed: Fixed 5 years ago Opened 6 years ago by ralph.

OK - In order to get the "demo" ready for Flock, we released some unmerged patches that we're not quite happy with:

  • One is the relevance_key and relevance_value which feels wooden and insufficiently generic to me.
  • One is the FedoraAtomicCIRule which is super custom and is there basically just to support a whitelist of packages.

So - let's figure how how we want to solve these two things. Here, let's take the first of the two.

Proposal, this can be put on solid footing if we add a new kind of YAMLObject that isn't a Rule, but is a new type called, say, Condition.

Condition is abstract and must be extended. To achieve parity with the awkward relevance_key and relevance_value, we will need something like a SubjectCondition that accepts arguments in the policy itself about how the policy should or should not apply to a specific subject entry.


See also #75 for the bit about the FedoraAtomicCIRule.

/cc @pingou, @bowlofeggs, @mjia, @dcallagh.

So ultimately the problem is that right now, we have got Bodhi asking Greenwave for a decision about:

{"type": "koji_build", "item": "packageA-1.1-1"}
{"type": "koji_build", "item": "packageB-2.0-1"}
{"type": "bodhi_update", "item": "FEDORA-2017-7e594f96bb"}

The two builds are the builds in the update. Bodhi knows this. But Greenwave does not.

It was really a mistake in our original API design that we would just have Bodhi lump these in together when it asks for a decision. The decision is not about the two packages and the update. The decision is about the update which consists of the two packages. But that structure is not reflected in the subject when Bodhi asks for a decision.

(I'm ignoring the original_spec_nvr thing for now because I hope it can go away soon.)

The tests we expect to be run on individual builds are totally disjoint from the tests we expect to be run on the update as a whole. It's almost like there are two different decision contexts -- "are the set of packages in this update ready to be pushed stable?" and "is the update itself ready to be pushed stable?"

(I'm also bypassing the question of why we test an "update" and not just the set of packages in the update... This feels to me like a mistake in how OpenQA is reporting its results but that ship has sailed I think.)

Or if it's not two different decision contexts, then at least two different subjects -- a set of packages, and an update. With a separate policy applied to each. In fact that is what the rejected PR #63 is effectively doing. One policy defines the tests required for a build -- a second policy defines the tests required for an update. There's nothing wrong with having small, granular policies. We foresaw that, it's why Greenwave applies the union of all policies.

So is the issue with relevance_key and relevance_value just that it feels like a strange and awkward way of specifying which types of items a policy applies to?

It feels like the alternative proposal here (with Condition) is just trying to achieve the same thing but pushing the decision down into the individual rules, so that we can have a single policy instead of two? -- even though there is nothing wrong with having two.

What if we tackled the OpenQA side of things, so that we don't have to worry about results for "type": "bodhi_update" anymore?

Let's assume OpenQA is testing an update by just installing the builds in the update. (I think that's how it works but I honestly need to check.) It doesn't test each build individually because often they might not be individually installable, because one build in the update depends on the other and they are expecting to be pushed together. So that's fine.

But what if instead of reporting its results against the update, it reports them against the set of build NVRs that it installed?

We then arrive at a slightly different problem. The results wouldn't be for "type": "koji_build" because it's really a set of builds, not a single build. We could imagine storing the results somehow like:

{"type": "koji_builds", "item": "packageA-1.1-1,packageB-2.0-1"}

Could we then make Greenwave smart enough to find such results and consider them against the policy?

Is it maybe inevitable that Greenwave learn about the concept of "updates" -- in the sense of "a set of packages which get tested and shipped together with each other"? Right now we are kind of trying to make that Bodhi's problem and have Greenwave not know about it.

(There is another related problem which we haven't dealt with yet. Internally we have "type": "koji_build_pair" and Greenwave will probably need to know something about this structure as well, in order to query the right results.)

(I'm ignoring the original_spec_nvr thing for now because I hope it can go away soon.)

Could you expand on your ideas for this then? :)

Okay, our answer to this is #126 / PR#184.

The relevance_key / relevance_value attributes in the policy are replaced with a subject_type attribute, to indicate the subject type which the policy applies to (koji_build, bodhi_update, compose).

So both relevance_key: original_spec_nvr and relevance_value: koji_build become subject_type: koji_build. Internally when Greenwave is asked for a decision about subject type koji_build it will look for either results from Taskotron with item or results from the Atomic CI pipeline with original_spec_nvr. And if necessary we can teach Greenwave other ways of finding results for a build (for example, internally I think we have brew-build as an alias for koji_build so we will need to teach Greenwave about that soon).

Metadata Update from @dcallagh:
- Issue assigned to dcallagh

5 years ago

Metadata Update from @dcallagh:
- Issue set to the milestone: 0.8

5 years ago

Metadata Update from @lholecek:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)
- Issue tagged with: enhancement

5 years ago

Necro-ing this a bit:

"Let's assume OpenQA is testing an update by just installing the builds in the update. (I think that's how it works but I honestly need to check.) It doesn't test each build individually because often they might not be individually installable, because one build in the update depends on the other and they are expecting to be pushed together. So that's fine.

But what if instead of reporting its results against the update, it reports them against the set of build NVRs that it installed?"

I don't see how that's better. To me there is nothing wrong with what openQA is doing. The update is the thing that we are interested in all along. It is not the builds. The openQA update tests aren't unit tests or functionality tests for the individual packages; they are integration tests for the update as a whole. So to me it makes perfect sense that, all through the process, openQA treats the update as the thing-under-test: the update ID is used as the identifier for the thing-under-test in openQA itself, in the fedmsgs it sends out, and in the results it sends to resultsdb. The job openQA is doing here is to say "this update, whatever it consists of, does or does not break these significant things you can do with Fedora".

Note that bodhi_update is an official 'artifact' type in the CI messages spec, so that agrees with me. :)

Anyway, that aside - I think the changes here caused greenwave to completely ignore openQA results :( And that in turn means Bodhi no longer shows them in its GUI, now that it doesn't query resultsdb directly but relies on greenwave to tell it what test results are available.

I am trying to figure out how to fix this now. Presumably I need to add a policy to the Fedora config with subject_type: bodhi_update, or something. Note that the actual production Fedora policy seems to use both subject_type and relevance_value, I assume this was to make it compatible with both old and new greenwave?

OK, so it more or less turns out that Fedora's Greenwave policy just never really considered openQA results (as we never tried to gate on them), and all this change meant is the way to add them was a bit different.

I've added them to the infra policy, and I also removed all the relevance_* lines as they're no longer needed since this change is in both staging and production. With that change, Bodhi is showing openQA results now...yay.

Login to comment on this ticket.

Metadata