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

Improve handling of complex RPC's #591

Closed
wants to merge 2 commits into from

Conversation

TheMysteriousX
Copy link

These changes work for me to resolve the issues I noted in #589 - I believe them to be safe and not cause any changes in behaviour (mostly because it doesn't appear the current behaviour works) but I don't have an extensive library of tasks to check these against as I'm working on a brand new deployment. But you may want to rework them, as I'm not familiar with ansible's best practices for module development.

This change does three things:

  • Port usage of _check_type_dict to support Ansible 4+
    I've tried to maintain compatibility with Ansible 2.9 by adding a fallback.

  • Changes the type of attrs, kwargs, and filter to list, avoiding round-tripping them from list to string to list. This is a problem, because safe_eval will not process lists with certain keywords that are common in JunOS configs.
    I believe this is a safe change, as if a user has e.g. "kwarg: 'foo'" Ansible will transparently convert it to "['foo']".

  • Allows an RPC to have nested configuration - this is required for e.g. validate, which needs to generate:

<rpc>
  <validate>
    <candidate/>
  </validate>
</rpc>

This is done recursively.

@TheMysteriousX
Copy link
Author

Also includes a fix for #582 now

@chidanandpujar
Copy link
Collaborator

chidanandpujar commented Nov 11, 2022

Hi ,

Looks to be working fine without this fix ,Could you please verify once with latest Ansible collections.

- name: Test juniper.device.rpc module
  hosts: all
  connection: local
  gather_facts: no
  collections:
    - juniper.device

  tasks:
#################
    - name: "Check configuration for errors"
      rpc:
        rpcs:
          - "open-configuration"
          - "load-configuration"
          - "close-configuration"
        attrs:
          - {}
          - action: 'set'
            format: 'text'
          - {}
        kwargs:
          - private: true
          - configuration_set: 'set system syslog file test1 any any'
          - {}
      register: test9
      tags: [ test9 ]

    - name: Check TEST 9
      debug:
        var: test9
      tags: [ test9 ]

ansible-playbook -i inventory pb.juniper_junos_issue_589.yml 

PLAY [Test juniper.device.rpc module] **************************************************************************************************************************************************************

TASK [Check configuration for errors] **************************************************************************************************************************************************************
ok: [test]

TASK [Check TEST 9] ********************************************************************************************************************************************************************************
ok: [test] => {
    "test9": {
        "changed": false,
        "failed": false,
        "results": [
            {
                "attrs": {},
                "changed": false,
                "failed": false,
                "format": "xml",
                "kwargs": {
                    "private": true
                },
                "msg": "The RPC executed successfully.",
                "rpc": "open-configuration",
                "stdout": "",
                "stdout_lines": []
            },
            {
                "attrs": {
                    "action": "set",
                    "format": "text"
                },
                "changed": false,
                "failed": false,
                "format": "xml",
                "kwargs": {
                    "configuration_set": "set system syslog file test1 any any"
                },
                "msg": "The RPC executed successfully.",
                "parsed_output": {
                    "load-configuration-results": {
                        "ok": ""
                    }
                },
                "rpc": "load-configuration",
                "stdout": "<load-configuration-results>\n  <ok/>\n</load-configuration-results>\n",
                "stdout_lines": [
                    "<load-configuration-results>",
                    "  <ok/>",
                    "</load-configuration-results>"
                ]
            },
            {
                "attrs": {},
                "changed": false,
                "failed": false,
                "format": "xml",
                "kwargs": {},
                "msg": "The RPC executed successfully.",
                "rpc": "close-configuration",
                "stdout": "",
                "stdout_lines": []
            }
        ]
    }
}

PLAY RECAP *****************************************************************************************************************************************************************************************
test                       : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

Thanks

@chidanandpujar
Add heading text
Add bold text, <Ctrl+b>
Add italic text, <Ctrl+i>
Add a quote, <Ctrl+Shift+.>
Add code, <Ctrl+e>
Add a link, <Ctrl+k>
Add a bulleted list, <Ctrl+Shift+8>
Add a numbered list, <Ctrl+Shift+7>
Add a task list, <Ctrl+Shift+l>
Directly mention a user or team
Reference an issue, pull request, or discussion
Add saved reply
Attach files by dragging & dropping, selecting or pasting them.
Styling with Markdown is supported
Remember, contributions to this repository should follow our GitHub Community Guidelines.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
Notifications
Customize

You’re receiving notifications because you commented.
3 participants

@dineshbaburam91
Copy link
Collaborator

As per @chidanandpujar comment, it is working without your changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants