Skip to content

Conversation

@TR-SLimey
Copy link

@TR-SLimey TR-SLimey commented Feb 6, 2021

Docker containers meant for one arch can run on another using QEMU. Using this idea, I attempted to build images for multiple architectures by simply pulling the plugins/docker images corresponding to each version I wanted to build for. Unfortunately, when using an image of this plugin meant for a different architecture, the docker daemon would fail to start. More details here: https://discourse.drone.io/t/cross-platform-docker-build/8642.

Anyway, I finally got around to finding the root cause of the issue. Docker containers use the host's kernel, iptables interacts with the kernel, and the docker daemon uses iptables to set up docker networking. Something along the way (presumably iptables and kernel) is incompatible between arches which meant that the deamon failed to start. As such, a simple and elegant solution is to disable networking altogether for the daemon. This allows it to run using QEMU, and also possibly reduces its startup time slightly. Meanwhile, networking is not needed if we are simply building images, so we don't lose any functionality.

Note: This does conflict with #309 but #309 does not fix my issue as --bridge=none is also needed. I also believe the added complexity of making this optional is unnecessary as networking features are never needed for building containers.

TR-SLimey added a commit to 0x1a8510f2/DroneExternalConfig that referenced this pull request Feb 6, 2021
Also temporarily removed arm64 build until fix is hopefully merged
See: drone-plugins/drone-docker#316
@TR-SLimey
Copy link
Author

The failed build appears to be due to hitting the docker pull limit as opposed to an error in the PR btw

@tboerger
Copy link

tboerger commented Mar 3, 2021

@bradrydzewski do you think that would have some side effects for previous versions of this plugin?

@bradrydzewski
Copy link
Member

@tboerger yes I am definitely concerned with security implications as well as regressions, given this is our most used plugin. I do not have the time to dedicate toward researching and verifying this change, and would therefore recommend the author run a fork for now.

@TR-SLimey
Copy link
Author

@bradrydzewski running a fork sadly creates a bootstrap problem for me, because I need an ARM version of this container to build ARM versions of this container, and I don't have a suitable ARM build host, which is why I need this in the first place :P

However, I have been able to verify that this works, simply by manually executing the command with those flags within the container, so I can at least confirm that nothing seems to be too noticeably broken by this.

As for the security implications and regressions you mention, would you be able to elaborate on what issues you may be expecting this to cause? That way I could do some further research and maybe provide some sources I find to help decide whether this is an acceptable solution.

@TR-SLimey
Copy link
Author

Upon some further thought, a more generic and possibly preferable solution might be to allow adding to the daemon command-line with an environment variable. That way, I could edit the command-line for my use-case and it would stay unchanged for everyone else. Does that sound "merge-able"? :P or maybe that's already a feature which I overlooked?

@tphoney
Copy link

tphoney commented Jul 6, 2021

Thank you for bringing this to our attention, and spending the time to come up with multiple solutions to the problem. Looking deeper into the issue, we found that this issue does not affect enough of the user base to make this kind of change for now.

We have a proposal site https://github.com/drone/proposal where you can express the editable command line via ENV variable solutions suggested. From this site we can gauge an interest in this feature. Thank you for your patience.

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.

4 participants