#348 Don't attempt to make decisions from old-style compose fedmsgs
Merged by ralph. Opened by adamwill.
adamwill/greenwave compose-decision-fix  into  master

Download 348.patch

This is effectively a reversion of 414d392 (PR #262). That
attempted to 'fix' greenwave to make compose decisions based on
the old-style taskotron.result.new messages with type 'compose'.
In fact, however, it was intentional that greenwave does not
do this, because it is not possible to do it reliably. These
references have the back story:

https://pagure.io/greenwave/issue/122
https://pagure.io/taskotron/resultsdb/issue/92
https://pagure.io/taskotron/resultsdb/pull-request/101
https://pagure.io/greenwave/pull-request/262#comment-70350

For compose tests, Autocloud reports the 'item' as the image it
is testing. Some openQA tests report the 'item' as the image
under test, and some report the 'item' as the compose ID. Thus
you can never actually find all the results for a given compose
by searching for a given 'item'. In order to find all results
for a given compose, you must search for results with the extra
data key 'productmd.compose.id' set to the compose ID, because
all compose results do have that.

Old-style 'taskotron.result.new' fedmsgs, however, do not have
the extra data keys; they only include the 'item', 'type' and
'name', so far as identifying information goes. For a message
like the one added as a test in the initial commit (and kept as
a test here, with a different expected outcome), where the item
is not the compose ID, we simply cannot possibly discover all
the other relevant results. If the 'item' happens to be the
compose ID, we could use it to find all relevant results with a
ResultsDB query for productmd.compose.id (though we still
couldn't find all relevant results in the message cache), but
that doesn't really seem worth trying to support.

The new-style 'resultsdb.result.new' fedmsgs that were added
by https://pagure.io/taskotron/resultsdb/pull-request/101 do
always include the extra data. So we can reliably find all
relevant results if we're working with that message format, and
that's what we were actually intending to do. We just never, it
seems, flipped the Fedora greenwave instance over to using the
new message format instead of the old one.

So in the end we should just revert the change, which restores
the correct behaviour of only attempting to handle compose
decisions when we have productmd.compose.id, and add tests for
the intended behaviour and better comments, so no-one ever has
to go through this again! I will also try and get the Fedora
deployment's config changed so it works from new-style messages
instead of old-style messages; if we do this, it'll actually
publish useful compose decisions.

Signed-off-by: Adam Williamson awilliam@redhat.com

@lholecek @gnaponie

That seems reasonable.
It looks fine with messages with the topic "resultsdb.result.new".
I would prefer it to work also with the old topic, but I see the point. It would be nice if you would manage to change the Fedora config to use the new topic, so we can gate also for composes (in a correct way).

Can also @ralph have a look? Thanks

Thanks for the detective work and providing all the context @adamwill. LGTM! :+1:

Commit 6a0d9f96 fixes this pull-request

Pull-Request has been merged by ralph

Pull-Request has been merged by ralph

@gnaponie, is there an upcoming greenwave release when @adamwill can expect to see this in prod?

It should be in stg shortly.

@ralph I might release it today (if I have time - I think yes).
Anyhow I'll send an email to infrastructure@lists.fedoraproject.org before doing that.

Any news? It seems 0.9.12 was cut, and pushed to the stream that staging greenwave uses, but the 'prod' stream that production greenwave uses hasn't changed and is still, i think, 0.9.9.

Thanks!

Hello Adam, sorry I forgot to put a comment here.
I didn't continue with the release because of that: https://pagure.io/fedora-infrastructure/issue/7111#comment-545709
I think actually the release won't break anything, but I wanted to be sure before release that. I've put a new comment asking for a confirmation again.

Thank you

Hello, we released Greenwave new version. This should be in production now.

Metadata