Skip to content

Reduce the amount of duplicated code in hack #3447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
afbjorklund opened this issue Apr 20, 2025 · 8 comments
Open

Reduce the amount of duplicated code in hack #3447

afbjorklund opened this issue Apr 20, 2025 · 8 comments
Labels
expert help wanted Extra attention is needed kind/refactoring Refactoring

Comments

@afbjorklund
Copy link
Member

afbjorklund commented Apr 20, 2025

Description

Currently there is an almost identical copy of the update script, for each distribution.

   424 hack/cache-common-inc.sh
   287 hack/update-template-almalinux-kitten.sh
   287 hack/update-template-almalinux.sh
   311 hack/update-template-alpine.sh
   294 hack/update-template-archlinux.sh
   274 hack/update-template-centos-stream.sh
   440 hack/update-template-debian.sh
   283 hack/update-template-fedora.sh
   364 hack/update-template-opensuse.sh
   232 hack/update-template-oraclelinux.sh
   302 hack/update-template-rocky.sh
   258 hack/update-template.sh
   458 hack/update-template-ubuntu.sh
  4214 total

These should be made into a program with config instead, to share common functionality.

It would probably be better to convert them from shell scripts into a language like Python (or Go).

The current scripts are hard to write and test, and don't really have any unit tests (only functional)


It also fails on older platforms, due to not checking the versions or the functionality of the tools.

curl: unknown --write-out variable: 'json'
curl: unknown --write-out variable: 'header_json'
parse error: Expected value before ',' at line 2, column 10
@afbjorklund
Copy link
Member Author

afbjorklund commented Apr 20, 2025

I still think it would be easier to handle the images separately from the templates, but that's a different topic...

For now, we just have to replace the .images in the yaml. And be careful to not mess up the rest of the template.

Another thought, should the "latest" images be marked in the list somehow - instead of just by missing a digest?

@jandubois
Copy link
Member

It would probably be better to convert them from shell scripts into a language like Python (or Go).

I would much prefer if we can keep the languages in the project to just bash and Go, if reasonably possible.

I always thought the image updater should have been written in Go, but since I was not spending my time on it, it seemed fine to let somebody else write it in bash.

For now, we just have to replace the .images in the yaml. And be careful to not mess up the rest of the template.

The updating can easily be done using yq, which will keep the rest of the file (including the comments) as is:

yq '.images[0].location="https://example.com" | .images[0]' templates/default.yaml
# Try to use release-yyyyMMdd image if available. Note that release-yyyyMMdd will be removed after several months.
location: "https://example.com"
arch: "x86_64"
digest: "sha256:c2c3ed89097c5f5c90ebbe45216d1569e3ea2d3c8d0993eeae74f859f6467cdb"

This can be done by running yq as a subprocess; no need to call into yqlib in the updater.

@Yadavanurag13
Copy link

@afbjorklund @jandubois Can I pick this ?

@jandubois
Copy link
Member

Can I pick this ?

Fine with me, but please discuss first how you would implement, and only start writing code once there is agreement.


My ideas are that there should be some generalized update logic, and then specific updaters for each distro.

The distro updaters are each in a separate Go package, and "register" themselves in an init function.

The general updater will then process all templates passed to it on the commandline and will give each registered updater a chance to update each image.

The updater would either decline (next updater gets a chance), accept without a new image (stops checking this image because no update is available), or return the new image/digest.

The general updater will then update the template with the results.


But feel free to ignore if you have a better idea.

@Yadavanurag13
Copy link

Yadavanurag13 commented Apr 20, 2025

@jandubois, as far as I understand, many updated scripts need to be written in Go. We can generalise some business logic in some places (Base) , and there it can be inherited by scripts.

This is my current understanding. I have to go through your approach more clearly.

@jandubois
Copy link
Member

You could try a high-level proof-of-concept with a single distro, to show the direction you want to go.

I'm just trying to make sure you are not spending a lot of time on all the details before we have agreement on the general design.

I don't actually care too much about the details as long as it is modular and can easily be extended to cover additional distros without duplicating any code.

@afbjorklund
Copy link
Member Author

afbjorklund commented Apr 21, 2025

Some variables and function renaming would work as well, it is not so much the duplication but the little differences.

hack/update-template-almalinux-kitten.sh:$(basename "${BASH_SOURCE[0]}"): Update the AlmaLinux Kitten image location in the specified templates
hack/update-template-almalinux-kitten.sh:  This script updates the AlmaLinux Kitten image location in the specified templates.
hack/update-template-almalinux.sh:$(basename "${BASH_SOURCE[0]}"): Update the AlmaLinux image location in the specified templates
hack/update-template-almalinux.sh:  This script updates the AlmaLinux image location in the specified templates.
hack/update-template-alpine.sh:$(basename "${BASH_SOURCE[0]}"): Update the Alpine Linux image location in the specified templates
hack/update-template-alpine.sh:  This script updates the Alpine Linux image location in the specified templates.
hack/update-template-archlinux.sh:$(basename "${BASH_SOURCE[0]}"): Update the Arch-Linux image location in the specified templates
hack/update-template-archlinux.sh:  This script updates the Arch-Linux image location in the specified templates.
hack/update-template-centos-stream.sh:$(basename "${BASH_SOURCE[0]}"): Update the CentOS Stream image location in the specified templates
hack/update-template-centos-stream.sh:  This script updates the CentOS Stream image location in the specified templates.
hack/update-template-debian.sh:$(basename "${BASH_SOURCE[0]}"): Update the Debian image location in the specified templates
hack/update-template-debian.sh:  This script updates the Debian image location in the specified templates.
hack/update-template-fedora.sh:$(basename "${BASH_SOURCE[0]}"): Update the Fedora Linux image location in the specified templates
hack/update-template-fedora.sh:  This script updates the Fedora Linux image location in the specified templates.
hack/update-template-opensuse.sh:$(basename "${BASH_SOURCE[0]}"): Update the openSUSE Linux image location in the specified templates
hack/update-template-opensuse.sh:  This script updates the openSUSE Linux image location in the specified templates.
hack/update-template-oraclelinux.sh:$(basename "${BASH_SOURCE[0]}"): Update the Oracle Linux image location in the specified templates
hack/update-template-oraclelinux.sh:  This script updates the Oracle Linux image location in the specified templates.
hack/update-template-rocky.sh:$(basename "${BASH_SOURCE[0]}"): Update the Rocky Linux image location in the specified templates
hack/update-template-rocky.sh:  This script updates the Rocky Linux image location in the specified templates.
hack/update-template.sh:$(basename "${BASH_SOURCE[0]}"): Update the image location in the specified templates
hack/update-template.sh:  This script updates the image location in the specified templates.
hack/update-template-ubuntu.sh:$(basename "${BASH_SOURCE[0]}"): Update the Ubuntu image location in the specified templates
hack/update-template-ubuntu.sh:  This script updates the Ubuntu image location in the specified templates.

But I do think it should be possible to configure the images as configuration, without having to write code for each...

@afbjorklund
Copy link
Member Author

afbjorklund commented Apr 21, 2025

The code can get more complicated (for the "helper" functions), as long as the config gets simpler (for the future updates).

So the names and the regexes could be configured in a file, and then the code could parse that and run the updating?

But the "configuration" can be bash to start with, and just move more functions into the cache-common-inc.sh et al

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expert help wanted Extra attention is needed kind/refactoring Refactoring
Projects
None yet
Development

No branches or pull requests

4 participants