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 <[email protected]> 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
