Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Assure no_log is false on failed tasks #1828

Closed
wants to merge 1 commit into from
Closed

Assure no_log is false on failed tasks #1828

wants to merge 1 commit into from

Conversation

ssbarnea
Copy link
Member

Improves user experience by displaying the errors that caused key
tasks to fail.

Avoids adding extra verbosity by using no_log on successes.

Fixes: #1666
Signed-off-by: Sorin Sbarnea [email protected]

Please include details of what it is, how to use it, how it's been tested

PR Type

  • Bugfix Pull Request

@ssbarnea ssbarnea self-assigned this Mar 13, 2019
@ssbarnea ssbarnea requested a review from decentral1se March 13, 2019 12:37
@ssbarnea ssbarnea added the ux label Mar 13, 2019
decentral1se
decentral1se reviewed Mar 13, 2019
View reviewed changes
Copy link
Contributor

@decentral1se decentral1se left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another strategic patch!

Here's my manual test log:

Test logs dropdown 
➜  molecule (master) ✔ g c refs/pull/upstream/1828
Note: checking out 'refs/pull/upstream/1828'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at 2edbd002... Assure no_log is false on failed tasks
➜  molecule (2edbd002) ✔ sudo service docker stop
➜  molecule (2edbd002) ✔ source .venv/bin/activate
(.venv) ➜  molecule (2edbd002) ✔ pip install -e .
Obtaining file:///home/decentral1se/ansible/molecule
Requirement already satisfied: flake8<4,>=3.6.0 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (3.7.7)
Requirement already satisfied: yamllint<2,>=1.11.1 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (1.11.1)
Requirement already satisfied: ansible-lint<5,>=4.0.2 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (4.1.0)
Requirement already satisfied: anyconfig==0.9.7 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (0.9.7)
Requirement already satisfied: cerberus==1.2 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (1.2)
Requirement already satisfied: click==6.7 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (6.7)
Requirement already satisfied: click-completion==0.3.1 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (0.3.1)
Requirement already satisfied: colorama==0.3.9 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (0.3.9)
Requirement already satisfied: cookiecutter==1.6.0 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (1.6.0)
Requirement already satisfied: idna<2.8 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (2.7)
Requirement already satisfied: python-gilt==1.2.1 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (1.2.1)
Requirement already satisfied: Jinja2==2.10 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (2.10)
Requirement already satisfied: pbr==5.1.1 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (5.1.1)
Requirement already satisfied: pexpect==4.6.0 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (4.6.0)
Requirement already satisfied: psutil==5.4.6 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (5.4.6)
Requirement already satisfied: PyYAML==3.13 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (3.13)
Requirement already satisfied: sh==1.12.14 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (1.12.14)
Requirement already satisfied: six==1.11.0 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (1.11.0)
Requirement already satisfied: tabulate==0.8.2 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (0.8.2)
Requirement already satisfied: testinfra==1.19.0 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (1.19.0)
Requirement already satisfied: tree-format==0.1.2 in ./.venv/lib/python3.7/site-packages (from molecule==2.20.0.0a3.dev4) (0.1.2)
Requirement already satisfied: mccabe<0.7.0,>=0.6.0 in ./.venv/lib/python3.7/site-packages (from flake8<4,>=3.6.0->molecule==2.20.0.0a3.dev4) (0.6.1)
Requirement already satisfied: entrypoints<0.4.0,>=0.3.0 in ./.venv/lib/python3.7/site-packages (from flake8<4,>=3.6.0->molecule==2.20.0.0a3.dev4) (0.3)
Requirement already satisfied: pycodestyle<2.6.0,>=2.5.0 in ./.venv/lib/python3.7/site-packages (from flake8<4,>=3.6.0->molecule==2.20.0.0a3.dev4) (2.5.0)
Requirement already satisfied: pyflakes<2.2.0,>=2.1.0 in ./.venv/lib/python3.7/site-packages (from flake8<4,>=3.6.0->molecule==2.20.0.0a3.dev4) (2.1.1)
Requirement already satisfied: pathspec>=0.5.3 in ./.venv/lib/python3.7/site-packages (from yamllint<2,>=1.11.1->molecule==2.20.0.0a3.dev4) (0.5.9)
Requirement already satisfied: ruamel.yaml in ./.venv/lib/python3.7/site-packages (from ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (0.15.88)
Requirement already satisfied: ansible in ./.venv/lib/python3.7/site-packages (from ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (2.7.7)
Requirement already satisfied: whichcraft>=0.4.0 in ./.venv/lib/python3.7/site-packages (from cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (0.5.2)
Requirement already satisfied: requests>=2.18.0 in ./.venv/lib/python3.7/site-packages (from cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (2.21.0)
Requirement already satisfied: jinja2-time>=0.1.0 in ./.venv/lib/python3.7/site-packages (from cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (0.2.0)
Requirement already satisfied: binaryornot>=0.2.0 in ./.venv/lib/python3.7/site-packages (from cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (0.4.4)
Requirement already satisfied: future>=0.15.2 in ./.venv/lib/python3.7/site-packages (from cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (0.17.1)
Requirement already satisfied: poyo>=0.1.0 in ./.venv/lib/python3.7/site-packages (from cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (0.4.2)
Requirement already satisfied: fasteners in ./.venv/lib/python3.7/site-packages (from python-gilt==1.2.1->molecule==2.20.0.0a3.dev4) (0.14.1)
Requirement already satisfied: git-url-parse in ./.venv/lib/python3.7/site-packages (from python-gilt==1.2.1->molecule==2.20.0.0a3.dev4)(1.2.0)
Requirement already satisfied: MarkupSafe>=0.23 in ./.venv/lib/python3.7/site-packages (from Jinja2==2.10->molecule==2.20.0.0a3.dev4) (1.1.0)
Requirement already satisfied: ptyprocess>=0.5 in ./.venv/lib/python3.7/site-packages (from pexpect==4.6.0->molecule==2.20.0.0a3.dev4) (0.6.0)
Requirement already satisfied: pytest!=3.0.2 in ./.venv/lib/python3.7/site-packages (from testinfra==1.19.0->molecule==2.20.0.0a3.dev4) (4.3.0)
Requirement already satisfied: paramiko in ./.venv/lib/python3.7/site-packages (from ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (2.4.2)
Requirement already satisfied: setuptools in ./.venv/lib/python3.7/site-packages (from ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (40.8.0)
Requirement already satisfied: cryptography in ./.venv/lib/python3.7/site-packages (from ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (2.5)
Requirement already satisfied: urllib3<1.25,>=1.21.1 in ./.venv/lib/python3.7/site-packages (from requests>=2.18.0->cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (1.24.1)
Requirement already satisfied: chardet<3.1.0,>=3.0.2 in ./.venv/lib/python3.7/site-packages (from requests>=2.18.0->cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (3.0.4)
Requirement already satisfied: certifi>=2017.4.17 in ./.venv/lib/python3.7/site-packages (from requests>=2.18.0->cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (2018.11.29)
Requirement already satisfied: arrow in ./.venv/lib/python3.7/site-packages (from jinja2-time>=0.1.0->cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (0.13.1)
Requirement already satisfied: monotonic>=0.1 in ./.venv/lib/python3.7/site-packages (from fasteners->python-gilt==1.2.1->molecule==2.20.0.0a3.dev4) (1.5)
Requirement already satisfied: more-itertools>=4.0.0; python_version > "2.7" in ./.venv/lib/python3.7/site-packages (from pytest!=3.0.2->testinfra==1.19.0->molecule==2.20.0.0a3.dev4) (6.0.0)
Requirement already satisfied: attrs>=17.4.0 in ./.venv/lib/python3.7/site-packages (from pytest!=3.0.2->testinfra==1.19.0->molecule==2.20.0.0a3.dev4) (18.2.0)
Requirement already satisfied: pluggy>=0.7 in ./.venv/lib/python3.7/site-packages (from pytest!=3.0.2->testinfra==1.19.0->molecule==2.20.0.0a3.dev4) (0.8.1)
Requirement already satisfied: py>=1.5.0 in ./.venv/lib/python3.7/site-packages (from pytest!=3.0.2->testinfra==1.19.0->molecule==2.20.0.0a3.dev4) (1.7.0)
Requirement already satisfied: atomicwrites>=1.0 in ./.venv/lib/python3.7/site-packages (from pytest!=3.0.2->testinfra==1.19.0->molecule==2.20.0.0a3.dev4) (1.3.0)
Requirement already satisfied: bcrypt>=3.1.3 in ./.venv/lib/python3.7/site-packages (from paramiko->ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (3.1.6)
Requirement already satisfied: pynacl>=1.0.1 in ./.venv/lib/python3.7/site-packages (from paramiko->ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (1.3.0)
Requirement already satisfied: pyasn1>=0.1.7 in ./.venv/lib/python3.7/site-packages (from paramiko->ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (0.4.5)
Requirement already satisfied: cffi!=1.11.3,>=1.8 in ./.venv/lib/python3.7/site-packages (from cryptography->ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (1.12.1)
Requirement already satisfied: asn1crypto>=0.21.0 in ./.venv/lib/python3.7/site-packages (from cryptography->ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (0.24.0)
Requirement already satisfied: python-dateutil in ./.venv/lib/python3.7/site-packages (from arrow->jinja2-time>=0.1.0->cookiecutter==1.6.0->molecule==2.20.0.0a3.dev4) (2.8.0)
Requirement already satisfied: pycparser in ./.venv/lib/python3.7/site-packages (from cffi!=1.11.3,>=1.8->cryptography->ansible->ansible-lint<5,>=4.0.2->molecule==2.20.0.0a3.dev4) (2.19)
Installing collected packages: molecule
  Found existing installation: molecule 2.20.0.0a3.dev1
    Uninstalling molecule-2.20.0.0a3.dev1:
      Successfully uninstalled molecule-2.20.0.0a3.dev1
  Running setup.py develop for molecule
Successfully installed molecule
(.venv) ➜  molecule (2edbd002) ✔ molecule init role -r foobar
--> Initializing new role foobar...
Initialized role in /home/decentral1se/ansible/molecule/foobar successfully.
(.venv) ➜  molecule (2edbd002) ✗ cd foobar
(.venv) ➜  foobar (2edbd002) ✗ molecule test
--> Validating schema /home/decentral1se/ansible/molecule/foobar/molecule/default/molecule.yml.
Validation completed successfully.
--> Test matrix

└── default
    ├── lint
    ├── cleanup
    ├── destroy
    ├── dependency
    ├── syntax
    ├── create
    ├── prepare
    ├── converge
    ├── idempotence
    ├── side_effect
    ├── verify
    ├── cleanup
    └── destroy

--> Scenario: 'default'
--> Action: 'lint'
--> Executing Yamllint on files found in /home/decentral1se/ansible/molecule/foobar/...
Lint completed successfully.
--> Executing Flake8 on files found in /home/decentral1se/ansible/molecule/foobar/molecule/default/tests/...
Lint completed successfully.
--> Executing Ansible Lint on /home/decentral1se/ansible/molecule/foobar/molecule/default/playbook.yml...
    [701] Role info should contain platforms
    /home/decentral1se/ansible/molecule/foobar/meta/main.yml:1
    {'meta/main.yml': {'galaxy_info': {'author': 'your name', 'description': 'your description', 'company': 'your company (optional)', 'license': 'license (GPLv2, CC-BY, etc)', 'min_ansible_version': 1.2, 'galaxy_tags': [], '__line__': 2, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}, 'dependencies': [], '__line__': 1, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}}

    [703] Should change default metadata: author
    /home/decentral1se/ansible/molecule/foobar/meta/main.yml:1
    {'meta/main.yml': {'galaxy_info': {'author': 'your name', 'description': 'your description', 'company': 'your company (optional)', 'license': 'license (GPLv2, CC-BY, etc)', 'min_ansible_version': 1.2, 'galaxy_tags': [], '__line__': 2, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}, 'dependencies': [], '__line__': 1, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}}

    [703] Should change default metadata: description
    /home/decentral1se/ansible/molecule/foobar/meta/main.yml:1
    {'meta/main.yml': {'galaxy_info': {'author': 'your name', 'description': 'your description', 'company': 'your company (optional)', 'license': 'license (GPLv2, CC-BY, etc)', 'min_ansible_version': 1.2, 'galaxy_tags': [], '__line__': 2, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}, 'dependencies': [], '__line__': 1, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}}

    [703] Should change default metadata: company
    /home/decentral1se/ansible/molecule/foobar/meta/main.yml:1
    {'meta/main.yml': {'galaxy_info': {'author': 'your name', 'description': 'your description', 'company': 'your company (optional)', 'license': 'license (GPLv2, CC-BY, etc)', 'min_ansible_version': 1.2, 'galaxy_tags': [], '__line__': 2, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}, 'dependencies': [], '__line__': 1, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}}

    [703] Should change default metadata: license
    /home/decentral1se/ansible/molecule/foobar/meta/main.yml:1
    {'meta/main.yml': {'galaxy_info': {'author': 'your name', 'description': 'your description', 'company': 'your company (optional)', 'license': 'license (GPLv2, CC-BY, etc)', 'min_ansible_version': 1.2, 'galaxy_tags': [], '__line__': 2, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}, 'dependencies': [], '__line__': 1, '__file__': '/home/decentral1se/ansible/molecule/foobar/meta/main.yml'}}

An error occurred during the test sequence action: 'lint'. Cleaning up.
--> Scenario: 'default'
--> Action: 'destroy'

    PLAY [Destroy] *****************************************************************

    TASK [Destroy molecule instance(s)] ********************************************
    changed: [localhost] => (item=None)
    changed: [localhost]

    TASK [Wait for instance(s) deletion to complete] *******************************
    failed: [localhost] (item=None) => {"attempts": 1, "censored": "the output has been hidden due to the fact that 'no_log: true' was specified for this result", "changed": false}
    fatal: [localhost]: FAILED! => {"censored": "the output has been hidden due to the fact that 'no_log: true' was specified for this result", "changed": false}

    PLAY RECAP *********************************************************************
    localhost                  : ok=1    changed=1    unreachable=0    failed=1


ERROR:
[1]    30442 exit 2     molecule test
(.venv) ➜  foobar (2edbd002) ✗ molecule create
--> Validating schema /home/decentral1se/ansible/molecule/foobar/molecule/default/molecule.yml.
Validation completed successfully.
--> Test matrix

└── default
    ├── create
    └── prepare

--> Scenario: 'default'
--> Action: 'create'

    PLAY [Create] ******************************************************************

    TASK [Log into a Docker registry] **********************************************
    skipping: [localhost] => (item=None)

    TASK [Create Dockerfiles from image names] *************************************
    changed: [localhost] => (item={'name': 'instance', 'image': 'centos:7'})

    TASK [Discover local Docker images] ********************************************
    failed: [localhost] (item={'diff': [], 'dest': '/tmp/molecule/foobar/default/Dockerfile_centos_7', 'src': '/home/decentral1se/.ansible/tmp/ansible-tmp-1552483752.4769568-171672018164334/source', 'md5sum': '217113737142bc105653c8b795461b41', 'checksum': '99bf425df595f9603d6b09e627a0c750b04c0ee4', 'changed': True, 'uid': 1000, 'gid': 999, 'owner': 'decentral1se', 'group': 'docker', 'mode': '0644', 'state': 'file', 'size': 850, 'invocation': {'module_args': {'src': '/home/decentral1se/.ansible/tmp/ansible-tmp-1552483752.4769568-171672018164334/source', 'dest': '/tmp/molecule/foobar/default/Dockerfile_centos_7', 'mode': None, 'follow': False, '_original_basename': 'Dockerfile.j2', 'checksum': '99bf425df595f9603d6b09e627a0c750b04c0ee4', 'backup': False, 'force': True, 'content': None, 'validate': None, 'directory_mode': None, 'remote_src': None, 'local_follow': None, 'owner': None, 'group': None, 'seuser': None, 'serole': None, 'selevel': None, 'setype': None, 'attributes': None, 'regexp': None, 'delimiter': None, 'unsafe_writes': None}}, '_ansible_parsed': True, '_ansible_no_log': False, 'failed': False, 'item': {'name': 'instance', 'image': 'centos:7'}, '_ansible_item_result': True, '_ansible_ignore_errors': None, '_ansible_item_label': {'name': 'instance', 'image': 'centos:7'}}) => {"changed": false, "item": {"changed": true, "checksum": "99bf425df595f9603d6b09e627a0c750b04c0ee4", "dest": "/tmp/molecule/foobar/default/Dockerfile_centos_7", "diff": [], "failed": false, "gid": 999, "group": "docker", "invocation": {"module_args": {"_original_basename": "Dockerfile.j2", "attributes": null, "backup": false, "checksum": "99bf425df595f9603d6b09e627a0c750b04c0ee4", "content": null, "delimiter": null, "dest": "/tmp/molecule/foobar/default/Dockerfile_centos_7", "directory_mode": null, "follow": false, "force": true, "group": null, "local_follow": null, "mode": null, "owner": null, "regexp": null, "remote_src": null, "selevel": null, "serole": null, "setype": null, "seuser": null, "src": "/home/decentral1se/.ansible/tmp/ansible-tmp-1552483752.4769568-171672018164334/source", "unsafe_writes": null, "validate": null}}, "item": {"image": "centos:7", "name": "instance"}, "md5sum": "217113737142bc105653c8b795461b41", "mode": "0644", "owner": "decentral1se", "size": 850, "src": "/home/decentral1se/.ansible/tmp/ansible-tmp-1552483752.4769568-171672018164334/source", "state": "file", "uid": 1000}, "msg": "Error connecting: Error while fetching server API version: ('Connection aborted.', ConnectionRefusedError(111, 'Connection refused'))"}

    PLAY RECAP *********************************************************************
    localhost                  : ok=1    changed=1    unreachable=0    failed=1


ERROR:
[1]    30547 exit 2     molecule create
(.venv) ➜  foobar (2edbd002) ✗ cd ..
(.venv) ➜  molecule (2edbd002) ✗ g c master
Previous HEAD position was 2edbd002... Assure no_log is false on failed tasks
Switched to branch 'master'
(.venv) ➜  molecule (master) ✗ cd foobar
(.venv) ➜  foobar (master) ✗ molecule create
--> Validating schema /home/decentral1se/ansible/molecule/foobar/molecule/default/molecule.yml.
Validation completed successfully.
--> Test matrix

└── default
    ├── create
    └── prepare

--> Scenario: 'default'
--> Action: 'create'

    PLAY [Create] ******************************************************************

    TASK [Log into a Docker registry] **********************************************
    skipping: [localhost] => (item=None)

    TASK [Create Dockerfiles from image names] *************************************
    ok: [localhost] => (item=None)
    ok: [localhost]

    TASK [Discover local Docker images] ********************************************
    failed: [localhost] (item=None) => {"censored": "the output has been hidden due to the fact that 'no_log: true' was specified for this result", "changed": false}
    fatal: [localhost]: FAILED! => {"censored": "the output has been hidden due to the fact that 'no_log: true' was specified for this result", "changed": false}

    PLAY RECAP *********************************************************************
    localhost                  : ok=1    changed=0    unreachable=0    failed=1

ERROR:
[1]    31012 exit 2     molecule create

What about this Wait for instance(s) deletion to complete task as well?

But I have to ask, why the heck do we have no_log: True set by default? To hide credentials for creating instances with remote hosting providers?

Can we go the opposite direction? Instead of 'whitelisting' certain taks for no_log:False so we can get useful output, can we 'blacklist' specific tasks that use credentials with no_log:True and then just set the default of no_log: False?

EDIT: Ansible does this by default?

This is arguably one of the worst parts of using Molecule for new and existing users.

Thanks @ssbarnea!

@decentral1se
Copy link
Contributor

btw, if we don't have energy for the no_log:False debate now, I am happy to accept this patch.

@decentral1se decentral1se mentioned this pull request Mar 13, 2019
Closed
@decentral1se
Copy link
Contributor

Adding more data points for no_log:False default in #1666 👍

@decentral1se
Copy link
Contributor

This also addresses the first step of #1799

webknjaz
webknjaz reviewed Mar 13, 2019
View reviewed changes
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be linking credentials?

@ssbarnea
Assure no_log is false on failed tasks
04f0d06 
Improves user experience by displaying the errors that caused key
tasks to fail.

Avoids adding extra verbosity by using no_log on successes.

Fixes: #1666
Signed-off-by: Sorin Sbarnea <[email protected]>
@tehsmyers
Copy link
Contributor

linking

s/linking/leaking/ ?

@decentral1se
Copy link
Contributor

Oh hmmmmm ... :(

@decentral1se decentral1se self-requested a review March 13, 2019 16:26
@tehsmyers
Copy link
Contributor

An idea I've been toying with is adding something alongside MOLECULE_DEBUG and the --debug flag like MOLECULE_PROVISION_LOG, that can be derived from MOLECULE_DEBUG, altered by a --provision-log flag, and stored as the default config value for provisioner.log. That could then be used instead of the current no_log: "{{ not (lookup('env', 'MOLECULE_DEBUG') | bool or molecule_yml.provisioner.log|default(false) | bool) }}" handling. That could also be combined with the log on failure stuff seen in this PR, I think.

I don't actually like the envvar or flag name, as they're a bit long, but that's just a cosmetic thing.

@webknjaz
Copy link
Member

Yes, "leaking", thanks :)

gundalow
gundalow requested changes Mar 15, 2019
View reviewed changes
Copy link
Contributor

@gundalow gundalow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just putting -1 till we confirm if this is leaking credentials

@decentral1se
Copy link
Contributor

I think #1839 is another attempt at this from a different angle.

Can we close here?

@ssbarnea
Copy link
Member Author

Superceeded by another PR.

@ssbarnea ssbarnea closed this Mar 18, 2019
@decentral1se decentral1se deleted the fix/no-log branch March 18, 2019 11:01
@decentral1se decentral1se mentioned this pull request Apr 30, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webknjaz webknjaz webknjaz left review comments

@gundalow gundalow gundalow requested changes

@decentral1se decentral1se Awaiting requested review from decentral1se

Assignees

@ssbarnea ssbarnea

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

too many common errors are offuscated in ansible, requiring run with --debug
5 participants
@ssbarnea @decentral1se @tehsmyers @webknjaz @gundalow