search

Katello / katello-client-bootstrap

Bootstrap Script for migrating systems to Foreman & Katello
GNU General Public License v2.0
52 stars 63 forks source link

Restructure classes #331

Open bastian-src opened 4 years ago

bastian-src commented 4 years ago

Restructure the script and implement classes for Debian/Ubuntu and SUSE integration

sbernhard commented 4 years ago

ping @evgeni :-)

evgeni commented 4 years ago

Please don't ping me

evgeni commented 4 years ago

So, coming back to this. I don't think we should spend too much time on the current version of bootstrap.py as there is currently movement to revamp the whole workflow and ideally replace bootstrap.py with a tiny script that just pulls and executes templates from Foreman and also includes support for non-RHEL systems.

chris1984 commented 4 years ago

Why do we have to have objects and classes, can we just use one long function with if statements?

bastian-src commented 4 years ago

Let's assume we go with one long function and if statements, there will be (kind of the same) statement every time we want to differ between Debian, RHEL and SLES. This leads to duplicated code.
When we use classes, this choice must be made only once at the beginning. Afterwards, the corresponding class, which implements the OS specific calls (yum, zypper, apt) and configurations, is used. I suggest an abstract class OSHandler that implements the general procedure of the script and defines some empty function bodies like pm_install(package_name) (package manager install) or setup_repo(url, gpg). Then, the classes OSHandlerRHEL, OSHandlerSLES, OSHandlerDeb inherit from OSHandler and implement the specific functionalities.

This approach has no duplicated code and increases readability by grouping OS specific implementations under one class.

chris1984 commented 4 years ago

@bastian-src thanks for explaining it to me, I am still trying to get a grasp on OOP so that helps!

B3DTech commented 3 years ago

There is a problem with this. You clear out the apt sources.list file before installing the subscription manager packages, specifically python3-subscription-manager, which means that it is not able to install the prerequisite packages and the script fails.

bastian-src commented 3 years ago

@B3DTech Thanks for your reply! I appreciate that someone is having a look at this PR.

On Debian/Ubuntu, the subscription-manager package is usually not included in the default repositories. Therefore, you have to pass the --deps-repository-url and --deps-repository-gpg-key parameters when running the script (providing an alternative repo). If you do so, in line 1681, during _install_prereqs, these two arguments will be passed to _setup_repo which can be seen in line 1534. At this point, the sources.list is "cleared" as you mentioned, but the additional repo is added afterwards which shall provide all dependencies to install subscription-manager.

Have in mind, both parameters (-url and -gpg-key) contain URLs. The script downloads the gpg key from the given URL and runs apt-key add ... on the retrieved file.

chris1984 commented 3 years ago

What are your thoughts on rewriting this entire tool in Ruby?

B3DTech commented 3 years ago

Therefore, you have to pass the --deps-repository-url and --deps-repository-gpg-key parameters when running the script (providing an alternative repo).

I'm sure I'm missing something here coming from the RHEL/CentOS side of things where subscription-manager is built into the repos and I can use --deps-repository-url to specify the CentOS base repo for dependencies.

The --deps-repository-url is supposed to specify the location of the subscription-manager packages. I'm using Aptix repo as it's the only one for Debian/Ubuntu that I can see.

This is supposed to install python3-subscription-manager, which has the following dependencies.

Depends: python3 (<< 3.9), python3 (>= 3.8~), python3-dateutil, python3-ethtool, python3-iniparse, python3-six, python3:any (>= 3.2~), python3-dbus, python3-rpm, virt-what, python3-debian, python-gi, python3-decorator Suggests: python3-subscription-manager-doc

These dependencies are not available in the Aptix repo, and since the sources.list file with the default Ubuntu repos are removed, there is now no way to resolve these dependencies.

Any thoughts? Thanks!

bastian-src commented 3 years ago

These dependencies are not available in the Aptix repo, and since the sources.list file with the default Ubuntu repos are removed, there is now no way to resolve these dependencies.

When you are using an Orcharhino, the Orcharhino itself provides you with a repo which holds all dependencies for the bootstrap process. You can see it in the documentation right before the part Using Katello Agent on SLES 11 (compare docs).

So, the script is designed to get all dependencies via an extra repository. As a quick fix you can just put your normal Ubuntu repos in an extra file /etc/apt/source.list.d/ubuntu.list. Let me know if that works for you!

Nevertheless, it can be discussed whether it leads to problems if we remove the sources.list after the installation of the subscription manager instead of doing it before.

bastian-src commented 3 years ago

What are your thoughts on rewriting this entire tool in Ruby?

If we wanna do exactly the same in Ruby, we would need Ruby to be installed on the host. Then it could also be translated to Ruby, but I think there is also a discussion going on regarding the re-design of this whole process (compare forum).

B3DTech commented 3 years ago

When you are using an Orcharhino

Not everyone uses Orcharhino. In this case it’s Foreman/Katello proper. And since this script is going into the Katello project it should be made universal.

I will modify the bootstrap for my setup as I need this to work but I think this really would impact anyone using Ubuntu or Debian with this script.

chris1984 commented 3 years ago

That's fair I am not a fan of Python and Ruby works better. That is why I wanted to switch this to Ruby. We could also do C or C++ which would be native to the OS. With that we could get improved speed too.

bastian-src commented 3 years ago

When you are using an Orcharhino

Not everyone uses Orcharhino.

Maybe not today and also not tomorrow, but the day will come.

Okay, jokes aside. Of course you are completely right and this script must be unified such that it can work with any foreman installation. Nevertheless, the reason for this single deps repo implementation is the fact that we experienced issues with apt when dependencies were available via multiple sources/repos. That's why other repos in sources.list are disabled before the installation process of the subscription-manager.

Therefore, the usual case should be that the repo, you retrieve your software from, should also satisfy the dependencies for that piece of software. Since this is not always the case, what are your opinions on giving the option to add multiple deps repos? Or maybe just an additional parameter where you can tell the script to disable sources.list after the installation of the subscription-manager?

B3DTech commented 3 years ago

For Unbuntu/Debian - the only source for Subscription Manager is the Atix repository. If there are required pre-req's and versions, a solution would be to have Atix include them in their repo.

The other option would be to have Atix document the additional deb's needed and put it on their apt repo web page. Maybe a quick how-to create that Repo in a native Foreman/Katello install.

Doesn't seem like I can create the repo in Katello/Pulp since I'm using Pulp2 for now and don't have signed repos - although I guess I can delete all my repos, upgrade to pulp3 and re-create them and then manually sign them.