Revert tasksmax workaround to avoid unsupported bins#24100
Revert tasksmax workaround to avoid unsupported bins#24100thaJeztah merged 1 commit intomoby:masterfrom
Conversation
GordonTheTurtle
added
status/0-triage
dco/no
Signed-off-by: Stefan Scherer <[email protected]>
StefanScherer
force-pushed
the
revert-tasksmax-workaround-21628
branch
from
7bc10b1 to
6509cc4
Compare
GordonTheTurtle
removed
the
dco/no
cpuguy83
added
status/1-design-review
priority/P1
|
Tagging this as P1 since it effects release. I agree we should revert. |
|
I still wonder if the reason we removed the option was needed; IIRC it was only a warning that the option was not supported, but I wonder if it really affected running docker if the systemd version didn't know about that option |
|
@thaJeztah -- Systemd doesn't handle the situation where there's an option it doesn't recognize, so docker won't start. It's probably best to revert and figure out a better approach (like the one @tianon suggested). |
|
@StefanScherer nice hedgehog 💜 and thanks for the deeper investigation. It's crazy that this doesn't affect all the deb builds. Edit: I keep forgetting that it doesn't affect jessie b/c the sed doesn't apply there. But what I didn't try when I was testing this was installing the binary and checking the version output. I was purely looking at the package file name, so perhaps that is how I missed the problem. |
|
Thanks @clnperez, that's very unfortunate indeed. So, yes, I agree we should revert, but we need a different solution, as a lot of people get hit by the limit otherwise. |
|
LGTM 🐸 then 😉 |
|
Yes, package names look good, but the binaries inside have problems. BTW: What about adding some tests testing the DEB packages in a Docker container like this docker version serverspec test? 👼 And probably running such tests in CI if a PR affects the make-deb/make-rpm build steps would also help. |
|
@mlaventure I know you worked on some things for testing debs ^^ |
|
Uh, this shouldn't cause the build to be marked LMTAL. |
|
@StefanScherer can you open a separate issue for the CI suggestion? I like the idea, and think we can definitely improve in that area |
|
ok, my bad, I read it wrong, we change the LGTM (IANAM). Quick question though, aren't all the |
|
@mlaventure Only distros using systemd versions after (including) 228. You can read /pull/21297. |
|
@thaJeztah Just recognized that there also is a |
|
LGTM |
|
merging for now but lets find a better solution before GA (if possible) |
|
@thaJeztah I'm looking into it since (although this area is not my expertise really, so it's been a lot of googling and manpage reading today!) Testing something out now... |
|
opened #24197 |
8 participants
This PR reverts PR #21628 and does no longer modify the git source tree for any OS until we come up with a better solution setting tasksmax differently for specific OS.
The modification of 21628 while building the binaries for each DEB package result in binaries with the "unsupported" tag if checked with
docker --versionordocker version.- What I did
I compiled all DEB packages with
make debfor v1.12.0-rc2 and found out that some OS have unsupported binaries. So I reverted that commit that introduced the code change.- How I did it
- How to verify it
You can reproduce this problem by installing eg. the 1.12.0-rc2 DEB package on eg. ubuntu:xenial or debian:stretch with
curl -fsSL https://test.docker.com/ | sh.The same steps with a debian/jessie64 Vagrant box shows clean binaries.
- Description for the changelog
Revert tasksmax hack to avoid unsupported binaries in DEB packages.
- A picture of a cute animal (not mandatory but encouraged)
(mirrored image of 21628 :-) )
PS: You can't take a shortcut and remove some directories in the contrib/builder/deb/amd64 directory as this also leeds to a git code change and unsupported binaries.