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 89c2662604a24035db68d1848f38281c292de1bf Author: Alex Heneveld <[email protected]> AuthorDate: Thu Oct 20 16:39:14 2022 +0100 fix bug where location tags get set as config on some locations which is not what is intended eg for AWS where the tags are applied to VMs --- .../brooklyn/camp/brooklyn/LocationsYamlTest.java | 15 ++++++++++---- .../core/location/BasicLocationRegistry.java | 23 ++++++++++++++++++---- .../core/mgmt/rebind/RebindLocationTest.java | 3 +-- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/LocationsYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/LocationsYamlTest.java index bb2fbc4819..898fd9a084 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/LocationsYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/LocationsYamlTest.java @@ -297,16 +297,20 @@ public class LocationsYamlTest extends AbstractYamlTest { LocalhostMachineProvisioningLocation loc = (LocalhostMachineProvisioningLocation) Iterables.getOnlyElement(app.getLocations()); assertNotNull(loc); Assert.assertTrue(loc.tags().containsTag("foo"), "location tags missing: "+loc.tags().getTags()); + // ensure tags not set as config + Assert.assertNull(loc.config().getBag().getStringKey("tags")); } - @Test - public void testJcloudsLocationWithTagsDoesntWork() throws Exception { - // NOTE: 'tags' on jclouds is used to set a config, NOT brooklyn object tags + public void testJcloudsLocationWithTagsActsCorrectly() throws Exception { + // NOTE: 'tags' on jclouds _was_ used to set a config, NOT brooklyn object tags + // CHANGED 2022-10 to be tags on the location, otherwise spec_hierarchy tags get passed to VMs; use brooklyn.config String yaml = "location:\n"+ " jclouds:aws-ec2:\n"+ - " tags: [ foo ]\n"+ + " tags: [ bar ]\n"+ + " brooklyn.config:\n"+ + " tags: [ foo ]\n"+ "services:\n"+ "- type: org.apache.brooklyn.core.test.entity.TestEntity\n"; @@ -314,7 +318,10 @@ public class LocationsYamlTest extends AbstractYamlTest { JcloudsLocation loc = (JcloudsLocation) Iterables.getOnlyElement(app.getLocations()); assertNotNull(loc); Assert.assertFalse(loc.tags().containsTag("foo"), "location tags for jclouds shouldn't support 'tags' flag: "+loc.tags().getTags()); + Assert.assertTrue(loc.tags().containsTag("bar")); + Asserts.assertNull(loc.config().getBag().getStringKey("brooklyn.config")); Asserts.assertThat(loc.config().get(JcloudsLocation.STRING_TAGS), r -> r instanceof Collection && ((Collection)r).contains("foo")); + Asserts.assertThat(loc.config().get(JcloudsLocation.STRING_TAGS), r -> r instanceof Collection && !((Collection)r).contains("bar")); } @Test diff --git a/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java b/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java index b83d9d23d7..30143a0228 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java @@ -443,14 +443,29 @@ public class BasicLocationRegistry implements LocationRegistry { if (resolver != null) { try { + Object tags = locationFlags.remove("tags"); + Object brTags = locationFlags.remove("brooklyn.tags"); + Object brConfig = locationFlags.remove("brooklyn.config"); + LocationSpec<? extends Location> specO = resolver.newLocationSpecFromString(spec, locationFlags, this); + + if (tags!=null) { + if (tags instanceof Iterable) specO.tagsAdd((Iterable)tags); + else specO.configure("tags", tags); + } + if (brTags!=null) { + if (brTags instanceof Iterable) specO.tagsAdd((Iterable)brTags); + else specO.configure("brooklyn.tags", brTags); + } + if (brConfig!=null) { + if (brConfig instanceof Map) specO.configure((Map)brConfig); + else specO.configure("brooklyn.config", brConfig); + } + specO.configure(LocationInternal.ORIGINAL_SPEC, spec); specO.configure(LocationInternal.NAMED_SPEC_NAME, spec); - Object tags = locationFlags.get("brooklyn.tags"); - if (tags instanceof Iterable) { - specO.tagsAdd((Iterable<?>)tags); - } return (Maybe) Maybe.of(specO); + } catch (RuntimeException e) { return Maybe.absent(Suppliers.ofInstance(e)); } diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java index 9de240cf8d..926c7fd7ef 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindLocationTest.java @@ -260,8 +260,7 @@ public class RebindLocationTest extends RebindTestFixtureWithApp { @Test public void testLocationTags() throws Exception { - Location origLoc = origManagementContext.getLocationManager().createLocation(LocationSpec.create(MyLocation.class)); - origLoc.tags().addTag("foo"); + Location origLoc = origManagementContext.getLocationManager().createLocation(LocationSpec.create(MyLocation.class).tag("foo")); origLoc.tags().addTag(origApp); origApp.start(ImmutableList.of(origLoc));
