Append custom env vars instead of replace in Process (#171)

Append custom env vars instead of replace in Process (#168, #169)

PR #162 refactored the default configuration code. This
introduced a subtle bug where `env` became `[]string{}` instead of the
default of `nil`.

In golang, `exec.Cmd.Env == nil` means to use the "current process's
environment". By setting it to `[]string{}` as a default the Process's
environment was emptied out which caused an array of strange and
difficult to troubleshoot behaviour. See issues #168 and #169

This commit changes the behaviour to append model configured environment
variables to the default list rather than replace them.

.goreleaser.yaml

@@ -27,4 +27,4 @@ archives:
       owner: root
     format_overrides:
       - goos: windows
-        format: zip
+        formats: ['zip']

Makefile

@@ -29,9 +29,13 @@ test: proxy/ui_dist/placeholder.txt
 test-all: proxy/ui_dist/placeholder.txt
 	go test -v -count=1 ./proxy
 
+ui/node_modules:
+	cd ui && npm install
+
 # build react UI
-ui:
+ui: ui/node_modules
 	cd ui && npm run build
+
 # Build OSX binary
 mac: ui
 	@echo "Building Mac binary..."

README.md

@@ -40,36 +40,32 @@ In the most basic configuration llama-swap handles one model at a time. For more
 
 ## config.yaml
 
-llama-swap's configuration is purposefully simple:
+llama-swap is managed entirely through a yaml configuration file. 
+
+It can be very minimal to start: 
 
 ```yaml
 models:
   "qwen2.5":
     cmd: |
-      /app/llama-server
+      /path/to/llama-server
       -hf bartowski/Qwen2.5-0.5B-Instruct-GGUF:Q4_K_M
       --port ${PORT}
-
-  "smollm2":
-    cmd: |
-      /app/llama-server
-      -hf bartowski/SmolLM2-135M-Instruct-GGUF:Q4_K_M
-      --port ${PORT}
 ```
 
-.. but also supports many advanced features:
+However, there are many more capabilities that llama-swap supports: 
 
 - `groups` to run multiple models at once
-- `macros` for reusable snippets
 - `ttl` to automatically unload models
+- `macros` for reusable snippets
 - `aliases` to use familiar model names (e.g., "gpt-4o-mini")
-- `env` variables to pass custom environment to inference servers
+- `env` to pass custom environment variables to inference servers
+- `cmdStop` for to gracefully stop Docker/Podman containers
 - `useModelName` to override model names sent to upstream servers
 - `healthCheckTimeout` to control model startup wait times
 - `${PORT}` automatic port variables for dynamic port assignment
-- `cmdStop` for to gracefully stop Docker/Podman containers
 
-Check the [configuration documentation](https://github.com/mostlygeek/llama-swap/wiki/Configuration) in the wiki for all options.
+See the [configuration documentation](https://github.com/mostlygeek/llama-swap/wiki/Configuration) in the wiki all options and examples.
 
 ## Docker Install ([download images](https://github.com/mostlygeek/llama-swap/pkgs/container/llama-swap))
 
@@ -120,11 +116,11 @@ $ docker run -it --rm --runtime nvidia -p 9292:8080 \
 
 ## Bare metal Install ([download](https://github.com/mostlygeek/llama-swap/releases))
 
-Pre-built binaries are available for Linux, FreeBSD and Darwin (OSX). These are automatically published and are likely a few hours ahead of the docker releases. The baremetal install works with any OpenAI compatible server, not just llama-server.
+Pre-built binaries are available for Linux, Mac, Windows and FreeBSD. These are automatically published and are likely a few hours ahead of the docker releases. The baremetal install works with any OpenAI compatible server, not just llama-server.
 
-1. Create a configuration file, see [config.example.yaml](config.example.yaml)
 1. Download a [release](https://github.com/mostlygeek/llama-swap/releases) appropriate for your OS and architecture.
-1. Run the binary with `llama-swap --config path/to/config.yaml`.
+1. Create a configuration file, see the [configuration documentation](https://github.com/mostlygeek/llama-swap/wiki/Configuration).
+1. Run the binary with `llama-swap --config path/to/config.yaml --listen localhost:8080`.
    Available flags:
    - `--config`: Path to the configuration file (default: `config.yaml`).
    - `--listen`: Address and port to listen on (default: `:8080`).
@@ -133,16 +129,16 @@ Pre-built binaries are available for Linux, FreeBSD and Darwin (OSX). These are
 
 ### Building from source
 
-1. Install golang for your system
+1. Build requires golang and nodejs for the user interface.
 1. `git clone git@github.com:mostlygeek/llama-swap.git`
 1. `make clean all`
 1. Binaries will be in `build/` subdirectory
 
 ## Monitoring Logs
 
-Open the `http://<host>/logs` with your browser to get a web interface with streaming logs.
+Open the `http://<host>:<port>/` with your browser to get a web interface with streaming logs.
 
-Of course, CLI access is also supported:
+CLI access is also supported:
 
 ```shell
 # sends up to the last 10KB of logs

proxy/process.go

@@ -189,17 +189,19 @@ func (p *Process) start() error {
 	p.waitStarting.Add(1)
 	defer p.waitStarting.Done()
 	cmdContext, ctxCancelUpstream := context.WithCancel(context.Background())
+
 	p.cmd = exec.CommandContext(cmdContext, args[0], args[1:]...)
 	p.cmd.Stdout = p.processLogger
 	p.cmd.Stderr = p.processLogger
-	p.cmd.Env = p.config.Env
-
+	p.cmd.Env = append(p.cmd.Environ(), p.config.Env...)
 	p.cmd.Cancel = p.cmdStopUpstreamProcess
 	p.cmd.WaitDelay = p.gracefulStopTimeout
 	p.cancelUpstream = ctxCancelUpstream
 	p.cmdWaitChan = make(chan struct{})
 
 	p.failedStartCount++ // this will be reset to zero when the process has successfully started
+
+	p.proxyLogger.Debugf("<%s> Executing start command: %s, env: %s", p.ID, strings.Join(args, " "), strings.Join(p.config.Env, ", "))
 	err = p.cmd.Start()
 
 	// Set process state to failed

proxy/process_test.go

@@ -394,6 +394,9 @@ func TestProcess_StopImmediately(t *testing.T) {
 // Test that SIGKILL is sent when gracefulStopTimeout is reached and properly terminates
 // the upstream command
 func TestProcess_ForceStopWithKill(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("skipping SIGTERM test on Windows ")
+	}
 
 	expectedMessage := "test_sigkill"
 	binaryPath := getSimpleResponderPath()
@@ -405,7 +408,6 @@ func TestProcess_ForceStopWithKill(t *testing.T) {
 		Cmd:           fmt.Sprintf("%s --port %d --respond %s --silent --ignore-sig-term", binaryPath, port, expectedMessage),
 		Proxy:         fmt.Sprintf("http://127.0.0.1:%d", port),
 		CheckEndpoint: "/health",
-		CmdStop:       "taskkill /f /t /pid ${PID}",
 	}
 
 	process := NewProcess("stop_immediate", 2, config, debugLogger, debugLogger)
@@ -465,3 +467,27 @@ func TestProcess_StopCmd(t *testing.T) {
 	process.StopImmediately()
 	assert.Equal(t, process.CurrentState(), StateStopped)
 }
+
+func TestProcess_EnvironmentSetCorrectly(t *testing.T) {
+	expectedMessage := "test_env_not_emptied"
+	config := getTestSimpleResponderConfig(expectedMessage)
+
+	// ensure that the the default config does not blank out the inherited environment
+	configWEnv := config
+
+	// ensure the additiona variables are appended to the process' environment
+	configWEnv.Env = append(configWEnv.Env, "TEST_ENV1=1", "TEST_ENV2=2")
+
+	process1 := NewProcess("env_test", 2, config, debugLogger, debugLogger)
+	process2 := NewProcess("env_test", 2, configWEnv, debugLogger, debugLogger)
+
+	process1.start()
+	defer process1.Stop()
+	process2.start()
+	defer process2.Stop()
+
+	assert.NotZero(t, len(process1.cmd.Environ()))
+	assert.NotZero(t, len(process2.cmd.Environ()))
+	assert.Equal(t, len(process1.cmd.Environ())+2, len(process2.cmd.Environ()), "process2 should have 2 more environment variables than process1")
+
+}