Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

SEP 26: Package Salt with Tiamat #34

Merged
merged 2 commits into from
Jun 29, 2021

Conversation

bryceml
Copy link

@bryceml bryceml commented Sep 3, 2020

No description provided.

@bryceml bryceml requested a review from a team as a code owner September 3, 2020 17:07
@ghost ghost requested review from krionbsd and removed request for a team September 3, 2020 17:08
@bryceml bryceml changed the title add package-salt-with-tiamat.md SEP 25: Package Salt with Tiamat Sep 3, 2020
@raddessi
Copy link

raddessi commented Sep 3, 2020

Say I need to patch a bug in salt, is this still possible via some other method after this is packaged in to a frozen binary? I would love to say I don't do this but I've had multiple patches for every version of salt that we've used in prod so far.

In the current system I just use salt's own file.patch to self-patch.. would we need to say keep our own fork of the branch we're running then apply our patches and repackage it up there, then distribute internally? Or is there another method?

@Akm0d
Copy link
Contributor

Akm0d commented Sep 3, 2020

In the current system I just use salt's own file.patch to self-patch.. would we need to say keep our own fork of the branch we're running then apply our patches and repackage it up there, then distribute internally? Or is there another method?

That's exactly what you would need to do; fork -> apply patch -> repackage

I'm currently building a docker container for tiamat that uses a very old glibc for maximum portability (and a second container that builds with wine for easy windows .exe creation)

@waynew
Copy link
Contributor

waynew commented Sep 3, 2020

@raddessi the other option is to deploy this single binary as a "onedir" build - salt+all deps get extracted to one directory, so you would be able to patch those files.

@bryceml
Copy link
Author

bryceml commented Sep 3, 2020

@raddessi the other option is to deploy this single binary as a "onedir" build - salt+all deps get extracted to one directory, so you would be able to patch those files.

I heard that tiamat onedir only had pyc files, no py files. I checked https://artifactory.saltstack.net/ui/repos/tree/General/open-rpm-staging%2Fcentos%2F7%2Fx86_64%2Fsalt-3001-1.el7.x86_64.rpm and there are py files in there. We should get some clarification on this.

@Akm0d
Copy link
Contributor

Akm0d commented Sep 3, 2020

oh onedir builds can have py files, it's the single binary that you can't change

@raddessi
Copy link

raddessi commented Sep 3, 2020

Hey nice to hear! I do like options. That's right I remember Tom talking about the "onedir" builds somewhere before.. maybe saltair? OK, both of those are viable (assuming .py files are included). 👍 Thanks all for the details

@barneysowood
Copy link
Contributor

The SEP suggests that the single binary will have all required Python Libraries and any C Library deps bundled. Looking at the pyinstaller docs, I'm not totally sure this is correct - I don't think it goes as far as bundling glibc for example does it? Might be good to clarify.

I also think the proposal needs to cover what the process will be for tracking what libraries at what version are bundled and how Saltstack will handle the issue of a security vulnerability in a bundled library.

Is there a way for a tiamat binary to show what libraries (and their versions) are bundled?

@Akm0d - the work you're doing with trying to build stuff "with a very old glibc" sounds somewhat concerning. What distro are you using and how old is it? I assume it's something that's still getting security updates - otherwise, how are you ensuring you don't end up bundling something with a known vulnerability?

Fundamentally, I think if you're proposing to move to a model where you are asking people to install software with a number of libraries bundled in, in a way that is opaque and not patchable in using normal OS procedures, then it's got to be clear that there's strong processes in place around security.

If you go do this road you're going from being responsible for providing updates for vulnerabilities in Salt to being responsible for providing updates for vulnerabilities in Salt and all it's dependencies.

Copy link

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

  • It would be a learning curve for people to understand how to add python libraries to the new environment.

We could expand here that we will likely be providing a pip command to install additional packages, so, the process will be almost the same as pip install package

  • Adding python libraries to the new environment might be harder in an air gapped network.

A previous pip download package on a similar system(because of the arch) using the same python version as the tiamat salt package would facilitate this scenario.
Then the downloaded packages could be brought into the air gapped environment for install. Since the tiamat salt package will include a pip command, the pip download package could potentially be done by a salt tiamat package with the same version as that in the air gapped system in order not to worry about arch/python version compatibility.

  • People would have to re-install their python packages potentially if we updated the python in the tiamat environment on upgrades.

We could potentially have a directory which could keep the downloaded wheels/sdist since we will control how things get installed and use that on upgrades; If we can have the tiamat packaged salt detect that it's being upgraded, or by a special command that would reinstall everything on demand.

@s0undt3ch
Copy link

I also think the proposal needs to cover what the process will be for tracking what libraries at what version are bundled and how Saltstack will handle the issue of a security vulnerability in a bundled library.

For the salt tiamat package, all shipped dependencies will be locked to a version(and the dependencies of a dependcy too).
Look under requirements/static/py3.*/linux.txt on the salt repo for an example(this is what we use for reproducible testing)

Is there a way for a tiamat binary to show what libraries (and their versions) are bundled?

Since the salt tiamat package will likely have a pip command, pip list would likely be available.

If you go do this road you're going from being responsible for providing updates for vulnerabilities in Salt to being responsible for providing updates for vulnerabilities in Salt and all it's dependencies.

That's correct, and a .x release would be made whenever needed to address a dependency security issue.
We are screening our dependencies for security advisory issues and we would release updates when needed, or, information on how to proceed to upgrade a dependency.

@max-arnold
Copy link

max-arnold commented Sep 5, 2020

A couple of questions:

  1. Will minor Salt upgrades require reinstallation of 3rd-party python libraries? Any other potential upgrade issues?
  2. How this change will affect Salt branching model and/or release frequency? What is the general policy regarding dependency versions and upgrades? There are plenty of security and non-security package upgrades (distros often supply a couple of patches on top of upstream source code). Any plans to track those for bundled libraries? Should we expect more frequent minor releases due to dependency upgrades? Can the existing tag-based release procedure (as opposed to branch-based) handle the increased minor release frequency?
  3. It also means that we can have a single binary as an option. Could someone elaborate on this? I assumed that all Tiamat-based packages will be single-binary. Is that not the case? Which platforms will get the bundle and which one the single binary?
  4. Is there any performance impact? I suspect that single binary salt-call startup time can suffer. Any other potential issues?
  5. It would be a learning curve for people to understand how to add python libraries to the new environment. How the new way of adding libraries will look like?
  6. Where in the filesystem Tiamat-based Salt will be located? Something like /opt? Is it possible to install multiple major versions simultaneously?

@max-arnold
Copy link

In the current system I just use salt's own file.patch to self-patch.. would we need to say keep our own fork of the branch we're running then apply our patches and repackage it up there, then distribute internally? Or is there another method?

That's exactly what you would need to do; fork -> apply patch -> repackage

@Akm0d I also have this question. A quite common way to work around Salt bugs (and lack of frequent minor bugfix releases) is to self-patch it using the file.patch state and a diff file that is distributed through salt:// fileserver. This is much easier than repackaging (no need to have packaging pipelines for different OSes, manage repositories, etc.). Just install Salt from upstream repo and apply a state that patches it. Will this still be possible with Tiamat-based packages?

@OrangeDog
Copy link

Just on a personal opinion, one of the most annoying things as a linux admin is projects that bundle all their own dependencies, instead of working properly with the OS package system. I guess the tide is turning against this (for desktop users anyway) with e.g. snaps.

I am very sceptical that the salt team is capable of maintaining security patches for all targets in all the dependencies. Most distros are patching something at least every week, with thousands of people supporting it. Meanwhile we constantly hear from salt how it's not possible to do even minor releases more frequently than every couple of months. This proposal increases the maintenance burden by a huge amount.

I need to see a believable commitment to do this properly, and not just vague promises. How (for example) are you going to get patches for undisclosed major CVEs, the way the distributions do?

@bryceml
Copy link
Author

bryceml commented Sep 10, 2020

@OrangeDog this makes it easier on us to package more often because to install newer dependencies, we just upgrade via pip as part of a pipeline. The old way was really hard to update any dependencies that we added to the repo (take a look at amazon linux's dependencies in the salt repo) because we have to go find updated source rpms or source debs for each of those and then create the package. With all those rpm's and deb's, which we often had to create ourselves, it added a lot of manual effort.

Windows and mac we already bundle everything and we can usually create those packages and get them tested in a few hours. The linux ones take several days.

Hopefully this helps ease your concern.

@OrangeDog
Copy link

This issue affects regular users, formulas, and custom modules.

Many modules require an additional python dependency in order to work.
You can get it with either pkg.installed or pip.installed (which may also require pkg.installed for dev dependencies).

How is this going to work now?
Are all the formulas going to need to detect whether this is a tiamat installation in order to do the right thing?
Is the default environment for pip.installed going to be the system python, or the tiamat one (either way requires people to change something)?
Will system python packages be available in the tiamat environment?
What about compatibility between non-python system packages and those bundled by tiamat?

@waynew
Copy link
Contributor

waynew commented Sep 10, 2020

@max-arnold - I think #34 (comment) answers at least a few of your points.

With a single binary style deployment it's static (though custom _modules & the like would still work -- I can't see why they wouldn't), so patching a state would not be possible (without overriding it with a custom _state, say).

However, with the onedir style deployment (IIUC it would go under /opt, though there might be platform specific exceptions) you can manually patch files to your heart's content.

IIUC it's possible to take the same build and either execute it or have it unpackage itself into a onedir style deployment, but I might be misunderstanding that.

A couple of questions:

  1. Will minor Salt upgrades require reinstallation of 3rd-party python libraries? Any other potential upgrade issues?

With the Tiamat builds, IIUC, the 3rd party libs would be included - though I'm not sure about optional deps. @Akm0d if this isn't documented in the SEP or documented elsewhere and linked from here, it should be.

For source-only deps there shouldn't be an issue, but there may be issues with compiled deps. For sure there will be if the ABI has any changes, but there might not be otherwise.

  1. How this change will affect Salt branching model and/or release frequency? What is the general policy regarding dependency versions and upgrades?

It should reduce the burden of releasing Salt, since vagaries in dependencies would be eliminated - we'll just ship the deps that Salt needs.

Obviously that means that we would have to ensure that we're keeping up with security advisories and cutting new releases when dependencies have issues. That should definitely be addressed in this SEP.

There are plenty of security and non-security package upgrades (distros often supply a couple of patches on top of upstream source code). Any plans to track those for bundled libraries? Should we expect more frequent minor releases due to dependency upgrades?

I believe that Pedro's answer covers this

Can the existing tag-based release procedure (as opposed to branch-based) handle the increased minor release frequency?

We do already branch if we need to make a .2 release (or worse). Our existing approach to other releases should not have to change.

  1. It also means that we can have a single binary as an option. Could someone elaborate on this? I assumed that all Tiamat-based packages will be single-binary. Is that not the case? Which platforms will get the bundle and which one the single binary?

(see above)

  1. Is there any performance impact? I suspect that single binary salt-call startup time can suffer. Any other potential issues?

single binary startup time definitely suffers, for the obvious reason of extracting the binary. I'm not aware of any other known issues. I seem to recall hearing something about improvements in the one-dir approach over existing Salt, but I don't remember the specifics.

  1. It would be a learning curve for people to understand how to add python libraries to the new environment. How the new way of adding libraries will look like?

That should definitely be documented and either added to the SEP or linked from the SEP

  1. Where in the filesystem Tiamat-based Salt will be located? Something like /opt? Is it possible to install multiple major versions simultaneously?

For a single binary option it should definitely be possible. I mean, heck, you can already do that now with Salt if you install to a virtual environment, or otherwise munge your PATH.

@waynew
Copy link
Contributor

waynew commented Sep 10, 2020

What about compatibility between non-python system packages and those bundled by tiamat?

I'm not sure I understand the question. https://gitlab.com/saltstack/pop/tiamat/-/blob/master/docs/topics/quickstart.rst might help.

@OrangeDog
Copy link

@waynew that's the probably the same question as what you said about ABI issues.

@barneysowood
Copy link
Contributor

I also think the proposal needs to cover what the process will be for tracking what libraries at what version are bundled and how Saltstack will handle the issue of a security vulnerability in a bundled library.

For the salt tiamat package, all shipped dependencies will be locked to a version(and the dependencies of a dependcy too).
Look under requirements/static/py3.*/linux.txt on the salt repo for an example(this is what we use for reproducible testing)

If you go do this road you're going from being responsible for providing updates for vulnerabilities in Salt to being responsible for providing updates for vulnerabilities in Salt and all it's dependencies.

That's correct, and a .x release would be made whenever needed to address a dependency security issue.
We are screening our dependencies for security advisory issues and we would release updates when needed, or, information on how to proceed to upgrade a dependency.

@s0undt3ch - that sounds great and like you're covering the issues raised. Would it be possible to get that documented in the SEP as part of the detailed design? I think it's a fundamental part of what's being proposed so it would be good to have it covered in the SEP.

@barneysowood
Copy link
Contributor

  1. Is there any performance impact? I suspect that single binary salt-call startup time can suffer. Any other potential issues?

single binary startup time definitely suffers, for the obvious reason of extracting the binary. I'm not aware of any other known issues. I seem to recall hearing something about improvements in the one-dir approach over existing Salt, but I don't remember the specifics.

Is it possible to quantify this? If calling salt or any of the cli tools is going to require the extraction each time that's going to make commandline use clunky.

@waynew
Copy link
Contributor

waynew commented Sep 11, 2020

@barneysowood the extraction is transparent to the user, aside from the extra time - a couple of seconds on a reasonably modern system.

But if that's unacceptable, that's why onedir exists 🙃

@barneysowood
Copy link
Contributor

@barneysowood the extraction is transparent to the user, aside from the extra time - a couple of seconds on a reasonably modern system.

But if that's unacceptable, that's why onedir exists 🙃

@waynew are there any Tiamat builds of 3001.1 available to test with currently?

@cassandrafaris cassandrafaris changed the title SEP 25: Package Salt with Tiamat SEP 26: Package Salt with Tiamat Sep 11, 2020
@waynew
Copy link
Contributor

waynew commented Sep 12, 2020

I think that's a question for @bryceml. I know it's possible to build your own packages with Tiamat, and at least beardedeagle (on Slack) has been making their own internal Tiamat Salt builds. I don't know if there are instructions on the Tiamat page on how to build Salt, but the quickstart guide did look pretty straight forward (linked above in one of my other comments).

@bryceml
Copy link
Author

bryceml commented Sep 12, 2020

@waynew are there any Tiamat builds of 3001.1 available to test with currently?

Doesn't look like it. It should be as easy as tagging the salt-pkg repo and they should show up on the artifactory staging repos. We can probably do that without much difficulty next week.

Also, if it's not clear yet, all the rpms and debs will contain the onedir format with no/negligible performance hit. The single binary will be for things like salt-ssh and testing type scenarios with a slight performance hit because it has to extract it. I don't imagine it's much different than salt-ssh's existing performance.

@OrangeDog
Copy link

OrangeDog commented Oct 26, 2020

Is tiamat going to bundle non-python dependencies or not? For example libgit2 and libzmq.

None of the answers have made reference to them either way. If they're not bundled then I don't see how the dependency/testing problems are solved, and if they are then this need to talk about how they're upgraded, or new ones added, and whether the licensing is valid.

@s0undt3ch
Copy link

Is tiamat going to bundle non-python dependencies or not? For example libgit2 and libzmq.

Yes and No.
No, the python libraries that require those will link to them at install time. PyZMQ ships it's own linked zmq bindings on its wheel.

As for libgit2, we're currently not including it in the tiamat package. If we were and linking was not enough, ie, it actually needed libgit2 available, we would include it in the tiamat package.
Since we're not including pygit2, you'd have to install libgit2 on your system and then pip install pygit2, which either links or loads libgit2 at runtime.

If they're not bundled then I don't see how the dependency/testing problems are solved,

How come? I don't understand the problem you're trying to point out.

and if they are then this need to talk about how they're upgraded, or new ones added, and whether the licensing is valid.

@thatch45 has already addressed the licensing concerns. Are there any more concerns regarding licensing?

As for how and when they are upgraded, at the lack of a written plan, at least so far, I'd say we upgrade to latest releases every time we get out of code freeze to start working on the new release, or if/when needed because of a security release.

@sagetherage
Copy link
Contributor

Meeting notes from the Open Hour on 2020-NOV-19 discussion and communication of the poll in the above comment can be seen here: https://github.com/saltstack/community/wiki/Open-Hour-Agendas-and-Notes:-2020-11#existing-seps

@aplanas
Copy link

aplanas commented Feb 19, 2021

Just an update here from #34 (comment). We have a very rough and broken prototype of a bundled version of Salt, as described before.

The source code is living here:

https://build.opensuse.org/project/show/systemsmanagement:saltstack:bundle

The targets for the different OSs are living in subprojects:

https://build.opensuse.org/project/subprojects/systemsmanagement:saltstack:bundle

For example, this is for CentOS:

https://build.opensuse.org/project/show/systemsmanagement:saltstack:bundle:CentOS-7

The only required package is venv-salt-minion, the rest are build dependencies

https://build.opensuse.org/package/show/systemsmanagement:saltstack:bundle/venv-salt-minion

Some notes of this exercise:

  • Is cheap. Is a couple of weeks we bundled Salt from the code base of SLE15-SP3. The amount of patches required to target different distros is not very high!
  • Debian was a problem, but extending debbuild to support lua was critical to support the RPM macros.
  • Fix of CVE for dependencies comes for free! For example, meanwhile I was writing this comment, the repository received a fix for the CVE-2021-3177, without any involvement from any of the Salt maintainers.

There is another nice technical thing, that I do not know how Tiamat can address. There is this subtle problem with Python bindings that comes from system libraries.

For example python-rpm. CentOS / RHEL / SUSE uses this RPM package format, that provides Python bindings that are written in C. This binding connect in one side the librpm part from the system, and for the other the libpython. Because RPM, depending on the version, can store the rpmdb in different places (/usr, /var) and with different formats (ndb, bdb, sqlite), is really important to use the same librpm version that the system expect (you can break your system otherwise).

We addressed this problem in OBS importing the pristine src.rpm from the target system, and compiling it with the Python version that will live inside the bundled (that is totally different from the one in the system). This generate a new python-rpm that is included in the bundle, with the correct library links from both sides (system and internal Python). For example, the one for CentOS is here:

https://build.opensuse.org/package/view_file/systemsmanagement:saltstack:bundle:CentOS-7/rpm/_service?expand=1

@sagetherage sagetherage self-assigned this May 18, 2021
@sagetherage sagetherage merged commit aa5578e into saltstack:master Jun 29, 2021
@waynew waynew added Accepted and removed Final Comment Period Speak now or forever hold your peace. labels Feb 15, 2022
@garethgreenaway
Copy link
Contributor

Just wanted to give everyone a heads up on some recent changes you may or may not have seen related to the Salt bootstrap script or the Salt 3005 documentation. The change in question was to rename the wording around the new onedir packages that are being built for 3005 using the Tiamat tool. The core team discussed this topic at length and decided that it was more accurate and descriptive to refer to these packages as onedir rather than referring them to as tiamatpackages. This gives us a clear distinction between the new onedir packages and the classic packages by focusing on the concept used to build them rather than the tool used to build them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.