From edd88931960c1fb346ee5a53df5b0ac1d07df751 Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Fri, 20 Sep 2024 20:33:04 -0400 Subject: [PATCH] Allow assigning ports to service_group --- docs/itest.md | 10 ++++++++-- private/itest.bzl | 34 ++++++++++++++++++++++------------ runner/service_instance.go | 8 ++++++-- tests/BUILD.bazel | 19 +++++++++++++++++++ 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/docs/itest.md b/docs/itest.md index 74374e4..d9f9d05 100644 --- a/docs/itest.md +++ b/docs/itest.md @@ -78,7 +78,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio ## itest_service_group 
-itest_service_group(name, services)
+itest_service_group(name, autoassign_port, named_ports, services, so_reuseport_aware)
A service group is a collection of services/tasks. @@ -94,7 +94,10 @@ It can bring up multiple services with a single `bazel run` command, which is us | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | +| 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 | +| 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 | [] | | services | Services/tasks that comprise this group. Can be itest_service, itest_task, or itest_service_group. | List of labels | 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 | @@ -127,7 +130,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio ## service_test 
-service_test(name, data, env, services, test)
+service_test(name, autoassign_port, data, env, named_ports, services, so_reuseport_aware, test)
Brings up a set of services/tasks and runs a test target against them. @@ -162,9 +165,12 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | +| 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 | | data | - | List of labels | optional | [] | | env | The service manager will merge these variables into the environment when spawning the underlying binary. | Dictionary: String -> 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 | [] | | services | Services/tasks that comprise this group. Can be itest_service, itest_task, or itest_service_group. | List of labels | 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 | | test | The underlying test target to execute once the services have been brought up and healthchecked. | Label | optional | None | diff --git a/private/itest.bzl b/private/itest.bzl index 7ebb00e..4af6dbf 100644 --- a/private/itest.bzl +++ b/private/itest.bzl @@ -56,7 +56,7 @@ def _run_environment(ctx, service_specs_file): # Flags "SVCINIT_ALLOW_CONFIGURING_TMPDIR": str(ctx.attr._allow_configuring_tmpdir[BuildSettingInfo].value), "SVCINIT_ENABLE_PER_SERVICE_RELOAD": str(ctx.attr._enable_per_service_reload[BuildSettingInfo].value), - "SVCINIT_KEEP_SERVICES_UP": str(ctx.attr.keep_services_up[BuildSettingInfo].value), + "SVCINIT_KEEP_SERVICES_UP": str(ctx.attr._keep_services_up[BuildSettingInfo].value), "SVCINIT_TERSE_OUTPUT": str(ctx.attr._terse_svcinit_output[BuildSettingInfo].value), # Other configuration @@ -217,8 +217,8 @@ def _itest_service_impl(ctx): return _itest_binary_impl(ctx, extra_service_spec_kwargs, extra_exe_runfiles) -_itest_service_attrs = _itest_binary_attrs | { - # Note, autoassigning a port is a little racy. If you can stick to hardcoded ports and network namespace, you should prefer that. +_port_assignment_attrs = { + # Note, autoassigning a port is a little racy. If you can stick to hardcoded ports and network namespace, you should prefer that. "autoassign_port": attr.bool( doc = """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`. @@ -242,6 +242,16 @@ _itest_service_attrs = _itest_binary_attrs | { Named ports are accessible through the service-port mapping. For more details, see `autoassign_port`.""", ), + "so_reuseport_aware": attr.bool( + doc = """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.""", + ), +} + +_itest_service_attrs = _itest_binary_attrs | _port_assignment_attrs | { "health_check": attr.label( cfg = "target", mandatory = False, @@ -274,13 +284,6 @@ _itest_service_attrs = _itest_binary_attrs | { 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}",`""", ), - "so_reuseport_aware": attr.bool( - doc = """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.""", - ), } itest_service = rule( @@ -309,10 +312,17 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio def _itest_service_group_impl(ctx): services = _collect_services(ctx.attr.services) + + if ctx.attr.so_reuseport_aware and not (ctx.attr.autoassign_port or ctx.attr.named_ports): + fail("SO_REUSEPORT awareness only makes sense when using port autoassignment") + service = struct( type = "group", label = str(ctx.label), deps = [str(service.label) for service in ctx.attr.services], + autoassign_port = ctx.attr.autoassign_port, + so_reuseport_aware = ctx.attr.so_reuseport_aware, + named_ports = ctx.attr.named_ports, ) services[service.label] = service @@ -327,12 +337,12 @@ def _itest_service_group_impl(ctx): _ServiceGroupInfo(services = services), ] -_itest_service_group_attrs = { +_itest_service_group_attrs = _port_assignment_attrs | _svcinit_attrs | { "services": attr.label_list( providers = [_ServiceGroupInfo], doc = "Services/tasks that comprise this group. Can be `itest_service`, `itest_task`, or `itest_service_group`.", ), -} | _svcinit_attrs +} itest_service_group = rule( implementation = _itest_service_group_impl, diff --git a/runner/service_instance.go b/runner/service_instance.go index 5ad0852..0c54096 100644 --- a/runner/service_instance.go +++ b/runner/service_instance.go @@ -104,14 +104,18 @@ var httpClient = http.Client{ } func (s *ServiceInstance) HealthCheck(ctx context.Context, expectedStartDuration time.Duration) bool { - httpHealthCheckReq, _ := http.NewRequestWithContext(ctx, "GET", s.HttpHealthCheckAddress, nil) coloredLabel := s.Colorize(s.Label) - shouldSilence := s.startTime.Add(expectedStartDuration).After(time.Now()) isHealthy := true var err error if s.HttpHealthCheckAddress != "" { + httpHealthCheckReq, err := http.NewRequestWithContext(ctx, "GET", s.HttpHealthCheckAddress, nil) + if err != nil { + log.Printf("Failed to construct healthcheck request for %s: %v\n", coloredLabel, err) + return false + } + if !s.HealthcheckAttempted() || !shouldSilence { log.Printf("HTTP Healthchecking %s (pid %d) : %s\n", coloredLabel, s.Pid(), s.HttpHealthCheckAddress) } diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index c593b43..59a9666 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -112,3 +112,22 @@ itest_service( exe = "//go_service", http_health_check_address = "http://127.0.0.1:$${PORT}", ) + +# Test port assignment through a group +itest_service( + name = "_speedy_service2", + args = [ + "-port", + "$${@@//:speedy_service2:port}", + ], + env = {"foo": "bar"}, + exe = "//go_service", + http_health_check_address = "http://127.0.0.1:$${@@//:speedy_service2:port}", + tags = ["manual"], +) + +itest_service_group( + name = "speedy_service2", + services = [":_speedy_service"], + named_ports = ["port"], +) \ No newline at end of file