Aias00 commented on code in PR #995:
URL: https://github.com/apache/dubbo-go-pixiu/pull/995#discussion_r3565778773
##########
admin/core/server.go:
##########
@@ -48,9 +48,13 @@ type server interface {
}
// RunServer start server
-func RunServer() {
+func RunServer() error {
// load config
- global.VP = Viper()
+ vp, err := Viper()
Review Comment:
Fixed in `e6a14199`.
`configPath` is now threaded explicitly through the whole chain:
`Start(configPath)` → `RunServer(configPath)` → `Viper(configPath)`. The CLI's
`--config/-c` value (already held in `cmd/admin.configPath` and defaulted in
`initDefaultValue()`) is passed into `Start(configPath)` in `RunE`, so a
user-specified admin config file is loaded instead of being silently ignored.
In `Viper` I also removed the stdlib `flag.StringVar`/`flag.Parse()`
fallback (it conflicted with cobra's flag ownership and was the root cause of
the path being ignored). `Viper(configPath)` now resolves the path purely from
the argument, falling back to the `GVA_CONFIG` env var and then the default
`config.yaml` only when `configPath` is empty.
Tests added in `admin/core/server_test.go`:
- `TestViper_NonDefaultPathHonored` — writes a temp `config.yaml` with
`system.addr: 9999`, calls `Viper(tmpPath)`, asserts `global.CONFIG.System.Addr
== 9999` and that `vp.ConfigFileUsed()` is the explicit path (not the default).
- `TestViper_MissingFileReturnsError` — a non-existent explicit path returns
a non-nil error (not silently defaulted).
##########
admin/core/server.go:
##########
@@ -83,4 +87,5 @@ func RunServer() {
})
wg.Wait()
+ return nil
Review Comment:
Fixed in `e6a14199`.
Worker startup errors now propagate to the CLI. The HTTP server and xDS
server run under a new `runCoordinated(ctx, ...workers)` harness: the first
worker to return a non-nil error cancels its peer and that error is returned by
`RunServer`. Concretely:
- `runHTTPServer` returns `ListenAndServe` failures (e.g. port already in
use), wrapped as `"http server: %w"`, instead of only logging.
- `StartxDsServer(ctx)` / `runXDSServer(ctx, ...)` take the context; the
initial `net.Listen` error (port-bind failure) is returned, and on peer-cancel
the gRPC server does `GracefulStop()` with the `Serve` error drained.
- The HTTP peer shuts down via `http.Server.Shutdown(ctx)` when the xDS
server fails (first real `ErrServerClosed` trigger in the package), so neither
worker lingers and the process exits non-zero instead of hanging on `wg.Wait()`.
Tests added in `admin/core/server_test.go`:
`TestRunCoordinated_FirstErrorWins`,
`TestRunCoordinated_OnlyFirstErrorPropagated`,
`TestRunCoordinated_NoErrorReturnsNil`,
`TestRunCoordinated_ParentCancelPropagates`, and
`TestRunXDSServer_PortBindFailure` (occupies a port on all interfaces, asserts
`runXDSServer` returns the bind error rather than hanging).
Side effect: drops the `v.marlon.life/toolkit/util` (`WaitGroupWrapper`)
fork dependency from `server.go` in favor of `sync.WaitGroup` + `context`,
aligning with develop (PR #984).
`go build`, `go vet`, `go test -race`, `gofmt`, and `golangci-lint` are all
clean on `admin/core` and `cmd/admin`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
