You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2020/07/08 13:18:25 UTC

[brooklyn-server] 15/20: fix (and test for) location leak, add deployment test

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 04caacd6d1b58e97dd3cd2058e139c1db4e66b2c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Tue Jul 7 22:59:50 2020 +0100

    fix (and test for) location leak, add deployment test
---
 .../location/kubernetes/KubernetesLocation.java    | 14 +++++--
 .../kubernetes/KubernetesLocationYamlLiveTest.java | 46 ++++++++++++++++++++++
 .../src/test/resources/nginx-2-deployment.yaml     | 37 +++++++++++++++++
 .../src/test/resources/nginx-2-service.yaml        | 28 +++++++++++++
 4 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/locations/container/src/main/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocation.java b/locations/container/src/main/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocation.java
index a0466b1..4ab4ca0 100644
--- a/locations/container/src/main/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocation.java
+++ b/locations/container/src/main/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocation.java
@@ -216,6 +216,8 @@ public class KubernetesLocation extends AbstractLocation implements MachineProvi
         } else {
             deleteKubernetesContainerLocation(entity, machine);
         }
+        getPortForwardManager().forgetPortMappings(machine);
+        removeChild(machine);
     }
 
     protected void deleteKubernetesContainerLocation(Entity entity, MachineLocation machine) {
@@ -400,7 +402,8 @@ public class KubernetesLocation extends AbstractLocation implements MachineProvi
                     .configure(KubernetesMachineLocation.KUBERNETES_RESOURCE_TYPE, resourceType);
 
             KubernetesMachineLocation machine = getManagementContext().getLocationManager().createLocation(locationSpec);
-
+            addChild(machine);
+            
             if (resourceType.equals(KubernetesResource.SERVICE) && machine instanceof KubernetesSshMachineLocation) {
                 Service service = getService(namespace, resourceName, client);
                 registerPortMappings((KubernetesSshMachineLocation) machine, entity, service);
@@ -511,6 +514,7 @@ public class KubernetesLocation extends AbstractLocation implements MachineProvi
                 .configure(KubernetesMachineLocation.KUBERNETES_RESOURCE_TYPE, getContainerResourceType());
 
         KubernetesSshMachineLocation machine = getManagementContext().getLocationManager().createLocation(locationSpec);
+        addChild(machine);
         registerPortMappings(machine, entity, service);
         if (!isDockerContainer(entity)) {
             waitForSshable(machine, Duration.FIVE_MINUTES);
@@ -551,8 +555,7 @@ public class KubernetesLocation extends AbstractLocation implements MachineProvi
     }
 
     protected void registerPortMappings(KubernetesSshMachineLocation machine, Entity entity, Service service) {
-        PortForwardManager portForwardManager = (PortForwardManager) getManagementContext().getLocationRegistry()
-                .getLocationManaged(PortForwardManagerLocationResolver.PFM_GLOBAL_SPEC);
+        PortForwardManager portForwardManager = getPortForwardManager();
         List<ServicePort> ports = service.getSpec().getPorts();
         String publicHostText = machine.getSshHostAndPort().getHostText();
         LOG.debug("Recording port-mappings for container {} of {}: {}", machine, this, ports);
@@ -578,6 +581,11 @@ public class KubernetesLocation extends AbstractLocation implements MachineProvi
                 .configure(AbstractOnNetworkEnricher.MAP_MATCHING, "kubernetes.[a-zA-Z0-9][a-zA-Z0-9-_]*.port"));
     }
 
+    private PortForwardManager getPortForwardManager() {
+        return (PortForwardManager) getManagementContext().getLocationRegistry()
+                .getLocationManaged(PortForwardManagerLocationResolver.PFM_GLOBAL_SPEC);
+    }
+
     protected synchronized Namespace createOrGetNamespace(final String name, Boolean create) {
         try (KubernetesClient client = getClient()) {
             Namespace namespace = client.namespaces().withName(name).get();
diff --git a/locations/container/src/test/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocationYamlLiveTest.java b/locations/container/src/test/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocationYamlLiveTest.java
index b9019e9..5960e1b 100644
--- a/locations/container/src/test/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocationYamlLiveTest.java
+++ b/locations/container/src/test/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocationYamlLiveTest.java
@@ -35,10 +35,12 @@ import static org.apache.brooklyn.util.http.HttpAsserts.assertHttpStatusCodeEven
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.location.Location;
 import org.apache.brooklyn.api.location.MachineLocation;
 import org.apache.brooklyn.api.location.MachineProvisioningLocation;
 import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
@@ -49,7 +51,9 @@ import org.apache.brooklyn.core.entity.Attributes;
 import org.apache.brooklyn.core.entity.Dumper;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityPredicates;
+import org.apache.brooklyn.core.entity.StartableApplication;
 import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.location.access.PortForwardManagerImpl;
 import org.apache.brooklyn.core.network.OnPublicNetworkEnricher;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess;
@@ -57,6 +61,7 @@ import org.apache.brooklyn.entity.software.base.SoftwareProcess;
 import org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess;
 import org.apache.brooklyn.entity.stock.BasicStartable;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.net.Networking;
 import org.apache.brooklyn.util.text.Identifiers;
@@ -519,7 +524,48 @@ public class KubernetesLocationYamlLiveTest extends AbstractYamlTest {
         String httpPublicPort = assertAttributeEventuallyNonNull(nginxService, Sensors.newStringSensor("kubernetes.http.endpoint.mapped.public"));
         assertReachableEventually(HostAndPort.fromString(httpPublicPort));
     }
+    
+    @Test(groups = {"Live"})
+    public void testNginxService2() throws Exception {
+        String yaml = Joiner.on("\n").join(
+                locationYaml,
+                "services:",
+                "  - type: " + KubernetesResource.class.getName(),
+                "    name: \"nginx-2-deployment\"",
+                "    brooklyn.config:",
+                "      resource: classpath://nginx-2-deployment.yaml",
+                "  - type: " + KubernetesResource.class.getName(),
+                "    name: \"nginx-2-service\"",
+                "    brooklyn.config:",
+                "      resource: classpath://nginx-2-service.yaml");
+        Entity app = createStartWaitAndLogApplication(yaml);
+
+        Iterable<KubernetesResource> resources = Entities.descendantsAndSelf(app, KubernetesResource.class);
+        KubernetesResource nginxDeployment = Iterables.find(resources, EntityPredicates.displayNameEqualTo("nginx-2-deployment"));
+        KubernetesResource nginxService = Iterables.find(resources, EntityPredicates.displayNameEqualTo("nginx-2-service"));
 
+        assertEntityHealthy(nginxDeployment);
+        assertEntityHealthy(nginxService);
+
+        Dumper.dumpInfo(app);
+
+        Integer httpPort = assertAttributeEventuallyNonNull(nginxService, Sensors.newIntegerSensor("kubernetes.http.port"));
+        assertEquals(httpPort, Integer.valueOf(80));
+        String httpPublicPort = assertAttributeEventuallyNonNull(nginxService, Sensors.newStringSensor("kubernetes.http.endpoint.mapped.public"));
+        assertReachableEventually(HostAndPort.fromString(httpPublicPort));
+        
+        // also check locations are cleaned up
+        mgmt().getApplications().forEach(appI -> ((StartableApplication)appI).stop());
+        Collection<Location> ll = mgmt().getLocationManager().getLocations();
+        if (ll.isEmpty()) {
+            // okay
+        } else {
+            // should have an empty PortForwardingManager
+            Asserts.assertSize(ll, 1);  
+            Asserts.assertSize(((PortForwardManagerImpl)Iterables.getOnlyElement(ll)).getPortMappings(), 0);
+        }
+    }
+    
     protected void assertReachableEventually(final HostAndPort hostAndPort) {
         succeedsEventually(new Runnable() {
             public void run() {
diff --git a/locations/container/src/test/resources/nginx-2-deployment.yaml b/locations/container/src/test/resources/nginx-2-deployment.yaml
new file mode 100644
index 0000000..f53aeb4
--- /dev/null
+++ b/locations/container/src/test/resources/nginx-2-deployment.yaml
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-2-deployment
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app.kubernetes.io/name: nginx
+  template:
+    metadata:
+      name: nginx
+      labels:
+        app.kubernetes.io/name: nginx
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+          ports:
+            - containerPort: 80
diff --git a/locations/container/src/test/resources/nginx-2-service.yaml b/locations/container/src/test/resources/nginx-2-service.yaml
new file mode 100644
index 0000000..dae654b
--- /dev/null
+++ b/locations/container/src/test/resources/nginx-2-service.yaml
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+apiVersion: v1
+kind: Service
+metadata:
+  name: nginx-2-service
+spec:
+  type: NodePort
+  selector:
+    app.kubernetes.io/name: nginx
+  ports:
+    - port: 80
+      name: "http"    
\ No newline at end of file