Joshua Hesketh <joshua.hesk...@gmail.com> writes:

> 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/zuul.yaml@master:
- job:
    name: my-job
    vars: {jobvar: true}
            
- project-template:
    name: myjobs
    check:
      jobs:
        - my-job:
            vars: {templatevar: true}

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

project/zuul.yaml@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?

> 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 my statement and table below was erroneous:

>> 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 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-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to