It fails on fedora 29...

Feb 11 05:17:19 host-8-246-147.host.centralci.eng.rdu2.redhat.com systemd[1]: /usr/lib/systemd/system/dirsrv@.service:16: .include directives are deprecated, and support for them will be removed in a future version of systemd. Please use drop-in files instead.
Feb 11 05:17:19 host-8-246-147.host.centralci.eng.rdu2.redhat.com systemd[1]: dirsrv@test_dscreate.service: Failed to load environment files: No such file or directory
Feb 11 05:17:19 host-8-246-147.host.centralci.eng.rdu2.redhat.com systemd[1]: dirsrv@test_dscreate.service: Failed to run 'start-pre' task: No such file or directory
Feb 11 05:17:19 host-8-246-147.host.centralci.eng.rdu2.redhat.com systemd[1]: dirsrv@test_dscreate.service: Failed with result 'resources'.

Also, we probably should check if FreeIPA works correctly with the change.

I wonder if that failure is an error in our systemd unit file?

@spichugi IPA will fail as they write the KRB5_KTNAME to /etc/sysconfig/dirsrv-instance still, so they need to make changes on their end.

Okay, so this rebase fixes that, and also fixes some of the other warnings you are getting from systemd there.

My understanding is that we only will need to move KRB5_KTNAME definition to /etc/systemd/system/dirsrv@<INSTANCE>.service.d/ipa.conf so that it will automatically be loaded by systemd.

Or we can actually keep /etc/sysconfig/dirsrv* as it is and create /etc/systemd/system/dirsrv@INSTANCE.service.d/ipa-sysconfig.conf that has

EnvironmentFile=-/etc/sysconfig/dirsrv
EnvironmentFile=-/etc/sysconfig/dirsrv-%i

This way we keep the existing configuration at place and if they are missing, they'll get simply ignored. Any additional configuration options can be written directly to /etc/systemd/system/dirsrv@<INSTANCE>.service.d/ipa.conf

FreeIPA ticket for tracking: https://pagure.io/freeipa/issue/7860

@firstyear, let's coordinate this change with FreeIPA to avoid an unnecessary breakage.

@abbra That would work. But I'm not removing the EnvironmentFile lines - I'm just not creating /etc/sysconfig/dirsrv-* by default anymore. So your env file solution already is there in the change?

So a better idea is you create the ipa-sysconfig.conf that contains "Environment=KRB5_KTNAME=..." I think. Alternately, you could do something like /etc/ipa/ds.conf and then have that as the EnvironmentFile=- that we include.

Either way, I think this is pretty easy for IPA to solve. Nothing in this directly breaks existing IPA installs, only new ones when they are created, and only because they probably expect /etc/sysconfig/dirsrv-instance to be there - if IPA recreates that file of their own, that's not going to bother me, and should keep working.

The build fails on F29 now...

BUILDSTDERR: make[1]: *** No rule to make target 'wrappers/dirsrv.systemd', needed by 'all-am'.  Stop.
make[1]: Leaving directory '/builddir/build/BUILD/389-ds-base-1.4.1.1.20190212git0c3045cf1'
BUILDSTDERR: make: *** [Makefile:4451: all] Error 2

I must have missed something in the make file then :) I'll check this out (I don't have a systemd capable build system and haven't used one in a long time ...)

Okay, this should fix the issue you are seeing. I think that maybe the next step is open an issue on freeipa (blocking?) about the env file needing to be created by them, but otherwise, I think we can't do much more here. Just need to make sure they are good to go.

https://pagure.io/freeipa/issue/7860 was created by @vashirov, and I've commented on it to help make the changes required clear.

@firstyear, yes I have no problem with this move -- we have all the code and example how to do that as we did the move for httpd.service some time ago to use the same scheme. As long as we coordinate a release time to Fedora 30 repositories, I'm fine.

BUILDSTDERR: error: File not found: /builddir/build/BUILDROOT/389-ds-base-1.4.1.1-20190213git08cf40f48.fc29.x86_64/etc/sysconfig/dirsrv

I think it really makes sense for you to have some environment for testing Fedora part...

@abbra Great, in that case, I'll leave it to you in the IPA ticket that has been opened to finish up that side. I'm sure we'll commit this once I finish fixing my mistakes :)

@spichugi I think this goes to a point where, yes, most of the developers of this project are on Fedora/RH. That was fine for a number of years, but it was exclusionary to debian/SUSE and others. Linux is not Fedora centric. This project is now getting broader attention, and to a point where SUSE is now financially investing in me to develop it as their LDAP platform for the future.

As a result it is not feasible for everyone to install every permutation and combination of OS to test on. We are an upstream project, and we need to do things in that light. And the downstream integrators to distributions need to also be mindful in reviews and testing for integration challenges. It is not logistically possible to just ask someone "ohh hey, do you mind installing a whole new OS ...". A response like that says our upstream is deeply tied into a specific vendor - that's not sustainable at all.

We need to step away from the RH/Fedora only mindset, and start to think about broader development and deployment scenarios. Diversity will promote health. For example I am probably the only person developing on SUSE, so I am finding those problems. I'm also likely the only person developing on and integrating in containers so I am finding those issues. This makes the project more robust and solid. I don't ask you to test on SUSE - that's what I'm paid to do.

My goal is 389 on SUSE and in containers. As an upstream our collective input is what will make 389 work best on a variety of platforms.

Now, I think the error you have pasted is from the rpm build - I think I have corrected it in the last commit.

It would, perhaps, help to set a simple CI test to build packages. Pagure supports Fedora CI integration so a test can be run easily with a container of any choice. I might look into adding this test interface support next week -- purely to allow building packages as a test of a pull request

An alternative would be for @firstyear to set up a space in OBS and let us trigger builds there for any pull request. OBS can build for Fedora and openSUSE already.

I agree with @firstyear that 389-ds should not be Fedora centric and it would be a constraint to ask on each PR to run build/test on many distrib/plateform. @abbra is right, PR-CI is a good answer to this. Now I am not a PR-CI expert, @vashirov do you know our status on that side and how much effort it would be to make PR-CI basic tests (build/install/basic) on several distrib/plateform ?

The last commit fixed the issue. And the code looks good to me. We can proceed here (according to the agreement with FreeIPA).

@spichugi I think this goes to a point where, yes, most of the developers of this project are on Fedora/RH. That was fine for a number of years, but it was exclusionary to debian/SUSE and others. Linux is not Fedora centric. This project is now getting broader attention, and to a point where SUSE is now financially investing in me to develop it as their LDAP platform for the future.

Sure. For now, we don't have the PR CI and if you'd have the Fedora environment - it will speed up the process (our time zones are too different and it takes to much time). That was my point.

If we'll have the PR CI then the issue will be solved.

Sounds good. I'll merge this shortly then. Thanks,

@firstyear - this breaks "dsctl remove". I can no longer remove instances after this PR was merged. Please fix ASAP or we will have to revert it. Thanks

Here is example showing the issue:

[root@localhost memberof_plugin]# ls /etc/dirsrv/
config slapd-consumer1 slapd-localhost slapd-master1 slapd-standalone1 ssca
schema slapd-hub1 slapd-stadalone1

[root@localhost memberof_plugin]# dsctl consumer1 remove --do-it
No such instance consumer1
[root@localhost memberof_plugin]# dsctl dirsrv-consumer1 remove --do-it
No such instance dirsrv-consumer1
[root@localhost memberof_plugin]# dsctl slapd-consumer1 remove --do-it
No such instance slapd-consumer1
[root@localhost memberof_plugin]# remove-ds.pl -i slapd-consumer1
Instance slapd-consumer1 removed.

So the old perl tools still work, but dsctl remove is now broken on fedora(and RHEL)

@firstyear I know this was asked of you before, but can you please test your patches on a Fedora VM before you summit them? It will save us all a lot of time and frustration. Thanks for your cooperation!!!

@mreynolds I'll investigate this today. I think revert is the wrong answer though. In general I'd like to say there is code merged by everyone else here that has broken docker, suse, and without-systemd support multiple times. I don't revert every issue I find, I fix the issues that exist in front of me (or at least investigate enough cause to work out what's going on). Revert is a heavy handed tactic, and I don't approve of it.

I don't have the resources timewise to test on Fedora, just the same way that you probably don't have the time to test on docker, opensuse and suse leap. I think part of being a community is always looking after different aspects, and just picking up as we go.

@mreynolds I'll investigate this today. I think revert is the wrong answer though. In general I'd like to say there is code merged by everyone else here that has broken docker, suse, and without-systemd support multiple times. I don't revert every issue I find, I fix the issues that exist in front of me (or at least investigate enough cause to work out what's going on). Revert is a heavy handed tactic, and I don't approve of it.
I don't have the resources timewise to test on Fedora, just the same way that you probably don't have the time to test on docker, opensuse and suse leap. I think part of being a community is always looking after different aspects, and just picking up as we go.

Revert is the last resort of course, but if this doesn't get fixed soon there will not be an option as there are already upstream builds done that now have this issue. I am being pressured to fix this asap...

Sorry to hear you are unwilling to install a fedora VM, it just means we will now have to spend our time fully testing your patches before we can ack them. We are hoping to get this automatically running in pagure, but for now it is not, and it needs to be run manually...

https://pagure.io/389-ds-base/pull-request/50242
This resolves the issue.

I think that this is an over-reaction. As mentioned, I have to fix mistakes left by everyone else, quite frequently. I'm not asking you to "install suse" and every permutation of linux. I fix the issues I see, and I talk to people when they arise. I think that there are probably better solutions to this problem than saying you'll have to "check all my work" - which probably is just going to consume more of your time, than do good. Everyone in the team is human, everyone makes mistakes, we just deal with them as they come up.

The change to the spec file broke FreeIPA upgrades from Fedoras 29 to Fedora 30. Fresh install on F30 work fine, but upgrades fail, and this is now being marked as a blocker.

Looks like this was merged before IPA was ready to make the change (@abbra)

So I need to revert this change in the Fedora upstream spec file until FreeIPA has a chance to make the their changes

The change to the spec file broke FreeIPA upgrades from Fedoras 29 to Fedora 30. Fresh install on F30 work fine, but upgrades fail, and this is now being marked as a blocker.
Looks like this was merged before IPA was ready to make the change (@abbra)
So I need to revert this change in the Fedora upstream spec file until FreeIPA has a chance to make the their changes

FreeIPA merged their change and did a build so this is all good now!

Yay! Thanks!

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/3272

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.