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

nmcli - Use 'connection.autoconnect' instead of 'autoconnect' #478

Closed
wants to merge 1 commit into from

Conversation

Akasurde
Copy link
Member

@Akasurde Akasurde commented Jun 8, 2020

SUMMARY

Correct usage of autoconnect in nmcli module.

Fixes: #459

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

changelogs/fragments/63609_nmcli.yml
plugins/modules/net_tools/nmcli.py

@ansibullbot
Copy link
Collaborator

changelogs/fragments/58115_nmcli.yml Outdated Show resolved Hide resolved
changelogs/fragments/63609_nmcli.yml Outdated Show resolved Hide resolved
@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR stale_ci CI is older than 7 days, rerun before merging labels Jun 16, 2020
@gundalow
Copy link
Contributor

@Akasurde Would you be able to rebase and address the above review comments, we can review this during PR day later today

@Akasurde Akasurde force-pushed the i62609 branch 2 times, most recently from 9dcc506 to db0dab7 Compare June 23, 2020 05:29
@ansibullbot ansibullbot added community_review and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR stale_ci CI is older than 7 days, rerun before merging labels Jun 23, 2020
Correct usage of autoconnect in nmcli module.

Fixes: ansible-collections#459

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@felixfontein
Copy link
Collaborator

Changelog looks good. Can't judge the change...

@felixfontein felixfontein changed the base branch from master to main July 6, 2020 06:54
@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Jul 6, 2020
@mator
Copy link
Contributor

mator commented Oct 1, 2020

tested on centos-7 without and with patch applied, works in both cases... going to test on ubuntu 16.x as well...

@Andersson007
Copy link
Contributor

@mator didn't you try on ubuntu 16.x yet? thanks!

@ansibullbot ansibullbot added the unit tests/unit label Oct 20, 2020
@mator
Copy link
Contributor

mator commented Oct 22, 2020

i'm unable to reproduce on ubuntu 16.04 , works (without any additional patches) on latest git version of ansible (and tag v2.9.9) and C.G. :

ubuntu version:

root@ubuntu1604:~# cat /etc/os-release
NAME="Ubuntu"
VERSION="16.04.7 LTS (Xenial Xerus)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 16.04.7 LTS"
VERSION_ID="16.04"
HOME_URL="http://www.ubuntu.com/"
SUPPORT_URL="http://help.ubuntu.com/"
BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"
VERSION_CODENAME=xenial
UBUNTU_CODENAME=xenial

ansible environment (tested both versions of ansible, latest git and tag v2.9.9) :

[mator@archlinux ansible.dev]$ ansible --version
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features
under development. This is a rapidly changing source of code and can become unstable at any point.
ansible 2.11.0.dev0 (devel c20329a0f6) last updated 2020/10/21 14:07:52 (GMT +300)
  config file = /home/mator/ansible.dev/ansible.cfg
  configured module search path = ['/home/mator/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/mator/ansible.dev/ansible/lib/ansible
  ansible collection location = /home/mator/ansible.dev/collections
  executable location = /home/mator/ansible.dev/ansible/bin/ansible
  python version = 3.8.6 (default, Sep 30 2020, 04:00:38) [GCC 10.2.0]
  libyaml = True

$ cd community.general.git
[mator@archlinux community.general.git]$ git desc
0.1.4-260-g7caba156

playbook (tested with type generic and ethernet)

$ cat playbooks/nmcli.yml
---
- name: community.general/issues/459
  hosts: ubuntu1604
  become: true
  gather_facts: false
  tasks:
  - name: 'Use the nmcli module with default parameters'
    nmcli:
      conn_name: 'testcon1'
      type: 'generic'
      state: 'present'

before running playbook :

root@ubuntu1604:~# nmcli con show
NAME  UUID  TYPE  DEVICE
root@ubuntu1604:~#

playbook run:

$ ansible-playbook playbooks/nmcli.yml
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features
under development. This is a rapidly changing source of code and can become unstable at any point.

PLAY [community.general/issues/459] *************************************************************************************************************************************

TASK [Use the nmcli module with default parameters] *********************************************************************************************************************
changed: [ubuntu1604]

PLAY RECAP **************************************************************************************************************************************************************
ubuntu1604                 : ok=1    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

root@ubuntu1604:~# nmcli con show
NAME      UUID                                  TYPE     DEVICE
testcon1  9c3d1877-f518-44dd-9823-1d9f288d8775  generic  --

@Andersson007
Copy link
Contributor

@mator thanks for testing! So the patch doesn't fix anything?

@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Oct 28, 2020
@aminvakil
Copy link
Contributor

I guess this has been fixed in #1113.

def connection_options(self, detect_change=False):
# Options common to multiple connection types.
options = {
'connection.autoconnect': self.autoconnect,
}

@dmsimard dmsimard added the pr_day Has been reviewed during a PR review Day. https://github.com/ansible/community/issues/407 label Dec 1, 2020
@dmsimard
Copy link
Contributor

dmsimard commented Dec 1, 2020

@aminvakil thanks !

@dmsimard dmsimard closed this Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug has_issue module module needs_maintainer needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR pr_day Has been reviewed during a PR review Day. https://github.com/ansible/community/issues/407 stale_ci CI is older than 7 days, rerun before merging tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nmcli: Errors on Ubuntu 16.04 due to invalid 'autoconnect' parameter
8 participants