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

Add new Kaniko Dockerfile for PDNS builder #68

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

Conversation

aerique
Copy link
Member

@aerique aerique commented Aug 30, 2023

(I'm not sure @wojas needs to review this. Possibly the Kaniko build process was added after @pieterlexis took ZoneControl over.)

A custom Kaniko image for PDNS Builder, because the official Kaniko debug image only offers Busybox and the PDNS Builder also needs Bash, Git, Perl, rsync and perhaps more.

For now we put it besides Dockerfile-kaniko until it is clear that one can be removed.

AFAIK this file is only used for ZoneControl builds but I'm not entirely sure, that's why I do not want to remove Dockerfile-kaniko yet.

This new Dockerfile has been successfully used for the latest ZoneControl builds as part of the process to fix the flaky builds.

It is available at https://hub.docker.com/repository/docker/powerdns/kaniko-pdns-builder/ which is where the ZoneControl builds pull it from.

@aerique aerique requested review from wojas and Habbie August 30, 2023 14:02
@aerique aerique self-assigned this Aug 30, 2023
@aerique aerique force-pushed the update-kaniko-dockerfile branch from d24736c to d69bbde Compare August 30, 2023 14:15
@Habbie
Copy link
Member

Habbie commented Sep 4, 2023

AFAIK this file is only used for ZoneControl builds but I'm not entirely sure, that's why I do not want to remove Dockerfile-kaniko yet.

Do note that all users of pdns-builder import it as a git submodule, which is tied to a git commit. Removing things will not randomly break users; they'll find out when they try to bump to a new version, at which point we can revisit, restore, etc.

I'm not proposing "just edit the file", because that might lead to invisible and subtle problems. I am proposing "add the new file, and delete the old file, and users will notice, very clearly, at some point".

@Habbie
Copy link
Member

Habbie commented Sep 4, 2023

Do note that all users of pdns-builder import it as a git submodule

He said, before reading the bit about "it's published to Docker Hub".

@wojas
Copy link
Member

wojas commented Sep 5, 2023

I would like to get rid of Kaniko in all our build systems eventually, but preferably without causing any external users too much inconvenience. Having two of these Dockerfiles just seems confusing.

Perhaps remove the old Dockerfile, move the new one to an examples subdir and add a Kaniko deprecation warning to the README?

Why would anyone want to continue to use the old Dockerfile-kaniko?

@aerique
Copy link
Member Author

aerique commented Sep 5, 2023

Why would anyone want to continue to use the old Dockerfile-kaniko?

I'm just not 100% sure the ZoneControl build process is the only one using it.

@Habbie
Copy link
Member

Habbie commented Sep 5, 2023

I'm just not 100% sure the ZoneControl build process is the only one using it.

While I thought I laid out a safe alternative path above (and was wrong), this is not a thing that worries me. I suggest just updating the existing file & docker hub image.

@aerique
Copy link
Member Author

aerique commented Sep 5, 2023

I'm assuming I can ignore the failed tests?

@wojas
Copy link
Member

wojas commented Sep 5, 2023

I'm assuming I can ignore the failed tests?

They are not related to this PR, but do need fixing.

Put it besides `Dockerfile-kaniko` until it is clear that one can be
removed.
Move Kaniko Dockerfile to `docker` directory.
Remove old Kaniko Dockerfile.
@Habbie Habbie force-pushed the update-kaniko-dockerfile branch from 72b7f3d to f55a2cf Compare September 9, 2024 14:52
@Habbie
Copy link
Member

Habbie commented Sep 9, 2024

I took the liberty of rebasing this, although I'm not sure we need it any more.

@aerique
Copy link
Member Author

aerique commented Sep 10, 2024

I took the liberty of rebasing this, although I'm not sure we need it any more.

ZoneControl is currently not using this.

@Habbie
Copy link
Member

Habbie commented Sep 10, 2024

let's close this and at some point remove it all

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