Do not pass parent pid to child daemon process (#2376)

## Changes
This PR:
1. No longer sets the `DATABRICKS_CLI_PARENT_PID` environment variable
since it was never required in the first place and was mistakenly merged
in the inital PR.
2. Performs minor cleanup based on post merge feedback in
https://github.com/databricks/cli/pull/2354.

## Tests
N/A
This commit is contained in:
shreyas-goenka 2025-02-26 17:39:45 +05:30 committed by GitHub
parent fa79d04980
commit 9659f91c9f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 11 additions and 18 deletions

3
go.mod
View File

@ -31,14 +31,13 @@ require (
golang.org/x/mod v0.23.0 golang.org/x/mod v0.23.0
golang.org/x/oauth2 v0.26.0 golang.org/x/oauth2 v0.26.0
golang.org/x/sync v0.11.0 golang.org/x/sync v0.11.0
golang.org/x/sys v0.30.0
golang.org/x/term v0.29.0 golang.org/x/term v0.29.0
golang.org/x/text v0.22.0 golang.org/x/text v0.22.0
gopkg.in/ini.v1 v1.67.0 // Apache 2.0 gopkg.in/ini.v1 v1.67.0 // Apache 2.0
gopkg.in/yaml.v3 v3.0.1 gopkg.in/yaml.v3 v3.0.1
) )
require golang.org/x/sys v0.30.0
require ( require (
cloud.google.com/go/auth v0.4.2 // indirect cloud.google.com/go/auth v0.4.2 // indirect
cloud.google.com/go/auth/oauth2adapt v0.2.2 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.2 // indirect

View File

@ -8,8 +8,6 @@ import (
"strconv" "strconv"
) )
const DatabricksCliParentPid = "DATABRICKS_CLI_PARENT_PID"
type Daemon struct { type Daemon struct {
// If provided, the child process's pid will be written in the file at this // If provided, the child process's pid will be written in the file at this
// path. // path.
@ -33,22 +31,17 @@ type Daemon struct {
} }
func (d *Daemon) Start() error { func (d *Daemon) Start() error {
cli, err := os.Executable() var err error
if err != nil {
return err
}
executable := d.Executable executable := d.Executable
if executable == "" { if executable == "" {
executable = cli // If Executable is not provided, use the current CLI executable.
executable, err = os.Executable()
if err != nil {
return err
}
} }
d.cmd = exec.Command(executable, d.Args...) d.cmd = exec.Command(executable, d.Args...)
// Set environment variable so that the child process knows its parent's PID.
// In unix systems orphaned processes are automatically re-parented to init (pid 1)
// so we cannot rely on os.Getppid() to get the original parent's pid.
d.Env = append(d.Env, fmt.Sprintf("%s=%d", DatabricksCliParentPid, os.Getpid()))
d.cmd.Env = d.Env d.cmd.Env = d.Env
d.cmd.SysProcAttr = sysProcAttr() d.cmd.SysProcAttr = sysProcAttr()
@ -64,6 +57,7 @@ func (d *Daemon) Start() error {
return fmt.Errorf("failed to open log file: %w", err) return fmt.Errorf("failed to open log file: %w", err)
} }
// The file descriptor for the log file is closed in the [Daemon.Release] method.
d.cmd.Stdout = d.logFile d.cmd.Stdout = d.logFile
d.cmd.Stderr = d.logFile d.cmd.Stderr = d.logFile
} }
@ -101,7 +95,7 @@ func (d *Daemon) Release() error {
} }
} }
// Note that the child process will stream it's output directly to the log file. // Note that the child process will stream its output directly to the log file.
// So it's safe to close this file handle even if the child process is still running. // So it's safe to close this file handle even if the child process is still running.
if d.logFile != nil { if d.logFile != nil {
err := d.logFile.Close() err := d.logFile.Close()
@ -114,7 +108,7 @@ func (d *Daemon) Release() error {
return nil return nil
} }
// The docs for [os.Process.Release] recommend calling Release if Wait is not called. // The docs for [os.Process.Release] specify that we need to call Release if
// It's probably not necessary but we call it just to be safe. // Wait is not called.
return d.cmd.Process.Release() return d.cmd.Process.Release()
} }