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

Add documentation for tukit.conf (bsc#1192307) #76

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

aplanas
Copy link
Collaborator

@aplanas aplanas commented Nov 3, 2021

No description provided.

@laenion
Copy link
Collaborator

laenion commented Nov 10, 2021

I'll merge this for transactional-update 4.0 along with a man page for tukit itself (which is also still undocumented yet).

Just one note: I think we never want to add /var/lib to the list of bind mounts.
The whole concept of transactional updates is based around the concept that an update can not break the currently running system. By bind-mounting such a vital directory that whole concept is basically nullified - and on the contrary will cause significant side effects:

/var/lib contains a lot of data, including databases, docker images or sssd data. That data is often bound to a specific version of the software.
A simple example: Imagine that a database package is updated, and that update also includes a schema change which the post script will perform if a database is found. Now we run into the situation that the database is still running the old version in the currently active system and suddenly gets confronted with a changed and probably incompatible new data set belonging to the new version (with the new database daemon only available in the new snapshot which hasn't been booted yet). Due to the bind mount however the new data leaks into the current system. This is playing with fire.

Because of that I'm really interested to hear what the use case for adding /var/lib to the bind mounts would be.

@aplanas
Copy link
Collaborator Author

aplanas commented Nov 10, 2021

I'll merge this for transactional-update 4.0 along with a man page for tukit itself (which is also still undocumented yet).

Sure. Will you prepare this?

Just one note: I think we never want to add /var/lib to the list of bind mounts. The whole concept of transactional updates is based around the concept that an update can not break the currently running system. By bind-mounting such a vital directory that whole concept is basically nullified - and on the contrary will cause significant side effects:

Indeed.

The issue is that there are some kubernetes installers that are doing this: updating /var/lib, adding directories there (like /var/lib/rancher). This can be view as a bug in the installer (should use something different that /var/lib), but sadly is required as today.

/var/lib contains a lot of data, including databases, docker images or sssd data. That data is often bound to a specific version of the software. A simple example: Imagine that a database package is updated, and that update also includes a schema change which the post script will perform if a database is found. Now we run into the situation that the database is still running the old version in the currently active system and suddenly gets confronted with a changed and probably incompatible new data set belonging to the new version (with the new database daemon only available in the new snapshot which hasn't been booted yet). Due to the bind mount however the new data leaks into the current system. This is playing with fire.

I will add as a comment in the documentation.

Because of that I'm really interested to hear what the use case for adding /var/lib to the bind mounts would be.

Rancher.

@laenion
Copy link
Collaborator

laenion commented Nov 10, 2021

Because of that I'm really interested to hear what the use case for adding /var/lib to the bind mounts would be.

Rancher.

But I guess to bind mount /var/lib/rancher would be enough then? Otherwise the whole concept of transactional-update is spoiled...

@aplanas
Copy link
Collaborator Author

aplanas commented Nov 10, 2021

Because of that I'm really interested to hear what the use case for adding /var/lib to the bind mounts would be.

Rancher.

But I guess to bind mount /var/lib/rancher would be enough then? Otherwise the whole concept of transactional-update is spoiled...

IIUC not always, as this can be created by the installer. This should be OK if:

  1. The user create first the directory.

  2. There is an specific partition created during the installation, and is already in fstab

@aplanas
Copy link
Collaborator Author

aplanas commented Nov 10, 2021

I changed the example, and I try to integrate the dangers of exposing /var/lib

</programlisting>
</para>
<para>
Note that is some situations we should be tempted to

Choose a reason for hiding this comment

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

Just seeing this by chance and thought I could suggest fixing a typo and maybe a logical error (depending on how it's meant).
is -> in
should -> could

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found and fixed the "is -> in", but I am not sure about the second one.

Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree on the "should" -> "could" comment: One may or could be tempted, but one never should be tempted ;-)

Again, to avoid the "we" form I'd suggest you could write "... one may be tempted to ...".

Copy link
Collaborator

@laenion laenion left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the documentation! Generally I really like it, just a few nits inline.

man/tukit.conf.5.xml Outdated Show resolved Hide resolved
man/tukit.conf.5.xml Outdated Show resolved Hide resolved
man/tukit.conf.5.xml Outdated Show resolved Hide resolved
man/tukit.conf.5.xml Outdated Show resolved Hide resolved
man/tukit.conf.5.xml Outdated Show resolved Hide resolved
man/tukit.conf.5.xml Outdated Show resolved Hide resolved
man/tukit.conf.5.xml Outdated Show resolved Hide resolved
man/tukit.conf.5.xml Outdated Show resolved Hide resolved
</programlisting>
</para>
<para>
Note that is some situations we should be tempted to
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree on the "should" -> "could" comment: One may or could be tempted, but one never should be tempted ;-)

Again, to avoid the "we" form I'd suggest you could write "... one may be tempted to ...".

man/tukit.conf.5.xml Outdated Show resolved Hide resolved
@laenion laenion merged commit b872891 into openSUSE:master Jul 14, 2022
@laenion
Copy link
Collaborator

laenion commented Jul 14, 2022

The 4.0 release is very near, so finally merging this PR.

@aplanas aplanas deleted the fix_tukitconf branch November 10, 2022 12:57
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