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

builder: add arch-specific targets #10016

Merged
merged 3 commits into from
Feb 8, 2021
Merged

Conversation

Habbie
Copy link
Member

@Habbie Habbie commented Jan 26, 2021

Short description

This allows us to be specific about what architecture to build for. The 'base' target remains, for anybody that just wants to build for their default arch. It might make sense to disable the base targets at some point.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@Habbie Habbie added this to the auth-4.5.0 milestone Jan 26, 2021
@Habbie
Copy link
Member Author

Habbie commented Jan 26, 2021

I'll file a pdns-builder PR to list the targets vertically instead of on one line, and then I'll see if we can bump the submodule here.

FROM centos:7 as dist-base
@ENDIF
@IF [ ${BUILDER_TARGET} = centos-7-amd64 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

@Habbie Is there a reason why you are calling the arch specific image? Most of these images all support manifest and should pull down the correct image based on your architecture. It would make sense to have an if/branch if the specific images does not have support.

For example centos images do not support s390x. So a lot of the times I have logic that checks what architecture I am building for, if its s390x, than I would pull from clefos. ClefOS being a s390x CentOS clone.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi James, this PR has nothing to do with building images. We utilize docker to produce RPM/DEB packages for different distros (and soon architectures), but out scripts need to know exactly what they're building.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pieterlexis Thanks for providing the needed context. When building the RPM/DEB packages is calling specific architecture image needed? Unless you are using an emulation layer like QEMU, Docker should detect the correct architecture and pull the correct image from the manifest.

Are you building RPM/DEB packages using QEMU/emulation or on architecture currently?

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently (without this PR), we build amd64 and raspbian armhf images, on amd64 machines. For armhf, we use qemu.

After this PR (plus related work in our buildbot setup), the plan is to build amd64 on amd64, and arm64+armhf on Amazon Graviton2 ARM64 boxes (that, unlike some other ARM64 chips, can run arm32 natively too).

The original reason for the change you are asking about is that the chosen target also decides the tarball name for the built packages. As a bonus effect, it allows one to force building packages for different architectures than the host, possibly, indeed, involving qemu.

Note that the diff does not remove the unprefixed version, that would, as you say, pull the correct image.

And, I did not realise this before, indeed it would allow us to choose forks based on their architecture support. :)

But as Pieter pointed out, none of this is about our Docker images, which is what you filed a request for in a separate ticket.

@Habbie
Copy link
Member Author

Habbie commented Jan 29, 2021

I've bumped pdns-builder, and compared a bunch of the debian-buster and centos-8 packages. I found no differences.

@Habbie
Copy link
Member Author

Habbie commented Jan 29, 2021

This PR really wants PowerDNS/pdns-builder#46

@Habbie
Copy link
Member Author

Habbie commented Feb 1, 2021

This PR really wants PowerDNS/pdns-builder#46

(But can be merged independently)

@Habbie Habbie merged commit 96c98d5 into PowerDNS:master Feb 8, 2021
@Habbie Habbie deleted the builder-archs branch February 8, 2021 08:34
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