Open Stack

Hi,

A number of issues related to how jobs are defined and run on projects
with stable branches have come up recently.  I believe they are all
related, and they, as well as the solutions to them, must be considered
together.


The Problems
============

I've identified the following five problems:

1) Parents should have variants applied

Currently inheritance and variance are distinct.  Inheritance modifies a
job's configuration statically at the time the configuration is loaded.
Variants are applied dynamically right before the job runs.

That means that when a job starts, we pick a single job to start with,
then apply variants of that particular job.  But since the inheritance
has already been applied, any variants which a user may expect to apply
to a parent job will not be applied.  When the inheritance chain is
created at configuration time, only the "reference" definition of the
job is used -- that is, the first appearance of the job.

In other words, more than likely, most jobs defined in a stable branch
are going to inherit from a parent job defined on the master branch --
even if that parent job has a stable branch variant.

2) Variants may cause parents to be ignored

We currently ignore the parent attribute on a job variant.  If we did
not, then when the variant is applied, it would pull in all of the
attributes of its parent, which is likely to be on the master branch
(since its parent will, as in #1, be the reference definition).

This ignoring of the parent attribute happens at configuration time
(naturally, since that is when inheritance is applied).

This means that if the first job that matches a change is a variant
(i.e., the reference definition of a job has a non-matching branch
matcher), this top-level variant job will not actually have a parent.

3) Variants may add duplicate pre/post playbooks

Currently, the master branch does not have an implied branch matcher, so
jobs that exist on master and stable branches will generally be derived
from both.

If such a job adds a pre or post playbook, the job that is ultimately
created and run for a change on the stable branch may have those added
by both the variant defined on the master branch as well as that defined
on the stable branch (since pre and post playbooks are cumulative).

4) Variants on branches without explicit playbooks should use branch
   playbooks

Whenever a job includes a pre-run, run (including the implicit run), or
post-run playbook, Zuul remembers where and uses that branch of that
repo to run that playbook.  If a job were constructed from the master
branch, and then had a stable branch variant applied but did not repeat
the pre-run, run, or post-run attributes from the master, then Zuul
would end up attempting to run the playbook from the master branch
rather than the stable.

5) The master branch should have implied branch matchers

Currently jobs defined in an untrusted project on any branch other than
'master' have an implicit branch matcher applied to them.  This is what
allows the version of a job in a stable branch to only affect the stable
branch.  The fact that there is no implicit branch matcher applied to
the master branch is what allows jobs defined in zuul-jobs to run on
changes to any branch.

However, this also means that jobs on stable branches are frequently
built from variants on both the master and stable branch.  This may work
for a short while, but will fail as soon as someone wants to add
something to the master branch which should not exist on the stable
branch (e.g., enabling a new service by default).


The Design Considerations
=========================

In looking at these, I believe they have come about because of two
design goals which we did not appreciate were in moderate tension with
each other:

A) In-repo configuration for a project should apply to that branch.
Changing the behavior of a job on a stable branch should merely involve
changing the configuration or playbook in that stable branch.  When a
project branches master to create a new stable branch, both branches
should initially have the same content and behavior, but then evolve
independently.

B) We should be able to define jobs not only in a central repository
such as project-config, but a central *untrusted* repository such as
zuul-jobs or openstack-zuul-jobs.

I think these are valid and important to keep in mind as we consider
solutions.


The Changes
===========

I believe the following changes will address all five problems and
achieve both design goals:

a) Apply inheritance at the same time as variance

Rather than applying inheritance at configuration time, apply it at the
time the job is frozen before being run.  We can perform a depth-first
traversal up the hierarchy of parents, applying all of the matching
variants at each level as we return down.  With the following graph
(where lines indicate parent relationships):

        0 base
         /    \
1 devstack  2 devstack  4 altbase
         \    /             |
       3 tempest        5 tempest
          /  \
     6 foo  7 foo

Would have the following jobs applied in order:

0 base, 1 devstack, 2 devstack, 3 tempest, 4 altbase,
5 tempest, 6 foo, 7 foo

b) Add an implicit branch matcher to the master branch

Generally this will add clarity to projects with multiple branches,
however, if we always add the an implicit branch matcher, then it makes
it difficult to use repos like zuul-jobs to define jobs that run
everywhere.  So do this only if the project has multiple branches.  If
the project only has a single branch, omit the implicit branch matcher.

c) Add a config option to disable the implicit branch matcher

There are some times when an implicit branch matcher on master may be
undesirable.  For example when tempest was becoming branchless, it had
multiple branches, but we would have wanted the jobs defined on master
to be applicable everywhere.  Or if, for some reason, we wanted to
create a feature branch on zuul-jobs.  For these cases, it's necessary
to have an option to disable the implicit branch matcher.  We can add a
new kind of configuration object to zuul.yaml, for example:

  - meta:
      implicit-branch-matcher: False

Which would be intended only to apply to the current branch.  Or we
could add an option to the tenant config file, so the admin can indicate
that a certain project should not have an implicit branch matcher
applied to certain branches.


The Solutions
=============

Reconsidering the problems above with those three changes:

1) Parents should have variants applied

Change (a) is a straightforward solution to this problem.

Note that simply applying change (a) to our current configuration is
likely to, in some cases, bring in new parents on the master branch
which are not intended to apply to stable branches, and so change (a) is
best made with change (b) at the same time to reconcile this.

2) Variants may cause parents to be ignored

Change (a) means that it is safe to honor the parent attribute in all
variants.  Typically, all variants will have the same parent, and the
resulting job application will be straightforward.  In the example in
change (a), that would mean base, devstack, tempest, foo.  However, if a
variant did have a different parent, (e.g., tempest -> altbase in the
example), that case is handled as well.

In all cases, we will either be honoring the parent specified by the
user, or defaulting to the tenant-configured default base job.

3) Variants may add duplicate pre/post playbooks

Change (b) means that variants on the master branch will no longer (by
default) apply to variants on stable branches.  This is the most likely
route for duplicate pre/post playbooks to occur.  They can still occur
if a user explicitly adds them in two matching variants, however, this
will no longer automatically happen when a project creates a new branch,
which is the main concern.

4) Variants on branches without explicit playbooks should use branch
   playbooks

The main reason for a variant not to repeat the playbook is in an
attempt to solve problem (3).  With that solved by (b), there should be
little reason not to have explicit playbook attributes set on the job in
all branches.  By doing so, Zuul will use the appropriate
branch-specific playbook.

5) The master branch should have implied branch matchers

Changes (b) and (c) are straightforward solutions to this problem.


The Bonus Change
================

(d) Remove the implicit run playbook

This is not required to solve any of the problems stated, however, it
does make the solution to problem (4) even more explicit.

Moreover, despite being an initial proponent of the implicit playbook, I
have found that in practice, we have so many jobs that do not have
playbooks at all (i.e., we're making heavy use of inheritance and
variance) that it is becoming difficult to determine where to look for a
job's run playbook.  Declaring the run playbook explicitly will help
with discoverability.


The Proposal
============

I have been experimenting with late-binding inheritance (i.e., the
change described as (a)) in [1].  Change (b) was also required[2] in
order to reconcile all of the tests (as described in the solution to
problem (1)).

I have worked through these changes enough to feel comfortable that it
is a workable solution that addresses the issues without bringing new
problems to the table.

I think we should implement changes a-d as soon as possible -- ideally
before additional use of in-repo stable branch jobs cause more issues in
production.  At this stage, I don't think we need to make any allowances
for backwards compatibility -- the way jobs are constructed and being
ported into projects and across branches is largely compatible with
these changes already.  We would just be catching the implementation up
to the implicit understanding.

We do need a change to the infra-manual Zuul v3 migration page to
indicate that not only job definitions, but playbooks and roles should
appear in all branches of in-repo configuration.

I hope I have articulated the problems sufficiently to facilitate
discussion.  I'm happy to discuss them more in depth, either via email
or IRC.  Of course, these may not be the only solutions to these
problems.  If you see other possible solutions, I'm happy to discuss
them as well.

Thanks,

Jim

[1] https://review.openstack.org/511352
[2] https://review.openstack.org/514459