veyron/lib/{modules,exec}: tidy up env var handling.
Tidy up the handling of environment variables in particular to
make working with and implementing the node manager and associated
tests easier.
The exec package now unsets the VEYRON_EXEC_VERSION env var when
it no longer needs to avoid having it be accidentally passed on
to other process that are are not necessarily started by this
package.
The modules shell type now has a 'CommandEnvelope' method that can
be used by the node manager when constructing envelopes for use
with the app manager.
Change-Id: I86a896557bdf6b87f39523fb7399453cf7d2a0fe
diff --git a/lib/exec/child.go b/lib/exec/child.go
index 8beb45a..105d0d5 100644
--- a/lib/exec/child.go
+++ b/lib/exec/child.go
@@ -9,8 +9,8 @@
)
var (
- ErrNoVersion = errors.New(versionVariable + " environment variable missing")
- ErrUnsupportedVersion = errors.New("Unsupported version of veyron/lib/exec request by " + versionVariable + " environment variable")
+ ErrNoVersion = errors.New(VersionVariable + " environment variable missing")
+ ErrUnsupportedVersion = errors.New("Unsupported version of veyron/lib/exec request by " + VersionVariable + " environment variable")
)
type ChildHandle struct {
@@ -77,10 +77,11 @@
}
func createChildHandle() (*ChildHandle, error) {
- switch os.Getenv(versionVariable) {
+ switch os.Getenv(VersionVariable) {
case "":
return nil, ErrNoVersion
case version1:
+ os.Setenv(VersionVariable, "")
// TODO(cnicolaou): need to use major.minor.build format for
// version #s.
default:
diff --git a/lib/exec/exec_test.go b/lib/exec/exec_test.go
index 1e7662e..cc1b292 100644
--- a/lib/exec/exec_test.go
+++ b/lib/exec/exec_test.go
@@ -394,6 +394,13 @@
clean(t, ph)
}
+func verifyNoExecVariable() {
+ version := os.Getenv(vexec.VersionVariable)
+ if len(version) != 0 {
+ log.Fatal(os.Stderr, "Version variable %q has a value: %s", vexec.VersionVariable, version)
+ }
+}
+
// TestHelperProcess isn't a real test; it's used as a helper process
// for the other tests.
func TestHelperProcess(*testing.T) {
@@ -404,6 +411,11 @@
}
defer os.Exit(0)
+ version := os.Getenv(vexec.VersionVariable)
+ if len(version) == 0 {
+ log.Fatal(os.Stderr, "Version variable %q has no value", vexec.VersionVariable)
+ }
+
// Write errors to stderr or using log. since the parent
// process is reading stderr.
args := os.Args
@@ -427,12 +439,14 @@
if err != nil {
log.Fatal(os.Stderr, "%s\n", err)
}
+ verifyNoExecVariable()
fmt.Fprintf(os.Stderr, "never ready")
case "testTooSlowToReady":
ch, err := vexec.GetChildHandle()
if err != nil {
log.Fatal(os.Stderr, "%s\n", err)
}
+ verifyNoExecVariable()
// Wait for the parent to tell us when it's ok to proceed.
controlPipe := ch.NewExtraFile(0, "control_rd")
for {
@@ -458,6 +472,7 @@
if err != nil {
log.Fatal(os.Stderr, "%s", err)
}
+ verifyNoExecVariable()
if err := ch.SetFailed(fmt.Errorf("%s", strings.Join(args, " "))); err != nil {
fmt.Fprintf(os.Stderr, "%s\n", err)
@@ -467,6 +482,7 @@
if err != nil {
log.Fatal(os.Stderr, "%s", err)
}
+ verifyNoExecVariable()
ch.SetReady()
fmt.Fprintf(os.Stderr, ".")
case "testReadySlow":
@@ -474,6 +490,7 @@
if err != nil {
log.Fatal(os.Stderr, "%s", err)
}
+ verifyNoExecVariable()
// Wait for the parent to tell us when it's ok to proceed.
controlPipe := ch.NewExtraFile(0, "control_rd")
for {
@@ -493,6 +510,7 @@
if err != nil {
log.Fatal(os.Stderr, "%s\n", err)
}
+ verifyNoExecVariable()
ch.SetReady()
rc := make(chan int)
go func() {
@@ -511,6 +529,7 @@
if err != nil {
log.Fatalf("%v", err)
} else {
+ verifyNoExecVariable()
serialized, err := ch.Config.Serialize()
if err != nil {
log.Fatalf("%v", err)
@@ -522,6 +541,7 @@
if err != nil {
log.Fatalf("%s", err)
} else {
+ verifyNoExecVariable()
fmt.Fprintf(os.Stderr, "%s", ch.Secret)
}
case "testExtraFiles":
@@ -529,6 +549,7 @@
if err != nil {
log.Fatal("error.... %s\n", err)
}
+ verifyNoExecVariable()
err = ch.SetReady()
rd := ch.NewExtraFile(0, "read")
buf := make([]byte, 1024)
diff --git a/lib/exec/parent.go b/lib/exec/parent.go
index bfcba80..a3aebec 100644
--- a/lib/exec/parent.go
+++ b/lib/exec/parent.go
@@ -66,7 +66,7 @@
// NewParentHandle creates a ParentHandle for the child process represented by
// an instance of exec.Cmd.
func NewParentHandle(c *exec.Cmd, opts ...ParentHandleOpt) *ParentHandle {
- c.Env = append(c.Env, versionVariable+"="+version1)
+
cfg, secret := NewConfig(), ""
tk := timekeeper.RealTime()
for _, opt := range opts {
@@ -92,6 +92,7 @@
// Start starts the child process, sharing a secret with it and
// setting up a communication channel over which to read its status.
func (p *ParentHandle) Start() error {
+ p.c.Env = append(p.c.Env, VersionVariable+"="+version1)
// Create anonymous pipe for communicating data between the child
// and the parent.
dataRead, dataWrite, err := os.Pipe()
diff --git a/lib/exec/shared.go b/lib/exec/shared.go
index 08c54ad..5b4058f 100644
--- a/lib/exec/shared.go
+++ b/lib/exec/shared.go
@@ -1,9 +1,18 @@
package exec
const (
- version1 = "1.0.0"
- readyStatus = "ready::"
- failedStatus = "failed::"
- initStatus = "init"
- versionVariable = "VEYRON_EXEC_VERSION"
+ version1 = "1.0.0"
+ readyStatus = "ready::"
+ failedStatus = "failed::"
+ initStatus = "init"
+
+ // The exec package uses this environment variable to communicate
+ // the version of the protocol being used between the parent and child.
+ // It takes care to clear this variable from the child process'
+ // environment as soon as it can, however, there may still be some
+ // situations where an application may need to test for its presence
+ // or ensure that it doesn't appear in a set of environment variables;
+ // exposing the name of this variable is intended to support such
+ // situations.
+ VersionVariable = "VEYRON_EXEC_VERSION"
)
