Allow assigning ports to service_group

docs/itest.md

@@ -78,7 +78,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio
 ## itest_service_group
 
 <pre>
-itest_service_group(<a href="#itest_service_group-name">name</a>, <a href="#itest_service_group-services">services</a>)
+itest_service_group(<a href="#itest_service_group-name">name</a>, <a href="#itest_service_group-autoassign_port">autoassign_port</a>, <a href="#itest_service_group-named_ports">named_ports</a>, <a href="#itest_service_group-services">services</a>, <a href="#itest_service_group-so_reuseport_aware">so_reuseport_aware</a>)
 </pre>
 
 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 |
 | :------------- | :------------- | :------------- | :------------- | :------------- |
 | <a id="itest_service_group-name"></a>name |  A unique name for this target.   | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required |  |
+| <a id="itest_service_group-autoassign_port"></a>autoassign_port |  If true, the service manager will pick a free port and assign it to the service.         The port will be interpolated into <code>$${PORT}</code> in the service's <code>http_health_check_address</code> and <code>args</code>.         It will also be exported under the target's fully qualified label in the service-port mapping.<br><br>        The assigned ports for all services are available for substitution in <code>http_health_check_address</code> and <code>args</code> (in case one service needs the address for another one.)         For example, the following substitution: <code>args = ["-client-addr", "127.0.0.1:$${@@//label/for:service}"]</code><br><br>        The service-port mapping is a JSON string -&gt; int map propagated through the <code>ASSIGNED_PORTS</code> env var.         For example, a port can be retrieved with the following JS code:         <code>JSON.parse(process.env["ASSIGNED_PORTS"])["@@//label/for:service"]</code>.<br><br>        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:         <code>PORT=$($GET_ASSIGNED_PORT_BIN @@//label/for:service)</code>   | Boolean | optional | <code>False</code> |
+| <a id="itest_service_group-named_ports"></a>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 <code>named_ports = ["http_port"]</code> will be assigned a fully-qualified name of <code>@@//label/for:service:http_port</code>.<br><br>        Named ports are accessible through the service-port mapping. For more details, see <code>autoassign_port</code>.   | List of strings | optional | <code>[]</code> |
 | <a id="itest_service_group-services"></a>services |  Services/tasks that comprise this group. Can be <code>itest_service</code>, <code>itest_task</code>, or <code>itest_service_group</code>.   | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | <code>[]</code> |
+| <a id="itest_service_group-so_reuseport_aware"></a>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.<br><br>        Must only be set when <code>autoassign_port</code> is enabled or <code>named_ports</code> are used.   | Boolean | optional | <code>False</code> |
 
 
 <a id="itest_task"></a>
@@ -127,7 +130,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio
 ## service_test
 
 <pre>
-service_test(<a href="#service_test-name">name</a>, <a href="#service_test-data">data</a>, <a href="#service_test-env">env</a>, <a href="#service_test-services">services</a>, <a href="#service_test-test">test</a>)
+service_test(<a href="#service_test-name">name</a>, <a href="#service_test-autoassign_port">autoassign_port</a>, <a href="#service_test-data">data</a>, <a href="#service_test-env">env</a>, <a href="#service_test-named_ports">named_ports</a>, <a href="#service_test-services">services</a>, <a href="#service_test-so_reuseport_aware">so_reuseport_aware</a>, <a href="#service_test-test">test</a>)
 </pre>
 
 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 |
 | :------------- | :------------- | :------------- | :------------- | :------------- |
 | <a id="service_test-name"></a>name |  A unique name for this target.   | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required |  |
+| <a id="service_test-autoassign_port"></a>autoassign_port |  If true, the service manager will pick a free port and assign it to the service.         The port will be interpolated into <code>$${PORT}</code> in the service's <code>http_health_check_address</code> and <code>args</code>.         It will also be exported under the target's fully qualified label in the service-port mapping.<br><br>        The assigned ports for all services are available for substitution in <code>http_health_check_address</code> and <code>args</code> (in case one service needs the address for another one.)         For example, the following substitution: <code>args = ["-client-addr", "127.0.0.1:$${@@//label/for:service}"]</code><br><br>        The service-port mapping is a JSON string -&gt; int map propagated through the <code>ASSIGNED_PORTS</code> env var.         For example, a port can be retrieved with the following JS code:         <code>JSON.parse(process.env["ASSIGNED_PORTS"])["@@//label/for:service"]</code>.<br><br>        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:         <code>PORT=$($GET_ASSIGNED_PORT_BIN @@//label/for:service)</code>   | Boolean | optional | <code>False</code> |
 | <a id="service_test-data"></a>data |  -   | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | <code>[]</code> |
 | <a id="service_test-env"></a>env |  The service manager will merge these variables into the environment when spawning the underlying binary.   | <a href="https://bazel.build/rules/lib/dict">Dictionary: String -> String</a> | optional | <code>{}</code> |
+| <a id="service_test-named_ports"></a>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 <code>named_ports = ["http_port"]</code> will be assigned a fully-qualified name of <code>@@//label/for:service:http_port</code>.<br><br>        Named ports are accessible through the service-port mapping. For more details, see <code>autoassign_port</code>.   | List of strings | optional | <code>[]</code> |
 | <a id="service_test-services"></a>services |  Services/tasks that comprise this group. Can be <code>itest_service</code>, <code>itest_task</code>, or <code>itest_service_group</code>.   | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | <code>[]</code> |
+| <a id="service_test-so_reuseport_aware"></a>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.<br><br>        Must only be set when <code>autoassign_port</code> is enabled or <code>named_ports</code> are used.   | Boolean | optional | <code>False</code> |
 | <a id="service_test-test"></a>test |  The underlying test target to execute once the services have been brought up and healthchecked.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
 
 

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,

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)
 		}

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"],
+)