testutil/fakestorage: use local paths, fix port-mapping and optimize, and remove contrib/httpserver#50654
Conversation
bd4d31e to
c0eddf5
Compare
- always build the go binary, instead of trying to find if it exists and then trying to copy it. - write the Dockerfile ourselves, instead of trying to copy the one from contrib. - update the Dockerfile to be built "FROM scratch"; we don't need busybox here. - start the container with "PublishAllPorts: true", otherwise no ports would be mapped. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's 12 lines of code total; we may as well write it as part of building; it looks to be the only place this is used, so we can remove the contrib directory, which should not be used by anyone. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
c0eddf5 to
50789e2
Compare
| func main() { | ||
| fs := http.FileServer(http.Dir("/static")) | ||
| http.Handle("/", fs) | ||
| log.Panic(http.ListenAndServe(":80", nil)) // #nosec G114 -- Ignoring for test-code: G114: Use of net/http serve function that has no support for setting timeouts (gosec) | ||
| } | ||
| ` |
There was a problem hiding this comment.
It's slightly ugly to put this in a const, but with the current handling of "create a temporary build-context somewhere, add some other files, then do a build", it was easier to do a os.WriteFile() with the source then to move or copy files around.
We should probably have another look at this package and logic used in future.
| if len(ports) == 0 { | ||
| t.Fatalf("no ports mapped for 80/tcp for %s: %#v", ctrName, i.NetworkSettings.Ports) | ||
| } | ||
| // TODO(thaJeztah): this will be "0.0.0.0" or "::", is that expected, should this use the IP of the testEnv.Server? |
There was a problem hiding this comment.
HA! Yes, same; I started to look at this package when our CI broke, wondering if my changes from #50640 could possibly be related. But then realised that we likely never run this code because it's only used when running tests against a "remote" daemon.
In either case, these changes made it slightly more correct, and it cleaned up the contrib/ directory, so that's a "small win". But I left this TODO as a crumble-path to find back "this may still be broken".
It worked when using on my local daemon though, so 🤷
testutil/fakestorage: use local paths, fix port-mapping and optimize
and then trying to copy it.
from contrib.
busybox here.
ports would be mapped.
testutil/fakestorage: inline go code and remove contrib/httpserver
It's 12 lines of code total; we may as well write it as part of building;
it looks to be the only place this is used, so we can remove the contrib
directory, which should not be used by anyone.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)