From c8163463a1ecdbae9ee110b9fc61538232347839 Mon Sep 17 00:00:00 2001 From: Scott Pledger Date: Tue, 16 Jun 2026 10:40:13 -0600 Subject: [PATCH 1/2] feat: Allow for external services and declarative port binding --- cmd/svcinit/main.go | 204 +++++++++-------- docs/itest.md | 106 ++++++++- go.mod | 2 +- go.sum | 2 + itest.bzl | 49 +++++ private/itest.bzl | 269 ++++++++++++++++++++++- runner/runner.go | 45 ++-- runner/service_instance.go | 35 +++ svcctl/svcctl.go | 42 +++- svclib/ports.go | 47 ++++ svclib/types.go | 27 ++- tests/BUILD.bazel | 4 +- tests/ports/BUILD.bazel | 148 +++++++++++++ tests/ports/ports_test.go | 127 +++++++++++ tests/ports/tests.bzl | 23 ++ tests/so_reuseport/so_reuseport_test.go | 2 +- tests/so_reuseport/so_reuseport_test.mjs | 2 +- tests/svcctl/client.go | 56 +++++ 18 files changed, 1065 insertions(+), 125 deletions(-) create mode 100644 tests/ports/BUILD.bazel create mode 100644 tests/ports/ports_test.go create mode 100644 tests/ports/tests.bzl diff --git a/cmd/svcinit/main.go b/cmd/svcinit/main.go index cc40ec1..6a29366 100644 --- a/cmd/svcinit/main.go +++ b/cmd/svcinit/main.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "log" - "maps" "math" "net" "os" @@ -119,9 +118,19 @@ func main() { listener, err := net.Listen("tcp", "127.0.0.1:0") must(err) - ports, err := assignPorts(unversionedSpecs) + portsMap, servicesMap, err := assignPorts(unversionedSpecs) must(err) + // Expose the rich port/service maps. These are inherited by both the test binary and + // all child services since they spawn with os.Environ() as their base. + serializedPortsMap, err := portsMap.Marshal() + must(err) + os.Setenv("ITEST_PORTS_MAP", string(serializedPortsMap)) + + serializedServicesMap, err := servicesMap.Marshal() + must(err) + os.Setenv("ITEST_SERVICES_MAP", string(serializedServicesMap)) + svcctlPort := listener.Addr().(*net.TCPAddr).Port svcctlPortStr := strconv.Itoa(svcctlPort) os.Setenv("SVCCTL_PORT", svcctlPortStr) @@ -132,7 +141,7 @@ func main() { defer os.Remove("/tmp/svcctl_port") } - serviceSpecs, err := augmentServiceSpecs(unversionedSpecs, ports, svcctlPortStr) + serviceSpecs, err := augmentServiceSpecs(unversionedSpecs, portsMap, svcctlPortStr) must(err) ctx, cancelFunc := context.WithCancel(context.Background()) @@ -152,7 +161,7 @@ func main() { go func() { defer listener.Close() - err := svcctl.Serve(ctx, listener, r, ports, servicesErrCh) + err := svcctl.Serve(ctx, listener, r, portsMap, servicesMap, servicesErrCh) if err != nil { log.Fatalf("svcctl.Serve: %v", err) } @@ -205,7 +214,7 @@ func main() { // Bazel's args attribute converts $$ to $, so args arrive with // single-$ placeholders (e.g. ${@@//:svc}) unlike env/spec files // which preserve the literal $$ since they're read from JSON. - argReplacements := buildReplacements(ports, "${") + argReplacements := buildReplacements(portsMap, "${") testArgs := make([]string, len(os.Args[1:])) for i, arg := range os.Args[1:] { testArgs[i] = replaceAll(arg, argReplacements) @@ -213,7 +222,7 @@ func main() { testPath, err := runfiles.Rlocation(os.Getenv("SVCINIT_TEST_RLOCATION_PATH")) must(err) - testEnv, err := buildTestEnv(ports) + testEnv, err := buildTestEnv(portsMap) must(err) fmt.Println("") @@ -280,7 +289,7 @@ func main() { unversionedSpecs, err := readServiceSpecs(serviceSpecsPath) must(err) - serviceSpecs, err := augmentServiceSpecs(unversionedSpecs, ports, svcctlPortStr) + serviceSpecs, err := augmentServiceSpecs(unversionedSpecs, portsMap, svcctlPortStr) must(err) testCancel() @@ -370,21 +379,66 @@ func readServiceSpecs( func assignPorts( serviceSpecs map[string]svclib.ServiceSpec, ) ( - svclib.Ports, error, + svclib.PortsMap, svclib.ServicesMap, error, ) { var toClose []net.Listener - ports := svclib.Ports{} + portsMap := svclib.PortsMap{} + servicesMap := svclib.ServicesMap{} + + // Tracks which service bound each port target, so we can enforce that a port is only + // ever bound once. + boundBy := map[string]string{} + + // register binds a resolved port under its target label and every alias, in the rich + // port/service maps. The legacy string->port view (ASSIGNED_PORTS, substitution, + // /v0/port) is derived from portsMap on demand. + register := func(serviceLabel, portName, domain, portStr, target string, aliases []string) { + info := svclib.BindingInfo{ + Origin: net.JoinHostPort(domain, portStr), + Domain: domain, + Port: portStr, + } + + keys := append([]string{target}, aliases...) + for _, key := range keys { + portsMap[key] = info + } + servicesMap.Set(serviceLabel, portName, info) + } for label, spec := range serviceSpecs { - namedPorts := maps.Clone(spec.NamedPorts) - if spec.AutoassignPort { - namedPorts[""] = spec.Port + if len(spec.PortBindings) == 0 { + continue } - // Note, this can cause collisions. So be careful! - // To avoid port collisions, set the `so_reuseport_aware` option on the service definition - // and use the SO_REUSEPORT socket option in your services. - for portName, port := range namedPorts { + domain := spec.Domain + if domain == "" { + domain = "127.0.0.1" + } + + for _, binding := range spec.PortBindings { + if other, ok := boundBy[binding.Target]; ok && other != label { + return nil, nil, fmt.Errorf( + "port %q is bound by multiple services: %q and %q. A port may only be bound once", + binding.Target, other, label, + ) + } + boundBy[binding.Target] = label + + // External services are not managed by us; their ports are reachable as-is at the FQDN. + if spec.Type == "external_service" { + if !terseOutput { + log.Printf("Registering external port %s for %s (%s)\n", binding.Value, binding.Target, domain) + } + register(label, binding.Name, domain, binding.Value, binding.Target, binding.Aliases) + continue + } + + // Internal service: bind the port so we can discover an autoassigned one and reserve it. + // Note, this can cause collisions. So be careful! + // To avoid port collisions, set the `so_reuseport_aware` option on the service definition + // and use the SO_REUSEPORT socket option in your services. + // // We do a bit of a dance here to set SO_LINGER to 0. For details, see // https://stackoverflow.com/questions/71975992/what-really-is-the-linger-time-that-can-be-set-with-so-linger-on-sockets lc := net.ListenConfig{ @@ -403,39 +457,20 @@ func assignPorts( }, } - listener, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:"+port) + listener, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:"+binding.Value) if err != nil { - return nil, err + return nil, nil, err } - _, port, err = net.SplitHostPort(listener.Addr().String()) + _, portStr, err := net.SplitHostPort(listener.Addr().String()) if err != nil { - return nil, err - } - - qualifiedPortName := label - if portName != "" { - qualifiedPortName += "." + portName + return nil, nil, err } if !terseOutput { - log.Printf("Assigning port %s to %s\n", port, qualifiedPortName) + log.Printf("Assigning port %s to %s\n", portStr, binding.Target) } - ports.Set(qualifiedPortName, port) - - { - // TODO(zbarsky): Clean this up after April 2026 - qualifiedPortName := label - if portName != "" { - qualifiedPortName += ":" + portName - } - - if !terseOutput { - log.Printf("Assigning port %s to %s\n", port, qualifiedPortName) - } - - ports.Set(qualifiedPortName, port) - } + register(label, binding.Name, domain, portStr, binding.Target, binding.Aliases) if !spec.SoReuseportAware { toClose = append(toClose, listener) @@ -446,28 +481,23 @@ func assignPorts( for _, listener := range toClose { err := listener.Close() if err != nil { - return nil, err + return nil, nil, err } } + // Resolve service-group port aliases (re-exports of another service's port). for label, spec := range serviceSpecs { for portName, aliasedTo := range spec.PortAliases { - qualifiedPortName := label + // Zero value if the aliased target has no rich info; Port will be "". + info := portsMap[aliasedTo] + + qualifiedDot := label if portName != "" { - qualifiedPortName += "." + portName + qualifiedDot += "." + portName } + portsMap[qualifiedDot] = info - ports.Set(qualifiedPortName, ports[aliasedTo]) - - { - // TODO(zbarsky): Clean this up after April 2026 - qualifiedPortName := label - if portName != "" { - qualifiedPortName += ":" + portName - } - - ports.Set(qualifiedPortName, ports[aliasedTo]) - } + servicesMap.Set(label, portName, info) } } @@ -475,24 +505,21 @@ func assignPorts( // Give the kernel a bit of time to figure out what we've done. time.Sleep(10 * time.Millisecond) - serializedPorts, err := ports.Marshal() + serializedPorts, err := portsMap.AssignedPorts().Marshal() if err != nil { - return nil, err + return nil, nil, err } os.Setenv("ASSIGNED_PORTS", string(serializedPorts)) - return ports, nil + return portsMap, servicesMap, nil } func augmentServiceSpecs( serviceSpecs map[string]svclib.ServiceSpec, - ports svclib.Ports, + portsMap svclib.PortsMap, svcctlPort string, ) ( map[string]svclib.VersionedServiceSpec, error, ) { - tmpDir := os.Getenv("TMPDIR") - socketDir := os.Getenv("SOCKET_DIR") - versionedServiceSpecs := make(map[string]svclib.VersionedServiceSpec, len(serviceSpecs)) for label, serviceSpec := range serviceSpecs { s := svclib.VersionedServiceSpec{ @@ -504,11 +531,21 @@ func augmentServiceSpecs( continue } - exePath, err := runfiles.Rlocation(s.Exe) - if err != nil { - return nil, err + // Env is always present for spawned/external specs, but normalize defensively so the + // substitution and SVCCTL_PORT write below can assume a non-nil map. + if s.Env == nil { + s.Env = map[string]string{} + } + + // External services are not spawned, but their health-check address/args may still + // reference ports/origins, so they go through substitution below. + if s.Type != "external_service" { + exePath, err := runfiles.Rlocation(s.Exe) + if err != nil { + return nil, err + } + s.Exe = exePath } - s.Exe = exePath if s.HealthCheck != "" { healthCheckPath, err := runfiles.Rlocation(serviceSpec.HealthCheck) @@ -534,7 +571,7 @@ func augmentServiceSpecs( s.Color = logger.Colorize(s.Label) if s.AutoassignPort { - port := ports[s.Label] + port := portsMap.Port(s.Label) for i := range s.ServiceSpec.Args { s.Args[i] = strings.ReplaceAll(s.Args[i], "$${PORT}", port) } @@ -551,18 +588,7 @@ func augmentServiceSpecs( versionedServiceSpecs[label] = s } - replacements := make([]Replacement, 0, 2+len(ports)) - replacements = append(replacements, - Replacement{Old: "$${TMPDIR}", New: tmpDir}, - Replacement{Old: "$${SOCKET_DIR}", New: socketDir}, - ) - - for label, port := range ports { - replacements = append(replacements, Replacement{ - Old: "$${" + label + "}", - New: port, - }) - } + replacements := buildReplacements(portsMap, "$${") replaceAllPorts := func(s string) string { for _, r := range replacements { @@ -596,17 +622,19 @@ type Replacement struct { // buildReplacements creates port/env substitution pairs. // prefix is "$${" for values from JSON files (which preserve literal $$), // or "${" for values from Bazel args (where $$ is already collapsed to $). -func buildReplacements(ports svclib.Ports, prefix string) []Replacement { - replacements := make([]Replacement, 0, 2+len(ports)) +func buildReplacements(portsMap svclib.PortsMap, prefix string) []Replacement { + replacements := make([]Replacement, 0, 2+3*len(portsMap)) replacements = append(replacements, Replacement{Old: prefix + "TMPDIR}", New: os.Getenv("TMPDIR")}, Replacement{Old: prefix + "SOCKET_DIR}", New: os.Getenv("SOCKET_DIR")}, ) - for label, port := range ports { - replacements = append(replacements, Replacement{ - Old: prefix + label + "}", - New: port, - }) + for label, info := range portsMap { + replacements = append(replacements, + Replacement{Old: prefix + label + "}", New: info.Port}, + // Rich origin/domain tokens. A "::" delimiter is used since it can't appear in a label. + Replacement{Old: prefix + label + "::origin}", New: info.Origin}, + Replacement{Old: prefix + label + "::domain}", New: info.Domain}, + ) } return replacements } @@ -618,7 +646,7 @@ func replaceAll(s string, replacements []Replacement) string { return s } -func buildTestEnv(ports svclib.Ports) ([]string, error) { +func buildTestEnv(portsMap svclib.PortsMap) ([]string, error) { testEnvPath, err := runfiles.Rlocation(os.Getenv("SVCINIT_TEST_ENV_RLOCATION_PATH")) if err != nil { panic(err) @@ -635,7 +663,7 @@ func buildTestEnv(ports svclib.Ports) ([]string, error) { panic(err) } - replacements := buildReplacements(ports, "$${") + replacements := buildReplacements(portsMap, "$${") // Note, this can technically specify the same var multiple times. // Last one wins - hope that's what you wanted! diff --git a/docs/itest.md b/docs/itest.md index 0a8aeaa..be5b538 100644 --- a/docs/itest.md +++ b/docs/itest.md @@ -22,7 +22,7 @@ forward the ibazel hot-reload notification over stdin instead of restarting the # Service control The service manager exposes a HTTP server on `http://127.0.0.1:{SVCCTL_PORT}`. It can be used to -start / stop services during a test run. There are currently 5 API endpoints available. +start / stop services during a test run. There are currently 7 API endpoints available. All of them are GET requests: 1. `/v0/healthcheck?service={label}`: Returns 200 if the service is healthy, 503 otherwise. @@ -31,10 +31,36 @@ All of them are GET requests: You can optionally specify the signal to send to the service (valid values: SIGTERM and SIGKILL). 4. `/v0/wait?service={label}`: Wait for the service to exit and returns the exit code in the body. 5. `/v0/port?service={label}`: Returns the assigned port for the given label. May be a named port. +6. `/v0/ports`: Returns the full `ITEST_PORTS_MAP` as JSON (every port target/alias -> binding info). +7. `/v0/services`: Returns the full `ITEST_SERVICES_MAP` as JSON (every service -> port name -> binding info). In `bazel run` mode, the service manager will write the value of `SVCCTL_PORT` to `/tmp/svcctl_port`. This can be used in conjunction with the `/v0/port` API to let other tools interact with the managed services. +# Ports, hostnames, and external services + +Ports can be declared as first-class targets with `itest_port`. A port is a handle whose value is an `int` +build-setting flag (`0` = autoassign); the host/domain is supplied by the internal or external service that +binds it. The value can be pinned from the command line via `--//pkg:my_port=8080`. A given port may only +ever be bound once. + +`itest_external_service` points at a production or production-like instance reachable at a fixed FQDN. +It provides the same information as an `itest_service`, so it can be swapped in for a local service using +Bazel `select()` to run a test suite against production-like instances. External services are never started +or stopped by the service manager; if a health check is configured, it is run to verify reachability. + +Two environment variables describe all bound ports/services (both internal and external). They are injected +into the test binary and every child service, and are also available through the `/v0/ports` and `/v0/services` +control APIs: + +- `ITEST_PORTS_MAP`: a JSON object keyed by port target label (and aliases): + `{"@@//pkg:my_port": {"origin": "127.0.0.1:54321", "domain": "127.0.0.1", "port": "54321"}, ...}` +- `ITEST_SERVICES_MAP`: a JSON object keyed by service target label, then by port name (and aliases): + `{"@@//pkg:my_service": {"http": {"origin": "...", "domain": "...", "port": "..."}}, ...}` + +The legacy `ASSIGNED_PORTS` env var, the `GET_ASSIGNED_PORT_BIN` helper, and the `/v0/port` endpoint all +continue to work as before. + ## itest_service @@ -42,10 +68,10 @@ This can be used in conjunction with the `/v0/port` API to let other tools inter
 load("@rules_itest//private:itest.bzl", "itest_service")
 
-itest_service(name, autoassign_port, data, deps, enforce_graceful_shutdown, env, exe,
+itest_service(name, autoassign_port, data, deps, domain, enforce_graceful_shutdown, env, exe,
               expected_start_duration, health_check, health_check_args, health_check_interval,
               health_check_timeout, hot_reloadable, http_health_check_address, named_ports,
-              port, shutdown_signal, shutdown_timeout, so_reuseport_aware)
+              port, ports, shutdown_signal, shutdown_timeout, so_reuseport_aware)
 
An itest_service is a binary that is intended to run for the duration of the integration test. Examples include databases, HTTP/RPC servers, queue consumers, external service mocks, etc. @@ -60,6 +86,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio | name | A unique name for this target. | Name | required | | | deps | Services/tasks that must be started before this service/task can be started. Can be `itest_service`, `itest_task`, or `itest_service_group`. | List of labels | optional | `[]` | | data | - | List of labels | optional | `[]` | +| domain | The host that this service's ports are reachable on. Defaults to `127.0.0.1` for locally-managed services. | String | optional | `"127.0.0.1"` | | autoassign_port | If true, the service manager will pick a free port and assign it to the service. The port will be interpolated into `$${PORT}` in the service's `http_health_check_address` and `args`. It will also be exported under the target's fully qualified label in the service-port mapping.

The assigned ports for all services are available for substitution in `http_health_check_address` and `args` (in case one service needs the address for another one.) For example, the following substitution: `args = ["-client-addr", "127.0.0.1:$${@@//label/for:service}"]`

The service-port mapping is a JSON string -> int map propagated through the `ASSIGNED_PORTS` env var. For example, a port can be retrieved with the following JS code: `JSON.parse(process.env["ASSIGNED_PORTS"])["@@//label/for:service"]`.

Alternately, the env will also contain the location of a binary that can return the port, for contexts without a readily-accessible JSON parser. For example, the following Bash command: `PORT=$($GET_ASSIGNED_PORT_BIN @@//label/for:service)` | Boolean | optional | `False` | | enforce_graceful_shutdown | If set to True, the service manager will fail the service_test if the service had to be forcefully killed if the signal was not SIGKILL and after the shutdown timeout elapsed.

This needs to be False to have coverage of your services but don't want a them to be graceful at shutdown | Label | optional | `"@rules_itest//:enforce_graceful_shutdown"` | | env | The service manager will merge these variables into the environment when spawning the underlying binary. | Dictionary: String -> String | optional | `{}` | @@ -71,13 +98,84 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio | health_check_timeout | The timeout to wait for the health check. The syntax is based on common time duration with a number, followed by the time unit. For example, `200ms`, `1s`, `2m`, `3h`, `4d`. If empty or not set, the health check will not have a timeout. | String | optional | `""` | | hot_reloadable | If set to True, the service manager will propagate ibazel's reload notification over stdin instead of restarting the service. See the ruleset docstring for more info on using ibazel | Boolean | optional | `False` | | http_health_check_address | If set, the service manager will send an HTTP request to this address to check if the service came up in a healthy state. This check will be retried until it returns a 200 HTTP code. When used in conjunction with autoassigned ports, `$${@@//label/for:service:port_name}` can be used in the address. Example: `http_health_check_address = "http://127.0.0.1:$${@@//label/for:service:port_name}",` | String | optional | `""` | -| named_ports | For each element of the list, the service manager will pick a free port and assign it to the service. The port's fully-qualified name is the service's fully-qualified label and the port name, separated by a colon. For example, a port assigned with `named_ports = ["http_port"]` will be assigned a fully-qualified name of `@@//label/for:service:http_port`.

Named ports are accessible through the service-port mapping. For more details, see `autoassign_port`. | List of strings | optional | `[]` | +| named_ports | For each element of the list, the service manager will pick a free port and assign it to the service. The port's fully-qualified name is the service's fully-qualified label and the port name, separated by a dot. For example, a port assigned with `named_ports = ["http_port"]` will be assigned a fully-qualified name of `@@//label/for:service.http_port`.

Named ports are accessible through the service-port mapping. For more details, see `autoassign_port`. | List of strings | optional | `[]` | | port | Internal. | Label | optional | `None` | +| ports | Maps `itest_port` targets that this service binds to a port name. The port name is used as the inner key in `ITEST_SERVICES_MAP`. The desired value for each port is carried by the `itest_port` target itself (an `int` flag, default `0` = autoassign), and can be pinned from the command line via `--//pkg:my_port=8080`. | Dictionary: Label -> String | optional | `{}` | | shutdown_signal | The signal to send to the service when it needs to be shut down. Valid values are: SIGTERM and SIGKILL. SIGTERM is necessary to have proper coverage of services which needs to be gracefully terminated | String | optional | `"SIGTERM"` | | shutdown_timeout | The duration to wait by default after sending the shutdown signal before forcefully killing the service. The syntax is based on common time duration with a number, followed by the time unit. For example, `200ms`, `1s`, `2m`, `3h`, `4d`. If not defined, the value of `_default_shutdown_timeout` will be used. | String | optional | `""` | | so_reuseport_aware | If set, the service manager will not release the autoassigned port. The service binary must use SO_REUSEPORT when binding it. This reduces the possibility of port collisions when running many service_tests in parallel, or when code binds port 0 without being aware of the port assignment mechanism.

Must only be set when `autoassign_port` is enabled or `named_ports` are used. | Boolean | optional | `False` | + + +## itest_external_service + +
+load("@rules_itest//:itest.bzl", "itest_external_service")
+
+itest_external_service(name, data, deferred, deps, domain, expected_start_duration, health_check,
+                       health_check_args, health_check_interval, health_check_timeout, http_health_check_address,
+                       port_numbers, ports)
+
+ +An itest_external_service points at a production or production-like instance of a service that is +reachable at a fixed FQDN, rather than a binary that the service manager spawns locally. + +The service manager never starts or stops external services. If a health check is configured, it will be run +to verify the external service is reachable before dependent services/tests run. + +Because it provides the same information as an `itest_service`, it can be swapped in for a local service using +Bazel `select()` to run a test suite against production-like instances. + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| domain | The fully-qualified domain name (FQDN) that this external service is reachable on, e.g. `my_service.test.mycompany.com`. | String | required | | +| ports | Maps `itest_port` targets that this external service exposes to a port name. Provide the literal port number for each name via `port_numbers`. | Dictionary: Label -> String | optional | `{}` | +| port_numbers | Maps each port name (from `ports`) to the literal port number it is reachable on at the FQDN. | Dictionary: String -> String | optional | `{}` | +| data | - | List of labels | optional | `[]` | +| deps | Services/tasks that must be available before this external service can be used. | List of labels | optional | `[]` | +| deferred | If set, the external service will not be health-checked on boot up. | Boolean | optional | `False` | +| http_health_check_address | If set, the service manager will send an HTTP request to this address to verify the external service is reachable. Port substitutions (including `$${::origin}`) are supported. | String | optional | `""` | +| health_check | If set, the service manager will execute this binary to verify the external service is reachable. | Label | optional | `None` | +| health_check_args | Arguments to pass to the health_check binary. Port substitutions are applied prior to execution. | List of strings | optional | `[]` | +| health_check_interval | The duration between each health check. | String | optional | `"200ms"` | +| health_check_timeout | The timeout to wait for the health check. If empty, the health check will not have a timeout. | String | optional | `""` | +| expected_start_duration | How long the service is expected to take before passing a health check. Failing checks before this elapses are not logged. | String | optional | `"0s"` | + + + + +## itest_port + +
+load("@rules_itest//:itest.bzl", "itest_port")
+
+itest_port(name, aliases, build_setting_default)
+
+ +Declares a port as a first-class target. A port is a handle: the host is supplied by the internal or +external service that binds it, while the desired value is carried by the port target itself as an `int` +build-setting flag (`0` = autoassign for internal services). A given port may only be bound by a single +service. + +Because the port is a flag, its value can be pinned from the command line, e.g. `--//pkg:my_port=8080`. + +Ports are always referenced by their target label (e.g. via `port` / `port_ref`), and are bound exactly once. + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| aliases | Additional keys that must resolve to this port in `ITEST_PORTS_MAP` and the `ASSIGNED_PORTS` map. Useful for keeping backwards-compatible references working. The port target's own label is always bound, in addition to any aliases. | List of strings | optional | `[]` | +| build_setting_default | The default value of this port flag. `0` means autoassign for the internal service that binds it. | Integer | optional | `0` | + + ## itest_service_group diff --git a/go.mod b/go.mod index c9d8fde..f85f985 100644 --- a/go.mod +++ b/go.mod @@ -4,5 +4,5 @@ go 1.26.0 require ( github.com/bazelbuild/rules_go v0.61.0 - golang.org/x/sys v0.30.0 + golang.org/x/sys v0.33.0 ) diff --git a/go.sum b/go.sum index 1b3981b..11c311b 100644 --- a/go.sum +++ b/go.sum @@ -2,3 +2,5 @@ github.com/bazelbuild/rules_go v0.61.0 h1:Anz8trQHMdiCIjUzSjqL1JEZRFZZMYzKkIqwT7 github.com/bazelbuild/rules_go v0.61.0/go.mod h1:6YghDRf6l3FSiAwncK+Ww9jE1naoQzNq28gaKJeB67I= golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= +golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= diff --git a/itest.bzl b/itest.bzl index b24ab3d..db35e85 100644 --- a/itest.bzl +++ b/itest.bzl @@ -3,6 +3,8 @@ load("@bazel_skylib//rules:common_settings.bzl", "int_flag") load( "//private:itest.bzl", + _itest_external_service = "itest_external_service", + _itest_port = "itest_port", _itest_service = "itest_service", _itest_service_group = "itest_service_group", _itest_task = "itest_task", @@ -43,6 +45,33 @@ def named_port_alias(label, name): """ return _to_relative_named_port(label, name) +def port_ref(label): + """References the assigned port of an `itest_port` target in the `args` or `env` of an `itest_service`/`itest_task`. + + This is equivalent to `port`, but is named to make it clear it points at an `itest_port` target. + """ + return "$${%s}" % _to_relative_port(label) + +def port_origin(label): + """References the origin (`:`) of an `itest_port` target in `args`/`env`/`http_health_check_address`.""" + return "$${%s::origin}" % _to_relative_port(label) + +def port_domain(label): + """References the domain (host) of an `itest_port` target in `args`/`env`/`http_health_check_address`.""" + return "$${%s::domain}" % _to_relative_port(label) + +def itest_port(name, build_setting_default = 0, **kwargs): + """Declares a first-class port target. + + The port is an `int` build-setting flag; `0` (the default) means autoassign for the internal service + that binds it. The value can be pinned from the command line via `--//pkg:name=8080`. + """ + _itest_port( + name = name, + build_setting_default = build_setting_default, + **kwargs + ) + def itest_service(name, tags = [], hygienic = True, named_ports = [], **kwargs): if "port" in kwargs: fail("Do not specify `port`, instead set it via the `%s` flag" % (name + ".port")) @@ -76,6 +105,19 @@ def itest_service(name, tags = [], hygienic = True, named_ports = [], **kwargs): tags = tags, ) +def itest_external_service(name, tags = [], hygienic = True, **kwargs): + _itest_external_service( + name = name, + tags = tags + ["ibazel_notify_changes"], + **kwargs + ) + + if hygienic: + _hygiene_test( + name = name, + tags = tags, + ) + def itest_service_group(name, tags = [], hygienic = True, **kwargs): _itest_service_group( name = name, @@ -111,3 +153,10 @@ def _hygiene_test(name, **kwargs): ) service_test = _service_test + +# The underlying rules are exported so that they can be more easily extended by users +# (for example, to wrap them in their own macros). Prefer the macros above for normal usage. +itest_service_rule = _itest_service +itest_task_rule = _itest_task +itest_service_group_rule = _itest_service_group +itest_external_service_rule = _itest_external_service diff --git a/private/itest.bzl b/private/itest.bzl index 32712b0..4e51f79 100644 --- a/private/itest.bzl +++ b/private/itest.bzl @@ -21,7 +21,7 @@ forward the ibazel hot-reload notification over stdin instead of restarting the # Service control The service manager exposes a HTTP server on `http://127.0.0.1:{SVCCTL_PORT}`. It can be used to -start / stop services during a test run. There are currently 5 API endpoints available. +start / stop services during a test run. There are currently 7 API endpoints available. All of them are GET requests: 1. `/v0/healthcheck?service={label}`: Returns 200 if the service is healthy, 503 otherwise. @@ -30,9 +30,35 @@ All of them are GET requests: You can optionally specify the signal to send to the service (valid values: SIGTERM and SIGKILL). 4. `/v0/wait?service={label}`: Wait for the service to exit and returns the exit code in the body. 5. `/v0/port?service={label}`: Returns the assigned port for the given label. May be a named port. +6. `/v0/ports`: Returns the full `ITEST_PORTS_MAP` as JSON (every port target/alias -> binding info). +7. `/v0/services`: Returns the full `ITEST_SERVICES_MAP` as JSON (every service -> port name -> binding info). In `bazel run` mode, the service manager will write the value of `SVCCTL_PORT` to `/tmp/svcctl_port`. This can be used in conjunction with the `/v0/port` API to let other tools interact with the managed services. + +# Ports, hostnames, and external services + +Ports can be declared as first-class targets with `itest_port`. A port is a handle whose value is an `int` +build-setting flag (`0` = autoassign); the host/domain is supplied by the internal or external service that +binds it. The value can be pinned from the command line via `--//pkg:my_port=8080`. A given port may only +ever be bound once. + +`itest_external_service` points at a production or production-like instance reachable at a fixed FQDN. +It provides the same information as an `itest_service`, so it can be swapped in for a local service using +Bazel `select()` to run a test suite against production-like instances. External services are never started +or stopped by the service manager; if a health check is configured, it is run to verify reachability. + +Two environment variables describe all bound ports/services (both internal and external). They are injected +into the test binary and every child service, and are also available through the `/v0/ports` and `/v0/services` +control APIs: + +- `ITEST_PORTS_MAP`: a JSON object keyed by port target label (and aliases): + `{"@@//pkg:my_port": {"origin": "127.0.0.1:54321", "domain": "127.0.0.1", "port": "54321"}, ...}` +- `ITEST_SERVICES_MAP`: a JSON object keyed by service target label, then by port name (and aliases): + `{"@@//pkg:my_service": {"http": {"origin": "...", "domain": "...", "port": "..."}}, ...}` + +The legacy `ASSIGNED_PORTS` env var, the `GET_ASSIGNED_PORT_BIN` helper, and the `/v0/port` endpoint all +continue to work as before. """ load("@bazel_lib//lib:paths.bzl", "to_rlocation_path") @@ -46,12 +72,81 @@ _ServiceGroupInfo = provider( }, ) +_PortInfo = provider( + doc = "A handle for a port that can be bound by an internal or external service.", + fields = { + "label": "The canonical (fully-qualified) label of the port target.", + "aliases": "Additional keys that must resolve to the same port for backwards compatibility.", + }, +) + def _collect_services(deps): services = {} for dep in deps: services |= dep[_ServiceGroupInfo].services return services +def _validate_unique_port_bindings(services): + """Ensures every port target is bound by at most one service.""" + seen = {} + for label, service in services.items(): + bindings = getattr(service, "port_bindings", None) + if not bindings: + continue + for binding in bindings: + if binding.target in seen and seen[binding.target] != label: + fail("Port %s is bound by multiple services: %s and %s. A port may only be bound once." % ( + binding.target, + seen[binding.target], + label, + )) + seen[binding.target] = label + +def _port_binding(target, name, aliases, value): + """Constructs a single canonical port binding. Shared by internal and external services.""" + return struct( + target = target, + name = name, + aliases = aliases, + value = value, + ) + +def _bind_port_targets(ctx, value_fn): + """Builds bindings for each `itest_port` target in `ctx.attr.ports`. + + `value_fn(port_target, name)` returns the desired value string for that binding, letting + internal services read it from the port flag while external services read it from `port_numbers`. + """ + return [ + _port_binding( + port_target[_PortInfo].label, + name, + port_target[_PortInfo].aliases, + value_fn(port_target, name), + ) + for port_target, name in ctx.attr.ports.items() + ] + +def _compute_port_bindings(ctx): + """Builds the canonical, target-keyed list of port bindings for an internal service. + + Legacy `autoassign_port` / `named_ports` are translated into bindings keyed by the + service's own label (so historical references keep working), and any explicit + `itest_port` targets in `ports` are bound using their `_PortInfo`. + """ + bindings = [] + label = str(ctx.label) + + if ctx.attr.autoassign_port: + bindings.append(_port_binding(label, "", [], str(ctx.attr.port[BuildSettingInfo].value))) + + for port_flag, name in ctx.attr.named_ports.items(): + bindings.append(_port_binding(label + "." + name, name, [], str(port_flag[BuildSettingInfo].value))) + + bindings += _bind_port_targets(ctx, lambda port_target, _name: str(port_target[BuildSettingInfo].value)) + + return bindings + def _run_environment(ctx, service_specs_file): return { # Flags @@ -162,17 +257,22 @@ def _itest_binary_impl(ctx, extra_service_spec_kwargs, extra_exe_runfiles = []): **extra_service_spec_kwargs ) + direct_runfiles = [version_file] if version_file else [] + transitive_runfiles = _services_runfiles(ctx, "data") + _services_runfiles(ctx, "deps") + exe_runfiles + return _finalize_service(ctx, service, direct_runfiles, transitive_runfiles) + +def _finalize_service(ctx, service, direct_runfiles = [], transitive_runfiles = []): + """Collects transitive services, emits the svcinit spec file, and returns the providers. + + Shared by `_itest_binary_impl` (services/tasks) and `_itest_external_service_impl`. + """ services = _collect_services(ctx.attr.deps) services[service.label] = service service_specs_file = _create_svcinit_actions(ctx, services) - direct_runfiles = ctx.files.data + [service_specs_file] - if version_file: - direct_runfiles.append(version_file) - - runfiles = ctx.runfiles(direct_runfiles) - runfiles = runfiles.merge_all(_services_runfiles(ctx, "data") + _services_runfiles(ctx, "deps") + exe_runfiles) + runfiles = ctx.runfiles(ctx.files.data + [service_specs_file] + direct_runfiles) + runfiles = runfiles.merge_all(transitive_runfiles) return [ RunEnvironmentInfo(environment = _run_environment(ctx, service_specs_file)), @@ -196,7 +296,7 @@ def _itest_service_impl(ctx): if ctx.attr.health_check_timeout: _validate_duration("health_check_timeout", ctx.attr.health_check_timeout) - if ctx.attr.so_reuseport_aware and not (ctx.attr.autoassign_port or ctx.attr.named_ports): + if ctx.attr.so_reuseport_aware and not (ctx.attr.autoassign_port or ctx.attr.named_ports or ctx.attr.ports): fail("SO_REUSEPORT awareness only makes sense when using port autoassignment") shutdown_timeout = ctx.attr.shutdown_timeout or ctx.attr._default_shutdown_timeout[BuildSettingInfo].value @@ -208,6 +308,8 @@ def _itest_service_impl(ctx): "autoassign_port": ctx.attr.autoassign_port, "so_reuseport_aware": ctx.attr.so_reuseport_aware, "deferred": ctx.attr.deferred, + "domain": ctx.attr.domain, + "port_bindings": _compute_port_bindings(ctx), "named_ports": { name: str(port_flag[BuildSettingInfo].value) for port_flag, name in ctx.attr.named_ports.items() @@ -254,9 +356,20 @@ _itest_service_attrs = _itest_binary_attrs | { `PORT=$($GET_ASSIGNED_PORT_BIN @@//label/for:service)`""", ), "port": attr.label(doc = "Internal"), + "domain": attr.string( + default = "127.0.0.1", + doc = """The host that this service's ports are reachable on. Defaults to `127.0.0.1` for locally-managed services.""", + ), + "ports": attr.label_keyed_string_dict( + providers = [_PortInfo], + doc = """Maps `itest_port` targets that this service binds to a port name. The port name is used + as the inner key in `ITEST_SERVICES_MAP`. The desired value for each port is carried by the + `itest_port` target itself (an `int` flag, default `0` = autoassign), and can be pinned from the + command line via `--//pkg:my_port=8080`.""", + ), "named_ports": attr.label_keyed_string_dict( doc = """For each element of the list, the service manager will pick a free port and assign it to the service. - The port's fully-qualified name is the service's fully-qualified label and the port name, separated by a colon. + The port's fully-qualified name is the service's fully-qualified label and the port name, separated by a dot. For example, a port assigned with `named_ports = ["http_port"]` will be assigned a fully-qualified name of `@@//label/for:service.http_port`. Named ports are accessible through the service-port mapping. For more details, see `autoassign_port`.""", @@ -329,6 +442,142 @@ itest_service = rule( All [common binary attributes](https://bazel.build/reference/be/common-definitions#common-attributes-binaries) are supported including `args`.""", ) +def _external_port_value(ctx, name): + if name not in ctx.attr.port_numbers: + fail("itest_external_service %s: no port number provided in `port_numbers` for port %r" % (ctx.label, name)) + return ctx.attr.port_numbers[name] + +def _itest_external_service_impl(ctx): + _validate_deferred(ctx, ctx.attr.deps) + + if ctx.attr.health_check_interval: + _validate_duration("health_check_interval", ctx.attr.health_check_interval) + if ctx.attr.health_check_timeout: + _validate_duration("health_check_timeout", ctx.attr.health_check_timeout) + + bindings = _bind_port_targets(ctx, lambda _port_target, name: _external_port_value(ctx, name)) + + extra_service_spec_kwargs = { + "type": "external_service", + "domain": ctx.attr.domain, + "port_bindings": bindings, + "http_health_check_address": ctx.attr.http_health_check_address, + "expected_start_duration": ctx.attr.expected_start_duration, + "health_check_interval": ctx.attr.health_check_interval, + "health_check_timeout": ctx.attr.health_check_timeout, + "deferred": ctx.attr.deferred, + } + extra_exe_runfiles = [] + + if ctx.attr.health_check: + extra_service_spec_kwargs["health_check_label"] = str(ctx.attr.health_check.label) + extra_service_spec_kwargs["health_check"] = to_rlocation_path(ctx, ctx.executable.health_check) + extra_exe_runfiles.append(ctx.attr.health_check.default_runfiles) + extra_service_spec_kwargs["health_check_args"] = [ + ctx.expand_location(arg, targets = ctx.attr.data) + for arg in ctx.attr.health_check_args + ] + + service = struct( + label = str(ctx.label), + exe = "", + args = [], + env = {}, + deps = [str(dep.label) for dep in ctx.attr.deps], + **extra_service_spec_kwargs + ) + + return _finalize_service(ctx, service, transitive_runfiles = _services_runfiles(ctx, "deps") + extra_exe_runfiles) + +_itest_external_service_attrs = { + "domain": attr.string( + mandatory = True, + doc = "The fully-qualified domain name (FQDN) that this external service is reachable on, e.g. `my_service.test.mycompany.com`.", + ), + "ports": attr.label_keyed_string_dict( + providers = [_PortInfo], + doc = "Maps `itest_port` targets that this external service exposes to a port name. Provide the literal port number for each name via `port_numbers`.", + ), + "port_numbers": attr.string_dict( + doc = "Maps each port name (from `ports`) to the literal port number it is reachable on at the FQDN.", + ), + "data": attr.label_list(allow_files = True), + "deps": attr.label_list( + providers = [_ServiceGroupInfo], + doc = "Services/tasks that must be available before this external service can be used.", + ), + "deferred": attr.bool( + doc = "If set, the external service will not be health-checked on boot up.", + ), + "http_health_check_address": attr.string( + doc = "If set, the service manager will send an HTTP request to this address to verify the external service is reachable. Port substitutions (including `$${::origin}`) are supported.", + ), + "health_check": attr.label( + cfg = "target", + mandatory = False, + executable = True, + doc = "If set, the service manager will execute this binary to verify the external service is reachable.", + ), + "health_check_args": attr.string_list( + doc = "Arguments to pass to the health_check binary. Port substitutions are applied prior to execution.", + ), + "health_check_interval": attr.string( + default = "200ms", + doc = "The duration between each health check.", + ), + "health_check_timeout": attr.string( + default = "", + doc = "The timeout to wait for the health check. If empty, the health check will not have a timeout.", + ), + "expected_start_duration": attr.string( + default = "0s", + doc = "How long the service is expected to take before passing a health check. Failing checks before this elapses are not logged.", + ), +} | _svcinit_attrs + +itest_external_service = rule( + implementation = _itest_external_service_impl, + attrs = _itest_external_service_attrs, + executable = True, + doc = """An itest_external_service points at a production or production-like instance of a service that is +reachable at a fixed FQDN, rather than a binary that the service manager spawns locally. + +The service manager never starts or stops external services. If a health check is configured, it will be run +to verify the external service is reachable before dependent services/tests run. + +Because it provides the same information as an `itest_service`, it can be swapped in for a local service using +Bazel `select()` to run a test suite against production-like instances.""", +) + +def _itest_port_impl(ctx): + return [ + _PortInfo( + label = str(ctx.label), + aliases = ctx.attr.aliases, + ), + BuildSettingInfo(value = ctx.build_setting_value), + ] + +itest_port = rule( + implementation = _itest_port_impl, + build_setting = config.int(flag = True), + attrs = { + "aliases": attr.string_list( + doc = """Additional keys that must resolve to this port in `ITEST_PORTS_MAP` and the `ASSIGNED_PORTS` + map. Useful for keeping backwards-compatible references working. The port target's own label is + always bound, in addition to any aliases.""", + ), + }, + doc = """Declares a port as a first-class target. A port is a pure handle: the host is supplied by the +internal or external service that binds it, while the desired value is carried by the port target itself as +an `int` build-setting flag (`0` = autoassign for internal services). A given port may only be bound by a +single service. + +Because the port is a flag, its value can be pinned from the command line, e.g. `--//pkg:my_port=8080`. + +Ports are always referenced by their target label (e.g. via `port` / `port_ref`), and are bound exactly once.""", +) + def _itest_task_impl(ctx): return _itest_binary_impl(ctx, { "type": "task", @@ -398,6 +647,8 @@ It can bring up multiple services with a single `bazel run` command, which is us ) def _create_svcinit_actions(ctx, services): + _validate_unique_port_bindings(services) + ctx.actions.symlink( output = ctx.outputs.executable, target_file = ctx.executable._svcinit, diff --git a/runner/runner.go b/runner/runner.go index 5fd3cf7..ac9ecc7 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -50,6 +50,20 @@ func colorize(s svclib.VersionedServiceSpec) string { return s.Colorize(s.Label) } +// withHealthCheckTimeout wraps ctx with the service's health_check_timeout, if set. +// The returned cancel func is always safe to call. +func withHealthCheckTimeout(ctx context.Context, service *ServiceInstance) (context.Context, context.CancelFunc) { + if service.VersionedServiceSpec.HealthCheckTimeout == "" { + return context.WithCancel(ctx) + } + timeout, err := time.ParseDuration(service.VersionedServiceSpec.HealthCheckTimeout) + if err != nil { + log.Printf("failed to parse health check timeout, falling back to no timeout: %v", err) + return context.WithCancel(ctx) + } + return context.WithTimeout(ctx, timeout) +} + func (r *Runner) StartAll(serviceErrCh chan error) ([]topological.Task, error) { tasks := allTasks(r.serviceInstances, func(ctx context.Context, service *ServiceInstance) error { if service.Type == "group" { @@ -61,6 +75,16 @@ func (r *Runner) StartAll(serviceErrCh chan error) ([]topological.Task, error) { return nil } + // External services are not spawned by us. If a health check is configured, we + // verify the external instance is reachable so dependents can order against it. + // We still honor health_check_timeout so an unreachable external dependency fails + // after the configured timeout instead of hanging until the outer test timeout. + if service.Type == "external_service" { + ctx, cancel := withHealthCheckTimeout(ctx, service) + defer cancel() + return service.WaitUntilHealthy(ctx) + } + if terseOutput { log.Printf("Starting %s\n", colorize(service.VersionedServiceSpec)) } else { @@ -79,15 +103,8 @@ func (r *Runner) StartAll(serviceErrCh chan error) ([]topological.Task, error) { } }() - if service.VersionedServiceSpec.HealthCheckTimeout != "" { - timeout, err := time.ParseDuration(service.VersionedServiceSpec.HealthCheckTimeout) - if err != nil { - log.Printf("failed to parse health check timeout, falling back to no timeout: %v", err) - } - timeoutCtx, cancel := context.WithTimeout(ctx, timeout) - ctx = timeoutCtx - defer cancel() - } + ctx, cancel := withHealthCheckTimeout(ctx, service) + defer cancel() return service.WaitUntilHealthy(ctx) }) starter := topological.NewRunner(tasks) @@ -98,7 +115,7 @@ func (r *Runner) StartAll(serviceErrCh chan error) ([]topological.Task, error) { func (r *Runner) StopAll() (map[string]*os.ProcessState, error) { tasks := allTasks(r.serviceInstances, func(ctx context.Context, service *ServiceInstance) error { - if service.Type == "group" || service.Deferred { + if service.Type == "group" || service.Type == "external_service" || service.Deferred { return nil } log.Printf("Stopping %s\n", colorize(service.VersionedServiceSpec)) @@ -110,7 +127,7 @@ func (r *Runner) StopAll() (map[string]*os.ProcessState, error) { states := make(map[string]*os.ProcessState) for _, serviceInstance := range r.serviceInstances { - if serviceInstance.Type == "group" || serviceInstance.Deferred { + if serviceInstance.Type == "group" || serviceInstance.Type == "external_service" || serviceInstance.Deferred { continue } states[serviceInstance.Label] = serviceInstance.ProcessState() @@ -182,7 +199,7 @@ func (r *Runner) UpdateSpecs(serviceSpecs ServiceSpecs, ibazelCmd []byte) error for _, label := range updateActions.toStopLabels { serviceInstance := r.serviceInstances[label] - if serviceInstance.Type == "group" { + if serviceInstance.Type == "group" || serviceInstance.Type == "external_service" { continue } serviceInstance.Stop() @@ -223,10 +240,12 @@ func (r *Runner) UpdateSpecsAndRestart( } func prepareServiceInstance(ctx context.Context, s svclib.VersionedServiceSpec) (*ServiceInstance, error) { - if s.Type == "group" { + // Neither groups nor external services are spawned, so they have no managed command. + if s.Type == "group" || s.Type == "external_service" { return &ServiceInstance{ VersionedServiceSpec: s, startErrFn: sync.OnceValue(func() error { return nil }), + waitErrFn: sync.OnceValue(func() error { return nil }), }, nil } diff --git a/runner/service_instance.go b/runner/service_instance.go index 99b8cf6..4318b36 100644 --- a/runner/service_instance.go +++ b/runner/service_instance.go @@ -69,6 +69,19 @@ func (s *ServiceInstance) WaitUntilHealthy(ctx context.Context) error { return err } + // External services are not spawned by us, so there is no process to watch for exit. + // If a health check is configured, poll it; otherwise assume the service is reachable. + if s.Type == "external_service" { + if s.HttpHealthCheckAddress == "" && s.ServiceSpec.HealthCheck == "" { + return nil + } + if err := s.PollUntilHealthy(ctx); err != nil { + return err + } + log.Printf("%s healthy!\n", coloredLabel) + return nil + } + sleepDuration, err := time.ParseDuration(s.HealthCheckInterval) if err != nil { log.Printf("failed to parse health check time duration, falling back to 200ms: %v", err) @@ -109,6 +122,25 @@ func (s *ServiceInstance) WaitUntilHealthy(ctx context.Context) error { return nil } +// PollUntilHealthy repeatedly runs the configured health check until it passes, the context is +// cancelled/times out, or ctx errors. It does not watch a managed process, so it is suitable for +// services the manager does not spawn (e.g. external services). +func (s *ServiceInstance) PollUntilHealthy(ctx context.Context) error { + sleepDuration, err := time.ParseDuration(s.HealthCheckInterval) + if err != nil { + sleepDuration = 200 * time.Millisecond + } + for { + if err := ctx.Err(); err != nil { + return err + } + if s.HealthCheck(ctx, 0) { + return nil + } + time.Sleep(sleepDuration) + } +} + var httpClient = http.Client{ // It's important to have a reasonable timeout here since the connection may never get accepted // if it's to a port that is SO_REUSEPORT-aware. In that case, the healthcheck will hang forever @@ -317,6 +349,9 @@ func (s *ServiceInstance) Wait() error { } func (s *ServiceInstance) Pid() int { + if s.cmd == nil || s.cmd.Process == nil { + return -1 + } return s.cmd.Process.Pid } diff --git a/svcctl/svcctl.go b/svcctl/svcctl.go index 58e7093..07e6ce0 100644 --- a/svcctl/svcctl.go +++ b/svcctl/svcctl.go @@ -4,6 +4,7 @@ package svcctl import ( "context" + "encoding/json" "errors" "fmt" "log" @@ -187,7 +188,7 @@ func handleWait(ctx context.Context, r *runner.Runner, _ chan error, w http.Resp } type portHandler struct { - ports svclib.Ports + portsMap svclib.PortsMap } func (p portHandler) handle(ctx context.Context, r *runner.Runner, _ chan error, w http.ResponseWriter, req *http.Request) { @@ -198,22 +199,53 @@ func (p portHandler) handle(ctx context.Context, r *runner.Runner, _ chan error, return } - port, ok := p.ports[service] + info, ok := p.portsMap[service] if !ok { http.Error(w, "port is not autoassigned", http.StatusBadRequest) return } w.WriteHeader(http.StatusOK) - w.Write([]byte(port)) + w.Write([]byte(info.Port)) } -func Serve(ctx context.Context, listener net.Listener, r *runner.Runner, ports svclib.Ports, servicesErrCh chan error) error { +// portsHandler serves the full ITEST_PORTS_MAP (every port target/alias -> binding info). +type portsHandler struct { + portsMap svclib.PortsMap +} + +func (p portsHandler) handle(ctx context.Context, r *runner.Runner, _ chan error, w http.ResponseWriter, req *http.Request) { + writeJSON(w, p.portsMap) +} + +// servicesHandler serves the full ITEST_SERVICES_MAP (every service -> port name -> binding info). +type servicesHandler struct { + servicesMap svclib.ServicesMap +} + +func (s servicesHandler) handle(ctx context.Context, r *runner.Runner, _ chan error, w http.ResponseWriter, req *http.Request) { + writeJSON(w, s.servicesMap) +} + +func writeJSON(w http.ResponseWriter, v any) { + data, err := json.Marshal(v) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write(data) +} + +func Serve(ctx context.Context, listener net.Listener, r *runner.Runner, portsMap svclib.PortsMap, servicesMap svclib.ServicesMap, servicesErrCh chan error) error { mux := http.NewServeMux() handle(ctx, mux, r, servicesErrCh, "GET /v0/healthcheck", handleHealthCheck) handle(ctx, mux, r, servicesErrCh, "GET /v0/start", handleStart) handle(ctx, mux, r, servicesErrCh, "GET /v0/kill", handleKill) handle(ctx, mux, r, servicesErrCh, "GET /v0/wait", handleWait) - handle(ctx, mux, r, servicesErrCh, "GET /v0/port", portHandler{ports}.handle) + handle(ctx, mux, r, servicesErrCh, "GET /v0/port", portHandler{portsMap}.handle) + handle(ctx, mux, r, servicesErrCh, "GET /v0/ports", portsHandler{portsMap}.handle) + handle(ctx, mux, r, servicesErrCh, "GET /v0/services", servicesHandler{servicesMap}.handle) return http.Serve(listener, mux) } diff --git a/svclib/ports.go b/svclib/ports.go index 2431a4c..273626a 100644 --- a/svclib/ports.go +++ b/svclib/ports.go @@ -15,3 +15,50 @@ func (p Ports) Marshal() ([]byte, error) { func (p *Ports) Unmarshal(data []byte) error { return json.Unmarshal(data, p) } + +// BindingInfo is the rich, per-port description exported through ITEST_PORTS_MAP +// and ITEST_SERVICES_MAP. +type BindingInfo struct { + // Origin is ":". + Origin string `json:"origin"` + Domain string `json:"domain"` + Port string `json:"port"` +} + +// PortsMap is keyed by port target label (and its aliases). +type PortsMap map[string]BindingInfo + +func (m PortsMap) Marshal() ([]byte, error) { + return json.Marshal(m) +} + +// Port returns just the port number bound under the given label (or alias), or "" if unbound. +func (m PortsMap) Port(label string) string { + return m[label].Port +} + +// AssignedPorts derives the legacy label->port string map (used for the ASSIGNED_PORTS +// env var and the /v0/port endpoint) from the rich binding info. +func (m PortsMap) AssignedPorts() Ports { + ports := make(Ports, len(m)) + for label, info := range m { + ports[label] = info.Port + } + return ports +} + +// ServicesMap is keyed by service target label, then by port name (and aliases). +type ServicesMap map[string]map[string]BindingInfo + +func (m ServicesMap) Set(service, portName string, info BindingInfo) { + inner, ok := m[service] + if !ok { + inner = map[string]BindingInfo{} + m[service] = inner + } + inner[portName] = info +} + +func (m ServicesMap) Marshal() ([]byte, error) { + return json.Marshal(m) +} diff --git a/svclib/types.go b/svclib/types.go index bf1e2a2..422d87d 100644 --- a/svclib/types.go +++ b/svclib/types.go @@ -2,9 +2,27 @@ package svclib import "rules_itest/logger" +// PortBinding describes a single port that a service binds (internal) or points +// at (external). It is created by Starlark and carries the canonical port target +// label along with any backwards-compatible aliases that must resolve to the same +// port in the various port maps. +type PortBinding struct { + // Target is the canonical port target label (e.g. an `itest_port` label, or for + // legacy services the service's own label / `label.port_name`). + Target string `json:"target"` + // Name is the port name used as the inner key in ITEST_SERVICES_MAP. + Name string `json:"name"` + // Aliases are additional keys that must resolve to the same port (for example, + // user-declared `itest_port` aliases) for backwards compatibility. + Aliases []string `json:"aliases"` + // Value is the desired port. For internal services "0" means autoassign. For + // external services it is the literal port number on the remote host. + Value string `json:"value"` +} + // Created by Starlark type ServiceSpec struct { - // Type can be "service", "task", or "group". + // Type can be "service", "task", "group", or "external_service". Type string `json:"type"` Label string `json:"label"` Args []string `json:"args"` @@ -29,6 +47,13 @@ type ServiceSpec struct { ShutdownTimeout string `json:"shutdown_timeout"` EnforceForcefulShutdown bool `json:"enforce_graceful_shutdown"` Deferred bool `json:"deferred"` + // Domain is the host that this service's ports are reachable on. Internal + // services default to "127.0.0.1"; external services set it to their FQDN. + Domain string `json:"domain"` + // PortBindings is the canonical, target-keyed description of the ports owned by + // this service. It supersedes Port/NamedPorts (which are retained for + // backwards compatibility). + PortBindings []PortBinding `json:"port_bindings"` } // Our internal representation. diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 4d5a4e3..f23154d 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -89,7 +89,7 @@ itest_service( ], autoassign_port = True, exe = "//go_service", - http_health_check_address = "http://127.0.0.1:$${@@//:named_port:http_port}", + http_health_check_address = "http://127.0.0.1:$${@@//:named_port.http_port}", named_ports = ["http_port"], ) @@ -123,7 +123,7 @@ itest_service( autoassign_port = True, env = {"foo": "bar"}, exe = "//go_service", - http_health_check_address = "http://127.0.0.1:$${@@//:speedy_service2:port}", + http_health_check_address = "http://127.0.0.1:$${@@//:speedy_service2.port}", tags = ["manual"], ) diff --git a/tests/ports/BUILD.bazel b/tests/ports/BUILD.bazel new file mode 100644 index 0000000..47c109a --- /dev/null +++ b/tests/ports/BUILD.bazel @@ -0,0 +1,148 @@ +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") +load("@rules_go//go:def.bzl", "go_test") +load( + "@rules_itest//:itest.bzl", + "itest_external_service", + "itest_port", + "itest_service", + "itest_service_group", + "port_ref", + "service_test", +) +load(":tests.bzl", "tests") + +package(default_visibility = ["//visibility:public"]) + +NOT_WINDOWS = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], +}) + +tests() + +# An internal service binds a first-class itest_port. The port is referenced by its +# target label everywhere. +itest_port(name = "db_port") + +itest_service( + name = "db", + args = [ + "-port", + port_ref(":db_port"), + ], + exe = "//go_service", + http_health_check_address = "http://127.0.0.1:" + port_ref(":db_port"), + ports = {":db_port": "sql"}, +) + +# An external service points at a fixed FQDN and exposes a port at a literal number. +itest_port(name = "ext_db_port") + +itest_external_service( + name = "external_db", + domain = "db.test.invalid", + port_numbers = {"sql": "5432"}, + ports = {":ext_db_port": "sql"}, +) + +# SO_REUSEPORT awareness must be allowed with declarative `itest_port` bindings too, +# not just legacy autoassign_port / named_ports. +itest_port(name = "reuseport_port") + +itest_service( + name = "reuseport_db", + args = [ + "-so-reuseport", + "-port", + port_ref(":reuseport_port"), + ], + exe = "//go_service", + health_check_timeout = "5s", + http_health_check_address = "http://127.0.0.1:" + port_ref(":reuseport_port"), + ports = {":reuseport_port": "sql"}, + so_reuseport_aware = True, + target_compatible_with = NOT_WINDOWS, +) + +# select() lets a test suite be pointed at either the locally-managed service or the +# production-like external instance. +bool_flag( + name = "use_external", + build_setting_default = False, +) + +config_setting( + name = "external", + flag_values = {":use_external": "True"}, +) + +go_test( + name = "_ports_test", + srcs = ["ports_test.go"], + tags = ["manual"], + deps = ["//svcctl"], +) + +service_test( + name = "internal_test", + env = { + "EXPECT_PORT_TARGET": "@@//ports:db_port", + "EXPECT_SERVICE": "@@//ports:db", + "EXPECT_PORT_NAME": "sql", + "EXPECT_DOMAIN": "127.0.0.1", + }, + services = [":db"], + test = ":_ports_test", +) + +service_test( + name = "external_test", + env = { + "EXPECT_PORT_TARGET": "@@//ports:ext_db_port", + "EXPECT_SERVICE": "@@//ports:external_db", + "EXPECT_PORT_NAME": "sql", + "EXPECT_DOMAIN": "db.test.invalid", + "EXPECT_PORT": "5432", + }, + services = [":external_db"], + test = ":_ports_test", +) + +# Demonstrates swapping the service implementation with select(). +service_test( + name = "select_test", + services = select({ + ":external": [":external_db"], + "//conditions:default": [":db"], + }), + test = "@rules_itest//:exit0", +) + +# Targets used by the analysis-failure test in tests.bzl: binding the same port twice. +itest_service( + name = "binder_a", + exe = "@rules_itest//:exit0", + hygienic = False, + ports = {":shared_port": "a"}, + tags = ["manual"], +) + +itest_service( + name = "binder_b", + exe = "@rules_itest//:exit0", + hygienic = False, + ports = {":shared_port": "b"}, + tags = ["manual"], +) + +itest_port(name = "shared_port") + +itest_service_group( + name = "double_bind", + hygienic = False, + services = [ + ":binder_a", + ":binder_b", + ], + tags = ["manual"], +) diff --git a/tests/ports/ports_test.go b/tests/ports/ports_test.go new file mode 100644 index 0000000..e88b6da --- /dev/null +++ b/tests/ports/ports_test.go @@ -0,0 +1,127 @@ +package main + +import ( + "context" + "encoding/json" + "net/http" + "os" + "testing" + + "github.com/hermeticbuild/rules_itest/tests/svcctl" +) + +type bindingInfo struct { + Origin string `json:"origin"` + Domain string `json:"domain"` + Port string `json:"port"` +} + +func loadPortsMap(t *testing.T) map[string]bindingInfo { + t.Helper() + raw := os.Getenv("ITEST_PORTS_MAP") + if raw == "" { + t.Fatal("ITEST_PORTS_MAP is not set") + } + out := map[string]bindingInfo{} + if err := json.Unmarshal([]byte(raw), &out); err != nil { + t.Fatalf("failed to parse ITEST_PORTS_MAP %q: %v", raw, err) + } + return out +} + +func loadServicesMap(t *testing.T) map[string]map[string]bindingInfo { + t.Helper() + raw := os.Getenv("ITEST_SERVICES_MAP") + if raw == "" { + t.Fatal("ITEST_SERVICES_MAP is not set") + } + out := map[string]map[string]bindingInfo{} + if err := json.Unmarshal([]byte(raw), &out); err != nil { + t.Fatalf("failed to parse ITEST_SERVICES_MAP %q: %v", raw, err) + } + return out +} + +func TestPortsMap(t *testing.T) { + target := os.Getenv("EXPECT_PORT_TARGET") + wantDomain := os.Getenv("EXPECT_DOMAIN") + wantPort := os.Getenv("EXPECT_PORT") // optional exact match + + portsMap := loadPortsMap(t) + info, ok := portsMap[target] + if !ok { + t.Fatalf("ITEST_PORTS_MAP missing port target %q; got %v", target, portsMap) + } + if info.Domain != wantDomain { + t.Errorf("port %q domain = %q, want %q", target, info.Domain, wantDomain) + } + if info.Port == "" { + t.Errorf("port %q has empty port", target) + } + if wantPort != "" && info.Port != wantPort { + t.Errorf("port %q port = %q, want %q", target, info.Port, wantPort) + } + if want := info.Domain + ":" + info.Port; info.Origin != want { + t.Errorf("port %q origin = %q, want %q", target, info.Origin, want) + } +} + +func TestServicesMap(t *testing.T) { + service := os.Getenv("EXPECT_SERVICE") + portName := os.Getenv("EXPECT_PORT_NAME") + wantDomain := os.Getenv("EXPECT_DOMAIN") + + servicesMap := loadServicesMap(t) + ports, ok := servicesMap[service] + if !ok { + t.Fatalf("ITEST_SERVICES_MAP missing service %q; got %v", service, servicesMap) + } + info, ok := ports[portName] + if !ok { + t.Fatalf("service %q missing port name %q; got %v", service, portName, ports) + } + if info.Domain != wantDomain { + t.Errorf("service %q port %q domain = %q, want %q", service, portName, info.Domain, wantDomain) + } + if info.Port == "" { + t.Errorf("service %q port %q has empty port", service, portName) + } +} + +func TestSvcctlListAll(t *testing.T) { + svcctlPort := os.Getenv("SVCCTL_PORT") + if svcctlPort == "" { + t.Fatal("SVCCTL_PORT not set") + } + client := svcctl.NewSvcctlClient("http://127.0.0.1:"+svcctlPort, http.DefaultClient) + + target := os.Getenv("EXPECT_PORT_TARGET") + service := os.Getenv("EXPECT_SERVICE") + portName := os.Getenv("EXPECT_PORT_NAME") + + // /v0/ports should agree with the env-provided ITEST_PORTS_MAP. + envPorts := loadPortsMap(t) + apiPorts, err := client.Ports(context.Background()) + if err != nil { + t.Fatalf("GET /v0/ports failed: %v", err) + } + if apiPorts[target].Port != envPorts[target].Port { + t.Errorf("/v0/ports port for %q = %q, want %q", target, apiPorts[target].Port, envPorts[target].Port) + } + if apiPorts[target].Domain != envPorts[target].Domain { + t.Errorf("/v0/ports domain for %q = %q, want %q", target, apiPorts[target].Domain, envPorts[target].Domain) + } + + // /v0/services should expose the service's port. + apiServices, err := client.Services(context.Background()) + if err != nil { + t.Fatalf("GET /v0/services failed: %v", err) + } + svc, ok := apiServices[service] + if !ok { + t.Fatalf("/v0/services missing service %q; got %v", service, apiServices) + } + if _, ok := svc[portName]; !ok { + t.Fatalf("/v0/services service %q missing port %q; got %v", service, portName, svc) + } +} diff --git a/tests/ports/tests.bzl b/tests/ports/tests.bzl new file mode 100644 index 0000000..98d158d --- /dev/null +++ b/tests/ports/tests.bzl @@ -0,0 +1,23 @@ +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") + +def _double_bind_test(ctx): + """Verifies that binding the same port from two services fails analysis.""" + env = analysistest.begin(ctx) + + asserts.expect_failure( + env, + "Port @@//ports:shared_port is bound by multiple services: @@//ports:binder_a and @@//ports:binder_b. A port may only be bound once.", + ) + + return analysistest.end(env) + +double_bind_test = analysistest.make( + _double_bind_test, + expect_failure = True, +) + +def tests(): + double_bind_test( + name = "test_double_bind_should_fail", + target_under_test = ":double_bind", + ) diff --git a/tests/so_reuseport/so_reuseport_test.go b/tests/so_reuseport/so_reuseport_test.go index a22f0ce..0bd36a0 100644 --- a/tests/so_reuseport/so_reuseport_test.go +++ b/tests/so_reuseport/so_reuseport_test.go @@ -11,7 +11,7 @@ import ( func TestNo_SO_REUSEPORT(t *testing.T) { portNames := []string{ "@@//so_reuseport:reuseport_service", - "@@//so_reuseport:reuseport_service:named_port1", + "@@//so_reuseport:reuseport_service.named_port1", } t.Logf("ASSIGNED_PORTS: %v", os.Getenv("ASSIGNED_PORTS")) diff --git a/tests/so_reuseport/so_reuseport_test.mjs b/tests/so_reuseport/so_reuseport_test.mjs index a264a09..5b4c134 100644 --- a/tests/so_reuseport/so_reuseport_test.mjs +++ b/tests/so_reuseport/so_reuseport_test.mjs @@ -6,7 +6,7 @@ const ports = JSON.parse(process.env.ASSIGNED_PORTS) for (const portName of [ "@@//so_reuseport:reuseport_service", - "@@//so_reuseport:reuseport_service:named_port1", + "@@//so_reuseport:reuseport_service.named_port1", ]) { const port = ports[portName]; assert(port); diff --git a/tests/svcctl/client.go b/tests/svcctl/client.go index 075579b..b93a8ef 100644 --- a/tests/svcctl/client.go +++ b/tests/svcctl/client.go @@ -2,12 +2,20 @@ package svcctl import ( "context" + "encoding/json" "fmt" "log" "net/http" "net/url" ) +// BindingInfo mirrors svclib.BindingInfo for the entries returned by /v0/ports and /v0/services. +type BindingInfo struct { + Origin string `json:"origin"` + Domain string `json:"domain"` + Port string `json:"port"` +} + type SvcctlClient struct { baseURL string httpClient *http.Client @@ -90,4 +98,52 @@ func (c *SvcctlClient) HealthCheck(ctx context.Context, service string) (int, er } return resp.StatusCode, nil +} + +// Ports fetches the full ITEST_PORTS_MAP via /v0/ports. +func (c *SvcctlClient) Ports(ctx context.Context) (map[string]BindingInfo, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.baseURL+"/v0/ports", nil) + if err != nil { + return nil, err + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("Got status code %d, want %d", resp.StatusCode, http.StatusOK) + } + + out := map[string]BindingInfo{} + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + return nil, err + } + return out, nil +} + +// Services fetches the full ITEST_SERVICES_MAP via /v0/services. +func (c *SvcctlClient) Services(ctx context.Context) (map[string]map[string]BindingInfo, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.baseURL+"/v0/services", nil) + if err != nil { + return nil, err + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("Got status code %d, want %d", resp.StatusCode, http.StatusOK) + } + + out := map[string]map[string]BindingInfo{} + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + return nil, err + } + return out, nil } \ No newline at end of file From e0f453c3c80de428a86cd6ee15d38cd2d31c2490 Mon Sep 17 00:00:00 2001 From: Scott Pledger Date: Tue, 25 Aug 2026 13:08:46 -0600 Subject: [PATCH 2/2] Address review feedback: rename domain->hostname, external cmd health checks - Rename `domain` -> `hostname` across the Starlark attrs, Go ServiceSpec/ BindingInfo fields, the ITEST_PORTS_MAP/ITEST_SERVICES_MAP key, the `port_domain`->`port_hostname` helper and `::domain`->`::hostname` token, tests, and docs. `hostname` matches `net/url.URL.Hostname()` semantics (host-or-IP minus the port), whereas `domain` was misleading. - Drop the hardcoded 200ms fallback in ServiceInstance.PollUntilHealthy and rely on the rule-provided (and validated) health_check_interval default. - Add a shared `_named_port_target` helper instead of ad-hoc `label + "." + name` concatenation when building named-port bindings. - Demonstrate command-based health checks for itest_external_service with a new tcp_probe example (a DB-style service with no HTTP endpoint), verified by a hygiene test. Co-authored-by: Cursor --- cmd/svcinit/main.go | 24 ++++++++++++------------ docs/itest.md | 14 +++++++------- itest.bzl | 8 ++++---- private/itest.bzl | 24 ++++++++++++++++-------- runner/service_instance.go | 4 +++- svclib/ports.go | 8 ++++---- svclib/types.go | 4 ++-- tests/ports/BUILD.bazel | 23 ++++++++++++++++++++--- tests/ports/ports_test.go | 20 ++++++++++---------- tests/ports/tcp_probe/BUILD.bazel | 14 ++++++++++++++ tests/ports/tcp_probe/main.go | 25 +++++++++++++++++++++++++ tests/svcctl/client.go | 6 +++--- 12 files changed, 120 insertions(+), 54 deletions(-) create mode 100644 tests/ports/tcp_probe/BUILD.bazel create mode 100644 tests/ports/tcp_probe/main.go diff --git a/cmd/svcinit/main.go b/cmd/svcinit/main.go index 3df97ee..09d7878 100644 --- a/cmd/svcinit/main.go +++ b/cmd/svcinit/main.go @@ -399,11 +399,11 @@ func assignPorts( // register binds a resolved port under its target label and every alias, in the rich // port/service maps. The legacy string->port view (ASSIGNED_PORTS, substitution, // /v0/port) is derived from portsMap on demand. - register := func(serviceLabel, portName, domain, portStr, target string, aliases []string) { + register := func(serviceLabel, portName, hostname, portStr, target string, aliases []string) { info := svclib.BindingInfo{ - Origin: net.JoinHostPort(domain, portStr), - Domain: domain, - Port: portStr, + Origin: net.JoinHostPort(hostname, portStr), + Hostname: hostname, + Port: portStr, } keys := append([]string{target}, aliases...) @@ -418,9 +418,9 @@ func assignPorts( continue } - domain := spec.Domain - if domain == "" { - domain = "127.0.0.1" + hostname := spec.Hostname + if hostname == "" { + hostname = "127.0.0.1" } for _, binding := range spec.PortBindings { @@ -435,9 +435,9 @@ func assignPorts( // External services are not managed by us; their ports are reachable as-is at the FQDN. if spec.Type == "external_service" { if !terseOutput { - log.Printf("Registering external port %s for %s (%s)\n", binding.Value, binding.Target, domain) + log.Printf("Registering external port %s for %s (%s)\n", binding.Value, binding.Target, hostname) } - register(label, binding.Name, domain, binding.Value, binding.Target, binding.Aliases) + register(label, binding.Name, hostname, binding.Value, binding.Target, binding.Aliases) continue } @@ -492,7 +492,7 @@ func assignPorts( log.Printf("Assigning port %s to %s\n", portStr, binding.Target) } - register(label, binding.Name, domain, portStr, binding.Target, binding.Aliases) + register(label, binding.Name, hostname, portStr, binding.Target, binding.Aliases) if !spec.SoReuseportAware { toClose = append(toClose, reservedPort) @@ -664,9 +664,9 @@ func buildReplacements(portsMap svclib.PortsMap, prefix string) []Replacement { for label, info := range portsMap { replacements = append(replacements, Replacement{Old: prefix + label + "}", New: info.Port}, - // Rich origin/domain tokens. A "::" delimiter is used since it can't appear in a label. + // Rich origin/hostname tokens. A "::" delimiter is used since it can't appear in a label. Replacement{Old: prefix + label + "::origin}", New: info.Origin}, - Replacement{Old: prefix + label + "::domain}", New: info.Domain}, + Replacement{Old: prefix + label + "::hostname}", New: info.Hostname}, ) } return replacements diff --git a/docs/itest.md b/docs/itest.md index 1cefb20..6d9c322 100644 --- a/docs/itest.md +++ b/docs/itest.md @@ -47,7 +47,7 @@ This can be used in conjunction with the `/v0/port` API to let other tools inter # Ports, hostnames, and external services Ports can be declared as first-class targets with `itest_port`. A port is a handle whose value is an `int` -build-setting flag (`0` = autoassign); the host/domain is supplied by the internal or external service that +build-setting flag (`0` = autoassign); the hostname is supplied by the internal or external service that binds it. The value can be pinned from the command line via `--//pkg:my_port=8080`. A given port may only ever be bound once. @@ -61,9 +61,9 @@ into the test binary and every child service, and are also available through the control APIs: - `ITEST_PORTS_MAP`: a JSON object keyed by port target label (and aliases): - `{"@@//pkg:my_port": {"origin": "127.0.0.1:54321", "domain": "127.0.0.1", "port": "54321"}, ...}` + `{"@@//pkg:my_port": {"origin": "127.0.0.1:54321", "hostname": "127.0.0.1", "port": "54321"}, ...}` - `ITEST_SERVICES_MAP`: a JSON object keyed by service target label, then by port name (and aliases): - `{"@@//pkg:my_service": {"http": {"origin": "...", "domain": "...", "port": "..."}}, ...}` + `{"@@//pkg:my_service": {"http": {"origin": "...", "hostname": "...", "port": "..."}}, ...}` The legacy `ASSIGNED_PORTS` env var, the `GET_ASSIGNED_PORT_BIN` helper, and the `/v0/port` endpoint all continue to work as before. @@ -75,7 +75,7 @@ continue to work as before.
 load("@rules_itest//:itest.bzl", "itest_service")
 
-itest_service(name, autoassign_port, data, deps, domain, enforce_graceful_shutdown, env, exe,
+itest_service(name, autoassign_port, data, deps, hostname, enforce_graceful_shutdown, env, exe,
               expected_start_duration, health_check, health_check_args, health_check_interval,
               health_check_timeout, hot_reloadable, http_health_check_address, named_ports,
               port, ports, shutdown_signal, shutdown_timeout, so_reuseport_aware)
@@ -93,7 +93,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio
 | name |  A unique name for this target.   | Name | required |  |
 | deps |  Services/tasks that must be started before this service/task can be started. Can be `itest_service`, `itest_task`, or `itest_service_group`.   | List of labels | optional |  `[]`  |
 | data |  -   | List of labels | optional |  `[]`  |
-| domain |  The host that this service's ports are reachable on. Defaults to `127.0.0.1` for locally-managed services.   | String | optional |  `"127.0.0.1"`  |
+| hostname |  The host that this service's ports are reachable on. Defaults to `127.0.0.1` for locally-managed services.   | String | optional |  `"127.0.0.1"`  |
 | autoassign_port |  If true, the service manager will pick a free port and assign it to the service. The port will be interpolated into `$${PORT}` in the service's `http_health_check_address` and `args`. It will also be exported under the target's fully qualified label in the service-port mapping.

The assigned ports for all services are available for substitution in `http_health_check_address` and `args` (in case one service needs the address for another one.) For example, the following substitution: `args = ["-client-addr", "127.0.0.1:$${@@//label/for:service}"]`

The service-port mapping is a JSON string -> string map propagated through the `ASSIGNED_PORTS` env var. For example, a port (as a string) can be retrieved with the following JS code: `JSON.parse(process.env["ASSIGNED_PORTS"])["@@//label/for:service"]`.

Alternately, the env will also contain the location of a binary that can return the port, for contexts without a readily-accessible JSON parser. For example, the following Bash command: `PORT=$($GET_ASSIGNED_PORT_BIN @@//label/for:service)` | Boolean | optional | `False` | | enforce_graceful_shutdown | If set to True, the service manager will fail the service_test if the service had to be forcefully killed if the signal was not SIGKILL and after the shutdown timeout elapsed.

This needs to be False to have coverage of your services but don't want a them to be graceful at shutdown | Label | optional | `"@rules_itest//:enforce_graceful_shutdown"` | | env | The service manager will merge these variables into the environment when spawning the underlying binary. | Dictionary: String -> String | optional | `{}` | @@ -120,7 +120,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio
 load("@rules_itest//:itest.bzl", "itest_external_service")
 
-itest_external_service(name, data, deferred, deps, domain, expected_start_duration, health_check,
+itest_external_service(name, data, deferred, deps, hostname, expected_start_duration, health_check,
                        health_check_args, health_check_interval, health_check_timeout, http_health_check_address,
                        port_numbers, ports)
 
@@ -140,7 +140,7 @@ Bazel `select()` to run a test suite against production-like instances. | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| domain | The fully-qualified domain name (FQDN) that this external service is reachable on, e.g. `my_service.test.mycompany.com`. | String | required | | +| hostname | The fully-qualified domain name (FQDN) that this external service is reachable on, e.g. `my_service.test.mycompany.com`. | String | required | | | ports | Maps `itest_port` targets that this external service exposes to a port name. Provide the literal port number for each name via `port_numbers`. | Dictionary: Label -> String | optional | `{}` | | port_numbers | Maps each port name (from `ports`) to the literal port number it is reachable on at the FQDN. | Dictionary: String -> String | optional | `{}` | | data | - | List of labels | optional | `[]` | diff --git a/itest.bzl b/itest.bzl index db35e85..7d6bad4 100644 --- a/itest.bzl +++ b/itest.bzl @@ -53,12 +53,12 @@ def port_ref(label): return "$${%s}" % _to_relative_port(label) def port_origin(label): - """References the origin (`:`) of an `itest_port` target in `args`/`env`/`http_health_check_address`.""" + """References the origin (`:`) of an `itest_port` target in `args`/`env`/`http_health_check_address`.""" return "$${%s::origin}" % _to_relative_port(label) -def port_domain(label): - """References the domain (host) of an `itest_port` target in `args`/`env`/`http_health_check_address`.""" - return "$${%s::domain}" % _to_relative_port(label) +def port_hostname(label): + """References the hostname (host) of an `itest_port` target in `args`/`env`/`http_health_check_address`.""" + return "$${%s::hostname}" % _to_relative_port(label) def itest_port(name, build_setting_default = 0, **kwargs): """Declares a first-class port target. diff --git a/private/itest.bzl b/private/itest.bzl index d7010b6..1a864c8 100644 --- a/private/itest.bzl +++ b/private/itest.bzl @@ -46,7 +46,7 @@ This can be used in conjunction with the `/v0/port` API to let other tools inter # Ports, hostnames, and external services Ports can be declared as first-class targets with `itest_port`. A port is a handle whose value is an `int` -build-setting flag (`0` = autoassign); the host/domain is supplied by the internal or external service that +build-setting flag (`0` = autoassign); the hostname is supplied by the internal or external service that binds it. The value can be pinned from the command line via `--//pkg:my_port=8080`. A given port may only ever be bound once. @@ -60,9 +60,9 @@ into the test binary and every child service, and are also available through the control APIs: - `ITEST_PORTS_MAP`: a JSON object keyed by port target label (and aliases): - `{"@@//pkg:my_port": {"origin": "127.0.0.1:54321", "domain": "127.0.0.1", "port": "54321"}, ...}` + `{"@@//pkg:my_port": {"origin": "127.0.0.1:54321", "hostname": "127.0.0.1", "port": "54321"}, ...}` - `ITEST_SERVICES_MAP`: a JSON object keyed by service target label, then by port name (and aliases): - `{"@@//pkg:my_service": {"http": {"origin": "...", "domain": "...", "port": "..."}}, ...}` + `{"@@//pkg:my_service": {"http": {"origin": "...", "hostname": "...", "port": "..."}}, ...}` The legacy `ASSIGNED_PORTS` env var, the `GET_ASSIGNED_PORT_BIN` helper, and the `/v0/port` endpoint all continue to work as before. @@ -109,6 +109,14 @@ def _validate_unique_port_bindings(services): )) seen[binding.target] = label +def _named_port_target(label, name): + """Returns the port target label for a named port owned by `label` (e.g. `@@//pkg:svc.http`). + + This is the analysis-phase counterpart to `_to_relative_named_port` in `//:itest.bzl`, which can + only run during loading. Both must agree on the `