libnetwork: processSetKeyReexec() remove defer()

Split the function into a "backing" function that returns an error, and the
re-exec entrypoint, which handles the error to provide a more idiomatic approach.

This was part of a larger change accross multiple re-exec functions (now removed).

For history's sake; here's the description for that;

The `reexec.Register()` function accepts reexec entrypoints, which are a `func()`
without return (matching a binary's `main()` function). As these functions cannot
return an error, it's the entrypoint's responsibility to handle any error, and to
indicate failures through `os.Exit()`.

I noticed that some of these entrypoint functions had `defer()` statements, but
called `os.Exit()` either explicitly or implicitly (e.g. through `logrus.Fatal()`).
defer statements are not executed if `os.Exit()` is called, which rendered these
statements useless.

While I doubt these were problematic (I expect files to be closed when the process
exists, and `runtime.LockOSThread()` to not have side-effects after exit), it also
didn't seem to "hurt" to call these as was expected by the function.

This patch rewrites some of the entrypoints to split them into a "backing function"
that can return an error (being slightly more iodiomatic Go) and an wrapper function
to act as entrypoint (which can handle the error and exit the executable).

To some extend, I'm wondering if we should change the signatures of the entrypoints
to return an error so that `reexec.Init()` can handle (or return) the errors, so
that logging can be handled more consistently (currently, some some use logrus,
some just print); this would also keep logging out of some packages, as well as
allows us to provide more metadata about the error (which reexec produced the
error for example).

A quick search showed that there's some external consumers of pkg/reexec, so I
kept this for a future discussion / exercise.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2022-04-20 17:34:56 +02:00
parent 4597f50deb
commit e974599593
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C

View file

@ -30,15 +30,12 @@ const (
// Refer to https://github.com/opencontainers/runc/pull/160/ for more information
// The docker exec-root can be specified as "-exec-root" flag. The default value is "/run/docker".
func processSetKeyReexec() {
var err error
// Return a failure to the calling process via ExitCode
defer func() {
if err != nil {
logrus.Fatalf("%v", err)
}
}()
if err := setKey(); err != nil {
logrus.Fatal(err)
}
}
func setKey() error {
execRoot := flag.String("exec-root", defaultExecRoot, "docker exec root")
flag.Parse()
@ -46,22 +43,21 @@ func processSetKeyReexec() {
// (i.e. expecting 2 flag.Args())
args := flag.Args()
if len(args) < 2 {
err = fmt.Errorf("Re-exec expects 2 args (after parsing flags), received : %d", len(args))
return
return fmt.Errorf("re-exec expects 2 args (after parsing flags), received : %d", len(args))
}
containerID, shortCtlrID := args[0], args[1]
// We expect specs.State as a json string in <stdin>
stateBuf, err := io.ReadAll(os.Stdin)
if err != nil {
return
return err
}
var state specs.State
if err = json.Unmarshal(stateBuf, &state); err != nil {
return
return err
}
err = SetExternalKey(shortCtlrID, containerID, fmt.Sprintf("/proc/%d/ns/net", state.Pid), *execRoot)
return SetExternalKey(shortCtlrID, containerID, fmt.Sprintf("/proc/%d/ns/net", state.Pid), *execRoot)
}
// SetExternalKey provides a convenient way to set an External key to a sandbox