katello-client-bootstrap icon indicating copy to clipboard operation
katello-client-bootstrap copied to clipboard

Restructure classes

Open bastian-src opened this issue 4 years ago • 16 comments

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

  • Introduce function get_os to determine current OS (using /etc/os-release or the platform library as backup)
  • Create abstract class OSHandler
    • Move general functions into OSHandler (functions that don't use redhat specific paths/commands)
  • Create class OSHandlerRHEL (implementing OSHandler)
    • Move OS specific functions into OSHandlerRHEL
  • In __main__, distinguish between current OS and instantiate responding OSHandler implementation
  • Create class OSHandlerSLES (implementing OSHandler)
    • Implement OS specific functions
  • Create class OSHandlerDeb (implementing OSHandler)
    • Implement OS specific functions
  • Remove Python 2.5 support (and Centos5 test) for better exception handling

bastian-src avatar Aug 11 '20 12:08 bastian-src

ping @evgeni :-)

sbernhard avatar Aug 13 '20 08:08 sbernhard

Please don't ping me

evgeni avatar Aug 13 '20 08:08 evgeni

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.

evgeni avatar Aug 25 '20 11:08 evgeni

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

chris1984 avatar Aug 25 '20 13:08 chris1984

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.

bastian-src avatar Aug 26 '20 07:08 bastian-src

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

chris1984 avatar Aug 26 '20 14:08 chris1984

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.

B3DTech avatar Mar 29 '21 20:03 B3DTech

@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.

bastian-src avatar Mar 30 '21 07:03 bastian-src

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

chris1984 avatar Mar 30 '21 13:03 chris1984

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!

B3DTech avatar Mar 30 '21 15:03 B3DTech

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 avatar Mar 31 '21 06:03 bastian-src

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).

bastian-src avatar Mar 31 '21 07:03 bastian-src

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.

B3DTech avatar Mar 31 '21 10:03 B3DTech

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.

chris1984 avatar Mar 31 '21 13:03 chris1984

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?

bastian-src avatar Apr 01 '21 13:04 bastian-src

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.

B3DTech avatar Apr 01 '21 15:04 B3DTech