Re: [Xen-devel] [PATCH v6 19/20] osstest: save/retrieve the last successfully tested FreeBSD build

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Mon, Jul 24, 2017 at 04:56:25PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v6 19/20] osstest: save/retrieve the last 
> successfully tested FreeBSD build"):
> >  push=false
> > -if grep '^tolerable$' $mrof >/dev/null 2>&1; then push=$wantpush; fi
> > +if grep '^tolerable$' $mrof >/dev/null 2>&1; then
> > +    push=$wantpush;
> > +    case "$branch" in
> > +    freebsd-*)
> > +        # Save the output of successful FreeBSD build jobs to be re-used.
> > +        # NB: hardcode arch to amd64 since that's all osstest covers ATM.
> > +        ./mg-anoint anoint "freebsd build $branch amd64" \
> > +                           $flight build-freebsd-amd64
> > +        ;;
> > +    esac
> > +fi
> 
> No, please don't change this.  Instead I think this should be done in
> the section where we do the actual push ?  Or maybe we need another
> tracking variable `anoint'.  We need to think about the following
> cases:
> 
>   OSSTEST_PUSH=false    probably want not to anoint

I would rather rely on OSSTEST_ANOINT=false, which could be set
independently of OSSTEST_PUSH. Having anointed != pushed is confusing,
but should not cause issues as long as the anointed version doesn't
contain regressions from the previous version.

>   $branch.force         I think this should force an anoint

Hm, not sure. AFAICT (sorry I couldn't find the documentation about
.force), .force will force a push of the branch, regardless of the
results of the flight.

The problem with this is that osstest might shoot itself in the foot,
and end up with a set of anointed artifacts that doesn't even boot,
thus leaving the FreeBSD flight unable to produce new versions. This
state would then require manual intervention in order to fix.

>   OLD_REVISON=none      Now forces test, should force anoint

I don't think we should anoint if OLD_REVISION=none, because osstest
won't have anything to compare the current result, so it doesn't know
if there are regression or not.

> 
> What about supporting
> 
>   OSSTEST_ANOINT=false  ?
> 
> I think the right approach is probably to add code after $wantpush and
> $push are computed, which computes $anoint, in a similar way.
> 
> Also I would like you to discuss explicitly (in a comment or commit
> message) about whether push or anoint should come first.  If push
> comes first then we can end up pushed but not anointed; and, vice
> versa.  What are the recovery arrangements from such a failure ?

Hm, that's a hard one. I think push should be our primary goal, and as
such we should try to do the push first, so that a failed anoint
doesn't prevent a push.

OTOH, doing a push and failing on anoint doesn't seem that critical,
osstest can still use the oldish anointed artifacts and continue
working, hoping that on the next pass the anoint will succeed.

Let me know if that sounds OK to you, and I will formalize it in a
comment.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel