[openstack-dev] Overriding project-templates in Zuul

On Mon, Apr 30, 2018 at 8:58 AM, James E. Blair <***@inaugust.com> wrote:
[...]

Post by James E. Blair
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC ABC
irrelevant-files AB BC B
================ ======== ======= =======
I believe this will address the shortcoming identified above, but before
we get too far in implementing it, I'd like to ask folks to take a
moment and evaluate whether it will address the issues you've seen, or
if you foresee any problems which I haven't anticipated.

It'll address a need we have in TripleO where we have complex file rules
and heavily rely on templates.
The matrix proposal looks good to me and will allow us to simplify a bit
our templates.

Thanks,

--
Emilien Macchi

Joshua Hesketh

2018-05-01 03:43:23 UTC

Post by James E. Blair
Hi,
If you've had difficulty overriding jobs in project-templates, please
read and provide feedback on this proposed change.
We tried to make the Zuul v3 configuration language as intuitive as
possible, and incorporated a lot that we learned from our years running
Zuul v2. One thing that we didn't anticipate was how folks would end up
wanting to use a job in both project-templates *and* local project
stanzas.
Essentially, we had assumed that if you wanted to control how a job was
run, you would add it to a project stanza directly rather that use a
project-template. It's easy to do so if you use one or the other.
However, it turns out there are lots of good reasons to use both. For
example, in a project-template we may want to establish a recommended
way to run a job, or that a job should always be run with a set of
related jobs. Yet a project may still want to indicate that job should
only run on certain changes in that specific repo.
To be very specific -- a very commonly expressed frustration is that a
project can't specify a "files" or "irrelevant-files" matcher to
override a job that appears in a project-template.
Reconciling those is difficult, largely because once Zuul decides to run
a job (for example, by a declaration in a project-template) it is
impossible to dissuade it from running that job by adding any extra
configuration to a project. We need to tread carefully when fixing
this, because quite a number of related concepts could be affected. For
instance, we need to preserve branch independence (a change to stop
running a job in one branch shouldn't affect others). And we need to
preserve the ability for job variants to layer on to each other (a
project-local variant should still be able to alter a variant in a
project-template).
I propose that we remedy this by making a small change to how Zuul
When a job appears multiple times on a project (for instance if it
appears in a project-template and also on the project itself), all of
the project-local variants which match the item's branch must also match
the item in order for the job to run. In other words, if a job appears
in a project-template used by a project and on the project, then both
must match.

I might be misunderstanding at which point a job is chosen to be ran and
therefore when it's too late to dissuade it. However, if possible, would it
make more sense for the project-local copy of a job to overwrite the
supplied files and irrelevant-files? This would allow a project to run a
job when it otherwise doesn't match.

What happens when something is in both files and irrelevant-files? If the
project-template is trying to say A is in 'files', but the project-local
says A is in 'irrelevant-files', should that overwrite it?

Cheers,
Josh

Post by James E. Blair
This effectively causes the "files" and "irrelevant-files" attributes on
all of the project-local job definitions matching a given branch to be
combined. The combination of multiple files matchers behaves as a
union, and irrelevant-files matchers as an intersection.
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC ABC
irrelevant-files AB BC B
================ ======== ======= =======
I believe this will address the shortcoming identified above, but before
we get too far in implementing it, I'd like to ask folks to take a
moment and evaluate whether it will address the issues you've seen, or
if you foresee any problems which I haven't anticipated.
Thanks,
Jim
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

James E. Blair

2018-05-01 17:02:32 UTC

Post by Joshua Hesketh
I might be misunderstanding at which point a job is chosen to be ran and
therefore when it's too late to dissuade it. However, if possible, would it
make more sense for the project-local copy of a job to overwrite the
supplied files and irrelevant-files? This would allow a project to run a
job when it otherwise doesn't match.

Imagine that a project with branches has a job added via a template.

project-config/***@master:
- job:
name: my-job
vars: {jobvar: true}

- project-template:
name: myjobs
check:
jobs:
- my-job:
vars: {templatevar: true}

project/***@master:
- project:
templates:
- myjobs
check:
jobs:
- my-job:
vars: {projectvar: true}

project/***@stable:
- project:
templates:
- myjobs
check:
jobs:
- my-job:
vars: {projectvar: true}

The resulting project config is:

- project:
jobs:
- my-job (branches: master; project-local job)
- my-job (branches: master; project-template job)
- my-job (branches: stable; project-local job)
- my-job (branches: stable; project-template job)

When Zuul decides what to run, it goes through each of those in order,
evaluates their matchers, and pulls in parents and their variants for
each that matches. So a change on the master branch would collect the
following variants to apply:

my-job (branch: master; project-local job)
my-job (job)
base (job)
my-job (branch: master; project-template job)
my-job (job)
base (job)

It would then apply them in this order:

base (job)
my-job (job)
my-job (branch: master; project-template job)
my-job (branch: master; project-local job)

To further restrict a project-local job with a "files:" matcher would
cause the project-local job not to match, but the project-template job
will still match, so the job gets run.

That's the situation we have today, which is what I meant by "it's too
late to dissuade it".

Regarding the suggestion to overwrite it, we would need to decide which
of the possible variants to overwrite. Keep in mind that there are 3
independent matchers operating on all the variants (branches, files,
irrelevant-files). Does a project-local job with a "files:" matcher
overwrite all of the variants? Just the ones which match the same
branch? That would probably be the most reasonable thing to do.

In my mind, that effectively splits the matchers into two categories:
branch matchers, and file matchers. And they would behave differently.

Zuul could collect the variants as above, considering only the branch
matchers. It could then apply all of the variants in the normal manner,
treating files and irrelevant-files as normal attributes which can be
overwritten. Then, once it has composed the job to run based on all the
matching variants, it would only *then* evaluate the files matchers. If
they don't match, then it would not run the job after all.

I think that's a very reasonable way to solve the issue as well, and I
believe it would match people's expectations. Ultimately, the outcome
will be very similar to the proposal I made except that rather than
being combined, the matchers will be overwritten. That means that if
you want to expand the set of irrelevant-files for a job, you would have
to copy the set from the parent.

There's one other aspect to consider -- it's possible to create a job
like this:

- job:
name: doc-job

- jobs:
name: doc-job
files: docs/index.rst
vars: {rebuild_index: true}

Which means: there's a normal docs job with no variables, but if
docs/index.rst is changed, set the rebuild_index variable to true.
Either approach (combine vs overwrite) eliminates the ability to do this
within a project or project-template stanza. But the "combine" approach
still lets us do this at the job level. We could still support this in
the overwrite approach, however, I think it might be simpler to work
with if we eliminated it as well and just always treated files and
irrelevant-files matchers as overwriteable attributes. It would no
longer be possible to implement the above example, but I'm not sure it's
that useful anyway?

Post by Joshua Hesketh
What happens when something is in both files and irrelevant-files? If the
project-template is trying to say A is in 'files', but the project-local
says A is in 'irrelevant-files', should that overwrite it?

I think in actuality, both operations would end up as intersections:

================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC B
irrelevant-files AB BC B
================ ======== ======= =======

So with the "combine" method, it's always possible to further restrict
where the job runs, but never to expand it.

As to your question about what if something appears in both -- in my
"combine" proposal, I would continue to treat them independently, so if
a project-local variant has an "irrelevant-files:" matcher, and a
project-template variant has a "files:" matcher, then the usual nonsense
would happen: they'd both have to match. So a job with "files: tests/"
and "irrelevant-files: docs/" would never run because it's impossible to
satisfy both. That actually matches some current behavior we have -- a
job must match a "real" job definition as well as at least one
project-template or project-local job definition, so a "job" with
"files: tests/" and a project-template with "irrelevant-files: docs/"
will behave the same way today.

However, if we go with the "overwrite" proposal, since we're altering
the behavior of the non-branch matchers anyway, that might be a fine
time to pair them up and treat them as a unit. Then, whichever is the
latest would win. So a job with "files" and a project-template with
"irrelevant-files" would end up only having an irrelevant-files
attribute. Then a further project-local stanza with only "files" would
eliminate the irrelevant-files attribute leaving only the files value.

Okay, let's summarize:

Proposal 1: All project-template and project-local job variants matching
the item's branch must also match the item.

* Files and irrelevant-files on project-template and project stanzas are
essentially combined in a set intersection.
* It's possible to further reduce the scope of jobs, but not expand.
* Files and irrelevant-files are still independent matchers, and if both
are present, both must match.
* It's not possible to alter a job attribute by adding a project-local
variant with only a files matcher (it would cause the whole job to run
or not run). But it's still possible to do that in the main job
definition itself.

Proposal 2: Files and irrelevant-files are treated as overwriteable
attributes and evaluated after branch-matching variants are combined.

* Files and irrelevant-files are overwritten, so the last value
encountered when combining all the matching variants (looking only at
branches) wins.
* Files and irrelevant-files will be treated as a pair, so that if
"irrelevant-files" appears, it will erase a previous "files"
attribute.
* It's possible to both reduce and expand the scope of jobs, but the
user may need to manually copy values from a parent or other variant
in order to do so.
* It will no longer be possible to alter a job attribute by adding a
variant with only a files matcher -- in all cases files and
irrelevant-files are used solely to determine whether the job is run,
not to determine whether to apply a variant.

I think both would be good solutions to the problem. The key points for
me are whether we want to keep the "alter a job attribute with variant
with a files matcher" functionality (the "rebuild_index" example from
above), and whether the additional control of overwriting the matchers
(at the cost of redundancy in configuration) is preferable to combining
the matchers.

-Jim

__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-***@lists.openstack.org?subject:unsubscribe
http://lists.op

Emilien Macchi

2018-05-01 17:22:55 UTC

On Tue, May 1, 2018 at 10:02 AM, James E. Blair <***@inaugust.com> wrote:
[...]

Post by James E. Blair
Proposal 1: All project-template and project-local job variants matching
the item's branch must also match the item.
* Files and irrelevant-files on project-template and project stanzas are
essentially combined in a set intersection.
* It's possible to further reduce the scope of jobs, but not expand.
* Files and irrelevant-files are still independent matchers, and if both
are present, both must match.
* It's not possible to alter a job attribute by adding a project-local
variant with only a files matcher (it would cause the whole job to run
or not run). But it's still possible to do that in the main job
definition itself.
Proposal 2: Files and irrelevant-files are treated as overwriteable
attributes and evaluated after branch-matching variants are combined.
* Files and irrelevant-files are overwritten, so the last value
encountered when combining all the matching variants (looking only at
branches) wins.
* Files and irrelevant-files will be treated as a pair, so that if
"irrelevant-files" appears, it will erase a previous "files"
attribute.
* It's possible to both reduce and expand the scope of jobs, but the
user may need to manually copy values from a parent or other variant
in order to do so.
* It will no longer be possible to alter a job attribute by adding a
variant with only a files matcher -- in all cases files and
irrelevant-files are used solely to determine whether the job is run,
not to determine whether to apply a variant.
I think both would be good solutions to the problem. The key points for
me are whether we want to keep the "alter a job attribute with variant
with a files matcher" functionality (the "rebuild_index" example from
above), and whether the additional control of overwriting the matchers
(at the cost of redundancy in configuration) is preferable to combining
the matchers.

In the case of TripleO, I think proposal 2 is what we want.
We have stanzas defined in the templates definitions in
openstack-infra/tripleo-ci repo, but really want to override the file rules
per repo (openstack/tripleo-quickstart for example) and I don't think we
want to have them both matching but so the last value encountered would win.
I'll let TripleO CI squad to give more thoughts though.

Thanks,

--
Emilien Macchi

Wesley Hayutin

2018-05-01 19:29:08 UTC

Post by Emilien Macchi
[...]

I agree,
Proposal #2 makes the most sense to me and seems more straight forward in
that you have the ability to override and that the project overriding would
need to handle both files and irrelevant-files from scratch.

Nice write up

Post by Emilien Macchi
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Joshua Hesketh

2018-05-02 08:46:43 UTC

Post by James E. Blair
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC B
irrelevant-files AB BC B
================ ======== ======= =======
So with the "combine" method, it's always possible to further restrict
where the job runs, but never to expand it.

Ignoring the 'files' above, in the example of 'irrelevant-files' haven't
you just combined the results to expand when it runs? ie, A and C are /not/
excluded and therefore the job will run when there are changes to A or C?

I would expect the table to be something like:
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC B
irrelevant-files AB BC ABC
================ ======== ======= =======

So a job with "files: tests/" and "irrelevant-files: docs/" would do
whatever it is that happens when you specify both.

tests/", but I've never claimed to understand irrelevant-files and I
won't start now.

Yes, I think you are right that this would reduce to that. However, what
about the use case of:
files: tests/*
irrelevant-files: tests/docs/*

I could see a use case where both of those would be helpful. Yes you could
describe that as one regex but to the end user the above may be expected to
work. Unless we make the two options mutually exclusive I feel like this is
a feature we should support. (That said, it's likely a separate
feature/functionality than what is being described now).

Anyway, I feel like Proposal #2 is more how I would expect the system to
behave.

I can see an argument for combining the results (and feel like you could
evaulate that at the end after combining the branch-matching variants) to
give something like:
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC ABC
irrelevant-files AB BC ABC
================ ======== ======= =======

However, that gives the user no way to remove a previously listed option.
Thus overwriting may be the better solution (ie proposal #2 as written)
unless we want to explore the option of allowing a syntax that says
"extend" or "overwrite".

Yours in hoping that made sense,
Josh

James E. Blair

2018-05-02 14:21:40 UTC

Ignoring the 'files' above, in the example of 'irrelevant-files' haven't
you just combined the results to expand when it runs? ie, A and C are /not/
excluded and therefore the job will run when there are changes to A or C?
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC B
irrelevant-files AB BC ABC
================ ======== ======= =======

Sure, we'll go with that. :)

So a job with "files: tests/" and "irrelevant-files: docs/" would do
whatever it is that happens when you specify both.

tests/", but I've never claimed to understand irrelevant-files and I
won't start now.

Yes, I think you are right that this would reduce to that. However, what
files: tests/*
irrelevant-files: tests/docs/*
I could see a use case where both of those would be helpful. Yes you could
describe that as one regex but to the end user the above may be expected to
work. Unless we make the two options mutually exclusive I feel like this is
a feature we should support. (That said, it's likely a separate
feature/functionality than what is being described now).

Today, that means: run the job if a file in tests/ is changed AND any
file outside of tests/docs/* is changed. A change to tests/foo matches
the irrelevant-files matcher, and also the files matcher, so it runs. A
change to tests/docs/foo matches the files matcher but not the
irrelevant-files matcher, so it doesn't run. I really hope I got that
right. Anyway, that is an example of something that's possible to
express with both.

I lumped in the idea of pairing files/irrelevant-files with Proposal 2
because I thought that being able to override them is key, and switching
from one to the other was part of that, and, to be honest, I don't think
people should ever combine them because it's hard enough to deal with
one, but maybe that's too much of an implicit behavior change, and
instead we should separate that out and consider it as its own change
later. I believe a user could still stop a the matchers by saying
"files: .*" and "irrelevant-files: ^$" in the project-local variant.

Let's revise Proposal #2 to omit that:

Proposal 2: Files and irrelevant-files are treated as overwriteable
attributes and evaluated after branch-matching variants are combined.

* Files and irrelevant-files are overwritten, so the last value
encountered when combining all the matching variants (looking only at
branches) wins.
* It's possible to both reduce and expand the scope of jobs, but the
user may need to manually copy values from a parent or other variant
in order to do so.
* It will no longer be possible to alter a job attribute by adding a
variant with only a files matcher -- in all cases files and
irrelevant-files are used solely to determine whether the job is run,
not to determine whether to apply a variant.

Post by Joshua Hesketh
Anyway, I feel like Proposal #2 is more how I would expect the system to
behave.
I can see an argument for combining the results (and feel like you could
evaulate that at the end after combining the branch-matching variants) to
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC ABC
irrelevant-files AB BC ABC
================ ======== ======= =======
However, that gives the user no way to remove a previously listed option.
Thus overwriting may be the better solution (ie proposal #2 as written)
unless we want to explore the option of allowing a syntax that says
"extend" or "overwrite".
Yours in hoping that made sense,
Josh

As much as anything with irrelevant-files does, yes. :)

-Jim

__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-***@lists.openstack.org?subject:unsubscribe
http://lists

Ghanshyam Mann

2018-05-10 06:02:42 UTC

Ignoring the 'files' above, in the example of 'irrelevant-files' haven't
you just combined the results to expand when it runs? ie, A and C are /not/
excluded and therefore the job will run when there are changes to A or C?
================ ======== ======= =======
Matcher Template Project Result
================ ======== ======= =======
files AB BC B
irrelevant-files AB BC ABC
================ ======== ======= =======

Sure, we'll go with that. :)

So a job with "files: tests/" and "irrelevant-files: docs/" would do
whatever it is that happens when you specify both.

tests/", but I've never claimed to understand irrelevant-files and I
won't start now.

Today, that means: run the job if a file in tests/ is changed AND any
file outside of tests/docs/* is changed. A change to tests/foo matches
the irrelevant-files matcher, and also the files matcher, so it runs. A
change to tests/docs/foo matches the files matcher but not the
irrelevant-files matcher, so it doesn't run. I really hope I got that
right. Anyway, that is an example of something that's possible to
express with both.
I lumped in the idea of pairing files/irrelevant-files with Proposal 2
because I thought that being able to override them is key, and switching
from one to the other was part of that, and, to be honest, I don't think
people should ever combine them because it's hard enough to deal with
one, but maybe that's too much of an implicit behavior change, and
instead we should separate that out and consider it as its own change
later. I believe a user could still stop a the matchers by saying
"files: .*" and "irrelevant-files: ^$" in the project-local variant.
Proposal 2: Files and irrelevant-files are treated as overwriteable
attributes and evaluated after branch-matching variants are combined.
* Files and irrelevant-files are overwritten, so the last value
encountered when combining all the matching variants (looking only at
branches) wins.
* It's possible to both reduce and expand the scope of jobs, but the
user may need to manually copy values from a parent or other variant
in order to do so.
* It will no longer be possible to alter a job attribute by adding a
variant with only a files matcher -- in all cases files and
irrelevant-files are used solely to determine whether the job is run,
not to determine whether to apply a variant.

This is limitation for this Proposal but i am not sure how many use
case of this features. I have not seen till now in jobs.
Yes, Proposal#2 looks good for nova use case [1] also where
integrated-gate templates job needs to be controlled by nova pipeline
definition mainly for 'irrelevant-files'. This approach gives benefit
of Easy to read from one place that this job is controlled by these
value of overridden var ('files', 'irrelevant-files').

-gmann

As much as anything with irrelevant-files does, yes. :)
-Jim

..1 https://bugs.launchpad.net/nova/+bug/1745431 ,
https://bugs.launchpad.net/nova/+bug/1745405

Post by James E. Blair
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-***@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/ma

James E. Blair

2018-05-01 17:53:12 UTC