Skip to content
This repository has been archived by the owner on Mar 21, 2022. It is now read-only.

Empty exposedPorts: ImmutableSet serialization not working as configured by docker-client, with Jackson 2.9.x #893

Closed
flozano opened this issue Sep 20, 2017 · 12 comments

Comments

@flozano
Copy link

flozano commented Sep 20, 2017

Description

I am creating a container with the exposedPorts property filled. However, it's not being used:

10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "POST /containers/create?name=test-kafka-1392566563 HTTP/1.1[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "Accept: application/json[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "Content-Type: application/json[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "User-Agent: Jersey/2.25.1 (Apache HttpClient 4.5.3)[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "Transfer-Encoding: chunked[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "Host: 192.168.99.100:2376[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "Connection: Keep-Alive[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "Accept-Encoding: gzip,deflate[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "12[\r][\n]"
10:59:20.247 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "{"ExposedPorts":{}[\r][\n]"
10:59:20.294 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "215[\r][\n]"
10:59:20.294 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> ","Env":["SSL_TRUSTSTORE_LOCATION=/var/private/ssl/server.truststore.jks","ADVERTISED_HOST=192.168.99.100","SSL_CLIENT_AUTH=required","SSL_KEYSTORE_LOCATION=/var/private/ssl/server.keystore.jks","ADVERTISED_SSL_PORT=22480","ADVERTISED_PORT=17186"],"Cmd":[],"Image":"jolivares/kafka:latest","Volumes":{"/tmp/some-strange-volume":{}},"HostConfig":{"Binds":["/Users/flozano/Projects/java/platform-common/test-container/bin/certs:/var/private/ssl"],"PortBindings":{},"PublishAllPorts":true,"NetworkMode":"docker-container-provider-test"}}[\r][\n]"
10:59:20.294 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "0[\r][\n]"
10:59:20.294 [jersey-client-async-executor-1] DEBUG org.apache.http.wire - http-outgoing-3 >> "[\r][\n]"

I am checking thru the debugger however that the ContainerConfig contains the right set of values.

I have Jackson 2.9.1 and Jersey 2.25.1 in classpath, and I can't use lower versions because of other dependencies. My guess is that it's related to this... with older versions (docker-client 8.8.0, jackson 2.8.9, hk2-2.5.0-b38) it didn't happen.

@flozano flozano changed the title Empty exposedPorts (jackson 2.9.1, jersey 2.25.1?) Empty exposedPorts (jackson 2.9.1, jersey 2.25.1, hk2-2.5.0-b54) Sep 20, 2017
@flozano
Copy link
Author

flozano commented Sep 20, 2017

I have isolated dependency issue to Jackson (2.8.X doesn't show this behaviour). Could it be possible that Jackson 2.9.1 is not detecting the ImmutableSet of exposedPorts as a collection?

@flozano
Copy link
Author

flozano commented Sep 20, 2017

Can't isolate in stand-alone test with any version of Jackson.

public class ImmutableSetIssue {
	ImmutableSet<String> set = ImmutableSet.copyOf(Arrays.asList("a", "b", "c"));

	@Test
	public void containerConfig() throws IOException {
		ObjectMapper om = new ObjectMapper();
		ContainerConfig c = ContainerConfig.builder().exposedPorts(set).build();
		ObjectNode on = (ObjectNode) om.readTree(om.writeValueAsString(c));
		ArrayNode n = (ArrayNode) on.get("ExposedPorts");
		assertEquals("a", n.get(0).asText());
		assertEquals("b", n.get(1).asText());
		assertEquals("c", n.get(2).asText());
	}
}

@flozano
Copy link
Author

flozano commented Sep 20, 2017

It seems there's a custom ImmutableSetSerializer. With it:

  private static class ImmutableSetSerializer extends JsonSerializer<ImmutableSet> {

    @Override
    public void serialize(final ImmutableSet value, final JsonGenerator jgen,
                          final SerializerProvider provider) throws IOException {
      final Map map = (value == null) ? null : Maps.asMap(value, VOID_VALUE);
      OBJECT_MAPPER.writeValue(jgen, map);
    }
  }

map value is the correct one in the debugger ({19763=null, 2362=null, 23425=null})
but just after stepping over OBJECT_MAPPER.writeValue(jgen, map);, this appears in the logs:

19:35:42.280 [jersey-client-async-executor-0] DEBUG org.apache.http.wire - http-outgoing-3 >> "12[\r][\n]"
19:35:42.281 [jersey-client-async-executor-0] DEBUG org.apache.http.wire - http-outgoing-3 >> "{"ExposedPorts":{}[\r][\n]"

it seems _suppressNulls is true in MapSerializer:

    /**
     * Flag that indicates what to do with `null` values, distinct from
     * handling of {@link #_suppressableValue}
     *
     * @since 2.9
     */
    protected final boolean _suppressNulls;

It's false in the MapSerializer member variable by default, but when instantiated from _checkMapContentInclusion:

MapSerializer.<init>(MapSerializer, TypeSerializer, Object, boolean) line: 226	
MapSerializer.withContentInclusion(Object, boolean) line: 291	
BeanSerializerFactory(BasicSerializerFactory)._checkMapContentInclusion(SerializerProvider, BeanDescription, MapSerializer) line: 853	

it's true

@flozano flozano changed the title Empty exposedPorts (jackson 2.9.1, jersey 2.25.1, hk2-2.5.0-b54) Empty exposedPorts (jackson 2.9.1): ImmutableSet serialization not working as configured by docker-client, with Jackson 2.9.x Sep 20, 2017
@flozano flozano changed the title Empty exposedPorts (jackson 2.9.1): ImmutableSet serialization not working as configured by docker-client, with Jackson 2.9.x Empty exposedPorts: ImmutableSet serialization not working as configured by docker-client, with Jackson 2.9.x Sep 20, 2017
@johnflavin
Copy link
Contributor

I'm not sure I understand exactly what you’re trying to do or what is going wrong. So I apologize if this question is off the mark. But are you using ContainerConfig.exposedPorts when what you really want to use is HostConfig.portBindings?

@flozano
Copy link
Author

flozano commented Sep 21, 2017

With PublishAllPorts: true and ExposedPorts as rendered in 2.8.x ({"ExposedPorts":{"18421":null,"21437":null,"2747":null}, when I retrieve the container I get the ports mapped, whereas with the ExposedPorts rendered by 2.9 ( {} ), I don't.

@johnflavin
Copy link
Contributor

Ok, I see. I wasn't aware of that combination of options. Could you give the snippet of code you’re using to set up your configs and create your container?

@johnflavin
Copy link
Contributor

I don't know if this is relevant at all, but in case this is something that needs to be refactored, I want to make a note about the docker-client code for this property vs. the docker API.

In their swagger.yaml, ExposedPorts is "an object mapping ports to an empty object...". I.e. it looks like {"1":{}, "2":{}, ...}. Whereas in docker-client it is a Set. Which means it looks like {"1", "2", ...}.

This seems like almost the right thing to do, because I think the "map of values to empty objects" is an attempt to hack a set type into go; see https://github.com/deckarep/golang-set, a go library which makes a set type by doing this exact "map of whatever to an empty struct" thing, and which claims to be used in docker.

But I don't know how our internal Set is being turned into the correct form of "map of ports to empty objects" when sent on the wire, or if it even is. Perhaps this Jackson version difference is just exposing a bug that has been there all along but has previously been unnoticed. For contrast, see ContainerConfig.volumes, which has the same form as ExposedPorts in the docker API but in docker-client is explicitly a ImmutableMap<String, Map> mapping volume strings to empty objects. Maybe changing ContainerConfig.exposedPorts from ImmutableSet<String> to ImmutableMap<String, Map> would solve this problem, regardless of the Jackson version.

@flozano
Copy link
Author

flozano commented Sep 21, 2017

Yes, it looks like it's the case, ImmutableSet is probably not the right value for this - or, if it is, it should be mapped to a map of String => Map (empty), not String => Map (null). That would fix the issue in this case.

@flozano
Copy link
Author

flozano commented Sep 22, 2017

With regard to:

But I don't know how our internal Set is being turned into the correct form of "map of ports to empty objects" when sent on the wire, or if it even is.

You have:

  private static class ImmutableSetSerializer extends JsonSerializer<ImmutableSet> {

    @Override
    public void serialize(final ImmutableSet value, final JsonGenerator jgen,
                          final SerializerProvider provider) throws IOException {
      final Map map = (value == null) ? null : Maps.asMap(value, VOID_VALUE);
      OBJECT_MAPPER.writeValue(jgen, map);
    }
  }

  private static class ImmutableSetDeserializer extends JsonDeserializer<ImmutableSet> {

    @Override
    public ImmutableSet<?> deserialize(final JsonParser jp, final DeserializationContext ctxt)
        throws IOException {
      final Map map = OBJECT_MAPPER.readValue(jp, Map.class);
      return (map == null) ? null : ImmutableSet.copyOf(map.keySet());
    }
  }

in your ObjectMapperProvider.java, and you register these in the object mapper you use.

@johnflavin
Copy link
Contributor

Ah, yes, thank you. That's exactly what I was looking for but couldn't find. My guess is that changing the VOID_VALUE to something else could set the value correctly.

I pushed a change for this to a branch on my fork, 893-set-to-map on johnflavin/docker-client. Is it possible for you to build a jar from that and test it out?

@flozano
Copy link
Author

flozano commented Oct 1, 2017

Yes, I could confirm that the issue goes away with the change in that branch. ExposedPorts is now properly filled:

08:44:44.936 [jersey-client-async-executor-0] DEBUG org.apache.http.wire - http-outgoing-3 >> "{"ExposedPorts":{"2427":{},"20452":{},"18081":{}}[\r][\n]"

@johnflavin
Copy link
Contributor

Thanks for checking that out. I’ll clean this up and open a PR for it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants