Remove redundant common play
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| kolla |
Fix Released
|
Medium
|
Unassigned | ||
Bug Description
In cf0c25c37 [0] a play was added to run the common role against all
hosts. This ends up being redundant since every role includes the common
role as a dependancy. The reasoning behind this change as pointed out by
the author in the review comments [1] was so that an operator could run
with '--tags common' and just have the common role applied.
To avoid redundancy, the common play has been removed and tags have been
added to the common role. This allows for just the common role to run
when another role is including it while reducing redundancy.
A side affect of removing the common playbook which runs against all
hosts is that not all facts on all hosts are gathered at the beginning
of the site.yml. This breaks the haproxy role since it relies heavily on
facts to build out the haproxy.cfg file.
Previously, the haproxy role would include several hosts purely for fact
gathering purposes as pointed out in c68c9d95 [2] and a guard was put in
place so that the tasks would only run against the 'haproxy' group. In
423e3f3f [3] these hosts were removed. After reading the review [4],
this seems to have been done without fully understanding why the hosts
were there in the first place.
This change did not break anything however since the common role that
ran on all hosts mentioned previously would gather all of the facts
necessary.
It looks like there have been other commits [5] to site.yml that remove
hosts that plays used for fact gathering.
Seems like a decision needs to be made about how fact gathering for Kolla
happens. Whether it is a single play that happens in the beginning of
site.yml gathering facts for all hosts or a more targeted approach where
each play includes the hosts it needs facts gathered for.
There are merits to each approach. The global play makes things much
cleaner from a code base standpoint and leaves less room for error going
forward when someone makes changes. However, it will cause fact gathering
to run on all nodes regardless of what task the operator is taking which
may not be efficient. The work around is to use '--limit' but that means
the operator has to understand what is happening under the covers.
The targeted approach is more efficient but does mean the list needs to
be kept accurate. It also prevents operators putting their own custom
configurations into some template files.
There were some good IRC [6] discussions about this that are worth
reading.
[0] https:/
[1] https:/
[2] https:/
[3] https:/
[4] https:/
[5] https:/
[6] http://
| Changed in kolla: | |
| status: | New → Confirmed |
| importance: | Undecided → High |
| milestone: | none → newton-rc2 |
| OpenStack Infra (hudson-openstack) wrote Fix merged to kolla (master) | #1 |
| Changed in kolla: | |
| status: | Confirmed → Fix Released |
| Changed in kolla: | |
| importance: | High → Medium |
| OpenStack Infra (hudson-openstack) wrote Fix included in openstack/kolla 3.0.0.0rc2 | #2 |
Reviewed: https:/
Committed: https:/
Submitter: Jenkins
Branch: master
commit 4963898b21d407a
Author: Andrew Widdersheim <email address hidden>
Date: Mon Sep 26 09:37:11 2016 -0400
Remove redundant common play and add haproxy hosts
In cf0c25c37 [0] a play was added to run the common role against all
hosts. This ends up being redundant since every role includes the common
role as a dependancy. The reasoning behind this change as pointed out by
the author in the review comments [1] was so that an operator could run
with '--tags common' and just have the common role applied.
To avoid redundancy, the common play has been removed and tags have been
added to the common role. This allows for just the common role to run
when another role is including it while reducing redundancy.
A side affect of removing the common playbook which runs against all
hosts is that not all facts on all hosts are gathered at the beginning
of the site.yml. This breaks the haproxy role since it relies heavily on
facts to build out the haproxy.cfg file.
Previously, the haproxy role would include several hosts purely for fact
gathering purposes as pointed out in c68c9d95 [2] and a guard was put in
place so that the tasks would only run against the 'haproxy' group. In
423e3f3f [3] these hosts were removed. After reading the review [4],
this seems to have been done without fully understanding why the hosts
were there in the first place.
This change did not break anything however since the common role that
ran on all hosts mentioned previously would gather all of the facts
necessary.
To fix this fact gathering issue replace the common role play with a
play that will simply gather facts with an 'always' tag to ensure it is
run regardless of what might be passed in the '--tags' argument by the
operator.
Kudos to Paul Bourke for helping identify many of these issues.
[0] https:/
[1] https:/
[2] https:/
[3] https:/
[4] https:/
TrivialFix
Closes-Bug: #1628472
Change-Id: Ia94146579e7439