Continue to backfill empty PortBindings in API 1.53

- introduced by commit 0ca7ac3 ("daemon: backfill empty PBs
  slices for backward compat")

Signed-off-by: Rob Murray <rob.murray@docker.com>

api/docs/CHANGELOG.md

@@ -33,7 +33,7 @@ keywords: "API, Docker, rcli, REST, documentation"
 * Deprecated: the Engine was automatically backfilling empty `PortBindings` lists with
   a PortBinding with an empty HostIP and HostPort when calling `POST /containers/{id}/start`.
   This behavior is now deprecated, and a warning is returned by `POST /containers/create`.
-  The next API version will drop empty `PortBindings` list altogether.
+  A future API version will drop empty `PortBindings` list altogether.
 * `GET /images/{name}/json` now omits the following `Config` fields when
   not set, to closer align with the implementation of the [OCI Image Specification](https://github.com/opencontainers/image-spec/blob/v1.1.1/specs-go/v1/config.go#L23-L62)
   `Cmd`, `Entrypoint`, `Env`, `Labels`, `OnBuild`, `User`, `Volumes`, and `WorkingDir`.

daemon/server/router/container/container_routes.go

@@ -903,18 +903,25 @@ func handlePortBindingsBC(hostConfig *container.HostConfig, version string) stri
 		if len(bindings) > 0 {
 			continue
 		}
-		if versions.GreaterThan(version, "1.52") && len(bindings) == 0 {
-			// Starting with API 1.53, no backfilling is done. An empty slice
-			// of port bindings is treated as "no port bindings" by the daemon,
-			// but it still needs to backfill empty slices when loading the
-			// on-disk state for containers created by older versions of the
-			// Engine. Drop the PortBindings entry to ensure that no backfilling
-			// will happen when restarting the daemon.
-			delete(hostConfig.PortBindings, port)
-			continue
-		}
+		/*
+			API 1.53 shipped in a minor release. We cannot introduce a breaking change there, so
+			we must still backfill empty port bindings. This change can be re-introduced for the
+			API version that ships in 30.x. Note that some networking tests will need fixing.
+			See https://github.com/moby/moby/issues/51727
 
-		if versions.Equal(version, "1.52") {
+			if versions.GreaterThan(version, "1.52") && len(bindings) == 0 {
+				// Starting with API 1.53, no backfilling is done. An empty slice
+				// of port bindings is treated as "no port bindings" by the daemon,
+				// but it still needs to backfill empty slices when loading the
+				// on-disk state for containers created by older versions of the
+				// Engine. Drop the PortBindings entry to ensure that no backfilling
+				// will happen when restarting the daemon.
+				delete(hostConfig.PortBindings, port)
+				continue
+			}
+		*/
+
+		if versions.GreaterThanOrEqualTo(version, "1.52") {
 			emptyPBs = append(emptyPBs, port.String())
 		}
 
@@ -922,7 +929,7 @@ func handlePortBindingsBC(hostConfig *container.HostConfig, version string) stri
 	}
 
 	if len(emptyPBs) > 0 {
-		return fmt.Sprintf("Following container port(s) have an empty list of port-bindings: %s. Starting with API 1.53, such bindings will be discarded.", strings.Join(emptyPBs, ", "))
+		return fmt.Sprintf("Following container port(s) have an empty list of port-bindings: %s. Such bindings will be discarded in a future version.", strings.Join(emptyPBs, ", "))
 	}
 
 	return ""

integration/network/bridge/bridge_linux_test.go

@@ -789,6 +789,8 @@ func TestPortMappingRestore(t *testing.T) {
 	const svrName = "svr"
 	cid := ctr.Run(ctx, t, c,
 		ctr.WithExposedPorts("80/tcp"),
+		// TODO(robmry): this test supplies an empty list of PortBindings.
+		// https://github.com/moby/moby/issues/51727 will break it.
 		ctr.WithPortMap(networktypes.PortMap{networktypes.MustParsePort("80/tcp"): {}}),
 		ctr.WithName(svrName),
 		ctr.WithRestartPolicy(containertypes.RestartPolicyUnlessStopped),
@@ -996,7 +998,7 @@ func TestEmptyPortBindingsBC(t *testing.T) {
 			{}, // An empty PortBinding is backfilled
 		}}
 		expWarnings := []string{
-			"Following container port(s) have an empty list of port-bindings: 80/tcp. Starting with API 1.53, such bindings will be discarded.",
+			"Following container port(s) have an empty list of port-bindings: 80/tcp. Such bindings will be discarded in a future version.",
 		}
 
 		mappings, warnings := createInspect(t, "1.52", []networktypes.PortBinding{})
@@ -1005,6 +1007,7 @@ func TestEmptyPortBindingsBC(t *testing.T) {
 	})
 
 	t.Run("no backfilling on API 1.53", func(t *testing.T) {
+		t.Skip("Backfilling was not removed in 1.53. See https://github.com/moby/moby/issues/51727")
 		expMappings := networktypes.PortMap{}
 		expWarnings := make([]string, 0)
 

integration/networking/bridge_linux_test.go

@@ -389,6 +389,8 @@ func TestBridgeINCRouted(t *testing.T) {
 			container.WithNetworkMode(netName),
 			container.WithName("ctr-"+gwMode),
 			container.WithExposedPorts("80/tcp"),
+			// TODO(robmry): this test supplies an empty list of PortBindings.
+			// https://github.com/moby/moby/issues/51727 will break it.
 			container.WithPortMap(networktypes.PortMap{networktypes.MustParsePort("80/tcp"): {}}),
 		)
 		t.Cleanup(func() {

integration/networking/port_mapping_linux_test.go

@@ -706,7 +706,9 @@ func TestDirectRoutingOpenPorts(t *testing.T) {
 			container.WithName("ctr-"+gwMode),
 			container.WithExposedPorts("80/tcp"),
 			container.WithPortMap(networktypes.PortMap{
-				networktypes.MustParsePort("80/tcp"): {},
+				// TODO(robmry): this test supplies an empty list of PortBindings.
+				// https://github.com/moby/moby/issues/51727 will break it.
+				networktypes.MustParsePort("80/tcp"): {{}},
 			}),
 		)
 		t.Cleanup(func() {