Skip to content

feat: move patch apply logic to patchable #1032

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
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

dervoeti
Copy link
Member

Description

This is a draft PR to solve #677. I replaced the apply_patches.sh script with Patchable for Zookeeper and Druid.

We decided in https://github.com/stackabletech/decisions/issues/44 that all sources should be mirrored on Github (to be implemented by #1011).

My idea was that we can just reuse the existing checkout logic, since it does exactly what we want: checkout a repo at a specific commit and apply a bunch of .patch files on top of it. In this PR, the original Apache repositories are used, those would need to be replaced by our mirror repos.

I added an optional flag --images_repo_root to manually specify a custom root for the images repo instead of relying on the auto-detection that Patchable currently does. We could also just use the auto-detection, but then we'd probably need to run git init && touch .patchable before running the patchable checkout command inside the container, which is a bit awkward. The extra flag isn't super pretty either though and I couldn't get around using PathBuf in the implementation, hence I made this a draft PR to serve as a PoC. This is just one way we could replace the patch scripts with Patchable. Maybe this can be solved in a more elegant way, I didn't find one yet though.

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Tasks

Preview Give feedback
TIP: Running integration tests with a new product image

The image can be built and uploaded to the kind cluster with the following commands:

bake --product <product> --image-version <stackable-image-version>
kind load docker-image <image-tagged-with-the-major-version> --name=<name-of-your-test-cluster>

See the output of bake to retrieve the image tag for <image-tagged-with-the-major-version>.

@dervoeti dervoeti requested a review from nightkr March 18, 2025 11:53
@dervoeti dervoeti force-pushed the feat/move-patch-apply-logic-to-patchable branch from 2d20b52 to 1efa478 Compare March 21, 2025 12:34
@dervoeti dervoeti force-pushed the feat/move-patch-apply-logic-to-patchable branch from 1efa478 to bdc6dfd Compare March 21, 2025 12:35
@dervoeti dervoeti requested a review from nightkr March 21, 2025 14:06
@dervoeti
Copy link
Member Author

@nightkr It's ready for another round of feedback 🙂 Once it's in a good shape I'll start adapting the patch processes of the other products.

Copy link
Member

@nightkr nightkr left a comment

Choose a reason for hiding this comment

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

Largely LGTM

@dervoeti dervoeti marked this pull request as ready for review March 31, 2025 10:49
@dervoeti dervoeti requested a review from nightkr March 31, 2025 10:49
@dervoeti
Copy link
Member Author

@nightkr I migrated all Dockerfiles now and tested all builds using the latest versions of the product, worked fine.

dervoeti added 3 commits April 9, 2025 15:06
…into feat/move-patch-apply-logic-to-patchable
…into feat/move-patch-apply-logic-to-patchable
…into feat/move-patch-apply-logic-to-patchable
@dervoeti dervoeti requested a review from nightkr April 14, 2025 15:35
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.

2 participants