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

Fix directory usage #207

Merged
merged 4 commits into from
Aug 14, 2018
Merged

Fix directory usage #207

merged 4 commits into from
Aug 14, 2018

Conversation

felixsch
Copy link
Contributor

@felixsch felixsch commented Aug 2, 2018

Change configuration, ssl and uuid path to make sure rmt also works on a read-only installation
yast/yast-rmt#26

Path changes:
/usr/share/rmt/ssl -> /etc/rmt/ssl
/usr/share/rmt/config/system_uuid -> /var/lib/rmt/system_uuid
/etc/rmt.conf -> /etc/rmt/rmt.conf

@@ -218,11 +224,25 @@ getent passwd %{rmt_user} >/dev/null || \
%post
%service_add_post rmt-server.target rmt-server.service rmt-server-migration.service rmt-server-mirror.service rmt-server-sync.service
cd %{_datadir}/rmt && runuser -u %{rmt_user} -g %{rmt_group} -- bin/rails secrets:setup >/dev/null
cd %{_datadir}/rmt && runuser -u %{rmt_user} -g %{rmt_group} -- bin/rails runner -e production "Rails::Secrets.write({'production' => {'secret_key_base' => SecureRandom.hex(64)}}.to_yaml)" >/dev/null
if [ $1 -eq 1 ] ; then
cd %{_datadir}/rmt && runuser -u %{rmt_user} -g %{rmt_group} -- bin/rails runner -e production "Rails::Secrets.write({'production' => {'secret_key_base' => SecureRandom.hex(64)}}.to_yaml)" >/dev/nulla
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ >/dev/nulla ⚠️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whaa good catch!

@felixsch felixsch force-pushed the align-directory-usage branch 2 times, most recently from 5d4d40f to b711294 Compare August 3, 2018 14:58
@felixsch felixsch removed the WIP Work in progress, do not merge. label Aug 3, 2018
Copy link
Contributor

@thutterer thutterer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Tested on a fresh SLE 15 VM.
Update from 1.0.0 to this 1.0.6 moved the files and everything.
Only one comment about the changelog entry. See inline.

And one question: Is the current behavior of doing a zypper in --oldpackage rmt-server-1.0.0 afterwards good enough? Since it does not move the files back to the old place, but it creates rpmsave file in the new place (since the old package version does not have these files) and creates new config files in the old place. So you basically loose your config and there is no RPM output about it.

Since I don't know if we should/must support downgrading as-well, I'm not yet approving this PR.

@ikapelyukhin Can you provide an answer to my question?

Thu Aug 2 16:19:35 UTC 2018 - fschnizlein@suse.com

- Version 1.0.6
- Change file paths to new locations to make read-only rmt servers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Change file paths to new locations to make RMT work with read-only rootfs would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes true I will change it. Regarding downgrading, good question!

@ikapelyukhin
Copy link
Contributor

@thutterer But the data is not really lost after the package is removed, it's just in a new location?
I think not having downgrade is fine -- the upgrade code will stay in the RPM spec file and will be invoked for future versions as well, if we add stuff for downgrade -- we will have to check the version we are downgrading to (if that's even possible).

You are supposed to upgrade, not downgrade!

@thutterer
Copy link
Contributor

You are supposed to upgrade, not downgrade!

Haha, totally fine with me 😃
I just asked so we consider the impact. But I agree it's not a big issue anyway.

@felixsch felixsch merged commit f2718fe into master Aug 14, 2018
@Sergeykot Sergeykot deleted the align-directory-usage branch August 24, 2018 12:40
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

3 participants