diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b1bd4f..22f3f9c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,83 @@ jobs: with: version: v2.12.2 + # sc.exe start returns while the service is Start Pending; run site at once + # with no readiness wait, so openService's own wait is what runs. The + # sequence is in docs/architecture/provisioning.md#the-service-restart. + - name: Restart the runner's PostgreSQL service through site + env: + SEMIBASE_SUPER_PASSWORD: root + SEMIBASE_WRITER_PASSWORD: ci-writer + SEMIBASE_PLOT_PASSWORD: ci-plot + PGPASSWORD: root + run: | + $service = 'postgresql-x64-17' + $psql = "$env:PGBIN\psql.exe" + Get-CimInstance Win32_Service -Filter "Name='$service'" | + Select-Object Name, State, StartMode, PathName | Format-List + go build -o semibase.exe ./cmd/semibase + if ($LASTEXITCODE -ne 0) { exit 1 } + + # stdout and stderr joined into one string: -like over an array filters + # elements instead of answering yes or no + function Invoke-Site([string] $name) { + $ErrorActionPreference = 'Continue' + $lines = & .\semibase.exe site --port 5432 --expected-major 17 --restart-service $name 2>&1 | + ForEach-Object { "$_" } + $lines | Write-Host + [pscustomobject]@{ Code = $LASTEXITCODE; Output = $lines -join "`n" } + } + function Assert-Refused($run, [string] $reason) { + if ($run.Code -ne 1 -or $run.Output -notlike "*$reason*" -or $run.Output -like '*ALTER SYSTEM SET*') { + Write-Host "::error::want exit 1 with '$reason' and no ALTER SYSTEM SET, got exit $($run.Code)" + exit 1 + } + } + + Assert-Refused (Invoke-Site $service) "service $service is Disabled" + + Set-Service $service -StartupType Manual + sc.exe start $service + if ($LASTEXITCODE -ne 0) { exit 1 } + Write-Host "state after sc.exe start: $((Get-Service $service).Status)" + + $first = Invoke-Site $service + if ($first.Code -ne 0) { Write-Host "::error::the first site run exited $($first.Code)"; exit 1 } + if ($first.Output -notlike "*restarting service $service*") { + Write-Host "::error::the first site run did not restart $service" + exit 1 + } + if ($first.Output -notlike "*service $service runs data directory C:\PostgreSQL\17\data, as the server on *:5432 does*") { + Write-Host "::error::the first site run did not tie $service to the server on port 5432" + exit 1 + } + if ($first.Output -notlike "*service $service stopped, nothing answers on *:5432*") { + Write-Host "::error::the first site run did not see the old server's port refused before the Start" + exit 1 + } + + Write-Host "data_directory: $(& $psql -h 127.0.0.1 -U postgres -d postgres -tAc 'SHOW data_directory')" + $listen = & $psql -h 127.0.0.1 -U postgres -d postgres -tAc 'SHOW listen_addresses' + if ($LASTEXITCODE -ne 0) { exit 1 } + $pending = & $psql -h 127.0.0.1 -U postgres -d postgres -tAc ` + 'SELECT count(*) FROM pg_settings WHERE pending_restart' + if ($LASTEXITCODE -ne 0) { exit 1 } + if ("$listen".Trim() -ne 'localhost' -or "$pending".Trim() -ne '0') { + Write-Host "::error::listen_addresses '$listen' (want localhost), pending_restart count '$pending' (want 0)" + exit 1 + } + + $second = Invoke-Site $service + if ($second.Code -ne 0) { Write-Host "::error::the second site run exited $($second.Code)"; exit 1 } + if ($second.Output -like '*restarting service*') { + Write-Host "::error::the second site run restarted $service with nothing pending" + exit 1 + } + + Assert-Refused (Invoke-Site 'no-such-service') 'service no-such-service does not exist' + # the runner exits with the last native exit code, which the refusal left at 1 + exit 0 + linux: runs-on: ubuntu-latest env: @@ -70,6 +147,25 @@ jobs: ./semibase site --database semiplot_dev --expected-major 17 ./semibase site --database semiplot_dev --expected-major 17 + # The service container is never restarted, so the loopback-only value + # stays pending. + - name: Require the listen_addresses delta + env: + PGHOST: localhost + PGUSER: postgres + PGPASSWORD: ci-super + run: | + set -euo pipefail + pending=$(psql -d postgres -tAc \ + "SELECT pending_restart FROM pg_settings WHERE name = 'listen_addresses'") + written=$(psql -d postgres -tAc \ + "SELECT setting FROM pg_file_settings WHERE name = 'listen_addresses' + AND sourcefile LIKE '%postgresql.auto.conf'") + if [[ "$pending" != t || "$written" != localhost ]]; then + echo "::error::listen_addresses pending_restart '$pending' (want t), auto.conf value '$written' (want localhost)" + exit 1 + fi + # Every other step runs semiplot_register_new_pens() over an empty # public.trends, so its INSERT adds nothing. - name: Register pens and probe the semiplot_tags constraints diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7e329d7..605538e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -121,7 +121,7 @@ jobs: echo "$DELIMITER" } >> "$GITHUB_OUTPUT" - # The module is pure pgx with no cgo, so one runner cross-compiles both + # The module is pure Go with no cgo, so one runner cross-compiles both # targets. Native-platform coverage is CI's job, not the release's. - name: Build env: diff --git a/CLAUDE.md b/CLAUDE.md index 568ddb3..e89ecf8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,9 +7,10 @@ CLI binary, `semibase.exe`, a container image carrying its Linux build, plus the architecture docs. Deployment target: Windows. Language: Go (module `github.com/Semiteq/SemiBase`), driver `pgx/v5`, the five `sql/semiplot_*.sql` files and `sql/trends.sql` embedded via `go:embed`. Entry point: -`cmd/semibase`. The module is pure `pgx` and compiles for any GOOS; Linux builds serve the -containerised test bench of consumers (SemiPlot runs `bench` against an ephemeral `postgres:17` -container). All commands run from the repository root. +`cmd/semibase`. The module is pure Go (`pgx/v5`, plus `golang.org/x/sys` for the Windows service +restart) and compiles for any GOOS; Linux builds serve the containerised test bench of consumers +(SemiPlot runs `bench` against an ephemeral `postgres:17` container). All commands run from the +repository root. ## Build @@ -24,7 +25,7 @@ Without ldflags, `version` falls back to the VCS revision recorded by the Go too `.github/workflows/release.yml` fires on a `v*` tag push. One `ubuntu-latest` job cross-compiles `windows/amd64` and `linux/amd64` with `CGO_ENABLED=0` — the module is pure -`pgx`, so native runners are CI's job, not the release's — and attaches +Go, so native runners are CI's job, not the release's — and attaches `semibase___amd64[.exe]` to the GitHub release — no extension on the Linux artifact, an ELF is executable by its mode bit. `-X main.revision=` is embedded, so `semibase version` prints the tag. @@ -79,10 +80,21 @@ golangci-lint run golangci-lint is pinned to 2.12.2 (`winget install GolangCI.golangci-lint --version 2.12.2`); config in `.golangci.yml`. CI (`.github/workflows/ci.yml`) runs `go build`, `go test -race`, -and the same lint on `windows-latest` and `ubuntu-latest` for every push and pull request; -the Linux job also provisions a `postgres:17-alpine` service container by running `site` -twice — the tuning path and the idempotency check in one step. -It then runs `bench` against a `postgres:14-alpine` container — the version floor, and the only +and the same lint on `windows-latest` and `ubuntu-latest` for every push and pull request. +The Windows job then runs the `Restart the runner's PostgreSQL service through site` step, the +only test of the service restart. It requires `site --restart-service postgresql-x64-17` to +refuse the runner's shipped Disabled service with no `ALTER SYSTEM SET` line, sets it to Manual +and starts it with `sc.exe start`, which returns while it is Start Pending. It then requires the +next run, started at once, to print `restarting service postgresql-x64-17`, the line tying the +service's `-D` to the server on port 5432, and `stopped, nothing answers on :5432`, and leave +`listen_addresses = localhost` with no setting pending, a second run not to restart the service, +and `--restart-service no-such-service` to be refused with no `ALTER SYSTEM SET` line. +The Linux job provisions a `postgres:17-alpine` service container by running `site` twice — the +tuning path and the idempotency check in one step. The `Require the listen_addresses delta` step +then requires `listen_addresses` still pending and `localhost` in `postgresql.auto.conf` +(`pg_file_settings`): the container is never restarted, so the pending value and the file line +prove `site` wrote the delta. +The job then runs `bench` against a `postgres:14-alpine` container — the version floor, and the only place `REVOKE CREATE ON SCHEMA public FROM PUBLIC` bites, because 15 and later withhold that grant by engine default — provisions a third container with `bench` as a `postgres` image init script over the unix socket, and builds the container image, so a broken `Dockerfile` fails on the pull @@ -99,12 +111,13 @@ password. The 14 step starts its server with all five statement-logging paramete `Keep the CREATE ROLE password statements out of the PostgreSQL 14 log` step reads that log. Both log steps run `.github/scripts/check-server-log.sh`, which requires `logging_collector = off`, so `docker logs` is the whole server log, requires the `semiplot` login's probe statement in it, and -requires the passwords absent. The pens and the 17 log steps need `psql` on the runner; to run -them locally, point `psql` at a container. +requires the passwords absent. The `listen_addresses`, the pens and the 17 log steps need `psql` +on the runner; to run them locally, point `psql` at a container. The Windows step uses the +runner's own `$env:PGBIN\psql.exe`. Unit tests live beside the source (`cmd/semibase/*_test.go`, `internal/provision/*_test.go`), table-driven. There are no database-touching tests in the repository; the integration checks are -the `semiplot` access check both commands run at the tail and the six CI database steps above, +the `semiplot` access check both commands run at the tail and the eight CI database steps above, against a live server. ## Format @@ -118,11 +131,12 @@ gofmt -w . # run before presenting changes; gofmt is authoritative ```powershell .\semibase.exe --help # commands: site | bench | version .\semibase.exe site # an installation machine: tuning, then everything else +.\semibase.exe site --restart-service postgresql-x64-17 # also restart the service when settings wait .\semibase.exe bench --port 15432 --database semiplot_dev --expected-major 17 .\semibase.exe version # print the build revision ``` -`site` and `bench` differ in one thing: `site` applies the `ALTER SYSTEM` memory constants, +`site` and `bench` differ in one thing: `site` applies the `ALTER SYSTEM` server settings, `bench` does not. Both create the database, the roles, the grants, the five SemiPlot tables (`semiplot_tags`, `semiplot_groups`, `semiplot_pen_groups`, `semiplot_markers`, `semiplot_meta`), the `semiplot_register_new_pens()` function and `public.trends`, and both end by actually reading @@ -135,6 +149,18 @@ in a rolled-back transaction, requiring 42501 for `INSERT`, `DELETE` and `UPDATE on the archive or on `semiplot_meta` and no `CREATE` on schema `public` (revoked from `PUBLIC` by `create`, since 14 still grants it). A failed check is a non-zero exit. +Two of the settings, `shared_buffers` and `listen_addresses = 'localhost'`, wait for a service +restart. Without a flag the run warns and the operator restarts. `site --restart-service ` +restarts that Windows service inside the tuning phase when settings wait, so `create` and the +tail check run against the restarted server. Only `site` registers the flag, and it refuses a +`--host` other than `localhost`, `127.0.0.1` or `::1`. The service is checked before anything +changes, including that the `-D` in its `PathName` is the `data_directory` of the server the run +connects to. The Start waits until the IP:port that connection reached refuses connections, since +`pg_ctl` reports Stopped before a slow postmaster exits, and once the service accepts the Stop the +restart runs to completion despite an interrupt +(`docs/architecture/provisioning.md#the-service-restart`, which also lists the cases that still +leave it stopped). + The `semiplot` grant on `semiplot_tags` is column-level: `SELECT` and `UPDATE` on the nine settings columns, never `id`, so `has_table_privilege(..., 'UPDATE')` answers false for that table and no check may ask it. @@ -168,9 +194,11 @@ superuser connection, so the table's owner is the role the SCADA writes with and ``` cmd/semibase/ CLI entry point: command dispatch, flags, usage text -internal/provision/ one file per phase behind the two commands: config.go (ALTER SYSTEM), - create.go (db/roles/grants/trends), schema.go (the semiplot schema files, - their grants, the pen-registration function's grant and its owner's +internal/provision/ one file per phase behind the two commands: config.go (ALTER SYSTEM and + the optional service restart, whose service control lives in + service_windows.go and service_other.go), create.go + (db/roles/grants/trends), schema.go (the semiplot schema files, their + grants, the pen-registration function's grant and its owner's registrarStatements), check.go (semiplot access). provision.go holds Options, Site/Bench and the connection plumbing they share sql/ embedded DDL: the five semiplot_* files (ours: the tables, the diff --git a/Dockerfile b/Dockerfile index 5931539..7a4a98a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ # The image carries the linux/amd64 binary and nothing else. Its only consumer # copies the binary out (COPY --from=ghcr.io/semiteq/semibase:latest /semibase), # so a base with a shell, CA certificates or a libc would ship weight nobody -# runs: the module is pure pgx, builds CGO_ENABLED=0 and speaks no TLS on the +# runs: the module is pure Go, builds CGO_ENABLED=0 and speaks no TLS on the # bench. The ENTRYPOINT costs one line and makes `docker run version` # a valid smoke check. # diff --git a/README.md b/README.md index 59bde3c..2b6a7d7 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ SemiBase — разворачиваемый PostgreSQL-сервис для ус # установка pgsql winget install --id PostgreSQL.PostgreSQL.17 --exact +# дальше всё в PowerShell от имени администратора: утилита перезапускает службу PostgreSQL # пароли ролей: каждая строка запрашивает пароль, в историю PowerShell он не попадает $env:SEMIBASE_SUPER_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Пароль postgres' -AsSecureString)).Password $env:SEMIBASE_WRITER_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Пароль scada_writer' -AsSecureString)).Password @@ -36,6 +37,6 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па # конфигурация сервера, архивная база, роли `scada_writer`, `semiplot` и # `semiplot_registrar`, права, таблица архива, пять таблиц SemiPlot и функция -# регистрации новых перьев -.\semibase.exe site +# регистрации новых перьев; перезапуск службы, если настройки его ждут +.\semibase.exe site --restart-service postgresql-x64-17 ``` diff --git a/cmd/semibase/main.go b/cmd/semibase/main.go index f5b3bb2..12c0cef 100644 --- a/cmd/semibase/main.go +++ b/cmd/semibase/main.go @@ -20,7 +20,7 @@ Usage: semibase [flags] Commands: - site an installation machine: apply the memory tuning (ALTER SYSTEM + reload), + site an installation machine: apply the server settings (ALTER SYSTEM + reload), then create the archive database, the roles, the access chain, public.trends, the five semiplot_* tables and semiplot_register_new_pens() bench a throwaway container: the same, without the tuning @@ -33,6 +33,11 @@ refused an insert, a delete or a key change on semiplot_tags; it writes neither archive nor semiplot_meta and holds no CREATE on schema public. A failed check is a non-zero exit. +The tuning includes settings that take effect only after a server restart. site +reports them and leaves the restart to the operator, unless --restart-service names +the Windows service to restart; the restart needs a loopback --host, because the +server then listens on the loopback interface only. + Every step checks before it creates; re-running either command is safe. Passwords of existing roles change only when the corresponding flag or variable is set. @@ -108,6 +113,10 @@ func run(ctx context.Context, arguments []string) int { defer cancel() if err := execute(commandContext, options); err != nil { fmt.Fprintln(errorOutput, "error:", err) + if errors.Is(commandContext.Err(), context.DeadlineExceeded) { + fmt.Fprintf(errorOutput, "%s ran out of its %s limit; every step checks before it creates, "+ + "so re-running it continues from where it stopped\n", command, phaseTimeout) + } return 1 } fmt.Fprintf(output, "\nDone: %s completed against %s.\n", @@ -146,10 +155,8 @@ func revisionFromSettings(settings []debug.BuildSetting) string { return vcsRevision } -// One flag set serves both commands: site and bench differ only in the tuning phase, -// which reads no flag of its own, so every flag below is used by either command. -// Password flags default to empty so the usage output can only ever show the variable -// names, never their values. +// Password flags default to empty so the usage output can only ever show the variable names, +// never their values. func newFlagSet(command string, options *provision.Options) *flag.FlagSet { flags := flag.NewFlagSet(command, flag.ContinueOnError) flags.SetOutput(errorOutput) @@ -166,6 +173,11 @@ func newFlagSet(command string, options *provision.Options) *flag.FlagSet { "semiplot password (env SEMIBASE_PLOT_PASSWORD)") flags.IntVar(&options.ExpectedMajor, "expected-major", 0, "required server major version; 0 accepts any major >= 14") + if command == "site" { + flags.StringVar(&options.RestartService, "restart-service", "", + "Windows service to restart when the server settings wait for a restart; "+ + "needs a loopback --host") + } return flags } diff --git a/cmd/semibase/main_test.go b/cmd/semibase/main_test.go index 9ce3346..21baa47 100644 --- a/cmd/semibase/main_test.go +++ b/cmd/semibase/main_test.go @@ -85,12 +85,61 @@ func TestFlagUsageHidesPasswordValues(t *testing.T) { func TestParseOptions(t *testing.T) { tests := []struct { name string + command string arguments []string environment map[string]string wantErr error wantAnyErr bool + wantErrText string check func(t *testing.T, options provision.Options) }{ + { + name: "site without --restart-service restarts nothing", + command: "site", + check: func(t *testing.T, options provision.Options) { + if options.RestartService != "" { + t.Errorf("RestartService = %q, want empty", options.RestartService) + } + }, + }, + { + name: "bench has no --restart-service", + command: "bench", + arguments: []string{"--restart-service", "x"}, + wantErrText: "flag provided but not defined: -restart-service", + }, + { + name: "site rejects --restart-service with a non-loopback host", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "db.internal"}, + wantErrText: "loopback", + }, + { + name: "site rejects --restart-service with a socket host", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "/var/run/postgresql"}, + wantErrText: "loopback", + }, + { + name: "site accepts --restart-service with the default host", + command: "site", + arguments: []string{"--restart-service", "postgresql-x64-17"}, + check: func(t *testing.T, options provision.Options) { + if options.RestartService != "postgresql-x64-17" { + t.Errorf("RestartService = %q, want %q", options.RestartService, "postgresql-x64-17") + } + }, + }, + { + name: "site accepts --restart-service with 127.0.0.1", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "127.0.0.1"}, + }, + { + name: "site accepts --restart-service with ::1", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "::1"}, + }, { name: "flag beats environment", arguments: []string{"--super-password", "from-flag"}, @@ -134,9 +183,17 @@ func TestParseOptions(t *testing.T) { t.Setenv(name, value) } - options, err := parseOptions("bench", tt.arguments) + command := tt.command + if command == "" { + command = "bench" + } + options, err := parseOptions(command, tt.arguments) switch { + case tt.wantErrText != "": + if err == nil || !strings.Contains(err.Error(), tt.wantErrText) { + t.Fatalf("parseOptions error = %v, want one containing %q", err, tt.wantErrText) + } case tt.wantErr != nil: if !errors.Is(err, tt.wantErr) { t.Fatalf("parseOptions error = %v, want %v", err, tt.wantErr) @@ -320,6 +377,28 @@ func TestRunReportsTheEndpointItRanAgainst(t *testing.T) { } } +func TestRunNamesTheUsedUpTimeLimit(t *testing.T) { + buffer := captureErrorOutput(t) + clearPasswordEnvironment(t) + + // mutates the shared commands map, so no t.Parallel here + commands["stub-deadline-exceeded"] = func(ctx context.Context, _ provision.Options) error { + <-ctx.Done() + return ctx.Err() + } + t.Cleanup(func() { delete(commands, "stub-deadline-exceeded") }) + + // an expired parent deadline stands in for a command that used up phaseTimeout + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + if got := run(ctx, []string{"stub-deadline-exceeded"}); got != 1 { + t.Fatalf("run(stub-deadline-exceeded) = %d, want exit code 1", got) + } + if want := "ran out of its " + phaseTimeout.String() + " limit"; !strings.Contains(buffer.String(), want) { + t.Errorf("error output %q does not contain %q", buffer.String(), want) + } +} + func TestRunReportsCommandFailure(t *testing.T) { buffer := captureErrorOutput(t) clearPasswordEnvironment(t) @@ -336,6 +415,9 @@ func TestRunReportsCommandFailure(t *testing.T) { if !strings.Contains(buffer.String(), "error: stub-failure sentinel") { t.Errorf("error output does not report the failure: %q", buffer.String()) } + if strings.Contains(buffer.String(), "limit") { + t.Errorf("error output blames the time limit for an ordinary failure: %q", buffer.String()) + } } func TestCancelledContextReturnsPromptly(t *testing.T) { diff --git a/docs/architecture/README.md b/docs/architecture/README.md index d368b06..9b053c8 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -18,11 +18,12 @@ Declarative architecture docs (English, present tense). These describe the syste | --- | --- | | Installation | PostgreSQL via `winget`, major version pinned, automatic upgrade disabled; engine install stays outside the tool | | Major version | 17 for new installs, pinned latest; floor 14 (SemiPlot needs `date_bin`; Simple-Scada 2 docs require 14+) | -| Provisioning tool | Go single binary `semibase.exe` (`pgx/v5`, embedded SQL); no runtime or `psql` on the target machine | -| Instance configuration | Fixed `ALTER SYSTEM` deltas applied by `semibase site` via `pg_reload_conf()`, never by editing `postgresql.conf`; `shared_buffers` takes effect at the next service restart, reported through `pg_settings.pending_restart` | +| Provisioning tool | Go single binary `semibase.exe` (`pgx/v5`, `golang.org/x/sys` for the Windows service, embedded SQL); no runtime or `psql` on the target machine | +| Instance configuration | Fixed `ALTER SYSTEM` deltas applied by `semibase site` via `pg_reload_conf()`, never by editing `postgresql.conf`; `shared_buffers` and `listen_addresses` take effect at the next service restart, reported through `pg_settings.pending_restart`; `site --restart-service ` performs that restart on Windows, opt-in | +| Network | Loopback only (`listen_addresses = 'localhost'`): every client runs on the installation machine | | Hardware floor | Installation machines guarantee 8 GB RAM and 50 GB database disk; memory settings are fixed constants sized to that floor, identical on every machine | | Roles | Four: the superuser the tool connects as, `scada_writer` (SCADA), `semiplot` (viewers), and `semiplot_registrar`, a `NOLOGIN` role that owns `semiplot_register_new_pens()` so its `SECURITY DEFINER` body never runs as the superuser. `semiplot` holds `SELECT` on the archive; `SELECT` on `semiplot_tags` and `UPDATE` on its settings columns, never on `id`; full row access to `semiplot_groups`, `semiplot_pen_groups` and `semiplot_markers`; `SELECT` on `semiplot_meta`; `EXECUTE` on the function; and no `CREATE` on schema `public` — revoked from `PUBLIC` by `create`, since PostgreSQL 14 still grants it by default. The superuser owns the tables, because it is the connection that creates them. One viewer role rather than a second write-only one: both passwords would share one plaintext file, so the split would separate nothing | -| Command surface | Two commands, `site` and `bench`, named for the situation rather than for the tool's internal steps; they differ in one thing, the memory tuning, which only `site` applies | +| Command surface | Two commands, `site` and `bench`, named for the situation rather than for the tool's internal steps; they differ in one thing, the server settings, which only `site` applies | | Archive read access | `ALTER DEFAULT PRIVILEGES FOR ROLE scada_writer` set **before** `public.trends` is created, and before the writer first runs. The `semiplot_*` tables are granted table by table instead, since the superuser creates them | | Objects we add | `semiplot_tags`, `semiplot_groups`, `semiplot_pen_groups`, `semiplot_markers`, `semiplot_meta`, the `semiplot_register_new_pens()` function the viewer calls, and the vendor-shaped `public.trends` — no triggers, scheduled jobs, extensions, or other functions; `messages` is not created, the SCADA makes it | | SemiPlot schema version | `semiplot_meta.schema_version`, `1` in this release, written by `provision` alone. A floor, not an equality: a viewer refuses a database below the version it needs and accepts one above, because the versions this repository issues grow by addition. When the rule starts, and what a schema change does before then: [provisioning.md](./provisioning.md#the-schema-version-is-a-floor) | diff --git a/docs/architecture/configuration.md b/docs/architecture/configuration.md index a297229..ada0cf6 100644 --- a/docs/architecture/configuration.md +++ b/docs/architecture/configuration.md @@ -1,8 +1,10 @@ # Instance configuration -Every setting the `site` command writes is listed. Most differ from the PostgreSQL default; -two (`effective_cache_size`, `checkpoint_completion_target`) equal it and are pinned -deliberately, so the values in force are visible here rather than implied. The workload is a +Every setting the `site` command writes is listed. Most differ from the PostgreSQL default. +Two (`effective_cache_size`, `checkpoint_completion_target`) equal it and are pinned +deliberately, so the values in force are visible here rather than implied. `listen_addresses` +equals the compiled default too, and its pin is what changes the server: it overrides the value +the installed `postgresql.conf` sets ([Network](#network)). The workload is a sustained append-only insert stream from one writer plus a small number of read-heavy analytical queries. @@ -12,21 +14,25 @@ configuration, and no setting depends on inspecting the host. `semibase site` writes these through `ALTER SYSTEM`, so they live in `postgresql.auto.conf` and survive a reinstall of `postgresql.conf`, and applies them with `pg_reload_conf()`. Every setting -below except `shared_buffers` is reload-context or weaker and takes effect immediately; -`shared_buffers` takes effect at the next service restart or reboot. The tuning phase prints the -server's own pending-restart list (`pg_settings.pending_restart`), and the tail of the run reads it -again as its last statement, so a missed restart is the last thing the run says. `bench` reads it +below except `shared_buffers` and `listen_addresses` is reload-context or weaker and takes effect +immediately; those two take effect at the next service restart or reboot. The tuning phase prints +the server's own pending-restart list (`pg_settings.pending_restart`), and the tail of the run +reads it again as its last statement, so a missed restart is the last thing the run says. +`site --restart-service ` restarts that Windows service inside the tuning phase when the +list is not empty, and the tail then reads it empty +([provisioning.md](./provisioning.md#the-service-restart)). `bench` reads it too — it tunes nothing, but it runs against existing servers, and one that a `site` run tuned can still be waiting for its restart. -`semibase bench` writes none of them: the values are constants sized to the installation -machine's hardware floor and mean nothing to a throwaway container. +`semibase bench` writes none of them: the memory values are constants sized to the installation +machine's hardware floor and mean nothing to a throwaway container, and a loopback-only +`listen_addresses` would shut out the clients that reach a bench through a published port. ## Server settings | Setting | Default | Ours | Why | | --- | --- | --- | --- | -| `shared_buffers` | 128 MB | 2 GB | The archive working set is far larger than the default cache. A quarter of the 8 GB hardware floor; the one setting that needs a service restart. | +| `shared_buffers` | 128 MB | 2 GB | The archive working set is far larger than the default cache. A quarter of the 8 GB hardware floor. Takes effect at a service restart. | | `effective_cache_size` | 4 GB | 4 GB | Planner hint only; set explicitly at half the hardware floor so it never falls below the value the readers' index scans depend on. | | `work_mem` | 4 MB | 64 MB | The readers' pixel-bucket query groups and sorts; at the default it spills to disk on wide windows. | | `maintenance_work_mem` | 64 MB | 512 MB | Index builds and vacuum on daily partitions. | @@ -37,6 +43,7 @@ machine's hardware floor and mean nothing to a throwaway container. | `random_page_cost` | 4.0 | 1.1 | The default assumes a spinning disk and discourages index scans. The machine has a solid-state drive. | | `log_min_duration_statement` | off | 1000 ms | A slow query must leave a trace; the only diagnostic that survives an operator restarting the client. The exception is `semibase`'s own superuser sessions, which set it to -1 to keep password statements out of the log ([provisioning.md](./provisioning.md#invariants-the-provision-package-owns)). | | `track_io_timing` | off | on | Makes `EXPLAIN (ANALYZE, BUFFERS)` usable when a query is slow in the field. | +| `listen_addresses` | `localhost` | `localhost` | Every client of the instance runs on the installation machine ([Network](#network)). Pinned because the installed `postgresql.conf` sets another value: on the Windows runner's PostgreSQL 17 and in the `postgres:17-alpine` container the setting is pending after `site` (measured in CI). Takes effect at a service restart. | Left at defaults deliberately: `max_connections` (a handful of clients), autovacuum (PostgreSQL 13+ already triggers vacuum on insert-only tables, which is what sets the visibility @@ -54,5 +61,11 @@ SCADA's writes: ## Network -The service runs under a dedicated Windows account and listens on the loopback interface plus the -operator network only. +The service runs under a dedicated Windows account and listens on the loopback interface only. +Every client of the instance runs on the same Windows machine: the SCADA, the viewers and +`semibase` itself. Nothing on the operator network may reach the server. + +`site` sets `listen_addresses = 'localhost'` through `ALTER SYSTEM`. The value lands in +`postgresql.auto.conf`, which the server reads after `postgresql.conf`, so it overrides whatever +the installer wrote there. Once the service restarts, a client that names the machine's own IP +address or host name instead of `localhost` or `127.0.0.1` is refused. diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 6878028..d5f47c0 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -56,10 +56,13 @@ The order matters, because the default privileges have to be in place before any 1. Install the engine: `winget install --id PostgreSQL.PostgreSQL.17 --exact`. 2. `semibase site` — tuning, then the archive database, the three created roles, default privileges, `public.trends`, the five SemiPlot tables and `semiplot_register_new_pens()`; it ends by proving `semiplot` reads `trends`, writes - what it owns, cannot add, delete or re-key a pen and cannot write the archive, and warns while `shared_buffers` waits for a service - restart. -3. Restart the PostgreSQL service or reboot the machine, so `shared_buffers` takes effect. -4. Point the Simple-Scada project at the database and start it once. It is expected to find + what it owns, cannot add, delete or re-key a pen and cannot write the archive, and warns while `shared_buffers` or + `listen_addresses` waits for a service restart. With `--restart-service ` it restarts that Windows service itself + and continues against the restarted server (`provisioning.md#the-service-restart`). +3. Without `--restart-service`, restart the PostgreSQL service or reboot the machine, so `shared_buffers` and + `listen_addresses` take effect. +4. Point the Simple-Scada project at the database on `localhost` or `127.0.0.1`, since the server listens on loopback + only, and start it once. It is expected to find `trends` in place, write into it, and create the day partitions and `messages` itself — an **unverified** assumption, with the experiment that settles it, in `provisioning.md` ("Assumption: the SCADA meeting an existing `trends`"). Set diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 959e455..2bb2a0b 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -78,7 +78,7 @@ situations it is run in, not the phases it runs through. | Command | What it does | When | | --- | --- | --- | -| `site` | The memory tuning, then everything else | An installation machine | +| `site` | The server settings, then everything else | An installation machine | | `bench` | Everything else, without the tuning | A throwaway container: a consumer's test bench or a development database | The two differ in exactly one thing, the tuning. The rest is identical, which is what makes a bench @@ -86,12 +86,13 @@ a test of what a site runs: | Step | `site` | `bench` | | --- | --- | --- | -| `ALTER SYSTEM` constants from `configuration.md`, applied with `pg_reload_conf()` | yes | no — the constants are sized for an installation machine and mean nothing to a container | +| `ALTER SYSTEM` deltas from `configuration.md`, applied with `pg_reload_conf()` | yes | no: the memory constants are sized for an installation machine and mean nothing to a container, and a loopback-only `listen_addresses` would shut out the clients that reach a bench through a published port | +| Restarting the Windows service the deltas wait for ([The service restart](#the-service-restart)) | only with `--restart-service` | no: `bench` does not register the flag | | Archive database, the three created roles, grants, default privileges | yes | yes | | `public.trends` with the `tpdefault` partition, created as `scada_writer` | yes | yes | | The five SemiPlot tables and `semiplot_register_new_pens()`, after `public.trends` because the function body names it | yes | yes | | The `semiplot` access checks at the tail | yes | yes | -| Pending-restart warning at the tail | yes | yes — `bench` tunes nothing, but it runs against existing servers too, and one a `site` run tuned can still be waiting for its restart | +| Pending-restart warning at the tail | yes, and after a `--restart-service` restart the list is empty | yes — `bench` tunes nothing, but it runs against existing servers too, and one a `site` run tuned can still be waiting for its restart | `version` (or `--version`) prints the build revision: the ldflags value (`-X main.revision=...`) when a release sets it, otherwise the VCS revision from @@ -179,13 +180,137 @@ The package that runs the SQL owns its guards; the CLI is a thin dispatcher. - **Cancellable execution.** Every phase honors its context: cancellation stops the in-flight query through pgx instead of killing the process mid-DDL. The context itself — `signal.NotifyContext(..., os.Interrupt)` plus the five-minute per-command timeout - (`phaseTimeout`) — is owned by the CLI dispatcher in `cmd/semibase`. -- **Pending-restart reporting.** The tool never touches the service manager. The tuning phase - applies its deltas with `pg_reload_conf()` and reads `pg_settings.pending_restart` back from - the server; both commands read it again as the **last** statement of the tail, after the - access checks, so a pending `shared_buffers` is named in the run's last line. The operator - restarts the service or reboots the machine — the one manual step, and the tool says so - instead of doing it. + (`phaseTimeout`) — is owned by the CLI dispatcher in `cmd/semibase`. The one exception is a + service restart: once the service accepts the Stop, the waits, the port probe and the Start + ignore the command's cancellation, so that an interrupt does not leave the service stopped + ([The service restart](#the-service-restart)). A run that used up `phaseTimeout` says so in a + second error line. +- **Pending-restart reporting.** The tuning phase applies its deltas with `pg_reload_conf()` and + reads `pg_settings.pending_restart` back from the server; both commands read it again as the + **last** statement of the tail, after the access checks, so a pending `shared_buffers` or + `listen_addresses` is named in the run's last line. Without `--restart-service` the tool never + touches the service manager: the operator restarts the service or reboots the machine, and the + tool says so instead of doing it. With the flag, `site` restarts the named Windows service + itself, and the tail reads an empty list. + +## The service restart + +`site --restart-service ` restarts that Windows service when, and only when, the tuning +phase leaves settings waiting for a restart, then waits for the server to accept connections and +continues. Without the flag the run reports the pending settings and changes nothing else. + +Only `site` registers the flag: `bench` applies no settings, so nothing it does waits for a +restart, and `bench --restart-service` fails as an undefined flag. `Options.Validate` refuses a +`--host` other than `localhost`, `127.0.0.1` or `::1`, because after the restart the server listens +on the loopback interface only. + +`config` opens the service before `applySettings` connects, so a service it cannot restart, or one +that is not the run's server, fails the run with nothing changed. `openService` +(`service_windows.go`) refuses a service that does not exist, a service manager or service the run +may not control (the message says to run `semibase` as an administrator), and a Disabled start +type, which would leave the service stopped after the Stop. A service in Start Pending gets up to +2 minutes to reach Running: an installer may run `site` right after it started the service, and an +EDB service (`pg_ctl runservice ... -w`, the `PathName` the Windows CI step prints) reports Running +only once the server accepts connections. That step starts the service with `sc.exe start`, which +returns while it is Start Pending, and runs `site` at once. Stopped, Stop Pending, Paused, or a +start that ends in Stopped, is refused. + +Once the service is Running, `openService` ties it to the server the run connects to. It connects +as the superuser, reads `SHOW data_directory` and the connection's remote IP:port, and compares +the directory with the `-D` argument of the service's `PathName`, split with +`windows.DecomposeCommandLine`. `pg_ctl register` writes that argument quoted, absolute and with +backslashes; the server reports forward slashes. The comparison runs `filepath.Clean` on both +paths and ignores case. A mismatch, or a `PathName` with no `-D`, is refused with nothing +changed: a Stop sent to another service would leave that service stopped, because the run's own +server would keep answering the probe. Without a `-D`, `pg_ctl` takes the data directory from the +service's environment, which the tool does not read. Off Windows, `service_other.go` refuses the +flag the same way and names the platform, so the module stays one cross-platform build. + +The restart runs inside `config`, right after `pendingRestartSettings` returns a non-empty list, so +`create` and `check` run against the restarted server. `applySettings` closes the phase connection +before it returns; then: + +1. Stop, then wait for Stopped and then for a refused TCP connect to the IP:port the identity + check's connection reached, both within one bound of 3 minutes from the Stop. If the bound runs + out, the Start is not sent. +2. Start, then wait up to 2 minutes from the Start for Running. Stopped after the first query fails + at once, and one more connect to that address names the cause (see below). +3. Connect to `postgres` as the superuser every second, up to 60 s. Every error that is not a + server error, a refused connection for one, is retried, and so is 57P03; any other server + error, a wrong password for one, fails at once. A timeout reports the last connect error the + deadline did not cut short; when no dial failed before the deadline, it reports the error of + the dial the deadline cut short. +4. Read `pending_restart` on the new connection. A non-empty list is an error naming the settings. + +Stopped does not mean that the postmaster has exited. What follows was read in +`src/bin/pg_ctl/pg_ctl.c` and `src/backend/postmaster/postmaster.c` of PostgreSQL REL_17_STABLE: + +- On a Stop, `pgwin32_ServiceMain` sends the postmaster SIGINT (fast shutdown) and waits for its + process at most 12 times 5 s. Then it reports Stopped and exits, whether the postmaster is gone + or not. The job object it puts the postmaster in has no `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, so + the postmaster outlives it and finishes its shutdown checkpoint, which with + `max_wal_size = 8GB` can take longer than that minute. +- A Start in that window launches a postmaster that finds the old one's live PID in + `postmaster.pid` and exits FATAL "lock file already exists" (`CreateLockFile` in `miscinit.c`). +- The postmaster keeps its listening sockets until its exit callbacks close them, just before + they remove `postmaster.pid` (`CloseServerPorts`). In `PM_WAIT_DEAD_END` it stops accepting + but keeps the sockets open, and the kernel still completes a TCP handshake into the listen + backlog. So a refused connect (`WSAECONNREFUSED`) means the + postmaster is gone, and step 1 waits for it. Any other connect error is retried. +- Step 1 probes once a second, because each accepted connect costs the server a process. The + probe dials the one literal IP:port the identity check's connection reached, never the name + `localhost`. Go dials a name that resolves to both `::1` and `127.0.0.1` in parallel and, when + both fail, returns the error of the first family (`dialParallel` in `net/dial.go`, Go 1.27). + An IPv6 failure other than a refusal would then hide the IPv4 refusal, and the Start would never + be sent. A literal address has one family. +- Each probe dial has a 15 s timeout. Windows resends a SYN after an RST, so a refused loopback + connect can take seconds (Daniel Stenberg, + `daniel.haxx.se/blog/2024/08/14/slow-tcp-connect-on-windows`). Measured on Windows 11 build + 26300, a refusal took under 2 ms; the resends were not reproduced here. The 3-minute stop bound + still limits the whole wait. +- The service runs `pg_ctl runservice ... -w` with no `-t`, so `wait_for_postmaster_start` gives + up after `DEFAULT_WAIT` = 60 s. It then reports Stopped and leaves a slower postmaster running + outside the service. A postmaster that exits during its startup also ends in Stopped. In step 2 + the probe tells the two apart: an answer means the timed-out startup, a refusal means the exit. + Only a hung `pg_ctl` reaches the 2-minute bound. + +Once the service accepts the Stop, steps 1 and 2 run on `context.WithoutCancel`, each to its own +bound. An interrupt or the command timeout during them does not stop the sequence: the service is +started, and the run then ends with the cancellation. A wait that runs out its bound names the +service and the last state it observed. Four cases still leave the service stopped: + +| Case | What the run says | Recovery | +| --- | --- | --- | +| The old postmaster still runs 3 minutes after the Stop, or `pg_ctl` itself has not reported Stopped by then | the service is stopped but the server still answers, or the service is still stopping; no Start was sent and the service stays stopped | once a connect to the port is refused (`Test-NetConnection localhost -Port `) and `Get-Service ` shows Stopped, `Start-Service ` | +| The process ends without running its code: console window closed, logoff, shutdown, a killed process. `signal.NotifyContext` catches `os.Interrupt` only, and Windows ends a console process a few seconds after a close event, which no handler can extend | nothing | `Start-Service ` | +| The postmaster exits during its startup, for example it cannot allocate `shared_buffers = 2GB` | the service stopped again after the Start and nothing answers on the port; the message names the recovery | read the server log in the data directory's `log` folder, remove the line it names from `postgresql.auto.conf`, then `Start-Service `. A re-run of `site` writes the line again and stops the service the same way, so fix the cause first | +| The startup takes longer than `pg_ctl`'s 60 s wait | the service stopped again after the Start but the server answers on the port: it runs outside the service | once it accepts connections, `pg_ctl stop -D `, then `Start-Service ` | + +The command's five-minute `phaseTimeout` also covers the restart. A stop and a start that use +their whole bounds leave no time for the ready wait, `create` and `check`; the run then fails with +the service running, says it ran out of its limit, and a re-run finishes the work. + +Service control goes through `golang.org/x/sys/windows/svc/mgr`, not `net stop` and `net start`. +Its errors do not depend on the Windows display language, it reads the start type, so a Disabled +service is refused before any change, and it reports the service state for the bounded waits. +`x/sys` is pure Go, so the release still cross-compiles with `CGO_ENABLED=0`. + +A restart drops every open connection to the server, the SCADA's included. + +CI's Linux job runs `site` without the flag: its server is a GitHub Actions service container, not +a Windows service, and nothing in the job restarts it. The `Require the listen_addresses delta` +step then reads `listen_addresses` still pending and `localhost` in `postgresql.auto.conf` +through `pg_file_settings`, which proves `site` wrote the delta. The Windows job's +`Restart the runner's PostgreSQL service through site` step is the one test of the restart +itself. The runner image ships PostgreSQL 17 as `postgresql-x64-17`, Stopped and Disabled. The +step first requires `site --restart-service postgresql-x64-17` to refuse the Disabled service +with no `ALTER SYSTEM SET` line in its output. It then sets the service to Manual, starts it with +`sc.exe start`, and runs `site` at once, while the service is Start Pending. That run must print +`restarting service postgresql-x64-17`, the identity line +`runs data directory C:\PostgreSQL\17\data, as the server on :5432 does` and +`stopped, nothing answers on :5432`, and leave `listen_addresses = localhost` with no setting +pending. A second run must not restart the service, and `--restart-service no-such-service` must +be refused with no `ALTER SYSTEM SET` line. ## The roles @@ -492,8 +617,9 @@ promises. They are knowable at exit precisely because this tool creates `public. was given, and on a socket host — `peer` is the platform default there and no consumer reads over the socket. 9. No setting waits for a service restart (`pg_settings.pending_restart`) — a warning, not a - failure, so a pending `shared_buffers` never blocks the access checks. Both commands read it, - last, after the access checks. + failure, so a pending `shared_buffers` or `listen_addresses` never blocks the access checks. + Both commands read it, last, after the access checks. After a `--restart-service` restart it + reads an empty list. ## Development benches diff --git a/docs/deployment.md b/docs/deployment.md index af76fec..ec25351 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -32,7 +32,7 @@ SemiBase разворачивается одной утилитой — `semibas | Команда | Действие | | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `site` | Машина установки: настройки сервера через `ALTER SYSTEM`, затем архивная база, роли, права доступа, пять таблиц SemiPlot и `public.trends` | -| `bench` | Временный контейнер (тестовый стенд, база для разработки): то же самое без настроек сервера — константы памяти рассчитаны на машину установки и контейнеру ничего не дают | +| `bench` | Временный контейнер (тестовый стенд, база для разработки): то же самое без настроек сервера: память в них рассчитана на машину установки, а прослушивание только локального интерфейса закрыло бы контейнер от клиентов | | `version` | Печать ревизии сборки | Обе команды заканчиваются проверкой: `semiplot` читает `public.trends` и версию схемы, вызывает @@ -156,8 +156,9 @@ Simple-Scada 2 на той же машине, и ей доверяют: она winget install --id PostgreSQL.PostgreSQL.17 --exact ``` -2. Задать пароли ролей переменными окружения в том окне PowerShell, из которого запускается - утилита. Каждая строка запрашивает пароль, и в историю команд PowerShell попадает только +2. Открыть PowerShell от имени администратора и задать в этом окне пароли ролей переменными + окружения: из него же запускается утилита, а перезапуск службы требует прав администратора. + Каждая строка запрашивает пароль, и в историю команд PowerShell попадает только сама строка, без пароля. Переменные живут, пока окно открыто. ```powershell @@ -169,9 +170,14 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па 3. Выполнить одну команду: ```powershell -.\semibase.exe site +.\semibase.exe site --restart-service postgresql-x64-17 ``` +`postgresql-x64-17` — имя службы, которую создаёт установщик PostgreSQL 17. Проверить его можно +командой `Get-Service postgresql*`. Перед любыми изменениями утилита сверяет каталог после `-D` в +пути к исполняемому файлу службы с каталогом данных сервера на порту. Если они не совпадают, флаг +называет не ту службу, и утилита завершается с ошибкой, ничего не изменив. + На выходе утилита гарантирует: настройки сервера применены, архивная база, три роли, права и права по умолчанию созданы, пять таблиц SemiPlot, функция `semiplot_register_new_pens()` и `public.trends` (с партицией `tpdefault`) на месте, `semiplot` читает `public.trends`, вызывает @@ -179,12 +185,44 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па удалить или перенумеровать перо и не может писать в архив. Отдельной проверки после этого не требуется — она уже выполнена, её результат в коде возврата. -4. Перезапустить службу PostgreSQL или перезагрузить машину. Все настройки, кроме - `shared_buffers`, применяются сразу; `shared_buffers` — только после перезапуска. - Утилита предупреждает об этом в последних строках вывода, пока перезапуск не сделан. - -5. Запустить проект Simple-Scada против базы. Мы рассчитываем, что таблицу `trends` он застанет - готовой, оставит её как есть и дальше создаст дневные партиции и `messages` сам. **Это +4. Убедиться, что служба перезапущена. `shared_buffers` и `listen_addresses` применяются только + после перезапуска службы, остальные настройки сразу. С флагом `--restart-service` утилита на + шаге 3 перезапускает службу сама, если настройки ждут перезапуска, ждёт, пока сервер снова + примет подключения, и продолжает работу. Без флага перезапустите службу или перезагрузите + машину; пока перезапуск не сделан, утилита предупреждает об этом в последних строках вывода. + + Утилита запускает службу, только когда старый сервер завершил работу и порт перестал + отвечать. Если она сообщает, что служба остановлена и такой останется, дождитесь, пока + `Test-NetConnection localhost -Port 5432` покажет `TcpTestSucceeded : False` и + `Get-Service postgresql-x64-17` покажет `Stopped`, и запустите службу командой + `Start-Service postgresql-x64-17`. + + Если служба остановилась сразу после запуска, утилита сообщает, отвечает ли сервер на порту. + Если ничего не отвечает, сервер завершился при старте. Причина записана в журнале сервера, в + папке `log` каталога данных. Если журнал называет настройку, которую записала утилита + (например, серверу не хватило памяти на `shared_buffers = 2GB`), удалите эту строку из + `postgresql.auto.conf` в каталоге данных и запустите службу командой + `Start-Service postgresql-x64-17`. Повторный запуск `site` запишет строку снова и так же + остановит службу, поэтому сначала устраните причину. Если сервер отвечает, служба не + дождалась конца его старта, и сервер работает вне службы. Когда он начнёт принимать + подключения, остановите его командой `pg_ctl stop -D <каталог данных>` (`pg_ctl.exe` лежит в + папке `bin` установки PostgreSQL, каталог данных указан после `-D` в пути к исполняемому файлу + службы) и запустите службу командой `Start-Service postgresql-x64-17`. + + Если во время перезапуска закрыли окно PowerShell, вышли из системы или завершили процесс + утилиты, служба может остаться остановленной. Проверьте её командой + `Get-Service postgresql-x64-17` и, если она остановлена, запустите командой + `Start-Service postgresql-x64-17`. + + Перезапуск разрывает подключения SCADA. Повторный запуск `site` на работающей установке + перезапускает службу, только если какие-то настройки ждут перезапуска. Их список выводит + запрос `SELECT name FROM pg_settings WHERE pending_restart`; если он не пуст, остановите SCADA + перед запуском. + +5. Запустить проект Simple-Scada против базы. Проект подключают к `localhost` или `127.0.0.1`: + после перезапуска сервер принимает подключения только с этой машины. Мы рассчитываем, что + таблицу `trends` он застанет готовой, оставит её как есть и дальше создаст дневные партиции и + `messages` сам. **Это предположение не проверено на реальном проекте.** На первом запуске включите `log_statement = 'all'` и прочитайте в журнале PostgreSQL, какие DDL-команды SCADA выдала: если она создаёт или изменяет `trends`, порядок ввода в эксплуатацию нужно @@ -209,11 +247,12 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па ## Основные флаги -| Флаг | По умолчанию | Назначение | -| ------------------ | --------------- | ------------------------------------------------------------- | -| `--host` | `localhost` | Адрес сервера или каталог unix-сокета (`/var/run/postgresql`) | -| `--port` | `5432` | Порт сервера | -| `--database` | `scada_archive` | Имя архивной базы | -| `--expected-major` | не проверяется | Требуемая мажорная версия сервера | +| Флаг | По умолчанию | Назначение | +| ------------------- | --------------- | ------------------------------------------------------------- | +| `--host` | `localhost` | Адрес сервера или каталог unix-сокета (`/var/run/postgresql`) | +| `--port` | `5432` | Порт сервера | +| `--database` | `scada_archive` | Имя архивной базы | +| `--expected-major` | не проверяется | Требуемая мажорная версия сервера | +| `--restart-service` | не задан | Только `site` в Windows: служба PostgreSQL, которую утилита перезапускает, если настройки ждут перезапуска. Требует `--host` `localhost`, `127.0.0.1` или `::1` и запуска от имени администратора. Отключённую, остановленную или обслуживающую другой каталог данных службу утилита не трогает и завершается с ошибкой; только что запущенную службу ждёт до двух минут | Полный перечень — `semibase <команда> --help`. diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md new file mode 100644 index 0000000..7c0228e --- /dev/null +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -0,0 +1,283 @@ +# Site: loopback-only listening and an optional service restart + +## Overview +- `semibase site` sets `listen_addresses = 'localhost'`: every client of the instance runs on the + same Windows machine, and nothing on the operator network may reach the server. +- `semibase site --restart-service ` restarts that Windows service when, and only when, the + tuning phase leaves settings waiting for a restart, then waits for the server to accept + connections and continues. Without the flag the run behaves as today and warns. +- The Windows installer (`20261019-windows-installer.md`) is the caller of the flag. CI's Linux job + runs `site` against a service container the tool cannot restart, so the restart stays opt-in. + +## Context (from discovery) +- `internal/provision/config.go:16-28` — the `settings` list applied with `ALTER SYSTEM`; + `config.go:30-71` applies it, reloads, and reports `pending_restart` (`config.go:60-70`); + `config.go:36-40` opens the one connection the phase uses. +- `internal/provision/provision.go:39-44` — `Options.Validate`; `provision.go:48-56` — `Site` runs + `config`, `create`, `check` in that order; `provision.go:26-35` — `Options`. +- `internal/provision/check.go:422` — `reportPendingRestart` at the tail of every run. +- `cmd/semibase/main.go:158-175` — one flag set shared by `site` and `bench`; the comment at + `main.go:154-155` states every flag is used by either command; `main.go:184` calls `Validate`. +- `cmd/semibase/main.go:60,112` — one five-minute timeout covers the whole command. +- `go.mod:5` — the only direct dependency is `pgx/v5`. +- `.github/workflows/ci.yml:11-29` — the Windows job builds, tests and lints only; + `ci.yml:31-47` — the Linux job's service container and its password `ci-super`; + `ci.yml:65-71` — the Linux job runs `site` twice against `postgres:17-alpine`. +- `docs/architecture/provisioning.md:160-165` — "The tool never touches the service manager." +- `docs/architecture/configuration.md:57-58` — "listens on the loopback interface plus the operator + network only"; this no longer holds. +- Read 2026-10-19 from `actions/runner-images` `images/windows/Windows2025-Readme.md` + (`windows-latest`): PostgreSQL 17.11, service `postgresql-x64-17`, status Stopped, start type + Disabled, user `postgres`, password `root`, `PGBIN=C:\Program Files\PostgreSQL\17\bin`, + `PGDATA=C:\PostgreSQL\17\data`. + +## Development Approach +- **testing approach**: Regular (code first, then tests in the same task) +- complete each task fully before moving to the next +- every task includes new or updated tests for the code it changes +- all tests pass before the next task starts +- update this plan when scope changes + +## Testing Strategy +- unit tests beside the source, table-driven (`cmd/semibase/main_test.go`, + `internal/provision/*_test.go`); platform-specific halves live in build-tagged test files +- integration: CI steps against live servers; the repository has no database-touching unit tests + (CLAUDE.md, Test section) + +## Acceptance Evidence +- Linux CI, after the two `site` runs (`ci.yml:70-71`): + `PGHOST=localhost PGUSER=postgres PGPASSWORD=ci-super psql -d postgres -tAc "SELECT pending_restart FROM pg_settings WHERE name = 'listen_addresses'"` + prints `t`, and `SELECT setting FROM pg_file_settings WHERE name = 'listen_addresses' AND + sourcefile LIKE '%postgresql.auto.conf'` prints `localhost`. The container is never restarted, + so the value stays pending; this proves the delta is written. +- Windows CI, new step against the runner's PostgreSQL 17, with `$psql = "$env:PGBIN\psql.exe"`: + 1. `semibase site --port 5432 --expected-major 17 --restart-service postgresql-x64-17` exits 0, + and its output contains `restarting service postgresql-x64-17`, the identity line + `service postgresql-x64-17 runs data directory C:\PostgreSQL\17\data, as the server on + :5432 does` and `service postgresql-x64-17 stopped, nothing answers on :5432`, where + `` is the literal address the identity check's connection reached. + 2. `& $psql -h 127.0.0.1 -U postgres -d postgres -tAc "SHOW listen_addresses"` prints + `localhost`, and `SELECT count(*) FROM pg_settings WHERE pending_restart` prints `0`. + 3. A second identical run exits 0, and its output does not contain `restarting service`. + 4. `site --restart-service` against the shipped Disabled service, and against + `no-such-service`, exits 1 with no `ALTER SYSTEM SET` line in its output. The restart run + follows `sc.exe start` with no other readiness wait, so it begins while the service may + still be Start Pending. +- Unit tests: `bench --restart-service x` is a usage error; `--restart-service` with a + non-loopback `--host` is a usage error; `site --restart-service x` with a loopback host parses; + the restart sends Start after the Stop when the command context is already cancelled or + expired. + +## Solution Overview +- `listen_addresses` joins the `settings` list. It is postmaster-context, so it lands in + `pending_restart` exactly like `shared_buffers`, and the existing reporting covers it. +- The restart happens inside the tuning phase, right after `pendingRestartSettings` returns a + non-empty list, so `create` and `check` run against the restarted server and the tail's + `reportPendingRestart` reads an empty list. +- Service control uses `golang.org/x/sys/windows/svc/mgr` behind a `windows` build tag, not + `net stop`/`net start`: its errors do not depend on the Windows display language, it reads the + service's start type, so a Disabled service is refused before any change, and it exposes the + service state for bounded waits. `x/sys` is pure Go, so cross-compilation without cgo stays. +- The flag parses on every platform, for `site` only. The service is opened and checked at the + start of `config`, before it connects: a wrong name, a missing right, a Disabled start type, a + state other than Running once a Start Pending has finished, or a non-Windows build + (`service_other.go` returns "service restart is not supported on ") fails with nothing + changed. The module stays one cross-platform build: the Linux binary is what the docker benches + run. +- The service must be the run's server. Once it is Running, `openService` connects, reads + `SHOW data_directory` and the connection's remote IP:port, and compares the directory with the + `-D` in the service's `PathName` (`windows.DecomposeCommandLine`, `filepath.Clean` on both + sides, case ignored). A mismatch or a `PathName` with no `-D` fails with nothing changed: a Stop + sent to another service would leave it stopped while the run's server keeps answering. +- Once the service accepts the Stop, the exit wait, the Start and the Running wait run on a + context detached from the command's cancellation and timeout (`context.WithoutCancel`), each + wait with its own bound. An interrupt or the command timeout during them does not stop the + sequence. +- SCM Stopped does not prove the postmaster is gone: `pg_ctl runservice` reports Stopped after at + most 12 x 5 s of a stop and leaves a slower postmaster running (`pg_ctl.c`, REL_17_STABLE). So + the Start waits, within the stop bound, for a TCP connect to be refused. The probe dials the + literal IP:port the identity check's connection reached: Go dials `localhost` to `::1` and + `127.0.0.1` in parallel and reports the first family's error, so a non-refusal on one family + would hide the refusal on the other. Each dial has a 15 s timeout, since Windows resends a SYN + after an RST. +- `pg_ctl -w` without `-t` gives up on a startup after 60 s and reports Stopped while the + postmaster keeps starting. Stopped after the Start therefore probes the port once: an answer + names that timeout, a refusal names a postmaster that exited during its startup. +- Four cases still leave the service stopped, each with its recovery in + `docs/architecture/provisioning.md#the-service-restart`: an old postmaster still running at the + stop bound, a process ended by the console close, logoff, shutdown or a kill, a postmaster that + exits during its startup, and a startup longer than `pg_ctl`'s 60 s wait. +- After the service reports Running, the tool reconnects in a bounded loop. An EDB service runs + `pg_ctl runservice ... -w` and reports Running only once the postmaster accepts connections + (read with CIM on the development machine, and printed by the Windows CI step for + `postgresql-x64-17`: `pg_ctl.exe runservice -N "postgresql-x64-17" -D "C:\PostgreSQL\17\data" -w`), + so the loop normally costs one connect. + +## Technical Details +- `Options.RestartService string`; empty means no restart. +- `--restart-service `, registered by `site` only: `bench` has no tuning, so nothing it + does creates a pending restart, and the flag is undefined for it. `Validate` refuses it when + `--host` is not `localhost`, `127.0.0.1` or `::1`: after the restart the server listens on + loopback only. +- The restart runs when the pending list is non-empty and a service was opened; the condition + sits inline in `config`'s switch. +- `openService` waits up to 2 minutes for a Start Pending service to reach Running, then refuses + any state other than Running. It then calls `describeServer` (`config.go`), which connects as + the superuser and returns `SHOW data_directory` and `RemoteAddr()`, and refuses a service whose + `-D` differs or is absent. The address becomes the probe's target. +- Restart sequence in `config`: + 1. Close the phase connection. + 2. `Stop` on the command context. From here on, steps 3 and 4 run on + `context.WithoutCancel(ctx)`. + 3. Wait for `Stopped`, then for a refused TCP connect to the identity check's IP:port + (`WSAECONNREFUSED`; any other connect error is retried; each dial times out after 15 s), + probing once a second, both bounded by 3 minutes from + the Stop: a shutdown checkpoint with `max_wal_size = 8GB` can be long. If the wait gives up, + Start is not sent and the error says the service will stay stopped and how to start it. + 4. `Start`, then wait for `Running`, bounded by 2 minutes from the Start. Stopped from the + second query on fails at once; one more probe decides the message: an answer means + `pg_ctl`'s 60 s startup wait ran out and the server runs outside the service, a refusal + means the server exited during its startup and points to the server log, to removing the + line it names from `postgresql.auto.conf` before `Start-Service`, and warns that a re-run + writes the line again. + 5. The ready wait connects to `postgres` as the superuser every second, bounded by 60 s. It + retries every error that is not a server error, and 57P03, returns the new connection, and + the last connect error the deadline did not cut short goes into the timeout message. + 6. `pendingRestartSettings` runs on the new connection. A non-empty list is an error naming the + settings. +- Service control goes through a four-method seam over `*mgr.Service` (`Query`, `Control`, + `Start`, `Config`), the port probe is a function field on `managedService`, and the server read + is a function `openService` receives, so the check and the sequence are unit-tested with + scripted states, configurations, servers and probe results. +- A wait that runs out its bound names the service and its last observed state. +- Output: `step("restarting service %s: %s waiting for a restart", name, list)`, then `ok` lines + for stopped, running, accepting connections. + +## What Goes Where +- Implementation Steps: code, tests, CI steps and docs in this repository. +- Post-Completion: checks on an installation machine. + +## Implementation Steps + +### Task 1: Listen on loopback only + +**Files:** +- Modify: `internal/provision/config.go` +- Modify: `.github/workflows/ci.yml` + +- [x] add `{"listen_addresses", "localhost"}` to `settings` (`config.go:16-28`) +- [x] add the Linux CI check from Acceptance Evidence after `ci.yml:71`; it is this task's test, + since a unit test of a constant list proves nothing +- [x] run `go test ./...` and `go vet ./...` - must pass before task 2 + +### Task 2: Parse and validate `--restart-service` + +**Files:** +- Modify: `internal/provision/provision.go` +- Modify: `cmd/semibase/main.go` +- Modify: `cmd/semibase/main_test.go` + +- [x] add `RestartService` and the command name `Validate` needs to `Options` + (`provision.go:26-35`) +- [x] extend `Validate` (`provision.go:39-44`) with the two refusals from Technical Details +- [x] register `--restart-service` in `newFlagSet` (`main.go:158`); rewrite the comment at + `main.go:154-155`, which stops being true; add the flag to `usage` (`main.go:20-55`) +- [x] write table tests: `bench` rejects the flag, a non-loopback host rejects it, `site` with a + loopback host accepts it, no flag parses as today +- [x] run `go test ./...` - must pass before task 3 + +### Task 3: Restart the service and wait for the server + +**Files:** +- Create: `internal/provision/service_windows.go`, `internal/provision/service_other.go` +- Modify: `internal/provision/config.go` +- Create: `internal/provision/config_test.go` +- Modify: `go.mod`, `go.sum` + +- [x] add `golang.org/x/sys`; in `service_windows.go` implement opening and checking the service + (exists, access, not Disabled) and the stop-start sequence with the bounds above +- [x] `service_other.go` returns an error naming the platform from the same check function +- [x] in `config`: check the service before connecting; after + `pendingRestartSettings` (`config.go:60`) call `restartNeeded`, run the sequence, re-read + on the new connection +- [x] extract the ready wait over an injected `func(ctx) (*pgx.Conn, error)` dialer +- [x] write tests: `restartNeeded` table; ready wait succeeds at once, succeeds after N failures, + times out with the last error, stops on context cancellation +- [x] run `go test ./...`, `go vet ./...`, `golangci-lint run`; `GOOS=linux go build ./...` - + must pass before task 4 + +### Task 4: Exercise the restart on the Windows runner + +**Files:** +- Modify: `.github/workflows/ci.yml` + +- [x] add a step to the Windows job: `Set-Service postgresql-x64-17 -StartupType Manual`, + `Start-Service postgresql-x64-17` +- [x] run `semibase site` with `SEMIBASE_SUPER_PASSWORD=root`, writer and plot passwords set + (the first run creates those roles), and the flags from Acceptance Evidence +- [x] run the three Windows checks from Acceptance Evidence; this step is the only test of the + service sequence itself +- [x] push the branch and read the CI run - both jobs green before task 5 + +### Task 5: Verify acceptance criteria +- [x] every item in Acceptance Evidence holds on the CI run (run 39249102852 at `9577716`, both + jobs green: Linux `Require the listen_addresses delta` passed; Windows first `site` printed + `restarting service postgresql-x64-17: listen_addresses, shared_buffers waiting for a + restart`, the `SHOW`/`count(*)` check passed, the second run printed no restart line) +- [x] `go test -race ./...`, `go vet ./...`, `golangci-lint run`, `gofmt -l .` prints nothing + (locally `-race` needs cgo and this host has no C compiler, so `go test ./...` ran without + it; both CI jobs run `go test -race ./...` green; `GOOS=linux go vet ./...` clean) + +### Task 6: [Final] Update documentation +- [x] `docs/architecture/provisioning.md:160-165`: the opt-in restart, where it runs, the + Start guarantee, why CI's Linux job does not use it; `provisioning.md:94` follows + (the bullet now at `:183`, plus a new "The service restart" section and a table row) +- [x] `docs/architecture/configuration.md`: `listen_addresses` in the deltas table; + `configuration.md:15-16` and `:29` stop calling `shared_buffers` the only restart setting; + `configuration.md:57-58` becomes loopback only, single machine, and notes that + `postgresql.auto.conf` overrides `postgresql.conf` +- [x] `docs/architecture/README.md:22` and `docs/architecture/overview.md:61` (provisioning + step 3) name the flag +- [x] `docs/deployment.md`, in Russian: the flags table gets `--restart-service`; step 4 says the + flag does the restart; one sentence that a restart drops the SCADA's connections, so the + SCADA is stopped before a re-run on a working site +- [x] "memory constants" and "memory tuning" become "server settings" in `CLAUDE.md`, + `main.go:26`; "pure `pgx`" becomes "pure Go" in `CLAUDE.md`, `release.yml:124` and + `Dockerfile:3-4`; the Run section of `CLAUDE.md` mentions the flag (also the same phrases + in `provision.go:65,77`, `README.md:25` and `provisioning.md:81`; the CLAUDE.md Test + section gains both new CI steps and counts eight database steps) +- [ ] move this plan to `docs/plans/completed/` (left for the delivery step) + +## Post-Completion +- On an installation machine, confirm the Simple-Scada project connects to `localhost` or + `127.0.0.1`. A project that names the machine's own IP or host name stops writing after the + restart. + +**Executed by exec:** +- branch: site-loopback-and-restart + +## Verify it yourself + +1. The loopback delta is written. In CI run 39264258290 (HEAD e6e7a4e), the Linux step "Require + the listen_addresses delta" is green: `pending_restart` for `listen_addresses` is `t` and + `postgresql.auto.conf` holds `localhost`. On `password-hygiene` (3cf9cd9) the setting is not in + `config.go`, so the same queries print `f` and nothing. +2. The restart runs once and only when needed. In the same run, the Windows step "Restart the + runner's PostgreSQL service through site" prints, in order: the Disabled refusal with no + `ALTER SYSTEM SET` line; `state after sc.exe start: StartPending`; the identity line + `service postgresql-x64-17 runs data directory C:\PostgreSQL\17\data, as the server on + [::1]:5432 does`; `restarting service postgresql-x64-17: listen_addresses, shared_buffers + waiting for a restart`; `stopped, nothing answers on [::1]:5432`; `running`; `all settings + active`. The second run prints no restart line, and `no-such-service` is refused. +3. Start always follows an accepted Stop when only the command is interrupted. Run + `go test ./internal/provision -run TestRestart -v` on Windows: the rows "an interrupt after the + Stop still starts the service" and "the command timeout after the Stop still starts the + service" pass, and every row asserts that Start comes only after the port refused a connect. +4. The flag is scoped: `go test ./cmd/semibase -run TestParseOptions -v` shows `bench + --restart-service` refused as an undefined flag, non-loopback and socket hosts refused, and + `localhost`, `127.0.0.1`, `::1` accepted. +5. Manual, on a machine with an EDB PostgreSQL 17 and an elevated PowerShell: run `semibase site + --restart-service postgresql-x64-17` with the passwords in `$env:SEMIBASE_*`, then + `psql -h 127.0.0.1 -U postgres -c "SHOW listen_addresses"` prints `localhost`, and + `Test-NetConnection -Port 5432` fails. The residual cases that can still leave + the service stopped are listed in `docs/architecture/provisioning.md#the-service-restart`. diff --git a/go.mod b/go.mod index 93865b2..edff430 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,10 @@ module github.com/Semiteq/SemiBase go 1.26 -require github.com/jackc/pgx/v5 v5.10.0 +require ( + github.com/jackc/pgx/v5 v5.10.0 + golang.org/x/sys v0.47.0 +) require ( github.com/jackc/pgpassfile v1.0.0 // indirect diff --git a/go.sum b/go.sum index c0e505b..c48822d 100644 --- a/go.sum +++ b/go.sum @@ -18,6 +18,8 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= golang.org/x/sync v0.17.0 h1:l60nONMj9l5drqw6jlhIELNv9I0A4OFgRsG9k2oT9Ug= golang.org/x/sync v0.17.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/provision/check.go b/internal/provision/check.go index 26b8d3c..afe03eb 100644 --- a/internal/provision/check.go +++ b/internal/provision/check.go @@ -163,7 +163,7 @@ func (o Options) check(ctx context.Context) error { if err := o.checkPlotLogin(ctx); err != nil { return err } - // last, so a pending shared_buffers is the last thing the run prints + // last, so a setting still waiting for a restart is the last thing the run prints return reportPendingRestart(ctx, archive) } @@ -418,8 +418,7 @@ func reportPendingRestart(ctx context.Context, conn *pgx.Conn) error { return err } if len(pending) > 0 { - warn("settings waiting for a service restart: %s - restart the PostgreSQL service or reboot the machine.", - strings.Join(pending, ", ")) + warn("settings waiting for a service restart: %s. %s", strings.Join(pending, ", "), restartAdvice(pending)) } return nil } diff --git a/internal/provision/config.go b/internal/provision/config.go index 0bebb71..86f8f71 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -2,10 +2,14 @@ package provision import ( "context" + "errors" "fmt" + "slices" "strings" + "time" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" ) type setting struct { @@ -25,51 +29,178 @@ var settings = []setting{ {"random_page_cost", "1.1"}, {"log_min_duration_statement", "1000"}, {"track_io_timing", "on"}, + {"listen_addresses", "localhost"}, } +const ( + readyTimeout = 60 * time.Second + readyInterval = time.Second +) + func (o Options) config(ctx context.Context) error { if err := o.Validate(); err != nil { return err } step("Phase config: ALTER SYSTEM deltas") - conn, err := o.connectSuperuser(ctx, "postgres") + var service *managedService + if o.RestartService != "" { + var err error + // SA4023: off Windows openService always fails, by design + if service, err = openService(ctx, o.RestartService, o.describeServer); err != nil { //nolint:staticcheck + return err + } + defer service.close() + } + + pending, err := o.applySettings(ctx) if err != nil { return err } + switch { + case len(pending) > 0 && service != nil: + return o.restartService(ctx, service, pending) + case len(pending) > 0: + note("waiting for a service restart: %s. %s", strings.Join(pending, ", "), restartAdvice(pending)) + default: + ok("all settings active") + } + return nil +} + +// runningServer is what a connection to --host:--port learns about the server behind it. +type runningServer struct { + dataDirectory string + // address is the IP:port the connection reached, one address of one family + address string +} + +func (o Options) describeServer(ctx context.Context) (runningServer, error) { + conn, err := o.connectSuperuser(ctx, "postgres") + if err != nil { + return runningServer{}, err + } + defer conn.Close(ctx) + var dataDirectory string + if err := conn.QueryRow(ctx, "SHOW data_directory").Scan(&dataDirectory); err != nil { + return runningServer{}, fmt.Errorf("reading data_directory: %w", err) + } + return runningServer{ + dataDirectory: dataDirectory, + address: conn.PgConn().Conn().RemoteAddr().String(), + }, nil +} + +// applySettings closes its connection before returning: a restart must not find it open. +func (o Options) applySettings(ctx context.Context) ([]string, error) { + conn, err := o.connectSuperuser(ctx, "postgres") + if err != nil { + return nil, err + } defer conn.Close(ctx) for _, s := range settings { statement := fmt.Sprintf("ALTER SYSTEM SET %s = '%s'", s.Name, s.Value) if _, err := conn.Exec(ctx, statement); err != nil { - return fmt.Errorf("%s: %w", statement, err) + return nil, fmt.Errorf("%s: %w", statement, err) } ok("ALTER SYSTEM SET %s = '%s'", s.Name, s.Value) } if _, err := conn.Exec(ctx, "SELECT pg_reload_conf()"); err != nil { - return fmt.Errorf("reloading the configuration: %w", err) + return nil, fmt.Errorf("reloading the configuration: %w", err) } // pg_reload_conf signals asynchronously; the pause lets the server processes apply // the new files before pending_restart is read if _, err := conn.Exec(ctx, "SELECT pg_sleep(0.5)"); err != nil { - return fmt.Errorf("waiting for the reload: %w", err) + return nil, fmt.Errorf("waiting for the reload: %w", err) } ok("configuration reloaded") - pending, err := pendingRestartSettings(ctx, conn) + return pendingRestartSettings(ctx, conn) +} + +func restartAdvice(pending []string) string { + advice := "Restart the PostgreSQL service or reboot the machine." + if slices.Contains(pending, "listen_addresses") { + advice += " After the restart the server accepts connections from this machine only." + } + return advice +} + +func (o Options) restartService(ctx context.Context, service *managedService, pending []string) error { + step("restarting service %s: %s waiting for a restart", o.RestartService, strings.Join(pending, ", ")) + if err := service.restart(ctx); err != nil { + return err + } + conn, err := waitReady(ctx, readyTimeout, readyInterval, func(ctx context.Context) (*pgx.Conn, error) { + return o.connectSuperuser(ctx, "postgres") + }) if err != nil { return err } - if len(pending) == 0 { - ok("all settings active") - return nil + defer conn.Close(ctx) + ok("server accepting connections") + + stillPending, err := pendingRestartSettings(ctx, conn) + if err != nil { + return err + } + if len(stillPending) > 0 { + return fmt.Errorf("after restarting service %s, settings still wait for a restart: %s", + o.RestartService, strings.Join(stillPending, ", ")) } - note("waiting for a service restart: %s. Restart the PostgreSQL service or reboot the machine.", - strings.Join(pending, ", ")) + ok("all settings active") return nil } +func waitReady( + ctx context.Context, + timeout, interval time.Duration, + dial func(context.Context) (*pgx.Conn, error), +) (*pgx.Conn, error) { + waitCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + var lastErr error + for { + conn, err := dial(waitCtx) + if err == nil { + return conn, nil + } + if waitCtx.Err() != nil { + // a dial cut short by the deadline reports the context, not why the server refused + if lastErr == nil { + lastErr = err + } + return nil, waitReadyFailure(ctx, timeout, lastErr) + } + if !retryableConnectError(err) { + return nil, err + } + lastErr = err + select { + case <-waitCtx.Done(): + return nil, waitReadyFailure(ctx, timeout, lastErr) + case <-time.After(interval): + } + } +} + +func waitReadyFailure(ctx context.Context, timeout time.Duration, lastErr error) error { + if ctx.Err() != nil { + return fmt.Errorf("waiting for the server to accept connections: %w; last connect error: %w", + ctx.Err(), lastErr) + } + return fmt.Errorf("the server accepted no connection within %s: %w", timeout, lastErr) +} + +const cannotConnectNow = "57P03" + +func retryableConnectError(err error) bool { + var pgErr *pgconn.PgError + return !errors.As(err, &pgErr) || pgErr.Code == cannotConnectNow +} + func pendingRestartSettings(ctx context.Context, conn *pgx.Conn) ([]string, error) { rows, err := conn.Query(ctx, "SELECT name FROM pg_settings WHERE pending_restart ORDER BY name") if err != nil { diff --git a/internal/provision/config_test.go b/internal/provision/config_test.go new file mode 100644 index 0000000..be1d674 --- /dev/null +++ b/internal/provision/config_test.go @@ -0,0 +1,141 @@ +package provision + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" +) + +func TestRestartAdvice(t *testing.T) { + const loopbackNote = "from this machine only" + tests := []struct { + name string + pending []string + wantLoopback bool + }{ + {"shared_buffers alone", []string{"shared_buffers"}, false}, + {"listen_addresses pending", []string{"listen_addresses", "shared_buffers"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := restartAdvice(tt.pending) + if !strings.Contains(got, "Restart the PostgreSQL service") { + t.Errorf("restartAdvice(%v) = %q, want the restart instruction", tt.pending, got) + } + if hasNote := strings.Contains(got, loopbackNote); hasNote != tt.wantLoopback { + t.Errorf("restartAdvice(%v) = %q, loopback note %v, want %v", tt.pending, got, hasNote, tt.wantLoopback) + } + }) + } +} + +var errRefused = errors.New("connection refused") + +func scriptedDialer( + failures int, + failure error, + conn *pgx.Conn, +) (dial func(context.Context) (*pgx.Conn, error), calls *int) { + calls = new(int) + dial = func(context.Context) (*pgx.Conn, error) { + *calls++ + if *calls <= failures { + return nil, failure + } + return conn, nil + } + return dial, calls +} + +func TestWaitReady(t *testing.T) { + conn := &pgx.Conn{} + wrongPassword := &pgconn.PgError{Code: "28P01", Message: "password authentication failed"} + startingUp := &pgconn.PgError{Code: cannotConnectNow, Message: "the database system is starting up"} + tests := []struct { + name string + failures int + failure error + timeout time.Duration + wantErr error + wantCalls func(calls int) bool + }{ + {"succeeds at once", 0, nil, time.Second, nil, func(calls int) bool { return calls == 1 }}, + {"succeeds after three refusals", 3, errRefused, time.Second, nil, func(calls int) bool { return calls == 4 }}, + {"retries 57P03", 2, startingUp, time.Second, nil, func(calls int) bool { return calls == 3 }}, + { + "times out with the last connect error", 1 << 30, errRefused, 50 * time.Millisecond, errRefused, + func(calls int) bool { return calls > 1 }, + }, + { + "gives up at once on another server error", 1 << 30, wrongPassword, time.Minute, wrongPassword, + func(calls int) bool { return calls == 1 }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dial, calls := scriptedDialer(tt.failures, tt.failure, conn) + got, err := waitReady(context.Background(), tt.timeout, time.Millisecond, dial) + if !tt.wantCalls(*calls) { + t.Errorf("dial called %d times", *calls) + } + if tt.wantErr != nil { + if !errors.Is(err, tt.wantErr) { + t.Fatalf("waitReady() error = %v, want it to wrap %v", err, tt.wantErr) + } + if got != nil { + t.Errorf("waitReady() returned a connection with error %v", err) + } + return + } + if err != nil { + t.Fatalf("waitReady() error = %v", err) + } + if got != conn { + t.Errorf("waitReady() returned %p, want the dialed connection %p", got, conn) + } + }) + } +} + +func TestWaitReadyKeepsTheLastRealErrorAtTheDeadline(t *testing.T) { + calls := 0 + dial := func(ctx context.Context) (*pgx.Conn, error) { + calls++ + if calls == 1 { + return nil, errRefused + } + <-ctx.Done() + return nil, ctx.Err() + } + _, err := waitReady(context.Background(), 50*time.Millisecond, time.Millisecond, dial) + if !errors.Is(err, errRefused) { + t.Fatalf("waitReady() error = %v, want it to wrap %v", err, errRefused) + } +} + +func TestWaitReadyStopsOnCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + calls := 0 + dial := func(context.Context) (*pgx.Conn, error) { + calls++ + if calls == 2 { + cancel() + } + return nil, errRefused + } + _, err := waitReady(ctx, time.Minute, time.Millisecond, dial) + if !errors.Is(err, context.Canceled) { + t.Fatalf("waitReady() error = %v, want it to wrap %v", err, context.Canceled) + } + if !errors.Is(err, errRefused) { + t.Errorf("waitReady() error = %v, want it to keep the last connect error %v", err, errRefused) + } + if calls != 2 { + t.Errorf("dial called %d times, want 2: no dial after the cancellation", calls) + } +} diff --git a/internal/provision/provision.go b/internal/provision/provision.go index 7874102..68d1aea 100644 --- a/internal/provision/provision.go +++ b/internal/provision/provision.go @@ -7,6 +7,7 @@ import ( "net" "net/url" "regexp" + "slices" "strconv" "strings" @@ -34,18 +35,26 @@ type Options struct { WriterPassword string PlotPassword string ExpectedMajor int + RestartService string } var databaseNamePattern = regexp.MustCompile(`^[a-z_][a-z0-9_]*$`) +var loopbackHosts = []string{"localhost", "127.0.0.1", "::1"} + func (o Options) Validate() error { if !databaseNamePattern.MatchString(o.Database) { return fmt.Errorf("database name %q must match %s", o.Database, databaseNamePattern) } + if o.RestartService != "" && !slices.Contains(loopbackHosts, o.Host) { + return fmt.Errorf("--restart-service needs --host to be one of %s, not %q: after the "+ + "restart the server listens on the loopback interface only", + strings.Join(loopbackHosts, ", "), o.Host) + } return nil } -// Site brings an installation machine to its commissioned state: memory tuning, then +// Site brings an installation machine to its commissioned state: server settings, then // everything the archive needs. func (o Options) Site(ctx context.Context) error { if err := o.config(ctx); err != nil { @@ -57,8 +66,8 @@ func (o Options) Site(ctx context.Context) error { return o.check(ctx) } -// Bench brings a throwaway container to the same state minus the tuning: its memory -// constants are sized for an installation machine and mean nothing to a container. +// Bench brings a throwaway container to the same state minus the server settings: they are +// sized for an installation machine, and a loopback-only listener would shut out its clients. func (o Options) Bench(ctx context.Context) error { if err := o.create(ctx); err != nil { return err diff --git a/internal/provision/provision_test.go b/internal/provision/provision_test.go index 5ce7ecb..2ad59cc 100644 --- a/internal/provision/provision_test.go +++ b/internal/provision/provision_test.go @@ -10,6 +10,35 @@ import ( "github.com/jackc/pgx/v5/pgconn" ) +func TestValidateRestartServiceHost(t *testing.T) { + tests := []struct { + host string + wantErr bool + }{ + {"localhost", false}, + {"127.0.0.1", false}, + {"::1", false}, + {"db.internal", true}, + {"192.168.1.10", true}, + {"/var/run/postgresql", true}, + } + for _, tt := range tests { + t.Run(tt.host, func(t *testing.T) { + options := Options{Database: "scada_archive", Host: tt.host, RestartService: "postgresql-x64-17"} + err := options.Validate() + if gotErr := err != nil; gotErr != tt.wantErr { + t.Errorf("Validate() with host %q: error = %v, wantErr %v", tt.host, err, tt.wantErr) + } + if err != nil && !strings.Contains(err.Error(), "loopback") { + t.Errorf("Validate() error = %q, want it to name the loopback rule", err) + } + }) + } + if err := (Options{Database: "scada_archive", Host: "db.internal"}).Validate(); err != nil { + t.Errorf("Validate() without --restart-service refused a remote host: %v", err) + } +} + func TestValidate(t *testing.T) { tests := []struct { name string diff --git a/internal/provision/service_other.go b/internal/provision/service_other.go new file mode 100644 index 0000000..7f9cff4 --- /dev/null +++ b/internal/provision/service_other.go @@ -0,0 +1,26 @@ +//go:build !windows + +package provision + +import ( + "context" + "errors" + "fmt" + "runtime" +) + +type managedService struct{} + +func openService( + _ context.Context, + name string, + _ func(context.Context) (runningServer, error), +) (*managedService, error) { + return nil, fmt.Errorf("--restart-service %s: service restart is not supported on %s", name, runtime.GOOS) +} + +func (*managedService) close() {} + +func (*managedService) restart(context.Context) error { + return errors.New("service restart is not supported on " + runtime.GOOS) +} diff --git a/internal/provision/service_other_test.go b/internal/provision/service_other_test.go new file mode 100644 index 0000000..50800ad --- /dev/null +++ b/internal/provision/service_other_test.go @@ -0,0 +1,25 @@ +//go:build !windows + +package provision + +import ( + "context" + "runtime" + "strings" + "testing" +) + +func TestOpenServiceIsRefusedOffWindows(t *testing.T) { + describe := func(context.Context) (runningServer, error) { + t.Fatal("openService() described the server off Windows") + return runningServer{}, nil + } + service, err := openService(context.Background(), "postgresql-x64-17", describe) + if err == nil { + service.close() + t.Fatal("openService() succeeded off Windows") + } + if want := "not supported on " + runtime.GOOS; !strings.Contains(err.Error(), want) { + t.Errorf("openService() error = %q, want it to contain %q", err, want) + } +} diff --git a/internal/provision/service_windows.go b/internal/provision/service_windows.go new file mode 100644 index 0000000..02bd440 --- /dev/null +++ b/internal/provision/service_windows.go @@ -0,0 +1,373 @@ +//go:build windows + +package provision + +import ( + "context" + "errors" + "fmt" + "net" + "path/filepath" + "strings" + "time" + + "golang.org/x/sys/windows" + "golang.org/x/sys/windows/svc" + "golang.org/x/sys/windows/svc/mgr" +) + +// serviceControl is the part of *mgr.Service the restart drives, so tests can script the states. +type serviceControl interface { + Query() (svc.Status, error) + Control(c svc.Cmd) (svc.Status, error) + Start(args ...string) error + Config() (mgr.Config, error) +} + +type serviceTimings struct { + // a shutdown checkpoint with max_wal_size = 8GB can be long + stop time.Duration + start time.Duration + poll time.Duration + // each accepted probe connection costs the server a process + probe time.Duration +} + +func defaultServiceTimings() serviceTimings { + return serviceTimings{ + stop: 3 * time.Minute, + start: 2 * time.Minute, + poll: 500 * time.Millisecond, + probe: time.Second, + } +} + +// Windows resends a SYN after an RST, so even a refused loopback connect can take seconds +const listenerDialTimeout = 15 * time.Second + +type managedService struct { + name string + control serviceControl + address string + // listening reports whether a TCP connect to address succeeds; false with a nil error means + // the connect was refused, so no postmaster listens there + listening func(context.Context) (bool, error) + timings serviceTimings + release func() +} + +// openService refuses a service the restart could not complete or that is not the server the run +// connects to. +func openService( + ctx context.Context, + name string, + describe func(context.Context) (runningServer, error), +) (*managedService, error) { + manager, err := mgr.Connect() + if err != nil { + if errors.Is(err, windows.ERROR_ACCESS_DENIED) { + return nil, fmt.Errorf("service %s: access to the service control manager denied; "+ + "run semibase as an administrator", name) + } + return nil, fmt.Errorf("service %s: connecting to the service control manager: %w", name, err) + } + handle, err := manager.OpenService(name) + if err != nil { + _ = manager.Disconnect() + switch { + case errors.Is(err, windows.ERROR_SERVICE_DOES_NOT_EXIST): + return nil, fmt.Errorf("service %s does not exist", name) + case errors.Is(err, windows.ERROR_ACCESS_DENIED): + return nil, fmt.Errorf("service %s: access denied; run semibase as an administrator", name) + } + return nil, fmt.Errorf("opening service %s: %w", name, err) + } + service := &managedService{ + name: name, + control: handle, + timings: defaultServiceTimings(), + release: func() { + _ = handle.Close() + _ = manager.Disconnect() + }, + } + if err := service.check(ctx, describe); err != nil { + service.close() + return nil, err + } + service.listening = dialListener(service.address) + return service, nil +} + +func (s *managedService) check( + ctx context.Context, + describe func(context.Context) (runningServer, error), +) error { + config, err := s.control.Config() + if err != nil { + return fmt.Errorf("reading the configuration of service %s: %w", s.name, err) + } + if config.StartType == mgr.StartDisabled { + return fmt.Errorf("service %s is Disabled, so it could not be started again after the stop; "+ + "set its start type to Automatic or Manual", s.name) + } + if err := s.awaitRunning(ctx); err != nil { + return err + } + return s.confirmServes(ctx, config.BinaryPathName, describe) +} + +// confirmServes ties the service to the server behind --host:--port through the data directory: +// stopping another service would leave it stopped, since the probe would keep finding the +// run's server answering. +func (s *managedService) confirmServes( + ctx context.Context, + pathName string, + describe func(context.Context) (runningServer, error), +) error { + serviceDirectory, found := dataDirectoryFlag(pathName) + if !found { + return fmt.Errorf("the PathName of service %s carries no -D, so site cannot tell which data "+ + "directory it runs; site restarts a service registered with pg_ctl register -D", s.name) + } + server, err := describe(ctx) + if err != nil { + return err + } + if !sameDataDirectory(serviceDirectory, server.dataDirectory) { + return fmt.Errorf("service %s runs data directory %s, but the server on %s uses %s: "+ + "--restart-service names another server. Find the service whose PathName carries "+ + "-D %s with Get-CimInstance Win32_Service", s.name, serviceDirectory, server.address, + server.dataDirectory, filepath.Clean(server.dataDirectory)) + } + ok("service %s runs data directory %s, as the server on %s does", s.name, serviceDirectory, + server.address) + s.address = server.address + return nil +} + +// dataDirectoryFlag reads the -D argument of a service command line, as pg_ctl register writes it. +func dataDirectoryFlag(pathName string) (string, bool) { + args, err := windows.DecomposeCommandLine(pathName) + if err != nil { + return "", false + } + for i, arg := range args { + switch { + case arg == "-D" && i+1 < len(args): + return args[i+1], true + case strings.HasPrefix(arg, "-D") && len(arg) > len("-D"): + return strings.TrimPrefix(arg, "-D"), true + } + } + return "", false +} + +// sameDataDirectory compares the service's native path with the server's, which PostgreSQL +// reports with forward slashes. +func sameDataDirectory(servicePath, serverPath string) bool { + return strings.EqualFold(filepath.Clean(servicePath), filepath.Clean(serverPath)) +} + +func dialListener(address string) func(context.Context) (bool, error) { + return func(ctx context.Context) (bool, error) { + dialer := net.Dialer{Timeout: listenerDialTimeout} + conn, err := dialer.DialContext(ctx, "tcp", address) + switch { + case err == nil: + _ = conn.Close() + return true, nil + case errors.Is(err, windows.WSAECONNREFUSED): + return false, nil + } + return false, err + } +} + +// awaitRunning lets a start in progress finish: an installer may run site right after it started +// the service, and an EDB service reports Running only once the server accepts connections. +func (s *managedService) awaitRunning(ctx context.Context) error { + waitCtx, cancel := context.WithTimeout(ctx, s.timings.start) + defer cancel() + state, err := s.pollUntil(waitCtx, func(state svc.State) bool { return state != svc.StartPending }) + switch { + case errors.Is(err, context.DeadlineExceeded): + return fmt.Errorf("service %s is still start pending after %s: %w", s.name, s.timings.start, err) + case err != nil: + return err + case state != svc.Running: + return fmt.Errorf("service %s is %s, not running; site restarts a running service only", + s.name, stateName(state)) + } + return nil +} + +func (s *managedService) close() { + s.release() +} + +// restart detaches from the command's cancellation once the Stop is accepted: the exit wait, +// the Start and the Running wait each run to their own bound, so an interrupt or the command +// timeout does not leave the service stopped. Residual cases: docs/architecture/provisioning.md. +func (s *managedService) restart(ctx context.Context) error { + if _, err := s.control.Control(svc.Stop); err != nil { + return fmt.Errorf("stopping service %s: %w", s.name, err) + } + detached := context.WithoutCancel(ctx) + if err := s.awaitServerExit(detached); err != nil { + return err + } + ok("service %s stopped, nothing answers on %s", s.name, s.address) + if err := s.control.Start(); err != nil { + return fmt.Errorf("starting service %s: %w; it stays stopped: start it with Start-Service %s", + s.name, err, s.name) + } + if err := s.awaitStarted(detached); err != nil { + return err + } + ok("service %s running", s.name) + if err := ctx.Err(); err != nil { + return fmt.Errorf("service %s restarted, then the run stopped: %w", s.name, err) + } + return nil +} + +// awaitServerExit gives Stopped and the closed port one bound from the Stop: pg_ctl reports +// Stopped after at most a minute even while the postmaster still shuts down, and a Start then +// would meet the old postmaster's lock file. +func (s *managedService) awaitServerExit(ctx context.Context) error { + waitCtx, cancel := context.WithTimeout(ctx, s.timings.stop) + defer cancel() + if err := s.awaitStopped(waitCtx); err != nil { + return err + } + return s.awaitListenerClosed(waitCtx) +} + +func (s *managedService) awaitStopped(ctx context.Context) error { + state, err := s.pollUntil(ctx, func(state svc.State) bool { return state == svc.Stopped }) + switch { + case errors.Is(err, context.DeadlineExceeded): + return fmt.Errorf("service %s is still %s %s after the Stop and will stay stopped; once "+ + "Get-Service %s shows Stopped, start it with Start-Service %s: %w", + s.name, stateName(state), s.timings.stop, s.name, s.name, err) + case err != nil: + return fmt.Errorf("%w after the Stop; check it with Get-Service %s and, if it is stopped, "+ + "start it with Start-Service %s", err, s.name, s.name) + } + return nil +} + +func (s *managedService) awaitListenerClosed(ctx context.Context) error { + answered := false + var lastErr error + for { + answers, err := s.listening(ctx) + if !answers && err == nil { + return nil + } + // a dial cut short by the deadline says nothing about the server + if ctx.Err() == nil { + answered, lastErr = answers, err + } + select { + case <-ctx.Done(): + return s.serverOutlivedStop(answered, lastErr, ctx.Err()) + case <-time.After(s.timings.probe): + } + } +} + +func (s *managedService) serverOutlivedStop(answered bool, lastErr, cause error) error { + observed := "the server still answers on " + s.address + if !answered { + observed = fmt.Sprintf("connecting to %s still fails without a refusal (%v)", s.address, lastErr) + } + return fmt.Errorf("service %s is stopped, but %s %s after the Stop: the old server may still be "+ + "shutting down, so site sends no Start and the service stays stopped. Once a connection to "+ + "%s is refused, start it with Start-Service %s: %w", + s.name, observed, s.timings.stop, s.address, s.name, cause) +} + +// awaitStarted fails at once on Stopped from the second query on: pg_ctl reports Stopped when the +// postmaster exits during its startup or when its own startup wait runs out. +func (s *managedService) awaitStarted(ctx context.Context) error { + waitCtx, cancel := context.WithTimeout(ctx, s.timings.start) + defer cancel() + queries := 0 + state, err := s.pollUntil(waitCtx, func(state svc.State) bool { + queries++ + return state == svc.Running || (state == svc.Stopped && queries > 1) + }) + switch { + case errors.Is(err, context.DeadlineExceeded): + return fmt.Errorf("service %s did not reach running within %s, last state %s: %w", + s.name, s.timings.start, stateName(state), err) + case err != nil: + return err + case state == svc.Stopped: + return s.stoppedAfterStart(ctx) + } + return nil +} + +func (s *managedService) stoppedAfterStart(ctx context.Context) error { + answers, err := s.listening(ctx) + switch { + case answers: + return fmt.Errorf("service %s stopped again after the Start, but the server answers on %s: "+ + "pg_ctl ran out of its startup wait (60 s unless the service's PathName sets -t) and "+ + "left the server running outside the service. Once it accepts connections, stop it "+ + "with pg_ctl stop -D , then start it with Start-Service %s", + s.name, s.address, s.name) + case err != nil: + return fmt.Errorf("service %s stopped again after the Start, and checking %s failed: %w. "+ + "Read the server log in the data directory's log folder and the Windows Application "+ + "event log", s.name, s.address, err) + } + return fmt.Errorf("service %s stopped again after the Start, and nothing answers on %s: the "+ + "server exited during its startup. Read the server log in the data directory's log folder "+ + "and the Windows Application event log. If it names a setting this run wrote, remove that "+ + "line from postgresql.auto.conf, then start the service with Start-Service %s; a re-run "+ + "of site writes the line again and stops the service the same way until the cause is fixed", + s.name, s.address, s.name) +} + +func (s *managedService) pollUntil(ctx context.Context, done func(svc.State) bool) (svc.State, error) { + var last svc.State + for { + status, err := s.control.Query() + if err != nil { + return last, fmt.Errorf("querying service %s: %w", s.name, err) + } + last = status.State + if done(last) { + return last, nil + } + select { + case <-ctx.Done(): + return last, ctx.Err() + case <-time.After(s.timings.poll): + } + } +} + +func stateName(state svc.State) string { + switch state { + case svc.Stopped: + return "stopped" + case svc.StartPending: + return "start pending" + case svc.StopPending: + return "stop pending" + case svc.Running: + return "running" + case svc.ContinuePending: + return "continue pending" + case svc.PausePending: + return "pause pending" + case svc.Paused: + return "paused" + } + return fmt.Sprintf("state %d", state) +} diff --git a/internal/provision/service_windows_test.go b/internal/provision/service_windows_test.go new file mode 100644 index 0000000..439a60b --- /dev/null +++ b/internal/provision/service_windows_test.go @@ -0,0 +1,420 @@ +//go:build windows + +package provision + +import ( + "cmp" + "context" + "errors" + "net" + "strings" + "testing" + "time" + + "golang.org/x/sys/windows/svc" + "golang.org/x/sys/windows/svc/mgr" +) + +type probeResult struct { + answers bool + err error +} + +var ( + answering = probeResult{answers: true} + refusing = probeResult{} +) + +// fakeService answers Query with states and the listener probe with probes, each in order and +// repeating its last entry; no probes means a refused connect. +type fakeService struct { + config mgr.Config + states []svc.State + probes []probeResult + queries int + probeCalls int + controlErr error + startErr error + stops int + starts int + probesAtStart int + startAfterRefusal bool +} + +func (f *fakeService) Query() (svc.Status, error) { + state := f.states[min(f.queries, len(f.states)-1)] + f.queries++ + return svc.Status{State: state}, nil +} + +func (f *fakeService) Config() (mgr.Config, error) { + return f.config, nil +} + +func (f *fakeService) Control(svc.Cmd) (svc.Status, error) { + f.stops++ + return svc.Status{State: svc.StopPending}, f.controlErr +} + +func (f *fakeService) Start(...string) error { + f.starts++ + f.probesAtStart = f.probeCalls + f.startAfterRefusal = f.probeCalls > 0 && f.lastProbe() == refusing + return f.startErr +} + +func (f *fakeService) listening(context.Context) (bool, error) { + f.probeCalls++ + result := f.lastProbe() + return result.answers, result.err +} + +func (f *fakeService) lastProbe() probeResult { + if len(f.probes) == 0 { + return refusing + } + return f.probes[min(f.probeCalls, len(f.probes))-1] +} + +func newFakeManagedService(fake *fakeService) *managedService { + return &managedService{ + name: "pg", + control: fake, + address: "[::1]:5432", + listening: fake.listening, + timings: serviceTimings{ + stop: 50 * time.Millisecond, start: 50 * time.Millisecond, + poll: time.Millisecond, probe: time.Millisecond, + }, + release: func() {}, + } +} + +func cancelledContext() context.Context { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + return ctx +} + +func expiredContext() context.Context { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + cancel() + return ctx +} + +var fullRestart = []svc.State{svc.StopPending, svc.StopPending, svc.Stopped, svc.StartPending, svc.Running} + +func TestRestart(t *testing.T) { + tests := []struct { + name string + ctx context.Context + states []svc.State + probes []probeResult + controlErr error + startErr error + wantStarts int + wantProbesAtStart int + wantErr []string + wantNotErr string + wantCause error + }{ + {name: "stops and starts", ctx: context.Background(), states: fullRestart, wantStarts: 1}, + { + name: "an interrupt after the Stop still starts the service", ctx: cancelledContext(), + states: fullRestart, wantStarts: 1, wantErr: []string{"restarted, then the run stopped"}, + wantCause: context.Canceled, + }, + { + name: "the command timeout after the Stop still starts the service", ctx: expiredContext(), + states: fullRestart, wantStarts: 1, wantErr: []string{"restarted, then the run stopped"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "a refused Stop changes nothing", ctx: context.Background(), states: []svc.State{svc.Running}, + controlErr: errors.New("access denied"), wantErr: []string{"stopping service pg"}, + }, + { + name: "a stop past its bound says the service will stay stopped", ctx: context.Background(), + states: []svc.State{svc.StopPending}, wantErr: []string{"will stay stopped", "Start-Service pg"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "Stopped while the server still answers: Start waits for the refusal", ctx: context.Background(), + states: fullRestart, probes: []probeResult{answering, answering, refusing}, + wantStarts: 1, wantProbesAtStart: 3, + }, + { + name: "a probe error is retried until the refusal", ctx: context.Background(), + states: fullRestart, probes: []probeResult{{err: errors.New("i/o timeout")}, refusing}, + wantStarts: 1, wantProbesAtStart: 2, + }, + { + name: "a server that outlives the stop bound gets no Start", ctx: context.Background(), + states: []svc.State{svc.Stopped}, probes: []probeResult{answering}, + wantErr: []string{"still answers on [::1]:5432", "sends no Start", "Start-Service pg"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "a probe that never gets a refusal gets no Start", ctx: context.Background(), + states: []svc.State{svc.Stopped}, probes: []probeResult{{err: errors.New("i/o timeout")}}, + wantErr: []string{"without a refusal (i/o timeout)", "Start-Service pg"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "a refused Start says how to start it", ctx: context.Background(), + states: []svc.State{svc.Stopped}, startErr: errors.New("start refused"), wantStarts: 1, + wantErr: []string{"stays stopped", "Start-Service pg"}, + }, + { + name: "a server that exits during its startup fails at once", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.StartPending, svc.Stopped}, wantStarts: 1, + wantErr: []string{ + "stopped again after the Start", "nothing answers", "postgresql.auto.conf", + "Start-Service pg", "writes the line again", + }, + }, + { + name: "a startup pg_ctl stopped waiting for is not blamed on the settings", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.StartPending, svc.Stopped}, + probes: []probeResult{refusing, answering}, wantStarts: 1, + wantErr: []string{"stopped again after the Start", "outside the service", "pg_ctl stop -D"}, + wantNotErr: "postgresql.auto.conf", + }, + { + name: "Stopped right after the Start is given one more query", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.Stopped, svc.Running}, wantStarts: 1, + }, + { + name: "a start past its bound names the last state", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.StartPending}, wantStarts: 1, + wantErr: []string{"did not reach running", "start pending"}, wantCause: context.DeadlineExceeded, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fake := &fakeService{ + states: tt.states, probes: tt.probes, controlErr: tt.controlErr, startErr: tt.startErr, + } + err := newFakeManagedService(fake).restart(tt.ctx) + if fake.stops != 1 { + t.Errorf("Stop sent %d times, want 1", fake.stops) + } + if fake.starts != tt.wantStarts { + t.Errorf("Start sent %d times, want %d", fake.starts, tt.wantStarts) + } + if fake.starts > 0 && !fake.startAfterRefusal { + t.Errorf("Start sent after %d probes, the last not a refusal", fake.probesAtStart) + } + if tt.wantProbesAtStart > 0 && fake.probesAtStart != tt.wantProbesAtStart { + t.Errorf("Start sent after %d probes, want %d", fake.probesAtStart, tt.wantProbesAtStart) + } + assertServiceError(t, err, tt.wantErr, tt.wantCause) + if tt.wantNotErr != "" && err != nil && strings.Contains(err.Error(), tt.wantNotErr) { + t.Errorf("error = %q, want it not to contain %q", err, tt.wantNotErr) + } + }) + } +} + +const runnerPathName = `"C:\Program Files\PostgreSQL\17\bin\pg_ctl.exe" runservice -N "pg" -D "C:\PostgreSQL\17\data" -w` + +var runnerServer = runningServer{dataDirectory: "C:/PostgreSQL/17/data", address: "[::1]:5432"} + +func TestCheck(t *testing.T) { + tests := []struct { + name string + startType uint32 + pathName string + states []svc.State + server runningServer + describeErr error + wantErr []string + wantCause error + wantDescribe bool + }{ + { + name: "the running service of the run's server", pathName: runnerPathName, + states: []svc.State{svc.Running}, server: runnerServer, wantDescribe: true, + }, + { + name: "a start in progress is waited out", pathName: runnerPathName, + states: []svc.State{svc.StartPending, svc.StartPending, svc.Running}, server: runnerServer, + wantDescribe: true, + }, + { + name: "Disabled is refused", startType: mgr.StartDisabled, pathName: runnerPathName, + states: []svc.State{svc.Running}, wantErr: []string{"is Disabled"}, + }, + { + name: "a start past its bound is refused", pathName: runnerPathName, + states: []svc.State{svc.StartPending}, wantErr: []string{"still start pending"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "stopped is refused", pathName: runnerPathName, states: []svc.State{svc.Stopped}, + wantErr: []string{"is stopped, not running"}, + }, + { + name: "stopping is refused", pathName: runnerPathName, states: []svc.State{svc.StopPending}, + wantErr: []string{"is stop pending"}, + }, + { + name: "paused is refused", pathName: runnerPathName, states: []svc.State{svc.Paused}, + wantErr: []string{"is paused"}, + }, + { + name: "a start that fails is refused", pathName: runnerPathName, + states: []svc.State{svc.StartPending, svc.Stopped}, wantErr: []string{"is stopped, not running"}, + }, + { + name: "a service with no -D is refused", pathName: `C:\Scada\scada.exe --service`, + states: []svc.State{svc.Running}, wantErr: []string{"carries no -D"}, + }, + { + name: "a service of another data directory is refused", pathName: runnerPathName, + states: []svc.State{svc.Running}, wantDescribe: true, + server: runningServer{dataDirectory: "C:/Program Files/PostgreSQL/14/data", address: "[::1]:5432"}, + wantErr: []string{ + `runs data directory C:\PostgreSQL\17\data`, "names another server", + `-D C:\Program Files\PostgreSQL\14\data`, + }, + }, + { + name: "a failed read of the server is returned", pathName: runnerPathName, + states: []svc.State{svc.Running}, describeErr: errors.New("connection refused"), + wantDescribe: true, wantErr: []string{"connection refused"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fake := &fakeService{ + config: mgr.Config{StartType: cmp.Or(tt.startType, mgr.StartManual), BinaryPathName: tt.pathName}, + states: tt.states, + } + described := false + describe := func(context.Context) (runningServer, error) { + described = true + return tt.server, tt.describeErr + } + service := newFakeManagedService(fake) + service.address = "" + err := service.check(context.Background(), describe) + assertServiceError(t, err, tt.wantErr, tt.wantCause) + if described != tt.wantDescribe { + t.Errorf("described the server = %v, want %v", described, tt.wantDescribe) + } + if err == nil && service.address != tt.server.address { + t.Errorf("address = %q, want the connection's %q", service.address, tt.server.address) + } + if fake.stops != 0 || fake.starts != 0 { + t.Errorf("the check sent %d Stop and %d Start, want none", fake.stops, fake.starts) + } + }) + } +} + +func TestDataDirectoryFlag(t *testing.T) { + tests := []struct { + name string + pathName string + want string + wantFound bool + }{ + { + name: "pg_ctl register, quoted with spaces", + pathName: `"C:\Program Files\PostgreSQL\14\bin\pg_ctl.exe" runservice -N "postgresql-14" -D "C:\Program Files\PostgreSQL\14\data" -w`, + want: `C:\Program Files\PostgreSQL\14\data`, wantFound: true, + }, + { + name: "unquoted", + pathName: `C:\pg\bin\pg_ctl.exe runservice -D C:\pg\data -w`, + want: `C:\pg\data`, wantFound: true, + }, + { + name: "value attached to the flag", + pathName: `C:\pg\bin\pg_ctl.exe runservice -DC:\pg\data`, + want: `C:\pg\data`, wantFound: true, + }, + {name: "no -D", pathName: `"C:\pg\bin\pg_ctl.exe" runservice -N "pg" -w`}, + {name: "-D with no value", pathName: `C:\pg\bin\pg_ctl.exe runservice -D`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, found := dataDirectoryFlag(tt.pathName) + if got != tt.want || found != tt.wantFound { + t.Errorf("dataDirectoryFlag() = %q, %v, want %q, %v", got, found, tt.want, tt.wantFound) + } + }) + } +} + +func TestSameDataDirectory(t *testing.T) { + const server = "C:/PostgreSQL/17/data" + tests := []struct { + name string + servicePath string + want bool + }{ + {name: "forward slashes on the server side", servicePath: `C:\PostgreSQL\17\data`, want: true}, + {name: "case", servicePath: `c:\postgresql\17\DATA`, want: true}, + {name: "trailing separator", servicePath: `C:\PostgreSQL\17\data\`, want: true}, + {name: "another directory", servicePath: `C:\Program Files\PostgreSQL\14\data`}, + {name: "a longer name with the same prefix", servicePath: `C:\PostgreSQL\17\data2`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := sameDataDirectory(tt.servicePath, server); got != tt.want { + t.Errorf("sameDataDirectory(%q, %q) = %v, want %v", tt.servicePath, server, got, tt.want) + } + }) + } +} + +// The probe dials one literal address, so the other loopback family cannot decide the answer. +func TestDialListener(t *testing.T) { + for _, host := range []string{"127.0.0.1", "::1"} { + t.Run(host+" answers, then refuses", func(t *testing.T) { + listener, err := net.Listen("tcp", net.JoinHostPort(host, "0")) + if err != nil { + t.Skipf("no %s listener on this machine: %v", host, err) + } + probe := dialListener(listener.Addr().String()) + if answers, err := probe(context.Background()); !answers || err != nil { + t.Errorf("with a listener: answers = %v, error = %v, want true, nil", answers, err) + } + if err := listener.Close(); err != nil { + t.Fatal(err) + } + if answers, err := probe(context.Background()); answers || err != nil { + t.Errorf("after the listener closed: answers = %v, error = %v, want false, nil", answers, err) + } + }) + } + t.Run("a connect error other than a refusal is not taken as gone", func(t *testing.T) { + answers, err := dialListener("127.0.0.1:0")(context.Background()) + if answers || err == nil { + t.Errorf("answers = %v, error = %v, want false and an error", answers, err) + } + }) +} + +func assertServiceError(t *testing.T, err error, wantText []string, wantCause error) { + t.Helper() + if wantText == nil { + if err != nil { + t.Fatalf("error = %v, want nil", err) + } + return + } + if err == nil { + t.Fatalf("error = nil, want one containing %q", wantText) + } + for _, text := range wantText { + if !strings.Contains(err.Error(), text) { + t.Errorf("error = %q, want it to contain %q", err, text) + } + } + if wantCause != nil && !errors.Is(err, wantCause) { + t.Errorf("error = %v, want it to wrap %v", err, wantCause) + } +}