#49101 Python 2 generate example entries
Closed: Fixed None Opened 2 years ago by firstyear.

Generate the example entries in lib389 for testing.


Very nice improvements! I like the most parts. And I only have a few questions.

Now, as you've removed "if MAJOR < 3", '''lib389/tests/instance/setup_test.py''' are failing.
As I understand, "-" doesn't work for lists in both Python 2 and Python 3.

{{{
self = <lib389.instance.options.slapd2base object="" at="" 0x7fcd1ea5d4d0="">
d = {'backup_dir': '/var/lib/dirsrv/slapd-{instance_name}/bak', 'bin_dir': '/usr/bin', 'cert_dir': '/etc/dirsrv/slapd-{instance_name}', 'config_dir': '/etc/dirsrv/slapd-{instance_name}', ...}

def _format(self, d):
    new_d = {}
    ks = d.keys()
  no_format_keys = ks - format_keys

E TypeError: unsupported operand type(s) for -: 'list' and 'list'

lib389/instance/options.py:246: TypeError
}}}

Shouldn't we use paths from '''defaults.inf'''?

{{{
ktab = '%s/etc/dirsrv/slapd-%s/ldap.keytab' % (self.prefix, self.serverid)
}}}

Can you describe this part a bit more, please? Why do we have such strange module names? I am not sure that it is really required to version configurations like this... I need to think about it more.

{{{
from .config_001003006 import c001003006, c001003006_sample_entries
}}}

Will it work on RHEL 6? I am not sure because it doesn't have Python 3 installed and we still need to test it.

{{{

!/usr/bin/python3

}}}

Was it by mistake put here or on purpose?

{{{

vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4

}}}

Replying to [comment:2 spichugi]:

Very nice improvements! I like the most parts. And I only have a few questions.

Now, as you've removed "if MAJOR < 3", '''lib389/tests/instance/setup_test.py''' are failing.
As I understand, "-" doesn't work for lists in both Python 2 and Python 3.

{{{
self = <lib389.instance.options.slapd2base object="" at="" 0x7fcd1ea5d4d0="">
d = {'backup_dir': '/var/lib/dirsrv/slapd-{instance_name}/bak', 'bin_dir': '/usr/bin', 'cert_dir': '/etc/dirsrv/slapd-{instance_name}', 'config_dir': '/etc/dirsrv/slapd-{instance_name}', ...}

def _format(self, d):
    new_d = {}
    ks = d.keys()
  no_format_keys = ks - format_keys

E TypeError: unsupported operand type(s) for -: 'list' and 'list'

lib389/instance/options.py:246: TypeError
}}}

I believe I fix that in the python 2 support patch, I rely on that in this fix :) I just haven't merged it yet sorry :(

Shouldn't we use paths from '''defaults.inf'''?

{{{
ktab = '%s/etc/dirsrv/slapd-%s/ldap.keytab' % (self.prefix, self.serverid)
}}}

Why yes we should! I'll update that.

Can you describe this part a bit more, please? Why do we have such strange module names? I am not sure that it is really required to version configurations like this... I need to think about it more.

{{{
from .config_001003006 import c001003006, c001003006_sample_entries
}}}

Will it work on RHEL 6? I am not sure because it doesn't have Python 3 installed and we still need to test it.

They will work on python 2

The module names are related to the config version. The idea is that right now we are tied to a template ldif, horrid upgrade scripts, terrible sample entries. But people, and our tests depend on them.

These module names map to the version of the configuration and entries you want to use. So we decouple the installer from a version.

the c001003006 -> 1.3.6. The idea is we could get to version 999.999.999 without fear ;) It also means we can properly sort by version as we increment. IE 1.4.0 > 1.3.9, ergo 001004000 > 001003009.

The idea is in a test, we can say "hey this works with the 1.3.6 config" and we can stick to it. We can then have tests that rely on newer config, different sample entries. We can star to change and adapt our server, and the setup-py installer can provide this to clients who want to map there server to specific versions.

The other goal is that this replaces the awful perl upgrade scripts in DS, where we say "hey, you can upgrade from X to Y just by applying these stateful configs in order". We solve upgrades and installs in one!

{{{

!/usr/bin/python3

}}}

This one mistake.

Was it by mistake put here or on purpose?

{{{

vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4

}}}

That was purpose, but I can remove it.

Replying to [comment:3 firstyear]:

Replying to [comment:2 spichugi]:

Can you describe this part a bit more, please? Why do we have such strange module names? I am not sure that it is really required to version configurations like this... I need to think about it more.

{{{
from .config_001003006 import c001003006, c001003006_sample_entries
}}}

Will it work on RHEL 6? I am not sure because it doesn't have Python 3 installed and we still need to test it.

They will work on python 2
The last line was about next code block you've commented later. :)

The module names are related to the config version. The idea is that right now we are tied to a template ldif, horrid upgrade scripts, terrible sample entries. But people, and our tests depend on them.

These module names map to the version of the configuration and entries you want to use. So we decouple the installer from a version.

the c001003006 -> 1.3.6. The idea is we could get to version 999.999.999 without fear ;) It also means we can properly sort by version as we increment. IE 1.4.0 > 1.3.9, ergo 001004000 > 001003009.

The idea is in a test, we can say "hey this works with the 1.3.6 config" and we can stick to it. We can then have tests that rely on newer config, different sample entries. We can star to change and adapt our server, and the setup-py installer can provide this to clients who want to map there server to specific versions.

The other goal is that this replaces the awful perl upgrade scripts in DS, where we say "hey, you can upgrade from X to Y just by applying these stateful configs in order". We solve upgrades and installs in one!

It is okay, I agree. My concern is only about module name.
In the import we have:

{{{
from .config_001003006 import c001003006, c001003006_sample_entries
}}}

It seems redundant to me to have config_001003006.py and the classes within this module with the same 001003006 number.
I think, there is a number of ways to solve this redundancy (like maybe we can put all version configs into one .py module). What do you think?

Replying to [comment:4 spichugi]:

Replying to [comment:3 firstyear]:

Replying to [comment:2 spichugi]:

Can you describe this part a bit more, please? Why do we have such strange module names? I am not sure that it is really required to version configurations like this... I need to think about it more.

{{{
from .config_001003006 import c001003006, c001003006_sample_entries
}}}

Will it work on RHEL 6? I am not sure because it doesn't have Python 3 installed and we still need to test it.

They will work on python 2
The last line was about next code block you've commented later. :)

Now it makes more sense >.< Still I'll apply your feedback.

The module names are related to the config version. The idea is that right now we are tied to a template ldif, horrid upgrade scripts, terrible sample entries. But people, and our tests depend on them.

These module names map to the version of the configuration and entries you want to use. So we decouple the installer from a version.

the c001003006 -> 1.3.6. The idea is we could get to version 999.999.999 without fear ;) It also means we can properly sort by version as we increment. IE 1.4.0 > 1.3.9, ergo 001004000 > 001003009.

The idea is in a test, we can say "hey this works with the 1.3.6 config" and we can stick to it. We can then have tests that rely on newer config, different sample entries. We can star to change and adapt our server, and the setup-py installer can provide this to clients who want to map there server to specific versions.

The other goal is that this replaces the awful perl upgrade scripts in DS, where we say "hey, you can upgrade from X to Y just by applying these stateful configs in order". We solve upgrades and installs in one!

It is okay, I agree. My concern is only about module name.
In the import we have:

{{{
from .config_001003006 import c001003006, c001003006_sample_entries
}}}

It seems redundant to me to have config_001003006.py and the classes within this module with the same 001003006 number.
I think, there is a number of ways to solve this redundancy (like maybe we can put all version configs into one .py module). What do you think?

This is one point I don't want to change.

Straight up, look at init.py in the root of the project. It's a disaster. I will not put all of these into one file, because we'll have the same issue.

The idea is you can copy-paste the file, rename, and then add the new config for a new version.

As well, we need to be able to import every module at the same time into the configurations/init.py, so that get_version works. This necessitates unique names.

So I see that it's "redundant", but there is method to my madness :)

Sure, I get your point and mostly agree with it.
Let's leave the naming like this. You have my ack (after the changes we've agreed on).

New patch with your fixes, I'll let you flick the ack button to be sure though :)

Replying to [comment:7 firstyear]:

New patch with your fixes, I'll let you flick the ack button to be sure though :)

It seems like you haven't pushed some other patch also. I can't apply your patch, though I'd like to run tests with it...

{{{

Applying: Ticket 49101 - Python 2 generate example entries
error: examples/ds-setup.inf: does not exist in index
.git/rebase-apply/patch:265: new blank line at EOF.
+
.git/rebase-apply/patch:315: new blank line at EOF.
+
.git/rebase-apply/patch:422: new blank line at EOF.
+
.git/rebase-apply/patch:479: new blank line at EOF.
+
.git/rebase-apply/patch:567: new blank line at EOF.
+
Patch failed at 0001 Ticket 49101 - Python 2 generate example entries
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
}}}

With the new 49103 commit pushed by you I can apply this patch.
Now '''lib389/tests/configurations/config_001003006_test.py''' fails during the instance startup:

{{{
[07/Feb/2017:05:49:24.775802389 -0500] - ERR - make_sure_dir_exists - Error - Unable to create /var/lock/dirsrv/slapd-standalone_1/imports, Netscape Portable Runtime error -5966 (Access Denied.)
[07/Feb/2017:05:49:24.776259825 -0500] - CRIT - main - Shutting down due to possible conflicts with other slapd processes
}}}

Build tested: '''389-ds-base-1.3.6.1-20170207git5e46302.fc25.x86_64'''
I still haven't a time to take a deeper look at the problem though...

Did you run the test with sudo?

Replying to [comment:9 spichugi]:

With the new 49103 commit pushed by you I can apply this patch.
Now '''lib389/tests/configurations/config_001003006_test.py''' fails during the instance startup:

{{{
[07/Feb/2017:05:49:24.775802389 -0500] - ERR - make_sure_dir_exists - Error - Unable to create /var/lock/dirsrv/slapd-standalone_1/imports, Netscape Portable Runtime error -5966 (Access Denied.)
[07/Feb/2017:05:49:24.776259825 -0500] - CRIT - main - Shutting down due to possible conflicts with other slapd processes
}}}

Build tested: '''389-ds-base-1.3.6.1-20170207git5e46302.fc25.x86_64'''
I still haven't a time to take a deeper look at the problem though...

{{{
{william@ldapkdc 9:45} ~/development/389ds I0> sudo PYTHONPATH=pwd/lib389 py.test lib389/lib389/tests/configurations/config_001003006_test.py
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.5 -- py-1.4.27 -- pytest-2.7.0
rootdir: /home/william/development/389ds/lib389, inifile:
collected 1 items

lib389/lib389/tests/configurations/config_001003006_test.py .

}}}

Replying to [comment:10 firstyear]:

Did you run the test with sudo?
Yes, also I have it on a fresh OpenStack machine. There is output. I'll send you an email with our Downstream CI details to reproduce the issue.

{{{
[root@qeos-95 lib389]# py.test -s -v lib389/tests/configurations/
===================================== test session starts =====================================
platform linux2 -- Python 2.7.13, pytest-2.9.2, py-1.4.32, pluggy-0.3.1 -- /usr/bin/python2
cachedir: .cache
DS build: 1.3.6.1.20170208git61953fc B2017.039.745
389-ds-base: 1.3.6.1-20170208git61953fc.fc25
nss: 3.28.1-1.3.fc25
nspr: 4.13.1-1.fc25
openldap: 2.4.44-7.fc25
svrcore: 4.1.2-1.fc25

rootdir: /mnt/tests/rhds/tests/upstream/src/lib389, inifile:
plugins: html-1.13.0, cov-2.4.0, beakerlib-0.6
collected 1 items

lib389/tests/configurations/config_001003006_test.py::test_install_sample_entries Job for dirsrv@standalone_1.service failed because the control process exited with error code.
See "systemctl status dirsrv@standalone_1.service" and "journalctl -xe" for details.
ERROR

=========================================== ERRORS ============================================
___ ERROR at setup of test_install_sample_entries ___

request = <subrequest 'topology_st'="" for="" <function="" 'test_install_sample_entries'="">>

@pytest.fixture(scope="module")
def topology_st(request):
    """Create DS standalone instance"""

    if DEBUGGING:
        standalone = DirSrv(verbose=True)
    else:
        standalone = DirSrv(verbose=False)
    args_instance[SER_HOST] = HOST_STANDALONE1
    args_instance[SER_PORT] = PORT_STANDALONE1
    # args_instance[SER_SECURE_PORT] = SECUREPORT_STANDALONE1
    args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE1
    args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX
    args_standalone = args_instance.copy()
    standalone.allocate(args_standalone)
    instance_standalone = standalone.exists()
    if instance_standalone:
        standalone.delete()
  standalone.create(pyinstall=True, version=INSTALL_LATEST_CONFIG)

lib389/tests/configurations/config_001003006_test.py:76:


lib389/init.py:978: in create
self._createPythonDirsrv(version)
lib389/init.py:930: in _createPythonDirsrv
sds.create_from_args(general, slapd, backends, None)
lib389/instance/setup.py:296: in create_from_args
self._install_ds(general, slapd, backends)
lib389/instance/setup.py:410: in _install_ds
ds_instance.start(timeout=60)
lib389/init.py:1176: in start
"dirsrv@%s" % self.serverid])


popenargs = (['/usr/bin/systemctl', 'start', 'dirsrv@standalone_1'],), kwargs = {}, retcode = 1
cmd = ['/usr/bin/systemctl', 'start', 'dirsrv@standalone_1']

def check_call(*popenargs, **kwargs):
    """Run command with arguments.  Wait for command to complete.  If
    the exit code was zero then return, otherwise raise
    CalledProcessError.  The CalledProcessError object will have the
    return code in the returncode attribute.

    The arguments are the same as for the Popen constructor.  Example:

    check_call(["ls", "-l"])
    """
    retcode = call(*popenargs, **kwargs)
    if retcode:
        cmd = kwargs.get("args")
        if cmd is None:
            cmd = popenargs[0]
      raise CalledProcessError(retcode, cmd)

E CalledProcessError: Command '['/usr/bin/systemctl', 'start', 'dirsrv@standalone_1']' returned non-zero exit status 1

/usr/lib64/python2.7/subprocess.py:186
}}}
CalledProcessError
=================================== 1 error in 0.65 seconds ===================================

I know what the problem is. I'm not installing the unit file.

Will update tomorrow morning.

https://bugzilla.redhat.com/show_bug.cgi?id=1420612

You need to setenforce 0, or semanage permissive -a dirsrv_t to run this.

If I set "semanage permissive -a dirsrv_t" the new installer won't fail.

Thanks for your review, I really appreciated it!

commit 12e0b4db89fe1afd00232a77fb86f07b84008305
Writing objects: 100% (40/40), 12.53 KiB | 0 bytes/s, done.
Total 40 (delta 22), reused 1 (delta 0)
To ssh://git.fedorahosted.org/git/389/lib389.git
a76c649..12e0b4d master -> master

Metadata Update from @firstyear:
- Issue assigned to firstyear
- Issue set to the milestone: lib389 1.0.3

2 years ago

Login to comment on this ticket.

Metadata