openhab / openhab-snap

Packaging of openHAB for Ubuntu Core
Eclipse Public License 2.0
18 stars 12 forks source link

Adapt to new folder layout #2

Closed kaikreuzer closed 7 years ago

kaikreuzer commented 7 years ago

@kubiko & @r2geo Please note https://github.com/openhab/openhab-distro/pull/318, which has been merged yesterday. As far as I can see, this also requires some changes in the snap. Could you please take care of them?

kubiko commented 7 years ago

sorry was busy here. This should be easy change, any change I can get test build of openhab so i can verify my changes?

kaikreuzer commented 7 years ago

The builds available on https://openhab.ci.cloudbees.com/job/openHAB-Distribution/ already have this changed structure, so you can use them for testing your changes!

kubiko commented 7 years ago

OK I have it running now, just want to confirm, does running service ever edit at run time runtime/services.cfg as at the moment this is living in squashfs and it's only read only.

kaikreuzer commented 7 years ago

That's absolutely fine - the complete runtime folder can be considered read-only and it is fully replaced upon an upgrade.

kubiko commented 7 years ago

OK this is now done, I will do pull request to master today. Are you planning beta with new folder layout?

kubiko commented 7 years ago

one more question, how about userdata/etc is this modified at runtime and should it be writable?

And if it's writable what should happen at update, should default files from update be always copied over the user ones?

kubiko commented 7 years ago

hmm and same question about: 'conf' directory, what should happen when we update to newer version?

From what I can see on my device: new userdata/etc can be copied over existing one 'conf' should be left as it is?

kaikreuzer commented 7 years ago

Hi @kubiko, see here:

and yes, userdata must be writeable, conf and runtime can be read-only.

kubiko commented 7 years ago

hmm this does not match my testing here:

kaikreuzer commented 7 years ago

conf directory seems to change on running system

Good point, the conf/services folder is indeed an exceptions. Installed add-ons point a configuration template in there. So you are right, ideally, it should not be read-only - actually it definitely must be writable by the user himself as this is his place to do system configuration.

But for the upgrade process, it still holds what I said: The conf folder should simply be left untouched, no need to replace anything in there.

What would be safe way to migrate?

Actually, there is no migration path from beta4 to the current distro - this is what https://github.com/openhab/openhab-distro/pull/318 is all about. As we do not have any real user community on the snap yet, I think it is ok to not care for a migration path. But this actually means that the snap should already now take a snapshot build. I actually plan a beta5 by mid of December, this might be a good time to publish a new stable version of the snap, already through the openHAB Foundation account then.

kubiko commented 7 years ago

I guess better to explain what I'm doing at the moment. When user installs openhab, at first run it will check if there is any user data and configuration, if not it will populate/copy content of "conf" and "userdata" from install package to snap's writable path. openhab is then configured to use those paths.

So, ignoring beta4 to openhab/openhab-distro#318 migration, for the future updated wouldn't it be safer to actually copy files which do not exist in existing installation?

example:

new version of openHAB has built in new package, which has config file under conf/services/air-quality.cfg. Surely this new file should be copied over to user's writable path, where we store config from existing installation?

So proposed solution would be:

install time:

populate userdata and conf with data from install package

update:

kaikreuzer commented 7 years ago

for the future updated wouldn't it be safer to actually copy files which do not exist in existing installation?

We might consider this, but it should in line to what is done in the debian packaging as well (inviting @theoweiss to the discussion). I am not sure what is the best way to handle changes of files in conf and userdata/etc, if they become necessary. Only saying that we add files in case of an upgrade is imho just half way; we might also need to delete or overwrite (or even modify?) some files in rare cases. How do other applications handle this? Should we maintain a script in the packaging that can be called by apt and snap (or manually by the user) for the upgrade process?

r2geo commented 7 years ago

@kaikreuzer and @kubiko At the moment I am confused about launchpad: Kai, did you set up a new openhab account with openhab.org as owner?

Using 2.0.0.b4-5-offline I still see some unexpected response - do you see that too? status: Ignoring predefined value for KARAF_HOME mkdir: cannot create directory '/var/snap/openhab/common/userdata/log': Permission denied

Also I only see 2 UI's (classic and rest) but am missing panel and paper.

kaikreuzer commented 7 years ago

Kai, did you set up a new openhab account with openhab.org as owner?

Yes, I did. Right here: https://code.launchpad.net/~openhab/openhab-snap/+git/master And I just shared the credentials with you, so you should have access to it now.

theoweiss commented 7 years ago

Just my 2 cents.

At least you possibly have to deal with a three way merge. You have to consider the worst case where you have:

This state can be very complex (as you may have experienced it with git) and there are likely changes which you can't solve automatically but need some user interaction.

Debian solves this - as far as I understand - in this way. Normally all files are owned by the package and will be overwritten on updates. If you mark a file as conffile the decision making is a bit more complex.

https://www.debian.org/doc/debian-policy/ap-pkg-conffiles.html https://raphaelhertzog.com/2010/09/21/debian-conffile-configuration-file-managed-by-dpkg/

I think it is necessary to clearly differentiate between (user) conffiles, service configuration files and userdata files.

Therefore:

I'm really not sure but I think within the openHAB distribution maybe these parts are not clearly separated. This should be clarified independent of the packaging type.

from install package from userdata/etc/ copy only files which do not exist in users space, do not touch any of the files already there.

I think this would be a problem in cases like this: https://github.com/openhab/openhab-distro/pull/326

kaikreuzer commented 7 years ago

I'm really not sure but I think within the openHAB distribution maybe these parts are not clearly separated. This should be clarified independent of the packaging type.

Well, this is very much what I tried to do with the conf/runtime/userdata split described in http://docs.openhab.org/tutorials/demo.html#installation, right?

Unfortunately, this nice theory didn't survive in all its glory when meeting reality. We have files in userdata, mainly due to the fact that Karaf doesn't follow this principle - see the "etc" folder, which holds the configuration (both user and system) and which is updated during runtime (when changing configs through the UI) - so where does it belong to? conf/runtime/userdata are equally correct answers. I tried hard to resolve this, but the best I could come up with was to move it to userdata.

I think in general this setup works. A problem really only occurs when we decide to move, rename or dismiss formerly existing files (both within conf and userdata folders). We do not have any mechanism to deal with such cases.

kubiko commented 7 years ago

@kaikreuzer so I have done some changes to how I handle writable data in new version. Snappy provide facility to have data store per each revision, as it keep previous(3) revisions installed, to provide easy and fast rollback. I have tested this now and works nice :) So now whenever new revision is installed, data is migrated in configure hook. At the moment it it will do just simple data copy from previous version, and it will copy any new files form release which do not exist locally. Once some guideline about data migration is decided, we can update it there. It can be maintained also per version.

kubiko commented 7 years ago

@r2geo can start testing new version I pushed to the store today?

@r2geo I checked LP build and all looks good, build error there is because code is ready for new dir layout but url in snapcraft.yaml is still pointing to beta4 release package. Once beta5 is out, we will just edit snapcraft.yaml with new url and build should start working.

to your error mentioned above. Problem is that service is running as root, so you need to run $ sudo openhab. then you will not get warning about permission denied.

You mentioned that you can't see Paper UI, what revision are you running on? It seems to working fine for me here.

kaikreuzer commented 7 years ago

Once some guideline about data migration is decided, we can update it there. It can be maintained also per version.

Sounds good. Unfortunately, the upgrade process is still not clear to me - I suggest to discuss that further at https://github.com/openhab/openhab-distro/issues/299#issuecomment-268253707.

Once beta5 is out, we will just edit snapcraft.yaml with new url and build should start working.

FYI: I plan this for the end of this week, so we can forget about beta4 then.

kaikreuzer commented 7 years ago

@kubiko & @r2geo As per our discussion regarding offline vs. online, I would like to point you to https://community.openhab.org/t/a-single-distro-from-now-on/19903.

There is now only a single openHAB distro left, which is the online distro - so this is to be used by the snap. As the file name has changed, could one of you take care to adapt the snap accordingly and test it?

r2geo commented 7 years ago

OK, I will take that one

On 10 Jan 2017, at 16:53, Kai Kreuzer notifications@github.com wrote:

@kubiko & @r2geo As per our discussion regarding offline vs. online, I would like to point you to https://community.openhab.org/t/a-single-distro-from-now-on/19903.

There is now only a single openHAB distro left, which is the online distro - so this is to be used by the snap. As the file name has changed, could one of you take care to adapt the snap accordingly and test it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

kubiko commented 7 years ago

OK this should be easy. As for online version, we did not really use "online" wording, unlike for "offline" version where it was specified. I have built new snap version from distro build and it all seems to work as expected. I will push it to edge channel for testing. At the moment build is broken as there are changes in snap craft which broke my custom java plugin. I had today call with Azul, and hopefully with their support we can come up with more robust solution without need to update java minor versions from openHAB git repo. I hope to implement most of it tomorrow to have automated build back on.

r2geo commented 7 years ago

Ubuntu-snap is now updated to latest folder layout with the latest PR.