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

fix most ansible-test issues, suppress the rest #362

Merged
merged 1 commit into from
Apr 9, 2021
+206 −134
Merged

fix most ansible-test issues, suppress the rest #362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
toxenvs="py${toxpyver}"
case "$toxpyver" in
27) toxenvs="${toxenvs},coveralls,flake8,pylint" ;;
36) toxenvs="${toxenvs},coveralls,black,yamllint,ansible-lint,collection" ;;
36) toxenvs="${toxenvs},coveralls,black,yamllint,ansible-lint,collection,ansible-test" ;;
37) toxenvs="${toxenvs},coveralls" ;;
38) toxenvs="${toxenvs},coveralls" ;;
esac
Expand Down
48 changes: 48 additions & 0 deletions .sanity-ansible-ignore-2.9.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
plugins/module_utils/network_lsr/nm/__init__.py empty-init!skip
plugins/module_utils/network_lsr/nm/active_connection.py import-2.6!skip
plugins/module_utils/network_lsr/nm/active_connection.py import-2.7!skip
plugins/module_utils/network_lsr/nm/active_connection.py import-3.5!skip
plugins/module_utils/network_lsr/nm/active_connection.py import-3.6!skip
plugins/module_utils/network_lsr/nm/active_connection.py import-3.7!skip
plugins/module_utils/network_lsr/nm/active_connection.py import-3.8!skip
tyll marked this conversation as resolved.
Show resolved Hide resolved
plugins/module_utils/network_lsr/nm/client.py import-2.6!skip
plugins/module_utils/network_lsr/nm/client.py import-2.7!skip
plugins/module_utils/network_lsr/nm/client.py import-3.5!skip
plugins/module_utils/network_lsr/nm/client.py import-3.6!skip
plugins/module_utils/network_lsr/nm/client.py import-3.7!skip
plugins/module_utils/network_lsr/nm/client.py import-3.8!skip
plugins/module_utils/network_lsr/nm/connection.py import-2.6!skip
plugins/module_utils/network_lsr/nm/connection.py import-2.7!skip
plugins/module_utils/network_lsr/nm/connection.py import-3.5!skip
plugins/module_utils/network_lsr/nm/connection.py import-3.6!skip
plugins/module_utils/network_lsr/nm/connection.py import-3.7!skip
plugins/module_utils/network_lsr/nm/connection.py import-3.8!skip
plugins/module_utils/network_lsr/nm/provider.py import-2.6!skip
plugins/module_utils/network_lsr/nm/provider.py import-2.7!skip
plugins/module_utils/network_lsr/nm/provider.py import-3.5!skip
plugins/module_utils/network_lsr/nm/provider.py import-3.6!skip
plugins/module_utils/network_lsr/nm/provider.py import-3.7!skip
plugins/module_utils/network_lsr/nm/provider.py import-3.8!skip
plugins/modules/network_connections.py validate-modules:missing-examples
plugins/modules/network_connections.py validate-modules:missing-gplv3-license
plugins/modules/network_connections.py validate-modules:no-default-for-required-parameter
plugins/modules/network_connections.py validate-modules:parameter-type-not-in-doc
plugins/modules/network_connections.py validate-modules:undocumented-parameter
tests/network/covstats shebang!skip
Copy link
Member

Choose a reason for hiding this comment

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

Seems this just needs the whitespace to be removed from the script to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

apparently not, if you are talking about the shebang

Copy link
Member

Choose a reason for hiding this comment

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

This exception is about what is described in https://docs.ansible.com/ansible/latest/dev_guide/testing/sanity/shebang.html, isn't it? It states #!/bin/bash as a valid Shebang but the file uses #! /bin/bash. Not sure what I am missing here.

Copy link
Member

Choose a reason for hiding this comment

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

#! /bin/bash is valid for the Linux kernel, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the error, for example:

ERROR: tests/network/covstats:1:1: unexpected non-module shebang: b'#!/bin/bash'
See documentation for help: https://docs.ansible.com/ansible/2.9/dev_guide/testing/sanity/shebang.html

I think the problem is that ansible-test doesn't understand executable files outside of the expected module/plugin directories. So it thinks that tests/covstats is a module since it is executable and has a shebang, but it isn't in the right directory. This is because we need to have these helper scripts but there isn't a good place to keep them. There isn't a general Ansible role/collection convention for helper scripts used during build/test time. The ssh role uses .dev-tools for this - https://github.com/linux-system-roles/ssh/tree/master/.dev-tools - I think a dotfile directory is a good idea - we used to use .travis/ for these sorts of scripts.

Copy link
Member

Choose a reason for hiding this comment

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

The scripts are currently meant for a human to be used so hiding them in a dotdir seems to be wrong. I am fine with skipping them for now.

tests/network/ensure_provider_tests.py compile-2.6!skip
tests/network/ensure_provider_tests.py compile-2.7!skip
tests/network/ensure_provider_tests.py compile-3.5!skip
tests/network/ensure_provider_tests.py future-import-boilerplate!skip
tests/network/ensure_provider_tests.py metaclass-boilerplate!skip
tests/network/ensure_provider_tests.py shebang!skip
tests/network/get_coverage.sh shebang!skip
tests/network/get_total_coverage.sh shebang!skip
tyll marked this conversation as resolved.
Show resolved Hide resolved
tests/network/integration/conftest.py future-import-boilerplate!skip
tests/network/integration/conftest.py metaclass-boilerplate!skip
tests/network/integration/test_ethernet.py future-import-boilerplate!skip
tests/network/integration/test_ethernet.py metaclass-boilerplate!skip
tests/network/merge_coverage.sh shebang!skip
tyll marked this conversation as resolved.
Show resolved Hide resolved
tests/network/unit/test_network_connections.py future-import-boilerplate!skip
tests/network/unit/test_network_connections.py metaclass-boilerplate!skip
tests/network/unit/test_nm_provider.py future-import-boilerplate!skip
tests/network/unit/test_nm_provider.py metaclass-boilerplate!skip
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ a consequence, `state: up` always changes the system.
You can deactivate a connection profile, even if is currently not active. As a
consequence, `state: down` always changes the system.

Note that if the `state` option is unset, the connection profile’s runtime state will
Note that if the `state` option is unset, the connection profile's runtime state will
not be changed.

### `persistent_state`
Expand Down
92 changes: 60 additions & 32 deletions library/network_connections.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,30 @@
# -*- coding: utf-8 -*-
# SPDX-License-Identifier: BSD-3-Clause

from __future__ import absolute_import, division, print_function

__metaclass__ = type

DOCUMENTATION = """
---
module: network_connections
author: Thomas Haller (@thom311)
short_description: module for network role to manage connection profiles
requirements: [pygobject, dbus, NetworkManager]
version_added: "2.0"
description:
- "WARNING: Do not use this module directly! It is only for role internal use."
- |
Manage networking profiles (connections) for NetworkManager and
initscripts networking providers. Documentation needs to be written. Note
that the network_connections module tightly integrates with the network
role and currently it is not expected to use this module outside the role.
Thus, consult README.md for examples for the role. The requirements are
only for the NetworkManager (nm) provider.
options: {}
"""


import errno
import functools
import os
Expand All @@ -16,7 +40,7 @@
# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.network_lsr import ethtool # noqa:E501
from ansible.module_utils.network_lsr import MyError # noqa:E501
from ansible.module_utils.network_lsr.myerror import MyError # noqa:E501

from ansible.module_utils.network_lsr.argument_validator import ( # noqa:E501
ArgUtil,
Expand All @@ -30,22 +54,6 @@
# pylint: enable=import-error, no-name-in-module


DOCUMENTATION = """
---
module: network_connections
author: "Thomas Haller ([email protected])"
short_description: module for network role to manage connection profiles
requirements: for 'nm' provider requires pygobject, dbus and NetworkManager.
version_added: "2.0"
description: Manage networking profiles (connections) for NetworkManager and
initscripts networking providers.
options: Documentation needs to be written. Note that the network_connections
module tightly integrates with the network role and currently it is not
expected to use this module outside the role. Thus, consult README.md for
examples for the role.
"""


###############################################################################
PERSISTENT_STATE = "persistent_state"
ABSENT_STATE = "absent"
Expand Down Expand Up @@ -774,7 +782,7 @@ def connection_compare(
if compare_flags is None:
compare_flags = NM.SettingCompareFlags.IGNORE_TIMESTAMP

return not (not (con_a.compare(con_b, compare_flags)))
return con_a.compare(con_b, compare_flags)

def connection_is_active(self, con):
NM = Util.NM()
Expand Down Expand Up @@ -1401,7 +1409,7 @@ def _check_mode_changed(self, old_check_mode, new_check_mode, connections):
def check_mode_set(self, check_mode, connections=None):
c = self._check_mode
self._check_mode = check_mode
assert (
if not (
(c is None and check_mode in [CheckMode.PREPARE])
or (
c == CheckMode.PREPARE
Expand All @@ -1410,7 +1418,12 @@ def check_mode_set(self, check_mode, connections=None):
or (c == CheckMode.PRE_RUN and check_mode in [CheckMode.REAL_RUN])
or (c == CheckMode.REAL_RUN and check_mode in [CheckMode.DONE])
or (c == CheckMode.DRY_RUN and check_mode in [CheckMode.DONE])
)
):
raise AssertionError(
"updating check_mode value from {0} into {1} is incorrect".format(
c, check_mode
)
)
self._check_mode_changed(c, check_mode, connections)


Expand Down Expand Up @@ -1472,7 +1485,8 @@ def log(
warn_traceback=False,
force_fail=False,
):
assert idx >= -1
if not idx >= -1:
raise AssertionError("idx {0} is less than -1".format(idx))
self._log_idx += 1
self.run_results[idx]["log"].append((severity, msg, self._log_idx))
if severity == LogLevel.ERROR:
Expand Down Expand Up @@ -1609,14 +1623,15 @@ def connections(self):
def connections_data(self):
c = self._connections_data
if c is None:
assert self.check_mode in [
if self.check_mode not in [
CheckMode.DRY_RUN,
CheckMode.PRE_RUN,
CheckMode.REAL_RUN,
]
c = []
for _ in range(0, len(self.connections)):
c.append({"changed": False})
]:
raise AssertionError(
"invalid value {0} for self.check_mode".format(self.check_mode)
)
c = [{"changed": False}] * len(self.connections)
self._connections_data = c
return c

Expand All @@ -1625,11 +1640,14 @@ def connections_data_reset(self):
c["changed"] = False

def connections_data_set_changed(self, idx, changed=True):
assert self._check_mode in [
if self._check_mode not in [
CheckMode.PRE_RUN,
CheckMode.DRY_RUN,
CheckMode.REAL_RUN,
]
]:
raise AssertionError(
"invalid value {0} for self._check_mode".format(self._check_mode)
)
if not changed:
return
self.connections_data[idx]["changed"] = changed
Expand Down Expand Up @@ -1699,7 +1717,10 @@ def connection_modified_earlier(self, idx):
# modify the connection.

con = self.connections[idx]
assert con["state"] in ["up", "down"]
if con["state"] not in ["up", "down"]:
raise AssertionError(
"connection state {0} not 'up' or 'down'".format(con["state"])
)

# also check, if the current profile is 'up' with a 'type' (which
# possibly modifies the connection as well)
Expand Down Expand Up @@ -1747,7 +1768,9 @@ def check_mode_next(self):
elif self._check_mode != CheckMode.DONE:
c = CheckMode.DONE
else:
assert False
raise AssertionError(
"invalid value {0} for self._check_mode".format(self._check_mode)
)
self._check_mode = c
self.run_env.check_mode_set(c)
return c
Expand Down Expand Up @@ -1913,7 +1936,12 @@ def run_prepare(self):

name = connection["name"]
if not name:
assert connection["persistent_state"] == "absent"
if not connection["persistent_state"] == "absent":
richm marked this conversation as resolved.
Show resolved Hide resolved
raise AssertionError(
"persistent_state must be 'absent' not {0} when there is no connection 'name'".format(
connection["persistent_state"]
)
)
continue
if name in names:
exists = names[name]["nm.exists"]
Expand Down Expand Up @@ -1990,7 +2018,7 @@ def _check_ethtool_setting_support(self, idx, connection):
idx, "ethtool.%s specified but not supported by NM", specified
)

for option, _ in specified.items():
for option in specified.keys():
nm_name = nm_get_name_fcnt(option)
if not nm_name:
self.log_fatal(
Expand Down
7 changes: 0 additions & 7 deletions module_utils/network_lsr/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +0,0 @@
#!/usr/bin/python3 -tt
# vim: fileencoding=utf8
# SPDX-License-Identifier: BSD-3-Clause


class MyError(Exception):
pass
10 changes: 7 additions & 3 deletions module_utils/network_lsr/argument_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
#!/usr/bin/python3 -tt
# vim: fileencoding=utf8
# SPDX-License-Identifier: BSD-3-Clause

from __future__ import absolute_import, division, print_function

__metaclass__ = type

import posixpath
import socket
import re

# pylint: disable=import-error, no-name-in-module
from ansible.module_utils.network_lsr import MyError # noqa:E501
from ansible.module_utils.network_lsr.myerror import MyError # noqa:E501
from ansible.module_utils.network_lsr.utils import Util # noqa:E501

UINT32_MAX = 0xFFFFFFFF
Expand Down Expand Up @@ -72,7 +75,8 @@ def connection_get_non_absent_names(connections):

class ValidationError(MyError):
def __init__(self, name, message):
Exception.__init__(self, name + ": " + message)
# pylint: disable=non-parent-init-called
super(ValidationError, self).__init__(name + ": " + message)
self.error_message = message
self.name = name

Expand Down
6 changes: 5 additions & 1 deletion module_utils/network_lsr/ethtool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# SPDX-License-Identifier: BSD-3-Clause

from __future__ import absolute_import, division, print_function

__metaclass__ = type

import array
import struct
import fcntl
Expand Down Expand Up @@ -46,7 +50,7 @@ def get_perm_addr(ifname):
res = ecmd.tobytes()
except AttributeError: # tobytes() is not available in python2
res = ecmd.tostring()
_, size, perm_addr = struct.unpack("II%is" % MAX_ADDR_LEN, res)
unused, size, perm_addr = struct.unpack("II%is" % MAX_ADDR_LEN, res)
perm_addr = Util.mac_ntoa(perm_addr[:size])
except IOError:
perm_addr = None
Expand Down
10 changes: 10 additions & 0 deletions module_utils/network_lsr/myerror.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# vim: fileencoding=utf8
# SPDX-License-Identifier: BSD-3-Clause

from __future__ import absolute_import, division, print_function

__metaclass__ = type


class MyError(Exception):
pass
4 changes: 4 additions & 0 deletions module_utils/network_lsr/nm/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Relative import is not support by ansible 2.8 yet
# pylint: disable=import-error, no-name-in-module
from __future__ import absolute_import, division, print_function

__metaclass__ = type

from ansible.module_utils.network_lsr.nm import provider # noqa:E501

# pylint: enable=import-error, no-name-in-module
Expand Down
Loading