Skip to content
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

Introduce regsync, Go scripts, config.yaml and regsync.yaml #829

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

adamkpickering
Copy link

@adamkpickering adamkpickering commented Feb 6, 2025

This PR takes the first steps towards overhauling rancher/image-mirror in order to accomplish the goals set out by EDR 005. Here is the JIRA issue. This PR:

  • Adds Go scripts with generate-regsync subcommand that generate regsync.yaml (the regsync config) from config.yaml. These scripts will facilitate the writing of unit tests, automation, and better validation.
  • Updates README.md to reflect the current state of the repo.

The idea is to develop the new scripts and setup alongside the old setup. I want to gradually migrate configuration from images-list and images-list-daily to config.yaml and regsync.yaml. Once that is done, retrieve-image-tags/config.json and the associated scripts can also be migrated. The old setup should never stop working until we are ready to completely migrate to the new setup.

@adamkpickering adamkpickering marked this pull request as ready for review February 10, 2025 20:59
@adamkpickering adamkpickering requested a review from a team as a code owner February 10, 2025 20:59
Copy link
Member

@brandond brandond Feb 12, 2025

Choose a reason for hiding this comment

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

nit: should new tools perhaps live in a hack or cmd subdirectory, instead of at the root of the repo? We have scripts for more recent shell-based additions.

I generally don't like to keep source code and configuration/input side by side, once the project has grown past a loose collection of utility scripts.

Copy link
Author

@adamkpickering adamkpickering Feb 12, 2025

Choose a reason for hiding this comment

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

Hi @brandond! I'm glad you commented - I think you've done a bunch of work on this repo in the past and no doubt know more than me. If you are able to give this PR a full review it would help a lot... but no pressure of course 😄

As to your comment, I'm curious as to why? Has this bitten you in the past?

I was thinking that go run main.go would be the gateway to all scripting functionality eventually, but maybe that isn't as tidy as it could be. We could move all of the Go code (main.go, pkg/, go.mod, go.sum) into a cmd/ (or whatever) directory, and the only cost would be having to write go run cmd/main.go ... instead of go run main.go .... I don't think this would cause any issues when running go test, golangci-lint etc.

On the other hand, there are already Dockerfile.dapper, Makefile, and entrypoint.sh, which are arguably source code...

Copy link
Member

@mallardduck mallardduck Feb 18, 2025

Choose a reason for hiding this comment

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

On the other hand, there are already Dockerfile.dapper, Makefile, and entrypoint.sh, which are arguably source code...

Sidenote, because Dapper is not something with any maintainers we could/should probably even consider removing that at some point. I don't have strong feelings on if that should be done with this effort, or afterwards - I could see either one making sense.


Edit: Actually, after looking over the current repo more and considering what it would look like w/o dapper, all 3 of those files could be removed with removal of Dapper. The only concern that leaves is if the scripts dir code can run everywhere w/o dapper as a middle layer.

So in that scenario, the only top level files are the yaml metadata files; with the rest of the "tooling" sources in subdirectories. To me this creates a "nice" delineation of contexts and I think if possible continuing that with the new tooling could be "nice" as well. (Not a huge driver or issue though at the end of the day IMO).

Just so I understand, is the goal that the new golang tooling will replace all the tools in scripts too? Or will it only cover the new mirroring requirements? If it won't replace them, will any deduplication and updates be done to those existing scripts?

Copy link
Author

Choose a reason for hiding this comment

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

I would like to remove dapper. However, I don't think it is a good idea to do that while we are using the current scripts. dapper is necessary for the Python scripts, assuming we don't want to mess around with venvs et cetera. dapper might be needed for the shell scripts as well.

I intend to replace the current scripts with Go and regsync. Part of the reason for this is to remove dapper - if we are using Go, we don't need to worry (much) about runtime dependencies.

I'll take a look at putting the Go scripts in their own directory 👍

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

Successfully merging this pull request may close these issues.

3 participants