Fix broken debug_level #5716
Fix broken debug_level #5716
Conversation
michaelgugino
requested review from
ashcrow,
sdodson,
ewolinetz,
mtnbikenc and
ingvagabund
openshift-ci-robot
added
the
size/L
michaelgugino
force-pushed
the
fix-debug_level
branch
from
3ebbd65
to
cfdb4b6
Compare
|
/lgtm |
|
/kind bug |
openshift-ci-robot
added
the
kind/bug
| @@ -34,6 +9,11 @@ | |||
| when: | |||
| - openshift.common.is_containerized | bool | |||
There was a problem hiding this comment.
What about to set both in the defaults/main.yml as:
containerized_svc_dir: "{{ '/etc/systemd/system' if openshift.common.is_containerized | bool else '/usr/lib/systemd/system' }}"
ha_svc_template_path: "{{ 'docker-cluster' if openshift.common.is_containerized | bool else 'native-cluster' }}"? This way both variables can be overridden from the inventory file and there is only once place in the role where both variables are set.
| - name: Init HA Service Info | ||
| set_fact: | ||
| containerized_svc_dir: "{{ containerized_svc_dir | default('/usr/lib/systemd/system') }}" | ||
| ha_svc_template_path: "{{ ha_svc_template_path | default('native-cluster') }}" |
There was a problem hiding this comment.
Both of them are defined in the defaults/main.yml. Why do they need to be re-defined again to their default values?
There was a problem hiding this comment.
this upgrade_facts.yml only executes during upgrades. The reasoning is mentioned in the comment at the top of the file.
| @@ -1,4 +1,9 @@ | |||
| --- | |||
| # openshift_master_defaults_in_use is a workaround to detect if we are consuming | |||
| # the plays from the role or outside of the role. | |||
| openshift_master_defaults_in_use: True | |||
There was a problem hiding this comment.
Can you re-explaining the meaning of the variable? "if we are consuming the plays from the role" is confusing
There was a problem hiding this comment.
During upgrades, systemd_units.yml is included in a playbook outside the role. This means that the play is unable to consume the role's defaults/main.yml and vars/main.yml. We have to work around by setting facts for variables that would otherwise be defined. I'm using this because if 'openshift_master_defaults_in_use' is not defined, then that means someone is running the upgrade and the values in defaults/main.yml won't be detected.
|
bot, retest this please |
michaelgugino
force-pushed
the
fix-debug_level
branch
from
cfdb4b6
to
34dcd32
Compare
openshift-merge-robot
removed
the
lgtm
|
/lgtm |
|
I'm not see this job appear in Travis. It didn't appear travis ran this PR before I rebased it. Not sure what's going on with Travis. |
Currently, debug_level is documented as a way to change the debug output level for both masters and nodes. debug_level does not currently have any effect. This commit removes debug_level from openshift_facts and properly sets openshift_master_debug_level and openshift_node_debug_level to the value of debug_level specified in the inventory. This commit also reorganizes some set_fact tasks needed during master upgrades to put all work-around set-facts for undefined variables in one place, allowing for easier cleanup in the future. This includes an entry for openshift_master_debug_level. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1500164
michaelgugino
force-pushed
the
fix-debug_level
branch
from
34dcd32
to
bb1d5f4
Compare
Currently, debug_level is documented as a way to change
the debug output level for both masters and nodes.
debug_level does not currently have any effect.
This commit removes debug_level from openshift_facts
and properly sets openshift_master_debug_level and
openshift_node_debug_level to the value of debug_level
specified in the inventory.
This commit also reorganizes some set_fact tasks
needed during master upgrades to put all work-around
set-facts for undefined variables in one place, allowing
for easier cleanup in the future. This includes an
entry for openshift_master_debug_level.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1500164