Fix client setup for recovery after server timeout 49/4849/7
authorAndrew Gauld <agauld@att.com>
Fri, 12 Jul 2019 19:54:41 +0000 (19:54 +0000)
committerAndrew Gauld <agauld@att.com>
Mon, 22 Jul 2019 13:52:37 +0000 (13:52 +0000)
Change-Id: I44e5a9145962734ec1dfac3309d9910227579d96
Issue-ID: ACUMOS-3193
Signed-off-by: Andrew Gauld <agauld@att.com>
.gitignore
acumos-fgw-client/src/main/java/org/acumos/federation/client/ClientBase.java
docs/release-notes.rst
gateway/src/main/java/org/acumos/federation/gateway/Clients.java
gateway/src/main/java/org/acumos/federation/gateway/ContentServiceImpl.java
gateway/src/test/java/org/acumos/federation/gateway/ClientsTest.java
gateway/src/test/java/org/acumos/federation/gateway/GatewayControllerTest.java

index 83485fa..97d5c2c 100644 (file)
@@ -1,5 +1,6 @@
 .classpath
 .project
+.flattened-pom.xml
 /.settings/
 /bin/
 /logs/
index 50ecb23..8f47029 100644 (file)
@@ -32,15 +32,11 @@ import java.security.KeyStore;
 import java.util.Collections;
 
 import org.apache.http.client.HttpClient;
-import org.apache.http.config.RegistryBuilder;
-import org.apache.http.conn.socket.ConnectionSocketFactory;
-import org.apache.http.conn.socket.PlainConnectionSocketFactory;
 import org.apache.http.ssl.SSLContextBuilder;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.ssl.SSLContexts;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.client.HttpClients;
-import org.apache.http.impl.conn.BasicHttpClientConnectionManager;
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.DeserializationContext;
@@ -223,8 +219,6 @@ public class ClientBase {
                }
                SSLContextBuilder sslContextBuilder = SSLContexts.custom();
                TlsConfig tls = conf.getSsl();
-               RegistryBuilder<ConnectionSocketFactory> registryBuilder = RegistryBuilder.create();
-               registryBuilder.register("http", PlainConnectionSocketFactory.getSocketFactory());
                HttpClientBuilder clientBuilder = HttpClients.custom();
                try {
                        if (tls != null) {
@@ -239,8 +233,6 @@ public class ClientBase {
                                }
                        }
                        SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory(sslContextBuilder.build(), new String[] { "TLSv1.2" }, null, SSLConnectionSocketFactory.getDefaultHostnameVerifier());
-                       registryBuilder.register("https", sslSocketFactory);
-                       clientBuilder.setConnectionManager(new BasicHttpClientConnectionManager(registryBuilder.build()));
                        clientBuilder.setSSLSocketFactory(sslSocketFactory);
                } catch (Exception ex) {
                        throw new TlsConfigException("Invalid TLS configuration", ex);
index 9bb2f72..3903706 100644 (file)
@@ -23,8 +23,10 @@ Federation Gateway Release Notes
 This server is available as a Docker image in a Docker registry at the Linux Foundation.
 The image name is "federation-gateway" and the tag is a version string as shown below.
 
-Version 2.3.0, 2019-06-14
+Version 2.3.0, 2019-07-22
 -------------------------
+* Fix DI artifact create fail due to Federation use of a stale TCP stream (`ACUMOS-3193 <https://jira.acumos.org/browse/ACUMOS-3193>`_)
+
 * Publish E5 Federation client library (`ACUMOS-2760 <https://jira.acumos.org/browse/ACUMOS-2760>`_)
 
   3 new sub-projects are introduced, in addition to the existing "gateway" sub-project.
index 005b2d2..bd29c04 100644 (file)
@@ -38,6 +38,22 @@ import org.acumos.federation.client.FederationClient;
  * By mocking this bean, all external access can be stubbed out.
  */
 public class Clients {
+       /*
+        * Implementation note:
+        *
+        * Ideally, all clients would be created at startup, and the getXXX()
+        * methods would just return them, however, while the Spring framework
+        * guarantees that @Autowired fields have been populated, before
+        * invoking @PostConstruct annotated methods and the afterPropertiesSet
+        * method, it doesn't guarantee that properties in those beans have
+        * been set, and the outcome is unrelable.  @Lazy could have been used,
+        * but it would need to be set both here, and in all the @Autowired
+        * uses: missing a single one, in future changes, would produce
+        * mysterious unrelable results.  So, instead, this code
+        * creates clients on first use and, where possible, keeps them for
+        * future use.
+        */
+
        @Autowired
        private FederationConfig federation;
 
@@ -52,9 +68,18 @@ public class Clients {
 
        private ICommonDataServiceRestClient cdsClient;
        private NexusClient nexusClient;
-       private DockerClient dockerClient;
 
        public FederationClient getFederationClient(String url) {
+               /*
+                * The set of peers can change, at runtime, and there is no
+                * notification when one is deleted (or has its API URL
+                * changed).  It would have been possible to keep federation
+                * clients in a hash and fault them in, as needed, but, without
+                * a means for identifying clients that were no longer needed,
+                * that would have constituted a memory (and possibly a TCP/IP
+                * connection) leak.  So this code does not cache federation
+                * clients.
+                */
                return new FederationClient(url, federation);
        }
 
@@ -78,21 +103,26 @@ public class Clients {
        }
 
        public synchronized DockerClient getDockerClient() {
-               if (dockerClient == null) {
-                       dockerClient = DockerClientBuilder.getInstance(
-                           DefaultDockerClientConfig.createDefaultConfigBuilder()
-                               .withDockerHost(dockerConfig.getHost())
-                               .withDockerTlsVerify(dockerConfig.getTlsVerify())
-                               .withDockerConfig(dockerConfig.getDockerConfig())
-                               .withDockerCertPath(dockerConfig.getDockerCertPath())
-                               .withApiVersion(dockerConfig.getApiVersion())
-                               .withRegistryUsername(dockerConfig.getRegistryUsername())
-                               .withRegistryPassword(dockerConfig.getRegistryPassword())
-                               .withRegistryEmail(dockerConfig.getRegistryEmail())
-                               .withRegistryUrl(dockerConfig.getRegistryUrl())
-                               .build()
-                           ).build();
-               }
-               return dockerClient;
+               /*
+                * For some reason, the DockerClient seems to go stale,
+                * resulting in operations (like the docker pull command)
+                * unexpectedly hanging, with no error or indication of a
+                * problem.  Creating a fresh DockerClient on each
+                * upload/download of a Docker image artifact, as a
+                * workaround, seems to work.
+                */
+               return DockerClientBuilder.getInstance(
+                   DefaultDockerClientConfig.createDefaultConfigBuilder()
+                       .withDockerHost(dockerConfig.getHost())
+                       .withDockerTlsVerify(dockerConfig.getTlsVerify())
+                       .withDockerConfig(dockerConfig.getDockerConfig())
+                       .withDockerCertPath(dockerConfig.getDockerCertPath())
+                       .withApiVersion(dockerConfig.getApiVersion())
+                       .withRegistryUsername(dockerConfig.getRegistryUsername())
+                       .withRegistryPassword(dockerConfig.getRegistryPassword())
+                       .withRegistryEmail(dockerConfig.getRegistryEmail())
+                       .withRegistryUrl(dockerConfig.getRegistryUrl())
+                       .build()
+                   ).build();
        }
 }
index e4f3afa..249ee45 100644 (file)
@@ -81,7 +81,7 @@ public class ContentServiceImpl implements ContentService {
        @Override
        public void setArtifactUri(String solutionId, MLPArtifact artifact) {
                if (FederationClient.ATC_DOCKER.equals(artifact.getArtifactTypeCode())) {
-                       artifact.setUri(dockerConfig.getRegistryUrl() + "/" + artifact.getArtifactId() + ":" + artifact.getVersion());
+                       artifact.setUri(dockerConfig.getRegistryUrl() + "/" + artifact.getName().toLowerCase() + "_" + solutionId  + ":" + artifact.getVersion());
                        artifact.setDescription(artifact.getUri());
                } else {
                        artifact.setUri(makeNexusUri(solutionId, ((Artifact)artifact).getFilename(), artifact.getName(), artifact.getVersion()));
@@ -98,9 +98,9 @@ public class ContentServiceImpl implements ContentService {
                        if (image == null) {
                                throw new BadRequestException(400, "Could not find loaded docker image for " + artifact);
                        }
-                       String name = dockerConfig.getRegistryUrl() + "/" + artifact.getArtifactId();
+                       String name = artifact.getDescription().substring(0, artifact.getDescription().lastIndexOf(':'));
                        docker.tagImageCmd(image.getId(), name, artifact.getVersion()).exec();
-                       docker.removeImageCmd(artifact.getDescription()).withForce(true).exec();
+                       docker.removeImageCmd(tag).withForce(true).exec();
                        try (PushImageResultCallback result = new PushImageResultCallback()) {
                                AuthConfig auth = (new AuthConfig())
                                    .withUsername(dockerConfig.getRegistryUsername())
index 8c98344..28df0f0 100644 (file)
@@ -50,6 +50,19 @@ import org.springframework.test.context.junit4.SpringRunner;
     }
 )
 public class ClientsTest {
+       /*
+        * Implementation note:
+        *
+        * Since "Clients" is the common hub for outbound activity from the
+        * Federation Gateway software, and since the destinations for that
+        * activity don't exist during automated testing, Clients is mocked
+        * out in all the other tests.  Without those external systems, there
+        * still isn't a lot, that can actually be tested, here, but
+        * this does at least exercise the code by insuring that all the
+        * methods get invoked and that, for the clients that are cached,
+        * the methods get invoked twice and the answers tested for equality.
+        */
+
        @Autowired
        private Clients clients;
 
@@ -58,6 +71,6 @@ public class ClientsTest {
                assertNotNull(clients.getFederationClient("https://somepeer.example.org"));
                assertEquals(clients.getCDSClient(), clients.getCDSClient());
                assertEquals(clients.getNexusClient(), clients.getNexusClient());
-               assertEquals(clients.getDockerClient(), clients.getDockerClient());
+               assertNotNull(clients.getDockerClient());
        }
 }
index b61e2a8..9328417 100644 (file)
@@ -75,6 +75,7 @@ import static org.acumos.federation.client.test.ClientMocking.xq;
        "local.ssl.trust-store-password=acumos",
        "nexus.group-id=nxsgrpid",
        "nexus.name-separator=,",
+       "docker.registry-url=someregistry:9999",
        "federation.operator=defuserid"
     }
 )
@@ -192,7 +193,7 @@ public class GatewayControllerTest {
                    .on("POST /peer/register", xq("{ 'content': {}}"))
                    .on("GET /solutions?catalogId=somecatalog", xq("{ 'content': [ { 'solutionId': 'somesolution' } ]}"))
                    .on("GET /solutions/somesolution", xq("{ 'content': { 'picture': 'YXNkZg==', 'revisions': [ { 'revisionId': 'revid1' } ] }}"))
-                   .on("GET /solutions/somesolution/revisions/revid1?catalogId=somecatalog", xq("{ 'content': { 'solutionId': 'somesolution', 'revisionId': 'revid1', 'documents': [ { 'documentId': 'docid1', 'filename': 'docfile.doctype', 'version': 'docversion' } ], 'artifacts': [ { 'artifactId': 'artid1', 'filename': 'someimage', 'version': 'someversion', 'artifactTypeCode': 'DI', 'description': 'thisimage:thistag' } ], 'revCatDescription': { 'revisionId': 'revid1', 'catalogId': 'somecatalog', 'description': 'some description' }}}"))
+                   .on("GET /solutions/somesolution/revisions/revid1?catalogId=somecatalog", xq("{ 'content': { 'solutionId': 'somesolution', 'revisionId': 'revid1', 'documents': [ { 'documentId': 'docid1', 'filename': 'docfile.doctype', 'version': 'docversion' } ], 'artifacts': [ { 'artifactId': 'artid1', 'name': 'somename', 'filename': 'someimage', 'version': 'someversion', 'artifactTypeCode': 'DI', 'description': 'thisimage:thistag' } ], 'revCatDescription': { 'revisionId': 'revid1', 'catalogId': 'somecatalog', 'description': 'some description' }}}"))
                    .on("GET /artifacts/artid1/content", "Artifact Content")
                    .on("GET /artifacts/artid2/content", "Artifact Content 2")
                    .on("GET /documents/docid1/content", "Document Content")