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

Provide RFC for Salt SSH for TU systems #93

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

m-czernek
Copy link

@m-czernek m-czernek commented Oct 16, 2024

@m-czernek m-czernek marked this pull request as draft October 16, 2024 11:02
@m-czernek m-czernek marked this pull request as ready for review November 26, 2024 15:12
Copy link
Member

@rjmateus rjmateus left a comment

Choose a reason for hiding this comment

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

Just one note from my side, other then that looks good to me. Lets also wait for ION squad team members review

- `state` (e.g. `state.apply`)
- `transactional_update` (e.g. `transactional_update.apply`)

Users should use the `state` module modify OOT filesystem, and the `transactional_update` module to modify the IAT filesystem.
Copy link
Member

Choose a reason for hiding this comment

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

This means that they cannot be mixed together in the same state? So any OOT state should be defined in it's own sls file and executed with state.apply, and any states that needs to modify IAT should be in a different sls file and executed with transational_update.apply.
Do we have any state that modifies files/data in both locations?

Copy link
Author

Choose a reason for hiding this comment

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

Correct. Users must specify whether they want to run a state inside, or outside of a transaction, and execute either state.apply, or transactional_update.apply.

Do we have any state that modifies files/data in both locations?

This is not possible, at least not without some significant modifications to Salt at a quite low level. Salt does not give us control at a more fine-grained level than the whole execution.

@m-czernek m-czernek changed the title WIP: Provide RFC for Salt SSH for TU systems Provide RFC for Salt SSH for TU systems Nov 29, 2024
Currently, when Uyuni bootstraps a minion by using Salt SSH, Uyuni deploys a Salt client into `/var/tmp/venv-salt-minion`.
When Salt SSH makes a `state` call to a client, for example `state.sls example-sls-filename`, it additionally compiles the `example-sls-filename` state (and all its dependencies, such as files used by the state) into a tar file, and deploys it to a generated location in `/var/tmp`, such as `/var/tmp/.root_12345_salt/salt_state.tgz` (together with other data, such as external modules, grain info, etc).

TU minions are different from non-TU minions in that they mount some partitions as read-only, for example `/etc`. To modify such partitions, users use the `transactional-update` command.
Copy link
Contributor

Choose a reason for hiding this comment

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

/etc is not read only. etc is an overlay and is read-write.

Suggested change
TU minions are different from non-TU minions in that they mount some partitions as read-only, for example `/etc`. To modify such partitions, users use the `transactional-update` command.
TU minions are different from non-TU minions in that they mount some partitions as read-only, for example `/usr`. To modify such partitions, users use the `transactional-update` command.


TU minions are different from non-TU minions in that they mount some partitions as read-only, for example `/etc`. To modify such partitions, users use the `transactional-update` command.

The `transactional-update` command temporarily mounts the partitions as read-write, and after the modifications are done, it creates a new BTRFS snapshot that is activated upon restart (or upon using `transactional-update apply`).
Copy link
Contributor

Choose a reason for hiding this comment

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

This is technically not correct.
TU first creates a new read-write snapshot of currently running snapshot (or latest if -c is used) and at the end set this new snapshot as default one for next reboot.
It does not remount anything as rw even temporarily.

# Alternatives
[alternatives]: #alternatives

- Not supporting Salt SSH on TU systems.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of "not supporting" salt ssh on TU systems, could we instead suggest using transactional_update execution module? Or can we even provide a transactional_update state which would wrap TU execution module?

i.e. instead of pkg.installed suggest using transaction_pkg.installed?

Of course this would open another can of worms like:

  • users required to keep in mind mandatory reboot
  • detect we are already running in the transaction via tu executor in normal salt

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.

4 participants