Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deb files build initial commit #138

Closed
wants to merge 13 commits into from
Closed

Conversation

theoweiss
Copy link
Member

Do not merge yet!
This PR is meant to discuss the deb packaging #66. I will add some comments soon.

@theoweiss
Copy link
Member Author

start-stop-scripts:

  • to circumvent the arm issue of the karaf install and also because openhab:install-service is not scriptable
    I've used the following methods to start/stop/status openHAB
 * systemd: 
   start: /usr/share/openhab2/start.sh
   stop: /usr/share/openhab2/runtime/karaf/bin/stop
 * sysV: 
   start: /usr/share/openhab2/runtime/karaf/bin/{start, stop, status}

exit status of the openhab server:

  • if the service is started with /usr/share/openhab2/start.sh the exit status is 143 when it is stopped with /usr/share/openhab2/runtime/karaf/bin/stop
    or with a sigint signal. What is the best way to stop the daemon?

process owner:

  • I suggest not to add the ability to run the openHAB user as another user as the predefined openhab2 user, just to keep things simple.

setpermissions:

  • I suggest not to do any fixing of file ownerships before starting openHAB as it is done with the setpermissions script in openHAB1.
    The drawback is: if someone started openHAB as root from the commandline he/she may run into trouble.

log files:

  • until now log files will go to /var/lib/openhab2/logs for openHAB1 deb files it was /var/log/openhab/. I don't know how to change this.

runtime owner:

  • I get errors if I change the owner of the runtime files to root:root while the openhab process owner is openhab2:
Feb 22 22:25:44 debian8 start.sh[2353]: Launching the openHAB runtime...
Feb 22 22:25:46 debian8 start.sh[2353]: WARN: can't update etc/config.properties with the generated command shutdown. We advise to manually add the karaf.shutdown.co
Feb 22 22:25:46 debian8 start.sh[2353]: Unable to update instance pid: Unable to create directory /usr/share/openhab2/runtime/karaf/instances

@kaikreuzer
Copy link
Member

What is the best way to stop the daemon?
Does the problem also exist if you use this for starting the process?

    start: /usr/share/openhab2/runtime/karaf/bin/start

I suggest not to add the ability to run the openHAB user as another user as the predefined openhab2 user, just to keep things simple.

Fine for me. Is there a special reason why it is the "openhab2" user? Could we simply use "openhab" as a user instead?

I suggest not to do any fixing of file ownerships before starting openHAB

I didn't follow this on openHAB 1 and cannot judge pros/cons - so up to you what you think is best. In general, it sounds good to me not to do any unexpected permission changes when starting a service.

until now log files will go to /var/lib/openhab2/logs for openHAB1 deb files it was /var/log/openhab/. I don't know how to change this.

/var/log/openhab would sound better to me. The log file location is defined in here.
We could maybe introduce a new environment variable ${log.dir}, which we set by default to ${karaf.data}/logs, but gives you a chance to set it to a different value. @dvanherbergen Any comments?

I get errors if I change the owner of the runtime files to root:root while the openhab process owner is openhab2

This was an open issue that Karaf wants to write instance information in there. I don't actually remember the latest state of the discussion. @maggu2810 & @dvanherbergen, can you comment? Was that solved with 4.0.4 or still up to us to adapt the directory layout and environment settings?

@kaikreuzer
Copy link
Member

Btw, regarding the best update strategy, this seems to be a decent way to do it.

@kaikreuzer
Copy link
Member

@theoweiss Any news on this or anything I can help with?

@theoweiss
Copy link
Member Author

I thought to get the feedback from @maggu2810 & @dvanherbergen. There are some open questions:

/var/log/openhab would sound better to me. The log file location is defined in here.
We could maybe introduce a new environment variable ${log.dir}, which we set by default to ${karaf.data}/logs, but gives you a chance to set it to a different value. @dvanherbergen Any comments?

I'm not sure how to proceed with the log destination? If I should introduce the environment variable I would need some assistance on how this is done.

This was an open issue that Karaf wants to write instance information in there. I don't actually remember the latest state of the discussion. @maggu2810 & @dvanherbergen, can you comment? Was that solved with 4.0.4 or still up to us to adapt the directory layout and environment settings?

Is this issue solved? If not I would use openhab2 as owner of the runtime files.

Fine for me. Is there a special reason why it is the "openhab2" user? Could we simply use "openhab" as a user instead?

This is because the openHAB1 deb packages are responsible for the user named "openhab". If you purge a openHAB1 debian installation the user "openhab" will be deleted by the postremove script of the deb package. I think it's a legal case to have an openHAB 2 installation on the same host, which would be damaged by the openHAB(1) purge.

Does the problem also exist if you use this for starting the process?

start: /usr/share/openhab2/runtime/karaf/bin/start

systemd works best with services which don't daemonize therefore the background mode of karaf/bin/start is not the best choice. But nevertheless it seems to work now. May be I've tested this with an older karaf version?

@maggu2810
Copy link
Contributor

There are two files that will be written to "."

  • lock
  • instances

The directory for lock could be configured in the system.properties.
If instance could not be written, this i no problem, but ATM you will see this message on startup:

Unable to update instance pid: Unable to create directory /some/directory/for/the/distribution/instances

I did not have a look at the code, if you could also change this file location.

@kaikreuzer
Copy link
Member

The directory for lock could be configured in the system.properties.

Shouldn't we in any case have that pointing to userdata/tmp/lock for example?

If instance could not be written, this i no problem, but ATM you will see this message on startup:

Wasn't that fixed with 4.0.4?

@maggu2810
Copy link
Contributor

Karaf tries to create (if not exist) a directory instances that needs to be writable.

  • If the directory exists and is writable, all is fine.
  • If the directory does not exist and could be created, all is fine.
  • If the directory does not eixst and could not be created, it is working (at least with only one instance), but you see a warning.

@maggu2810
Copy link
Contributor

@theoweiss
Copy link
Member Author

FYI, I started working on the log directory case.

# Conflicts:
#	distributions/distribution-resources/src/main/resources/deb/bin/oh2_dir_layout
@theoweiss
Copy link
Member Author

I've pushed changes for handling the log directory and handling docker installations.

@kaikreuzer
Copy link
Member

Great thanks - so is this PR then ready to be merged from your point of view?

@theoweiss
Copy link
Member Author

I think a review of the logdir changes would be great! I've tested it on linux and mac os and it work well for me, but you never know. The changed files are: setenv, setenv.bat, oh2_dir_layout and oh2_dir_layout.bat and org.ops4j.pax.logging.cfg. These changes will also affect non deb package installations.

What's furthermore missing is the handling of karaf.instances, some minor tweaks of deb related things like conflict handling and a discussion about naming conventions openhab vs. openhab2. But maybe we could change these things with further PR('s) and give users a chance to test the current development.

Furthermore as soon as a deb "testing" package is published I could make a PR for the openhab-docker project with updates for the Dockerfile. I think the docker installation should also be based on the deb package installation.

@watou
Copy link
Contributor

watou commented Apr 11, 2016

a discussion about naming conventions openhab vs. openhab2

An item on my wishlist is to see .deb packages of 1.9 addons that can be installed from openHAB 1.x installations with no special steps, so they end up in the right place and have the right names in bintray. Any chance we can keep the sensible .deb names from OH1 to minimise disruption?

@kaikreuzer
Copy link
Member

@watou This issue here is about OH2 packaging and there won't be any Debian packages for addons anymore - only for the distro itself.

@theoweiss
Copy link
Member Author

I've tried to workaround the instance.properties problem by overriding the
-Dkaraf.instances="${KARAF_HOME}/instances" configuration in /bin/karaf (https://github.com/apache/karaf/blob/master/assemblies/features/base/src/main/resources/resources/bin/karaf#L479)
using KARAF_OPTS https://github.com/apache/karaf/blob/master/assemblies/features/base/src/main/resources/resources/bin/karaf#L477).

This works so far but it breaks checkRootInstance()
https://github.com/apache/karaf/blob/master/assemblies/features/base/src/main/resources/resources/bin/karaf#L327
because this function relies on a path relative to ${KARAF_HOME}. A proper solution needs fixes in bin/karaf.
What do the karaf experts think on how we should proceed?

Besides the karaf.instances problem the packaging is ready for testing from my point of view.

@theoweiss
Copy link
Member Author

Any comments from the karaf experts?

@kaikreuzer
Copy link
Member

Any comments from the karaf experts?

You cannot mean me here. Maybe @maggu2810 or @dvanherbergen?

@maggu2810
Copy link
Contributor

I have not followed this discussion the last time...
@theoweiss Perhaps you could use the Karaf mailing list or IRC chat to contact the real experts.

@theoweiss
Copy link
Member Author

Thanks @maggu2810 I've already checked if I could prepare a PR for Karaf, but it's to much work. I will try to solve the issue keeping karaf.instances at it's default value and provide a ${KARAF_HOME}/instances directory which is writeable for the openhab2 user.

@kaikreuzer
Copy link
Member

@theoweiss I finally took the time to look at it.
The changed regarding log location look fine to me.

What blows up the required disk space of the build is the tar.gz distro besides the zip - but I assume this is the only one you can use as an input for the deb package, right?

discussion about naming conventions openhab vs. openhab2

If I get you right, the only problem when using "openhab" is that if a user has OH1 and OH2 installed through apt-get and he then de-installs OH1, he will also break his OH2 installation, correct? I think this is something that can be documented and is acceptable. On the long run, I very much prefer to have the straight-forward user name "openhab" instead of a kind of tweaked name "openhab2".

Besides this, I would be fine to merge and have people start testing the package!

@theoweiss
Copy link
Member Author

@kaikreuzer yes only the tar.gz is working with jdeb.
Your also right with your assumptions concerning the openhab user name. I will change it to openhab.
I've some outstanding changes concerning the karaf instances problem. I solved this problem but run into other problems as soon as the openhab application directory is not writeable for the openhab user. I will give it another short trial but if I find no solution I would suggest to use the openhab user as the owner of the application directory.
Next week I've hopefully some spare time to work on this issue.

@theoweiss
Copy link
Member Author

@kaikreuzer I've changed the user and group name to openhab. The openhab application directory is now writable for the openhab user and no longer owned by root. I've also added the apt-repo plugin to the maven stack, thus it's possible to add the CI http url as apt-repository. The downside is more disk space usage for the build.
From my point of view it's ok to merge the PR and start the testing.

@kaikreuzer
Copy link
Member

Ok, many thanks! So let's merge and test.
Btw, I plan a beta3 release this weekend, it would be terrific if we had the first debian packages with that. Do you think you could reserve some time on Sunday to get that done with me?

@kaikreuzer
Copy link
Member

Btw, your commits are lacking the "Signed-off-by" footer (see here).
Could you please squash them and sign the resulting commit? Thanks!

@theoweiss
Copy link
Member Author

Unfortunately (for the beta3 release ;-) ) I was on vacation with my family. I've opened a new PR with squashed commits and Signed-off-by here #213 . It's meant as successor of this PR, therefore I'll close this one.
I suggest to discuss the release related things within the new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants