With True I get

[root@server ds]# dscreate install test1
READY: Preparing installation for localhost
READY: Beginning installation for localhost
Created symlink /etc/systemd/system/multi-user.target.wants/dirsrv@localhost.service → /usr/lib/systemd/system/dirsrv@.service.
Error: version True no match

According to src/lib389/lib389/configurations/__init__.py it should be a numerical value matching DS version, for example '001004000'

Hmm yeah I need to look into this. I'm not sure I like this numerical format...

Beside the Viktor's concern, the code looks good to me.

@spichugi @vashirov I made some more changes to this one so if you don't mind revewing it again I'd appreciate it!

You should not assert this by default as it causes more issues than good.

You shouldn't change this from the default, because people may not want randomly named instances. If I setup a prd cluster I don't want an instance on one calle "slapd-penny" and the other being "slapd-flower" or something. It's better to let people just choose, and have a sane default otherwise.

Also this needs to stay as localhost else it breaks the containerisation.

You need a default password here else you berka container builds

There is already support for mulitple backends in the installer and inf format. I'm not sure what you're trying to do here .....

This is just for the template - people can name it whatever they want. Every customer case I've ever looked at is using the hostname as the server id. That makes the most sense for a default value instead of defaulting to 'localhost' which no one uses except for development. But how does this break containers?

I just made this optional, not required. What is the concern here? That getfqdn() will fail or is unreliable? Apparently gethostnmae() is not the best option for getting the fqdn, so I switch it to getfqdn(). Just curious what issues you are concerned about.

This has been made into a required value. So there really is no default. How does this break containers?

These template files are meant to be edited, so I don't understand how anything done here in this PR is breaking containers. You still have to edit the template file regardless if it's in a container or not. With this PR there is less to edit actually.

There is already support for mulitple backends in the installer and inf format. I'm not sure what you're trying to do here .....

Ummm, it didn't work, and there was no validation of values/types ;-) This was also for the UI changes as well - don't be so quick to dismiss this work.

Going back to containers...

Okay, so now I see why you were having some issues with the changes to the options in the example template. In your docker file you just create template, do not edit it, and then install from it.

The issue we had was that this does not work. We get errors trying to just create template, not edit it, and install it (missing required settings, etc). So we assumed that it was NOT meant to work that way, and that it was expected that you HAVE to edit the template. So from our standpoint once you have to edit it, does it really matter what the instance name defaults to? :-)

Now that I looked at the dockerfile I understand what you were saying and I will make some changes to this PR so that it does not break the container model.

The code looks good to me.
CLI backend creation works, and sample entries too.

but when I try create it through Web UI - it fails:

Error: No option 'require_index' in section: 'backend-userroot'

I didn't find the field for 'require_index'.
And when I create the backend in CLI (I uncomment and change 'sample_entries' and 'suffix') it does complain about 'require_index'. I think it should use the default if we don't specify it overwise.

The code looks good to me.
CLI backend creation works, and sample entries too.
but when I try create it through Web UI - it fails:
Error: No option 'require_index' in section: 'backend-userroot'

I didn't find the field for 'require_index'.
And when I create the backend in CLI (I uncomment and change 'sample_entries' and 'suffix') it does complain about 'require_index'. I think it should use the default if we don't specify it overwise.

It's supposed to, I'll investigate....

@spichugi fixed!

LGTM! Ack

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2868

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.