-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
figure out DinD + podutils #8780
Comments
/assign |
The current implementation of the utils is super generic and there is merit to that, but if we wanted to have a |
@stevekuznetsov I don't think that will work properly due to volume mounts. See #8779 (comment) Seperately from DinD, most things we run fork, avoiding zombies is something to handle more generally. What needs to happen here specifically is a different snippet to boot docker (instead of having it in the |
We should be able to refactor out just the DinD parts from This just reminded me that zombies are also coming to eat our nodes. For that something like I'll handle the refactor bit now and circle back on zombies. Kubernetes may also soon have better ability to manage this problem as well... (it does have some alpha gated enhancements now IIRC) .. we're not the only ones with "the PID1 problem". |
I'd check where the |
It looks like we are waiting on kubernetes/kubernetes#1615 before the pause container can reap zombies. It has some logic already, but currently it doesn't work as the pod doesn't share a PID namespace yet. It will be a while before Prow is running on this though, as even 1.11 has it as alpha, 1.12 may have it as beta. |
You are stock GKE, right? I think we may be running with it right now on ours, but we fiddle with cluster flags a lot. |
Are there benefits from integrating more tightly for classes of jobs (DinD, jobs that want VMs, stuff like that) or do we just want to black-box everything and keep it as run by |
Stock GKE does not enable alpha features to my knowledge. We could almost certainly sidestep this but it would be hacky.
We should keep it flexible. I don't think we want to support VMs etc as part of PRow. I do think we need to support forking, as tons of useful utilities fork. This issue is really for tracking that our images also need an update because they assume they can do $magic in the entrypoint before running bootstrap. |
FYI, GKE supports alpha clusters, but they're only suitable for testing. @stevekuznetsov Are you running with PodShareProcessNamespace=true? I haven't received much feedback on this feature. I'd like to gather a few examples of people using it successfully. |
Yeah, alpha clusters are not a suitable option for Prow. Some end to end testing uses them though. |
/milestone v1.13 |
This is definitely v1.13 I expect to start poking this more next week :-) |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
/milestone 2019-goals @BenTheElder I punted this out given that I saw no activity here in the v1.13 cycle. Please add back to v1.14 or close out or do nothing as appropriate. |
pod-utils + dind works with our sysv dind, it's not quite ideal though, but we should probably track options for not hijacking ENTRYPOINT in another issue. |
This is a specific issue to track the need for Docker in Docker with the podutils. I am also filing a related but more general issue for the podutils and init / forking that may wind up solving this one. xref: #8779
In short, with the old bootstrap + scenario scripts and their images we have support for using docker at runtime in prowjobs, with the podutils we do not. We need to sort this out. A complication here is that the podutils now control the entrypoint, we can run a similar script under the entrypoint but before the actual test command, but that leaves other questions, like how we will handle zombie processes...
xref: #8763 (comment), #7844 (comment)
The text was updated successfully, but these errors were encountered: