teamdfir / sift

SIFT
MIT License
484 stars 67 forks source link

[CLI] sift --install breaks systems on which salt is already used #272

Closed ptitdoc closed 6 years ago

ptitdoc commented 6 years ago

sift --install can break systems that are actively using salt because: 1/ It stops and disable salt-minion daemon in the sift.config.salt-minion state 2/ It re-install salt by first removing salt-minion and then installing new salt packages, thus removing all third party packages depending on salt.

A workaround for 1/ is to add an option to ensure that sift cli tool is not managing the salt configuration.

The solution for 2/ is to ensure that sift-cli.js is only updating salt instead of removing it and reinstalling it.

ekristen commented 6 years ago

Hi @ptitdoc are you using a system already installed with saltstack? If so, I'm curious whats your install environment like and use case.

While both your points are correct, I have to be honest and say that this is most likely not going to change. The primary purpose of the CLI tool is to build VMs for the community and the classroom and keep them up-to-date. In the spirit of this, we have to control the environment to ensure compatibility.

With that said, it might be possible to introduce a flag that prevents 2 from happening and it could potentially prevent 1 from happening too, this would be a more advanced usage of the tool.

However to get around 1 you can set packages-only mode on the CLI. Or since you seem familiar with salt, you can simply call state.apply sift.pkgs, this will forgo and modifications to the user, the desktop, or configuration of any kind, it'll only install tools, scripts, packages and the like.

ptitdoc commented 6 years ago

Hello, here are two use cases: 1/ Using qubes OS, the Virtual Machines (also called the Qubes) are managed using salt. I try to install salt on such a Virtual Machine but salt-cli removes qubes specific salt packages. 2/ On my Lab, I'm already using salt to manage the whole Lab, so I just cloned sift salt repository on my salt master and try to apply the states on my lab VMs. However at some point, the sift.config state just stops the minion on the VM, thus stopping the whole sift install process.

Actually, I already tried to change the sift.config.salt-minion state to apply it only if the pillar value sift.manage_salt is not set to false and this works. I only discovered the second issue with salt-cli.js today.

If you want I can try to send a Pull Request later on, but I would like your feedback before trying to implement things.

ekristen commented 6 years ago

@ptitdoc thank you for the information. I'd definitely consider this an advanced use case.

Your best bet would be to use the states directly. We might be able to make some tweaks to allow for more configs to disable/enable certain things, but first and foremost we need to make sure things continue to work for the classroom.

Pull Requests welcome, but the current behavior has to be the default.

ptitdoc commented 6 years ago

The previous commit allows add sift_manage_salt pillar option allowing to skip salt-minion state if set to False. It however only fixes case 1/.

The default behavior is still to disable salt-minion service.

In the same time, I added pillar.example documentation that can be used as reference for people looking for available pillar options.

ptitdoc commented 6 years ago

Hello, I will refactor the commit. I think that following my use cases if a user already uses salt on a his system, he should avoid using salt-cli at all. So the 2/ can be ignored. The refactored pull request will also in this regard skip installing sift-cli if sift_salt_managed is set to False.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.