On Thu, Dec 06, 2018 at 12:13:59PM -0800, Adam Williamson wrote:
On Thu, 2018-12-06 at 19:23 +0100, Pierre-Yves Chibon wrote:
> On Thu, Dec 06, 2018 at 01:16:43PM +0100, Miroslav Vadkerti wrote:
> > Hi
> > On Wed, Dec 5, 2018 at 9:23 PM Adam Williamson
> > <adamwill(a)fedoraproject.org> wrote:
> >
> > On Wed, 2018-12-05 at 12:16 -0800, Adam Williamson wrote:
> > >
> > > To be clear, I can see why the pipeline might want to *emit a fedmsg
> > on
> > > its own topic* saying that it ignored a package. What doesn't
seem to
> > > be necessary is to *submit a result to ResultsDB* which looks like
> > > this:
> > >
> > >
https://taskotron.fedoraproject.org/resultsdb/results/25608385
> > >
> > > it's kind of a bizarre result, because it sort of looks like a
test
> > > passed, but really it just means...there was no test at all. Why
> > submit
> > > a 'test result' for a case where nothing was tested?
> > >
> > > I mentioned fedmsgs because I was looking at the fedmsgs *emitted by
> > > ResultsDB itself* when a result is created (as that's what
greenwave
> > is
> > > using - I was fixing stuff in greenwave). But I'm not suggesting
the
> > > pipeline shouldn't emit messages *on its own topic* when it
ignores
> > > something, just that it shouldn't submit a "test
result" to ResultsDB
> > > when it does that.
> >
> > Actually, thinking it through a little more, I can guess at a possible
> > reason: it may be for greenwave policy purposes. Perhaps the idea is
> > that you can set a rule like "either the pipeline tests must have
> > passed, or the package must have been ignored"? The idea might be
that
> > this is essentially a way to tell *greenwave* (which consumes fedmsgs
> > from resultsdb) that the pipeline did not test the package.
> >
> > I am not aware of any reason to keep in resultsdb ignore packages
> > really. OSCI does not have any use case for now.
> > TBH we did not write or contribute much the upstream resultsdb updater,
> > the SUCCESS state
> > is moreover completely misleading yeah.
> > Let's just fix upstream resultsdb listener to ignore these, I am
creating
> > a PR in:
> > Â Â
https://pagure.io/ci-resultsdb-listener
>
> The issue is greenwave, how does bodhi know if an update is waiting to be tested
> vs is ignored by the sytem?
Yeah, that's what I suggested above - but it seems this wasn't really
*intended* for that.
It very much was intended for this, back then there was no opting-in via a file
in dist-git so the only way to know for the decision system if a package is
gated and the tests haven't run vs is not gated at all was: have the CI system
tell that it ignores the package.
Still, if we do want to tackle that problem, we do need to come up
with
some ideas. It *is* a real problem, if we want to gate on tests which
are not run on all packages. I suggested the possibility that Greenwave
could have a 'test must pass or not have run' type of rule, but you or
someone else pointed out that comes with obvious holes, like Bodhi will
consider the update to be passing that rule if the test has been
*scheduled* but not *finished* yet.
I will not dispute that it can be causing problems, nor that we can improve it,
just stating the history of the current situation :)
Sending 'ignore' results or something like that is one
potential way to
deal with the problem, though I can see it getting messy. For instance,
openQA runs different sets of tests on different updates. I had to set
up a whitelist mechanism to get the server tests run on FreeIPA-related
updates that aren't on the critical path; as part of that whitelisting
mechanism, I set things up so it runs *only* the server tests on those
updates, it skips the desktop tests). What do we do if we want to gate
on the desktop tests? openQA would have to do something like send a
'test not run' result for *every individual test it *might* have run*,
or send something like a 'ignored desktop tests' result and greenwave
would have to somehow be able to "understand" that.
Conceptually this approach would be sort of mildly abusing ResultsDB as
a database of test *execution*, and that seems kinda the wrong design
to me.
We do in fact *have* a database of test execution! It's called ExecDB:
https://pagure.io/taskotron/execdb
But my understanding is that execdb follows the status of the running test, a
package that is ignored could be approach as a result (something along the lines
of "no results is a result").
and it's part of Taskotron. I believe right now it's not
really set up
to be usable entirely independently from the rest of Taskotron like
ResultsDB is (though jskladan and kparal might know better than me),
but then, ResultsDB wasn't initially either. Perhaps it *could* be made
more independent, along the lines of ResultsDB, and we could then have
openQA and the pipeline and Autocloud and so on report test execution
status to that db, just like they report results to ResultsDB.
Or, of course, if there's a similar component in the pipeline or
anywhere else which could be shared by other systems, we could use that
instead. Basically, I think the 'right' way to do this is to have some
kind of shared record of test execution status. Then we can more
reasonably have policies like "if this test is actually run for this
artifact, it must pass".
Another possibility here is to have Greenwave track test execution
status based on the standardized 'CI' fedmsgs; if we require that any
tests we want to gate on must be run in systems that emit messages in
the standardized format, it might be viable to work things that way.
I guess there's still a slightly fuzzy area around "is this test really
not being run, or is the test system just down or behind or missed
updating the status for some reason?", but I think that's manageable.
We could have some kind of configurable 'timeout' in Greenwave by which
time it assumes required tests must have been scheduled. For instance,
if the rule says "test 'foobar' must pass or not be run", for the
first
five minutes after some new artifact shows up, that rule is unsatisfied
(to give test systems time to notice the artifact and schedule the
tests); after five minutes, if no test system has registered that it
will be executing that test, Greenwave would count the rule as
satisfied on the basis that the test is not being run. The more
'robust' approach would be to require test systems to explicitly
register that they have *chosen not to run* the test, but that could be
difficult to implement (in fact, I can say confidently that for openQA
it *is* difficult to implement, as I can't really make the scheduler
ask openQA "OK, I'm not really going to run tests for this flavor, but
if I *did*, what tests *would* you run?" - that's just not a thing).
Anyone have any thoughts?
Another approach could also be: gate everything except what the packages set in
dist-git to not be gated (basically an opt-out instead of an opt-in), this way
there is no longer a need for a notification from the CI system about ignoring
since the decision maker will know not to look for test results.
This has of course a high cost: make packagers opt-out instead of opting in.
Thinking of it, since packages now opt-in, maybe we do not need the ignore
anymore, since the decision maker will know to gate package only if they have
asked to be gated, which means: if there are no test results, then either the
tests haven't run in which case it should be gated or the tests failed to run,
which should be notified to the CI pipeline admins and the best person to do
this would be the packager here as well.
Does that make sense?
I'd want to run this by Ralph or Luiz to ensure this line of thought isn't too
far in the weeds, but it doesn't sound too far in the weeds for me :)
Pierre