#293 Fix standard-test-source to work on Fedora or RHEL
Merged 5 years ago by astepano. Opened 5 years ago by stefw.
stefw/standard-test-roles source-lookaside-cache  into  master

@@ -0,0 +1,21 @@ 

+ Modules for this role

+ 

+ ## source-lookaside.py

+ 

+ This module is only used by the standard-test-source role. But further documentation

+ about how to call it is availeble in the module itself.

+ 

+ To hack on a module, create an args.json file like this:

+ 

+     {

+         "ANSIBLE_MODULE_ARGS": {

+             "package": "cockpit",

+             "target": "extracted"

+         }

+     }

+ 

+ Then call the module like this or this, depending on your Ansible version:

+ 

+     $ python2 roles/standard-test-source/library/source-lookaside.py args.json

+     $ python3 roles/standard-test-source/library/source-lookaside.py args.json

+ 

@@ -0,0 +1,234 @@ 

+ #!/usr/bin/python3

+ # This code runs under python2 and python3

+ 

+ #

+ # Copyright: (c) 2018, Stef Walter <stefw@redhat.com>

+ #

+ # The MIT License (MIT)

+ # Permission is hereby granted, free of charge, to any person obtaining a copy

+ # of this software and associated documentation files (the "Software"), to deal

+ # in the Software without restriction, including without limitation the rights

+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

+ # copies of the Software, and to permit persons to whom the Software is

+ # furnished to do so, subject to the following conditions:

+ #

+ # The above copyright notice and this permission notice shall be included in all

+ # copies or substantial portions of the Software.

+ #

+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE

+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE

+ # SOFTWARE.

+ #

+ 

+ from ansible.module_utils.basic import AnsibleModule

+ 

+ import errno

+ import logging

+ import glob

+ import re

+ import os

+ import shutil

+ 

+ try:

+     from urllib.request import urlopen

+     from urllib.error import URLError

+     from configparser import ConfigParser

+ except ImportError:

+     from urllib2 import urlopen, URLError

+     from ConfigParser import ConfigParser

+ 

+ ANSIBLE_METADATA = {

+     'metadata_version': '1.1',

+     'status': ['preview'],

+     'supported_by': 'community'

+ }

+ 

+ DOCUMENTATION = '''

+ ---

+ module: source-lookaside

+ 

+ short_description: Extract source code from Fedora or RHEL lookaside dist-git caches

+ 

+ version_added: "2.4"

+ 

+ description:

+     - "This module retrives source code artifacts from the Fedora or RHEL dist-git lookaside caches"

+ 

+ options:

+     package:

+         description:

+             - The package name

+         required: true

+     sources:

+         description:

+             - The sources file that includes hashes. Default: "./sources"

+         required: false

+     target:

+         description:

+             - Target directory to write sources to. Default: same directory as sources

+         required: false

+ 

+ author:

+     - Stef Walter (@stefwalter)

+ '''

+ 

+ EXAMPLES = '''

+ # Pull source tarball

+ - name: Pull sources

+   source-lookaside:

+     package: cockpit

+ 

+ # Pull a sources from the given sources file

+ - name: Pull sources

+   source-lookaside:

+     package: cockpit

+     sources: /path/to/cockpit/sources

+ 

+ # Pull the sources into to a specific directory

+ - name: Pull sources

+   source-lookaside:

+     package: cockpit

+     target: /path/to/source

+ '''

+ 

+ RETURN = '''

+ original_sources:

+     description: The path to the sources file

+ sources:

+     description: A list of source artifact files

+ '''

+ 

+ LOOKASIDES = (

+     "https://src.fedoraproject.org/repo/pkgs",

+ )

+ 

+ LOOKASIDE_URI = "/rpms/{name}/{filename}/{hashtype}/{hash}/{filename}"

+ 

+ # Location to find more lookaside URLs

+ LOOKASIDE_CONFIG = "/etc/rpkg/*.conf"

+ 

+ PATTERNS = (

+     re.compile(r'^(?P<hashtype>[^ ]+?) \((?P<filename>[^ )]+?)\) = (?P<hash>[^ ]+?)$'),

+     re.compile(r'^(?P<hash>[^ ]+?)  (?P<filename>[^ )]+?)$'),

+ )

+ 

+ # A logger to write to

+ logger = logging.getLogger(__name__)

+ logger.setLevel(logging.INFO)

+ 

+ 

+ # Because some people have other lookaside URLs configured, look them up

+ def lookasides():

+     for url in LOOKASIDES:

+         yield url + LOOKASIDE_URI

+     config = ConfigParser()

+     config.read(glob.glob(LOOKASIDE_CONFIG))

+     for section in config.sections():

+         if config.has_option(section, "lookaside"):

+             yield config.get(section, "lookaside") + LOOKASIDE_URI

+ 

+ 

+ # Parses the sources file into a list of possible urls to retrieve

+ def urls(package, sources):

+     if not os.path.exists(sources):

+         return

+     with open(sources, 'r') as fp:

+         for line in fp.readlines():

+             for pattern in PATTERNS:

+                 match = pattern.match(line.strip())

+                 if match is None:

+                     continue

+                 fields = match.groupdict()

+                 fields['hashtype'] = fields.get('hashtype', 'md5').lower()

+                 fields['name'] = package

+                 yield tuple(map(lambda url: url.format(**fields), lookasides()))

+ 

+ 

+ def mkdirs(directory):

+     try:

+         os.makedirs(directory)

+     except OSError as ex:

+         if ex.errno == errno.EEXIST and os.path.isdir(directory):

+             pass

+ 

+ 

+ def retrieve(url, target):

+     name = os.path.basename(url)

+     dest = os.path.join(target, name)

+ 

+     mkdirs(target)

+ 

+     try:

+         with open(dest, 'wb') as fp:

+             shutil.copyfileobj(urlopen(url), fp)

+     except URLError as ex:

+         if not hasattr(ex, "code") or ex.code != 404:

+             logger.error("{0}: {1} {2}".format(name, url, str(ex)))

+         return None

+ 

+     return dest

+ 

+ 

+ def run_module():

+     # define available arguments/parameters a user can pass to the module

+     module_args = dict(

+         package=dict(type='str', required=True),

+         sources=dict(type='str', required=False, default="./sources"),

+         target=dict(type='str', required=False, default=""),

+     )

+ 

+     module = AnsibleModule(argument_spec=module_args, supports_check_mode=True)

+ 

+     package = module.params['package'].strip()

+     sources = module.params['sources']

+     target = module.params['target']

+ 

+     # Default target to same directory as sources file

+     if not target:

+         target = os.path.dirname(sources)

+ 

+     # The results

+     result = dict(changed=False, original_sources=sources, sources=[])

+ 

+     if not os.path.exists(sources):

+         module.fail_json(msg='The sources file does not exist: {0}'.format(sources), **result)

+         return

+ 

+     # if the user is working with this module in only check mode we do not

+     # want to make any changes to the environment, just return the current

+     # state with no modifications

+     if module.check_mode:

+         return result

+ 

+     # Get a list of possible URLs for each of the sources

+     for possible in urls(package, sources):

+         name = None

+         for url in possible:

+             name = os.path.basename(url)

+             logger.info("{0}: {1}\n".format(name, url))

+ 

+             # Try to retrieve the possible url for this source

+             dest = retrieve(url, target)

+             if dest:

+                 result['changed'] = True

+                 result['sources'].append(dest)

+                 break

+ 

+         # We fail module execution if we cannot retrieve each file

+         else:

+             module.fail_json(msg="Unable to retrieve source file: {}".format(name), **result)

+ 

+     # Successful module execution

+     module.exit_json(**result)

+ 

+ 

+ def main():

+     run_module()

+ 

+ 

+ if __name__ == '__main__':

+     main()

@@ -13,14 +13,24 @@ 

      setup:

      delegate_facts: True

  

-   # The dist doesn't actually matter here

-   - name: Download sources, extract, and strip leading path

+   - name: Get the specfile package name

+     shell: rpm -q --specfile --queryformat="%{NAME}\n" {{pkgdir}}/*.spec | head -n1

+     args:

+       warn: false

+     register: name

+ 

+   - name: Pull down the source tarballs

+     source-lookaside:

+       package: "{{name.stdout}}"

+       sources: "{{pkgdir}}/sources"

+       target: "{{pkgdir}}/"

+ 

+   - name: Extract and setup the sources

      shell: |

-       set -e

-       rm -rf {{ srcdir }}

-       fedpkg --release=master prep --builddir={{ srcdir }}

+       rm -rf "{{srcdir}}"

+       rpmbuild -bp {{pkgdir}}/*.spec --nodeps --define "_sourcedir {{pkgdir}}/" --define "_builddir {{srcdir}}"

      args:

-       chdir: "{{playbook_dir}}/.."

+       warn: false

  

    - name: Flatten sources

      shell: |

@@ -1,3 +1,4 @@ 

  ---

+ pkgdir: "{{ playbook_dir }}/.."

  srcdir: "{{ playbook_dir }}/source"

  flatten: True

file added
+3
@@ -0,0 +1,3 @@ 

+ *.retry

+ test-source-zsh/*.tar.xz

+ source/

file added
+14
@@ -0,0 +1,14 @@ 

+ # Tests for standard-test-source role

+ - hosts: localhost

+   tags:

+   - atomic

+   - classic

+   - container

+   roles:

+   - role: standard-test-source

+     pkgdir: "{{playbook_dir}}/test-source-zsh"

+     srcdir: "{{playbook_dir}}/source"

+   tasks:

+   - name: "Check if sources were pulled for zsh"

+     shell: ls "{{playbook_dir}}/source/configure" "{{playbook_dir}}/source/aczsh.m4" "{{playbook_dir}}/source/patch-created.txt"

+ 

@@ -0,0 +1,21 @@ 

+ From 701936e50d4530b64d580d3fb1e3afbcb4fc1292 Mon Sep 17 00:00:00 2001

+ From: Stef Walter <stefw@redhat.com>

+ Date: Thu, 10 Jan 2019 07:02:48 +0100

+ Subject: [PATCH] Patch that creates a file in the sources

+ 

+ ---

+  patch-created.txt | 2 ++

+  1 file changed, 2 insertions(+)

+  create mode 100644 patch-created.txt

+ 

+ diff --git a/patch-created.txt b/patch-created.txt

+ new file mode 100644

+ index 0000000..6e6acb8

+ --- /dev/null

+ +++ b/patch-created.txt

+ @@ -0,0 +1,2 @@

+ +This is a file created by a patch

+ +It should be present in the extracted sources

+ -- 

+ 2.20.1

+ 

@@ -0,0 +1,1 @@ 

+ SHA512 (zsh-5.6.2.tar.xz) = f0a49e41b55eb478692ab5471d7c9828956b7e96bc82944202b0ef1c49a889b21a0e7682aa5f59fd0054ebfd866c2244c8a622e7aa46c13038af5c226c48a3a2

@@ -0,0 +1,160 @@ 

+ Summary: Spec file for zsh to test lookaside cache

+ Name: zsh

+ Version: 5.6.2

+ Release: 3%{?dist}

+ License: MIT

+ URL: http://zsh.sourceforge.net/

+ Source0: https://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.xz

+ Source1: zlogin.rhs

+ Source2: zlogout.rhs

+ Source3: zprofile.rhs

+ Source4: zshrc.rhs

+ Source5: zshenv.rhs

+ Source6: dotzshrc

+ 

+ Patch1:  create-file.patch

+ 

+ BuildRequires: autoconf

+ BuildRequires: coreutils

+ BuildRequires: gawk

+ BuildRequires: gcc

+ BuildRequires: gdbm-devel

+ BuildRequires: libcap-devel

+ BuildRequires: ncurses-devel

+ BuildRequires: pcre-devel

+ BuildRequires: sed

+ BuildRequires: texi2html

+ BuildRequires: texinfo

+ Requires(post): info grep

+ Requires(preun): info

+ Requires(postun): coreutils grep

+ 

+ # the hostname package is not available on RHEL-6

+ %if 12 < 0%{?fedora} || 6 < 0%{?rhel}

+ BuildRequires: hostname

+ %else

+ # /bin and /usr/bin are separate directories on RHEL-6

+ %define _bindir /bin

+ %endif

+ 

+ Provides: /bin/zsh

+ 

+ %description

+ Spec file for zsh to test lookaside cache

+ 

+ %package html

+ Summary: Zsh shell manual in html format

+ BuildArch:	noarch

+ 

+ %description html

+ Spec file for zsh to test lookaside cache

+ 

+ %prep

+ %autosetup -p1

+ 

+ # enable parallel build

+ sed -e 's|^\.NOTPARALLEL|#.NOTPARALLEL|' -i 'Config/defs.mk.in'

+ 

+ %build

+ # make build of run-time loadable modules work again (#1535422)

+ %undefine _strict_symbol_defs_build

+ 

+ # make loading of module's dependencies work again (#1277996)

+ export LIBLDFLAGS='-z lazy'

+ 

+ %configure \

+     --enable-etcdir=%{_sysconfdir} \

+     --with-tcsetpgrp \

+     --enable-maildir-support \

+     --enable-pcre

+ 

+ # prevent the build from failing while running in parallel

+ make -C Src headers

+ make -C Src -f Makemod zsh{path,xmod}s.h version.h

+ 

+ make %{?_smp_mflags} all html

+ 

+ %check

+ # Run the testsuite

+ make check

+ 

+ %install

+ %make_install install.info \

+   fndir=%{_datadir}/%{name}/%{version}/functions \

+   sitefndir=%{_datadir}/%{name}/site-functions \

+   scriptdir=%{_datadir}/%{name}/%{version}/scripts \

+   sitescriptdir=%{_datadir}/%{name}/scripts \

+   runhelpdir=%{_datadir}/%{name}/%{version}/help

+ 

+ rm -f $RPM_BUILD_ROOT%{_bindir}/zsh-%{version}

+ rm -f $RPM_BUILD_ROOT%{_infodir}/dir

+ 

+ mkdir -p ${RPM_BUILD_ROOT}%{_sysconfdir}

+ for i in %{SOURCE1} %{SOURCE2} %{SOURCE3} %{SOURCE4} %{SOURCE5}; do

+     install -m 644 $i $RPM_BUILD_ROOT%{_sysconfdir}/"$(basename $i .rhs)"

+ done

+ 

+ mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/skel

+ install -m 644 %{SOURCE6} $RPM_BUILD_ROOT%{_sysconfdir}/skel/.zshrc

+ 

+ # This is just here to shut up rpmlint, and is very annoying.

+ # Note that we can't chmod everything as then rpmlint will complain about

+ # those without a she-bang line.

+ for i in checkmail harden run-help zcalc zkbd; do

+     sed -i -e 's!/usr/local/bin/zsh!%{_bindir}/zsh!' \

+     $RPM_BUILD_ROOT%{_datadir}/zsh/%{version}/functions/$i

+     chmod +x $RPM_BUILD_ROOT%{_datadir}/zsh/%{version}/functions/$i

+ done

+ 

+ 

+ %post

+ if [ "$1" = 1 ]; then

+   if [ ! -f %{_sysconfdir}/shells ] ; then

+     echo "%{_bindir}/%{name}" > %{_sysconfdir}/shells

+     echo "/bin/%{name}" >> %{_sysconfdir}/shells

+   else

+     grep -q "^%{_bindir}/%{name}$" %{_sysconfdir}/shells || echo "%{_bindir}/%{name}" >> %{_sysconfdir}/shells

+     grep -q "^/bin/%{name}$" %{_sysconfdir}/shells || echo "/bin/%{name}" >> %{_sysconfdir}/shells

+   fi

+ fi

+ 

+ if [ -f %{_infodir}/zsh.info.gz ]; then

+ # This is needed so that --excludedocs works.

+ /sbin/install-info %{_infodir}/zsh.info.gz %{_infodir}/dir \

+   --entry="* zsh: (zsh).			An enhanced bourne shell."

+ fi

+ 

+ 

+ %preun

+ if [ "$1" = 0 ] ; then

+     if [ -f %{_infodir}/zsh.info.gz ]; then

+     # This is needed so that --excludedocs works.

+     /sbin/install-info --delete %{_infodir}/zsh.info.gz %{_infodir}/dir \

+       --entry="* zsh: (zsh).			An enhanced bourne shell."

+     fi

+ fi

+ 

+ %postun

+ if [ "$1" = 0 ] && [ -f %{_sysconfdir}/shells ] ; then

+   sed -i '\!^%{_bindir}/%{name}$!d' %{_sysconfdir}/shells

+   sed -i '\!^/bin/%{name}$!d' %{_sysconfdir}/shells

+ fi

+ 

+ 

+ %files

+ %doc README LICENCE Etc/BUGS Etc/CONTRIBUTORS Etc/FAQ FEATURES MACHINES

+ %doc NEWS Etc/zsh-development-guide Etc/completion-style-guide

+ %attr(755,root,root) %{_bindir}/zsh

+ %{_mandir}/*/*

+ %{_infodir}/*

+ %{_datadir}/zsh

+ %{_libdir}/zsh

+ %config(noreplace) %{_sysconfdir}/skel/.z*

+ %config(noreplace) %{_sysconfdir}/z*

+ 

+ %files html

+ %doc Doc/*.html

+ 

+ %changelog

+ * Fri Nov 30 2018 Kamil Dudka <kdudka@redhat.com> - 5.6.2-3

+ - return non-zero exit status on nested parse error (#1654989)

file modified
+1
@@ -3,3 +3,4 @@ 

  - import_playbook: basic.yml

  - import_playbook: beakerlib.yml

  - import_playbook: avocado.yml

+ - import_playbook: source.yml

Use the rather standard HTTP API for the sources file and the dist-git
lookaside cache to retrieve sources. Don't rely on the fedpkg or rhpkg
tooling because these are unnecessarily diverged between Fedora and RHEL

Solves Issue #271

Worked on this during the holidays. This needs a test if we agree with the general approach

Must be changed to specific version of Python Interpreter. #!/usr/bin/python3

We will be asked to remove this line. Internal RH names are prohibited to go public. Just a warning. I was asked to remove internal links a few times before.

Not sure, why independent function makes assumption that parent directory was created for before its call.

Please, go to Python 3, and do not import individual functions. This harms code standards.

Make urlopen as part of corresponding module. urllib.request.urlopen() or request.urlopen()

I wonder, if it is more appropriate use logger. logger can output code to stderr too.

In upstream code no place for such commented lines.

Just, a quick question: does this script will correctly process sources file with multi-lines, such as: https://src.fedoraproject.org/rpms/kernel/blob/master/f/sources ?

Definitely I would move above Ansible plugin to pre-defined path for third-part plugins dir and make in available to all system.

Must be changed to specific version of Python Interpreter. #!/usr/bin/python3

Can I drop python2 support all together? If so, that'd be great!

Definitely I would move above Ansible plugin to pre-defined path for third-part plugins dir and make in available to all system.

I think this module should only be exposed as a role. Ansible discourages writing modules, and roles are the preferred form for interacting modules. Lets move this into the pre-defined path once there are more than one role that needs to call this module.

Just, a quick question: does this script will correctly process sources file with multi-lines, such as: https://src.fedoraproject.org/rpms/kernel/blob/master/f/sources ?

Yes, it's written like that, but we can make sure it works with a test.

In upstream code no place for such commented lines.

Will add a verbose option.

Not sure, why independent function makes assumption that parent directory was created for before its call.

This is pretty normal, but I'll change it if it helps get this merged ;)

2 new commits added

  • FIXUP run on python3
  • FIXUP for review
5 years ago

python3 doesn't work for a ansible module, at least not on Fedora 29. The module appears to be loaded in python2 directly. It is not executed as a script. I have verified this, and marked the source-lookaside.py file as executable ... but it doesn't help.

Therefore I am retaining python2/python3 support in the source-lookaside.py file.

Any ideas how can I resolve this while remaining true to the Standard Test Interface, which perscribes invoking "ansible-playbook" as the entry point?

Make urlopen as part of corresponding module. urllib.request.urlopen() or request.urlopen()

Until we are able to use python3 modules via the standard 'ansible-playbook' command, I don't believe I can fix this.

rebased onto 8165be137fa158d4ea99c3c7da87d64f07d2f4b6

5 years ago

pretty please pagure-ci rebuild

5 years ago

pretty please pagure-ci rebuild

5 years ago

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

rebased onto b57b8df02630c70de04c83c1c8ff0724f0b43844

5 years ago

pretty please pagure-ci rebuild

5 years ago

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

pretty please pagure-ci rebuild

5 years ago

rebased onto 8d4bc96

5 years ago

pretty please pagure-ci rebuild

5 years ago

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

pretty please pagure-ci rebuild

5 years ago

pretty please pagure-ci rebuild

5 years ago

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

pretty please pagure-ci rebuild

5 years ago

Must be changed to specific version of Python Interpreter. #!/usr/bin/python3

Can I drop python2 support all together? If so, that'd be great!

Yes, we should pick up only Python3. Python2 is deprecated in Fedora now.

python3 doesn't work for a ansible module, at least not on Fedora 29. The module appears to be loaded in python2 directly. It is not executed as a script. I have verified this, and marked the source-lookaside.py file as executable ... but it doesn't help.
Therefore I am retaining python2/python3 support in the source-lookaside.py file.
Any ideas how can I resolve this while remaining true to the Standard Test Interface, which perscribes invoking "ansible-playbook" as the entry point?

Make urlopen as part of corresponding module. urllib.request.urlopen() or request.urlopen()

Until we are able to use python3 modules via the standard 'ansible-playbook' command, I don't believe I can fix this.

It should work.
Please make sure you call ansible-3 or ansible-playbook-3

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

It should work. Please make sure you call ansible-3 or ansible-playbook-3

Neither the standard test interface, nor developer documentation, describes calling "ansible-3" or "ansible-playbook-3". Until it does (and requires python3 and ansible3 everywhere), we should retain 2 and 3 compatibility.

All other review comments have been addressed.

It should work. Please make sure you call ansible-3 or ansible-playbook-3

The standard test interface does not describe calling "ansible-3" or "ansible-playbook-3". Until it does (and thus requires python3 and ansible3), we should retain 2 and 3 compatibility.
All other review comments have been addressed.

Personally I have opposite view.
Python2 is officially deprecated in Fedora.
All Fedora community moved toward Python3.
Python2 is NOT installed by default.
There is Ansible build that uses only Python3.

In my understanding STI says: run Ansible playbook.
STI doesn't mention about Python2.
Moreover, if Ansible command will always stay as ansible-3 and ansible-playbook-3 than you doom STI for using Python2 forever. Binding STI to specific Python version is bad.

Up to now, current master uses ONLY Python3.
There is released version 3.0 STR that uses only Python3.
With this approach you suggest to install in system two Pythons.
My point, let's be flexible, modern, and follow community tendencies.
I announced to mail list that STR was completely migrated to Python3.
All feedback was only positive.
My point: we should retain 2 and 3 compatibility. --- no, we should not.
In current Fedora/RHEL8 you MUST specify /usr/bin/python3
For me it is very strange to call deprecated Ansible for Python3 code.

Keeping this PR until receive some feedback from @dperpeet @bookwar @mvadkert @bgoncalv @plautrba @vdolezal or other people.

My point: we should retain 2 and 3 compatibility. --- no, we should not.
In current Fedora/RHEL8 you MUST specify /usr/bin/python3
For me it is very strange to call deprecated Ansible for Python3 code.

I'm with you: that we should move to python3. Happy you're planning ahead.

However, there is a lot of work necessary (to update documentation and reeducate maintainers) before the Standard Test Interface supports only Python3.

All of these document python 2 and ansible with python2:
* Landing page: https://fedoraproject.org/wiki/CI
* Our documented interface: https://fedoraproject.org/wiki/Changes/InvokingTests
* Our documented tutorials: https://fedoraproject.org/wiki/CI/Quick_Start_Guide
* Our test creation guide: https://fedoraproject.org/wiki/CI/Tests

Where is the work to migrate to python 3 being tracked?

As a real example, if I were to drop python 2 from this pull request, the standard-test-source role would just fail with a big exception for maintainers that are following our docs.

Again, I'm not against dropping python 2 support, and I'm really glad you're thinking ahead here. But that work must be done properly or it breaks the workflows of our users in an unacceptable way.

Checking which docs need to be updated. But to make it clear: Are we now supposed to use ansible-playbook-3 to run tests? There does not seem to be such binary in ansible-2.7.5-1.fc29.noarch. I've tested with ansible-playbook and it works fine.

@psss thank you, right

[root@host-8-254-147 standard-test-roles]# which ansible-playbook 
/usr/bin/ansible-playbook
[root@host-8-254-147 standard-test-roles]# rpm -qf /usr/bin/ansible-playbook
ansible-2.7.5-1.fc29.noarch
[root@host-8-254-147 standard-test-roles]# rpm -qR ansible
/usr/bin/python3
config(ansible) = 2.7.5-1.fc29
python(abi) = 3.7
python3-PyYAML
python3-crypto
python3-jinja2
python3-jmespath
python3-paramiko
python3-setuptools
python3-six
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1
sshpass

All are Python3

Checking which docs need to be updated. But to make it clear: Are we now supposed to use ansible-playbook-3 to run tests? There does not seem to be such binary in ansible-2.7.5-1.fc29.noarch. I've tested with ansible-playbook and it works fine.

It works fine with the pull request as posted. But when I make the following changes (to drop python2 support) it fails with this exception:

--- a/roles/standard-test-source/library/source-lookaside.py
+++ b/roles/standard-test-source/library/source-lookaside.py
@@ -33,13 +33,9 @@ import re
 import os
 import shutil

-try:
-    from urllib.request import urlopen
-    from urllib.error import URLError
-    from configparser import ConfigParser
-except ImportError:
-    from urllib2 import urlopen, URLError
-    from ConfigParser import ConfigParser
+import urllib.request
+import urllib.error
+import configparser

 ANSIBLE_METADATA = {
     'metadata_version': '1.1',
@@ -125,7 +121,7 @@ logger.setLevel(logging.INFO)
 def lookasides():
     for url in LOOKASIDES:
         yield url + LOOKASIDE_URI
-    config = ConfigParser()
+    config = configparser.ConfigParser()
     config.read(glob.glob(LOOKASIDE_CONFIG))
     for section in config.sections():
         if config.has_option(section, "lookaside"):
@@ -164,8 +160,8 @@ def retrieve(url, target):

     try:
         with open(dest, 'wb') as fp:
-            shutil.copyfileobj(urlopen(url), fp)
-    except URLError as ex:
+            shutil.copyfileobj(urllib.request.urlopen(url), fp)
+    except urllib.error.URLError as ex:
         if not hasattr(ex, "code") or ex.code != 404:
             logger.error("{0}: {1} {2}".format(name, url, str(ex)))
         return None

With this exception:

MODULE FAILURE
See stdout/stderr for the exact error


MODULE_STDERR:

Traceback (most recent call last):
  File "/root/.ansible/tmp/ansible-tmp-1547728140.5455825-143940266836305/AnsiballZ_source-lookaside.py", line 113, in <module>
    _ansiballz_main()
  File "/root/.ansible/tmp/ansible-tmp-1547728140.5455825-143940266836305/AnsiballZ_source-lookaside.py", line 105, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/root/.ansible/tmp/ansible-tmp-1547728140.5455825-143940266836305/AnsiballZ_source-lookaside.py", line 48, in invoke_module
    imp.load_module('__main__', mod, module, MOD_DESC)
  File "/tmp/ansible_source-lookaside_payload_s2CL_w/__main__.py", line 36, in <module>
ImportError: No module named request

This is because:

$ python2
Python 2.7.15 (default, Oct 15 2018, 15:26:09) 
[GCC 8.2.1 20180801 (Red Hat 8.2.1-2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import urllib.request
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named request

Whereas python3 works fine. Hence I added the python 2/3 try/import stuff in the original pull request.

1 new commit added

  • FIXUP remove Python 3 support
5 years ago

I've pushed the above patch as a FIXUP in this pull request. So you can see if you can reproduce the issue. I'm running on:

  • Fedora 29
  • ansible-2.7.5-1.fc29.noarch

And I run this command to reproduce the problem:

sudo ansible-playbook tests/source.yml

Or run tests.yml in any dist-git repo that uses standard-roles-source ... to also reproduce.

@stefw you are right.

Ansible plugins must be both Python2/Python3 compatible.

# ansible all -i localhost, -m setup --extra-vars "ansible_connection=local" | grep python
            "_": "/usr/bin/python"
        "ansible_python": {
            "executable": "/usr/bin/python",
        "ansible_python_version": "2.7.15",
        "ansible_selinux_python_present": false,

And, even Anisible-engine runs with help of Python3 all actions are done with help of Python2.

Could you please bring back compatibility with Python2 only plugin code? Thank you for your time.

Thank you. Force pushed. Hopefully this is ready to go in now.

2 new commits added

  • Fix standard-test-source to work on Fedora or RHEL
  • Basic test for standard-test-source
5 years ago

Commit 806aa1a fixes this pull-request

Pull-Request has been merged by astepano

5 years ago

Pull-Request has been merged by astepano

5 years ago

pretty please pagure-ci rebuild

Metadata