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

enable multi-arch build for -reactor and -injector #88

Merged
merged 1 commit into from
Aug 30, 2021
Merged

enable multi-arch build for -reactor and -injector #88

merged 1 commit into from
Aug 30, 2021

Conversation

toelke
Copy link

@toelke toelke commented Aug 22, 2021

See #74

I changed the Makefiles to build for the local architecture on make update and build for all specified architectures on make upload.

@toelke toelke marked this pull request as ready for review August 22, 2021 10:18
@rck
Copy link
Member

rck commented Aug 23, 2021

LGTM. As we are not in a rush I guess and with Moritz being the one that oversees the CI parts, I'd wait for him being back from vacation to actually hit the merge button. Guess it is fair to ping him next Tue.

Copy link
Member

@WanzenBug WanzenBug left a comment

Choose a reason for hiding this comment

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

Over all: looks good and I suspect it's working.

Any particular reason we need to use linux/arm64/v8 instead of just linux/arm64, like in the other PRs?

Also, it would be nice to re-use the same workflows as piraeus-client&piraeus-server. I understand that this is a bit harder for drbd-loader, since we have multiple dockerfiles for one "product". However, drbd-reactor should work 1:1 the same as piraeus-server?

Signed-off-by: Philipp Riederer <pr@tum.fail>
@toelke
Copy link
Author

toelke commented Aug 30, 2021

For the multiple Dockerfiles it seems as if we would need to have multiple build-blocks in the yaml; I'm not sure if that's nicer than having the loop in the Makefile.

What do you think?

@WanzenBug
Copy link
Member

I'd say: "good enough" 😉

LGTM

@WanzenBug WanzenBug merged commit 0806b95 into piraeusdatastore:master Aug 30, 2021
@WanzenBug
Copy link
Member

@toelke toelke mentioned this pull request Aug 30, 2021
@rck
Copy link
Member

rck commented Aug 31, 2021

just some thoughts on where the complexity should be: In general I'd like to see it (hidden) in the Makefiles if possible. They should be easy to use if necessary. Necessary for whatever reason: Somebody just wants a quick rebuild on the command line, or a quick test build. Or switches to another CI pipeline. Maybe one in house. Besides the other points to me it always has a certain degree of vendor lock-in vibes.

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.

None yet

3 participants