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.

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

To fix eos_vrf failure when transport method is eapi #41470

Merged
merged 5 commits into from
Jun 20, 2018

Conversation

justjais
Copy link
Contributor

SUMMARY

This PR is raised to resolve eos_vrf failure when the transport method is eapi, and this issue was raised in bug #40930

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

eos

ANSIBLE VERSION
2.6
ADDITIONAL INFORMATION
User faces `unconverted error` when transport method `eapi` is used in `eos_vrf` module and this happened because, in `eos`,  not all show commands have been converted
to return formatted data, and trying to run the command with the format parameter set to json will result in an error, and in such cases, output format needs to be changed `text`.

@ansibot
Copy link
Contributor

ansibot commented Jun 13, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. networking Network category support:core This issue/PR relates to code supported by the Ansible Engineering Team. support:network This issue/PR relates to code supported by the Ansible Network Team. labels Jun 13, 2018
@justjais justjais requested a review from ganeshrn June 13, 2018 06:16
@@ -189,7 +189,7 @@ def map_obj_to_commands(updates, module):

def map_config_to_obj(module):
objs = []
output = run_commands(module, ['show vrf'])
output = run_commands(module, ['show vrf | text'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than creating something not supported by the device which will inevitably confuse someone later, you should probably do run_commands(module, {'command': 'show vrf', 'output': 'text'})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep.. makes sense, I have updated the code accordingly.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. labels Jun 13, 2018
@ansibot
Copy link
Contributor

ansibot commented Jun 20, 2018

@justjais this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot ansibot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jun 20, 2018
@ansibot ansibot removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jun 20, 2018
@@ -409,7 +409,10 @@ def is_eapi(module):


def to_command(module, commands):
if is_eapi(module):
if isinstance(commands, dict):
Copy link
Contributor

Choose a reason for hiding this comment

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

transform(to_list(commands)) already does this, with the bonus of not breaking if output is not specified in the dictionary. You should not have to change this file for this to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my bad I'll revert the change and will upload again

@ansibot ansibot removed the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jun 20, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 20, 2018
@justjais justjais merged commit c989b62 into ansible:devel Jun 20, 2018
justjais added a commit to justjais/ansible that referenced this pull request Jun 21, 2018
* resolve bug 40930

* resolve bug 40930

* to fix review comments

* to fix review comments

* reverting the changes based on review

(cherry picked from commit c989b62)
mattclay pushed a commit that referenced this pull request Jun 21, 2018
* To fix eos_vrf failure when transport method is eapi (#41470)

* resolve bug 40930

* resolve bug 40930

* to fix review comments

* to fix review comments

* reverting the changes based on review

(cherry picked from commit c989b62)

* adding changelog for backport
@justjais justjais deleted the 29577 branch June 25, 2018 06:53
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
* resolve bug 40930

* resolve bug 40930

* to fix review comments

* to fix review comments

* reverting the changes based on review
kbreit pushed a commit to kbreit/ansible that referenced this pull request Jul 3, 2018
…nsible#41771)

* To fix eos_vrf failure when transport method is eapi (ansible#41470)

* resolve bug 40930

* resolve bug 40930

* to fix review comments

* to fix review comments

* reverting the changes based on review

(cherry picked from commit c989b62)

* adding changelog for backport
@ansible ansible locked and limited conversation to collaborators Jun 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. networking Network category support:network This issue/PR relates to code supported by the Ansible Network Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants