Initial move of the internal image generation code

[SchoolGuy]

This PR aims to move internal code for image building for salt-toaster to the public. Not all targets are available to the public because they would require access to internal resources.

• Edit 1: This will obsolete https://hub.docker.com/orgs/salttoaster because we will use the GH Registry directly.
• Edit 2: This PR should not yet clean up the templates and the currently commited dockerfiles. This will follow in a later PR.
• Edit 3: TODO List added

TODOs:

• Rebase this PR once CI: Change to GH Actions #92 is merged
• Add pylint linting to the new Python files
• Merge README.md file with the README.adoc file
• Remove GH Pages
• Fixup templating so all images succeed building
• Use the GH Action from Docker for registry login at least
• Rename VERSION to DISTRO
• Add image building for the other OSes
• Cleanup Git History

[SchoolGuy]

Okay so although this is green, it is not working because it is using my personal account for logging into ghcr.

[agraul]

I know it's still in "Draft mode", but I already took an early look and left a few comments.

[SchoolGuy]

Boom! It works with openSUSE now!

[SchoolGuy]

Okay so the substitution of the lowercased owner is not done correctly yet. Maybe in the next run or tomorrow.

[SchoolGuy]

Argh does not work. Sigh the small things...

[SchoolGuy]

Finally it works.

[agraul]

I'm sure I missed something but I've found a few things we can get rid off or that we need to fix with the README conversion

[agraul]

@meaksh how important is it to keep the old Dockerfiles in a repository? I agree with @SchoolGuy that we should generate them on the fly and use them, although we do lose the ability to do dirty hacks (or rather, we force ourselves to do even worse hacks e.g. sed -i s/foo/bar/ in Jenkins). But what about keeping them for archival purposes? We could also keep them in the old repo and make that one read-only (if possible) instead of removing it.

[SchoolGuy]

@agraul @meaksh I just talked to Julio and he meant it could be reasonable to build the Docker images inside the OBS because of automated rebuilds on package changes. This however means that we have to check them in and have to maintain a lot of images. Should I change the mechanism once more and talk to Jordi for that or do we want to do it here?

[SchoolGuy]

Okay next problem to make this work: We have https://github.com/dincamihai/saltrepoinspect used and this is where the image source is being built. The problem with the dependency is that the URL of the repository is hardcoded in the source code of the repository.

Since I believe it is only use who uses that repo, I believe it would be a good time to pull this also in this repo. Opinions @agraul @meaksh ?

[agraul]

I think that Python package was, or still is, installed into containers. I don't recall what we need it for at runtime, only that it's used when generating Dockerfiles. I wonder if we used it outside of the salt-toaster context or if we want to start using that for something else. If not, keeping it within this repository is ok for me. Is this something that's blocking this PR or can we decide that later?

[SchoolGuy]

@agraul It is blocking for this PR since the source of the Docker Image (FROM-step) is generated indirectly through an environment variable passed to Jinja2.

[SchoolGuy]

Note: The mechanism I would like to keep since it allows use to dynamically prefer our internal registry which saves us a lot of bandwidth because we have the images already internally mirrored once.

[agraul]

@agraul It is blocking for this PR since the source of the Docker Image (FROM-step) is generated indirectly through an environment variable passed to Jinja2.

Until now the generation was possible without having saltrepoinspect in the same repository as the python scripts that generate the Dockerfiles. Why is that different now?

[SchoolGuy]

It was agreed upon that we move the repository code into this repository, so we'll remove the dependency on an external source. I will do so on Monday.

[SchoolGuy]

Note: It was agreed upon by @agraul and me that we try to stick to OS packages as much as possible since this is the version we will have once it is built.

[SchoolGuy]

I rearranged the commits a bit to make it easier to clean the history in the future.

[agraul]

Just sending this part before I forget it, I'm not done yet.

[agraul]

I think the python code can still be improved, but for this PR I think it is ok. We can always follow up with additional PRs.

[SchoolGuy]

@agraul @meaksh I believe this is ready to be merged after I test this for the internal cases and images as well. Except smaller bugfixes the code should be final now. I must say I am not happy with the Git history but I don't know how to structure it better. Maybe a squash is acceptable in this case?

[agraul]

The builds are working, all of my review comments are addressed. Thanks for your work @SchoolGuy !

There are some improvements to be done later, but this PR is already pretty big. I agree that the git history is not the cleanest. I rebase my branches pretty often during development to keep them clean, fixing 42 commits into a few meaningful ones is not a nice task to have. For example, when I change something I have added in a previous commit of a PR, I normally squash them. Same for "WIP" commits. Review comments only make it into their own commits if I want to keep the co-author.
For a PR like this one, I would have aimed at something like 1 commit for docs, 1 commit for adding images/*.py, 1 for touching a group of templates doing the same change (each) and 1 for fixing a distro specific template (each). There probably would have been some other things to commit separately (e.g. VERSION to DISTRO renaming), I just wanted to give you an idea.

I'm fine with squashing this one as long as the squashed commit has a nice message 😄

[SchoolGuy]

Update: I tested the new generation and since the logic for pulling the images does not work for SLES and RHEL based containers, we need to wait a littlebit more before we merge this I believe.

[SchoolGuy]

Okay interesting... Now pylint finds new issues because I force pushed this to squash things.

[SchoolGuy]

This is the best I can do in terms of Git History without redoing the whole PR from scratch manually in my eyes. I am open for suggestions of course. However I see this as ready to be merged.

[SchoolGuy]

Okay local testing of the unit tests did succeed with the new template for openSUSE 15.2. Thus I believe we didn't break anything bigger. This should now really be ready!

[meaksh]

Good, thanks for the work here Enno!

However I am not really sure here about the change from VERSION to DISTRO. This will break all toaster jobs we currently have in our jenkins. All those jobs are currently passing VERSION as env variables when calling the toaster (not only for building images but also for running the tests).

I think we should do the necessary changes in the jenkins jobs if we want to go with this rename of the "VERSION" variable here to avoid breaking those jobs. At least for those jobs that we want to keep running in our jenkins, and then disabling the ones that we're moving to GH.

[agraul]

However I am not really sure here about the change from VERSION to DISTRO. This will break all toaster jobs we currently have in our jenkins. All those jobs are currently passing VERSION as env variables when calling the toaster (not only for building images but also for running the tests).

The ones for building images already use DISTRO, but the test jobs need to be adjusted at the time of merging this on.

I think we should do the necessary changes in the jenkins jobs if we want to go with this rename of the "VERSION" variable here to avoid breaking those jobs. At least for those jobs that we want to keep running in our jenkins, and then disabling the ones that we're moving to GH.

👍