Solaris: Correct version check in svcadm_supports_sync API

[guillermomolina]

SUMMARY

This is a bug and a fix, the service module fails in Solaris, the way the comparison of the version is implemented in lib/ansible/modules/service.py:

def svcadm_supports_sync(self): # Support for synchronous restart/refresh is only supported on # Oracle Solaris >= 11.2 for line in open('/etc/release', 'r').readlines(): m = re.match(r'\s+Oracle Solaris (\d+\.\d+).*', line.rstrip()) if m and m.groups()[0] >= 11.2: return True

It compares the string "11.X" with the float 11.2 raising an exception.
I propose to convert it to a float and guard it for exception with:

def svcadm_supports_sync(self): # Support for synchronous restart/refresh is only supported on # Oracle Solaris >= 11.2 for line in open('/etc/release', 'r').readlines(): m = re.match(r'\s+Oracle Solaris (\d+\.\d+).*', line.rstrip()) try: if m and float(m.groups()[0]) >= 11.2: return True except: pass

This new code is tested ok in Solaris 11.4

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

service

ADDITIONAL INFORMATION

[ansibot]

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/service.py:1362:12: bare-except: No exception type(s) specified

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/service.py:1362:13: E722: do not use bare 'except'

click here for bot help

[mattclay]

Several changes are needed here:

1. Use LooseVersion to compare versions.
2. Add tests.
3. Add a changelog.

[bcoca]

still needs changelog and tests

[guillermomolina]

still needs changelog and tests

What do you mean by changelog? do you want me to add a file at ansible/changelogs/fragments ?

Could you point me where this the service module is tested to insert a test there

Thanks

[Akasurde]

@guillermomolina Every PR requires a changelog. You can create it here. Take a look at the existing changelog entry to get an idea.

You can write tests here

[guillermomolina]

Please give me hints on how to create the module test, everything I tried didn't work.

[Akasurde]

Please give me hints on how to create the module test, everything I tried didn't work.

Give me some time I will try something and let you know. Thanks.

[Akasurde]

@guillermomolina Could you please take a look at the test and let me know? Thanks.

[guillermomolina]

@guillermomolina Could you please take a look at the test and let me know? Thanks.

@Akasurde Perfect, very well done, thanks.

[Akasurde]

@bcoca @mattclay Can you please review this? Thanks in advance.

[Akasurde]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Akasurde]

@guillermomolina Thanks for the contribution.

[guillermomolina]

@Akasurde you are welcome

[froebela]

Hello,

we've just installed the new quarterly Solaris 11.4 SRU patchset 32. As part of that, Solaris has changed the standard Python interpreter /usr/bin/python from Python 2 to Python 3. As a result we now also face this issue with our Ansible version 2.9 running on Debian 10 and we assume many more Solaris customers will follow. Therefore we'd kindly request a backport for this fix to Ansible 2.9 and 2.10 as soon as possible.

Thanks and kind regards,

Andre Fröbel!

[Akasurde]

@froebela Can you confirm if this fix works with 2.10?

FYI, Ansible 2.9 is only accepting security fixes.

[froebela]

@Akasurde We're a little bit afraid of getting newer Ansible major versions in an uncontrolled way, therefore we pin it. I'm in discussion with my colleagues in order to roll out version 2.10. After that we could test the fix.