Let's use 2 words separated by dash here too :)

Perhaps this should be log.error of log.fatal?

You may want to ask for something more than "Y" here. Like "are you sure?" "yes I'm sure". Copy-paste is a good way to check. Github does this by making you type the repo name before you are allowed to delete it.

I agree with @vashirov, this should be log.error

Maybe just do conf_dir = (prefix or "") + "/etc/dirsrv/".

In the usage, we have -
remove - Destroy an instance of Directory Server, and remove all data.

But in actual it takes "--doit" to do it -
[root@172 upstream]# dsctl server remove
Not removing: if you are sure, add --do-it

Whereas for the "--removeall" option, it asks like this -
Are you sure you want to remove all the Directory Server instances? (y/n): y

IMO, Some consistency would be better here.
I suggest to have - Are you sure you want to remove all the Directory Server instances? (yes/no):
for both the options for consistency.
Thanks.

In the usage, we have -
remove - Destroy an instance of Directory Server, and remove all data.
But in actual it takes "--doit" to do it -
[root@172 upstream]# dsctl server remove
Not removing: if you are sure, add --do-it
Whereas for the "--removeall" option, it asks like this -
Are you sure you want to remove all the Directory Server instances? (y/n): y
IMO, Some consistency would be better here.
I suggest to have - Are you sure you want to remove all the Directory Server instances? (yes/no):
for both the options for consistency.

This is actually being discussed in https://pagure.io/389-ds-base/issue/50022

Since no one was in agreement I did not change "dsctl remove". As for not using "--do-it" with "--remove-all" that is because from the usage (arg parsing) perspective to have "--remote-all" and "--do-it" does not make sense. - so prompting for confirmation made more sense. We should probably do it "dsctl remove", but like I said there is disagreement over that at the moment.

New changes applied and rebased, please review...

Typo...

If you create an instance with a custom port (like 38901) it will complain on the removal:

Removed /etc/systemd/system/multi-user.target.wants/dirsrv@localhost1.service.
ValueError: Port tcp/38901 is not defined
ValueError: Port tcp/38901 is not defined
ValueError: Port tcp/38901 is not defined
CRITICAL: Failed to remove all instances: Failed to mangle port label: Command '['semanage', 'port', '-d', '-t', 'ldap_port_t', '-p', 'tcp', '38901']' returned non-zero exit status 1.

But the removal still happens.

Another thing, the UI doesn't count the changes and fails while I try to create an instance.

The rest looks good to me! Thanks!

If you create an instance with a custom port (like 38901) it will complain on the removal:
Removed /etc/systemd/system/multi-user.target.wants/dirsrv@localhost1.service.
ValueError: Port tcp/38901 is not defined
ValueError: Port tcp/38901 is not defined
ValueError: Port tcp/38901 is not defined
CRITICAL: Failed to remove all instances: Failed to mangle port label: Command '['semanage', 'port', '-d', '-t', 'ldap_port_t', '-p', 'tcp', '38901']' returned non-zero exit status 1.

But the removal still happens.

This is being addressed in this PR: https://pagure.io/389-ds-base/pull-request/50045

Fixed the other issue in UI for "fromfile" to "from-file".

Sorry, I missed one more issue in the UI - '--doit' -> '--do-it'

The rest looks good to me! 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/3093

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.