[lxc-devel] [lxd/master] Move forkproxy to using subprocess
stgraber on Github
lxc-bot at linuxcontainers.org
Thu Aug 13 22:46:01 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200813/2972ba45/attachment.bin>
-------------- next part --------------
From eb97c1ef8f5f001d6eb4e9227eb554f9750ed04b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 13 Aug 2020 18:27:57 -0400
Subject: [PATCH 1/2] shared/subprocess: Add StartWithFiles
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
shared/subprocess/proc.go | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/shared/subprocess/proc.go b/shared/subprocess/proc.go
index 060637e681..f78b1a1f70 100644
--- a/shared/subprocess/proc.go
+++ b/shared/subprocess/proc.go
@@ -100,6 +100,15 @@ func (p *Process) Stop() error {
// Start will start the given process object
func (p *Process) Start() error {
+ return p.start(nil)
+}
+
+// StartWithFiles will start the given process object with extra file descriptors
+func (p *Process) StartWithFiles(fds []*os.File) error {
+ return p.start(fds)
+}
+
+func (p *Process) start(fds []*os.File) error {
var cmd *exec.Cmd
if p.Apparmor != "" && p.hasApparmor() {
@@ -117,6 +126,10 @@ func (p *Process) Start() error {
cmd.SysProcAttr.Credential.Gid = p.GID
}
+ if fds != nil {
+ cmd.ExtraFiles = fds
+ }
+
// Setup output capture.
if p.Stdout != "" {
out, err := os.Create(p.Stdout)
From 8e461e015c24fc4b3b02302fdf31fb2f55cddbca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 13 Aug 2020 18:28:13 -0400
Subject: [PATCH 2/2] lxd/forkproxy: Switch to using subprocess
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/device/proxy.go | 75 +++++++++++++++---------------------
lxd/main_forkproxy.go | 90 ++++++++-----------------------------------
2 files changed, 47 insertions(+), 118 deletions(-)
diff --git a/lxd/device/proxy.go b/lxd/device/proxy.go
index c3ef87019c..9d3420622e 100644
--- a/lxd/device/proxy.go
+++ b/lxd/device/proxy.go
@@ -2,9 +2,7 @@ package device
import (
"bufio"
- "bytes"
"fmt"
- "io/ioutil"
"net"
"os"
"path/filepath"
@@ -13,7 +11,6 @@ import (
"time"
"github.com/pkg/errors"
- "golang.org/x/sys/unix"
liblxc "gopkg.in/lxc/go-lxc.v2"
deviceConfig "github.com/lxc/lxd/lxd/device/config"
@@ -24,6 +21,7 @@ import (
"github.com/lxc/lxd/lxd/util"
"github.com/lxc/lxd/shared"
"github.com/lxc/lxd/shared/logger"
+ "github.com/lxc/lxd/shared/subprocess"
"github.com/lxc/lxd/shared/validate"
)
@@ -195,10 +193,9 @@ func (d *proxy) Start() (*deviceConfig.RunConfig, error) {
logFileName := fmt.Sprintf("proxy.%s.log", d.name)
logPath := filepath.Join(d.inst.LogPath(), logFileName)
- _, err = shared.RunCommandInheritFds(
- proxyValues.inheritFds,
- d.state.OS.ExecPath,
- "forkproxy",
+ // Spawn the daemon using subprocess
+ command := d.state.OS.ExecPath
+ forkproxyargs := []string{"forkproxy",
"--",
proxyValues.listenPid,
proxyValues.listenPidFd,
@@ -206,28 +203,43 @@ func (d *proxy) Start() (*deviceConfig.RunConfig, error) {
proxyValues.connectPid,
proxyValues.connectPidFd,
proxyValues.connectAddr,
- logPath,
- pidPath,
proxyValues.listenAddrGID,
proxyValues.listenAddrUID,
proxyValues.listenAddrMode,
proxyValues.securityGID,
proxyValues.securityUID,
proxyValues.proxyProtocol,
- )
+ }
+
+ p, err := subprocess.NewProcess(command, forkproxyargs, logPath, logPath)
+ if err != nil {
+ return fmt.Errorf("Failed to create subprocess: %s", err)
+ }
+
+ err = p.StartWithFiles(proxyValues.inheritFds)
+ if err != nil {
+ return fmt.Errorf("Failed to run: %s %s: %v", command, strings.Join(forkproxyargs, " "), err)
+ }
for _, file := range proxyValues.inheritFds {
file.Close()
}
+ err = p.Save(pidPath)
if err != nil {
- return fmt.Errorf("Error occurred when starting proxy device: %s", err)
+ // Kill Process if started, but could not save the file
+ err2 := p.Stop()
+ if err != nil {
+ return fmt.Errorf("Could not kill subprocess while handling saving error: %s: %s", err, err2)
+ }
+
+ return fmt.Errorf("Failed to save subprocess details: %s", err)
}
// Poll log file a few times until we see "Started" to indicate successful start.
for i := 0; i < 10; i++ {
started, err := d.checkProcStarted(logPath)
-
if err != nil {
+ p.Stop()
return fmt.Errorf("Error occurred when starting proxy device: %s", err)
}
@@ -238,6 +250,7 @@ func (d *proxy) Start() (*deviceConfig.RunConfig, error) {
time.Sleep(time.Second)
}
+ p.Stop()
return fmt.Errorf("Error occurred when starting proxy device, please look in %s", logPath)
},
}
@@ -514,45 +527,21 @@ func (d *proxy) setupProxyProcInfo() (*proxyProcInfo, error) {
}
func (d *proxy) killProxyProc(pidPath string) error {
- // Get the contents of the pid file
- contents, err := ioutil.ReadFile(pidPath)
- if err != nil {
- return err
- }
- pidString := strings.TrimSpace(string(contents))
-
- // Check if the process still exists
- if !shared.PathExists(fmt.Sprintf("/proc/%s", pidString)) {
- os.Remove(pidPath)
- return nil
- }
-
- // Check if it's forkdns
- cmdArgs, err := ioutil.ReadFile(fmt.Sprintf("/proc/%s/cmdline", pidString))
- if err != nil {
- os.Remove(pidPath)
+ // If the pid file doesn't exist, there is no process to kill.
+ if !shared.PathExists(pidPath) {
return nil
}
- cmdFields := strings.Split(string(bytes.TrimRight(cmdArgs, string("\x00"))), string(byte(0)))
- if len(cmdFields) < 5 || cmdFields[1] != "forkproxy" {
- os.Remove(pidPath)
- return nil
- }
-
- // Parse the pid
- pidInt, err := strconv.Atoi(pidString)
+ p, err := subprocess.ImportProcess(pidPath)
if err != nil {
- return err
+ return fmt.Errorf("Could not read pid file: %s", err)
}
- // Actually kill the process
- err = unix.Kill(pidInt, unix.SIGKILL)
- if err != nil {
- return err
+ err = p.Stop()
+ if err != nil && err != subprocess.ErrNotRunning {
+ return fmt.Errorf("Unable to kill forkproxy: %s", err)
}
- // Cleanup
os.Remove(pidPath)
return nil
}
diff --git a/lxd/main_forkproxy.go b/lxd/main_forkproxy.go
index 196c7c567c..49d810cb55 100644
--- a/lxd/main_forkproxy.go
+++ b/lxd/main_forkproxy.go
@@ -85,13 +85,12 @@ again:
void forkproxy(void)
{
unsigned int needs_mntns = 0;
- int connect_pid, connect_pidfd, listen_pid, listen_pidfd, log_fd;
+ int connect_pid, connect_pidfd, listen_pid, listen_pidfd;
size_t unix_prefix_len = sizeof("unix:") - 1;
ssize_t ret;
pid_t pid;
- char *connect_addr, *cur, *listen_addr, *log_path, *pid_path;
+ char *connect_addr, *cur, *listen_addr;
int sk_fds[2] = {-EBADF, -EBADF};
- FILE *pid_file;
// Get the pid
cur = advance_arg(false);
@@ -106,29 +105,6 @@ void forkproxy(void)
connect_pid = atoi(advance_arg(true));
connect_pidfd = atoi(advance_arg(true));
connect_addr = advance_arg(true);
- log_path = advance_arg(true);
- pid_path = advance_arg(true);
-
- close(STDIN_FILENO);
- log_fd = open(log_path, O_WRONLY | O_CREAT | O_CLOEXEC | O_TRUNC, 0600);
- if (log_fd < 0)
- _exit(EXIT_FAILURE);
-
- ret = dup3(log_fd, STDOUT_FILENO, O_CLOEXEC);
- if (ret < 0)
- _exit(EXIT_FAILURE);
-
- ret = dup3(log_fd, STDERR_FILENO, O_CLOEXEC);
- if (ret < 0)
- _exit(EXIT_FAILURE);
-
- pid_file = fopen(pid_path, "we+");
- if (!pid_file) {
- fprintf(stderr,
- "%s - Failed to create pid file for proxy daemon\n",
- strerror(errno));
- _exit(EXIT_FAILURE);
- }
if (strncmp(listen_addr, "udp:", sizeof("udp:") - 1) == 0 &&
strncmp(connect_addr, "udp:", sizeof("udp:") - 1) != 0) {
@@ -166,7 +142,6 @@ void forkproxy(void)
whoami = FORKPROXY_CHILD;
- fclose(pid_file);
ret = close(sk_fds[0]);
if (ret < 0)
fprintf(stderr, "%s - Failed to close fd %d\n",
@@ -269,41 +244,6 @@ void forkproxy(void)
// can just rely on init doing it's job and reaping the zombie
// process. So, technically unsatisfying but pragmatically
// correct.
-
- // daemonize
- pid = fork();
- if (pid < 0)
- _exit(EXIT_FAILURE);
-
- if (pid != 0) {
- ret = wait_for_pid(pid);
- if (ret < 0)
- _exit(EXIT_FAILURE);
-
- _exit(EXIT_SUCCESS);
- }
-
- pid = fork();
- if (pid < 0)
- _exit(EXIT_FAILURE);
-
- if (pid != 0) {
- ret = fprintf(pid_file, "%d", pid);
- fclose(pid_file);
- if (ret < 0) {
- fprintf(stderr, "Failed to write proxy daemon pid %d to \"%s\"\n",
- pid, pid_path);
- ret = EXIT_FAILURE;
- }
- close(STDOUT_FILENO);
- close(STDERR_FILENO);
- _exit(EXIT_SUCCESS);
- }
-
- ret = setsid();
- if (ret < 0)
- fprintf(stderr, "%s - Failed to setsid in proxy daemon\n",
- strerror(errno));
}
}
*/
@@ -338,7 +278,7 @@ func (c *cmdForkproxy) Command() *cobra.Command {
container, connecting one side to the host and the other to the
container.
`
- cmd.Args = cobra.ExactArgs(14)
+ cmd.Args = cobra.ExactArgs(12)
cmd.RunE = c.Run
cmd.Hidden = true
@@ -459,7 +399,7 @@ func (c *cmdForkproxy) Run(cmd *cobra.Command, args []string) error {
}
// Sanity checks
- if len(args) != 14 {
+ if len(args) != 12 {
cmd.Help()
if len(args) == 0 {
@@ -529,16 +469,16 @@ func (c *cmdForkproxy) Run(cmd *cobra.Command, args []string) error {
var err error
listenAddrGID := -1
- if args[8] != "" {
- listenAddrGID, err = strconv.Atoi(args[8])
+ if args[6] != "" {
+ listenAddrGID, err = strconv.Atoi(args[6])
if err != nil {
return err
}
}
listenAddrUID := -1
- if args[9] != "" {
- listenAddrUID, err = strconv.Atoi(args[9])
+ if args[7] != "" {
+ listenAddrUID, err = strconv.Atoi(args[7])
if err != nil {
return err
}
@@ -552,8 +492,8 @@ func (c *cmdForkproxy) Run(cmd *cobra.Command, args []string) error {
}
var listenAddrMode os.FileMode
- if args[10] != "" {
- tmp, err := strconv.ParseUint(args[10], 8, 0)
+ if args[8] != "" {
+ tmp, err := strconv.ParseUint(args[8], 8, 0)
if err != nil {
return err
}
@@ -621,16 +561,16 @@ func (c *cmdForkproxy) Run(cmd *cobra.Command, args []string) error {
// Drop privilege if requested
gid := uint64(0)
- if args[11] != "" {
- gid, err = strconv.ParseUint(args[11], 10, 32)
+ if args[9] != "" {
+ gid, err = strconv.ParseUint(args[9], 10, 32)
if err != nil {
return err
}
}
uid := uint64(0)
- if args[12] != "" {
- uid, err = strconv.ParseUint(args[12], 10, 32)
+ if args[10] != "" {
+ uid, err = strconv.ParseUint(args[10], 10, 32)
if err != nil {
return err
}
@@ -710,7 +650,7 @@ func (c *cmdForkproxy) Run(cmd *cobra.Command, args []string) error {
continue
}
- err := listenerInstance(epFd, lAddr, cAddr, curFd, srcConn, args[13] == "true")
+ err := listenerInstance(epFd, lAddr, cAddr, curFd, srcConn, args[11] == "true")
if err != nil {
fmt.Printf("Warning: Failed to prepare new listener instance: %s\n", err)
}
More information about the lxc-devel
mailing list