You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2016/07/18 20:15:49 UTC
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/266
BROOKLYN-264: refactor wait-for-provision
We now mark the internal state as provisioning entirely in
MachineLifecycleEffectorTasks, and that is also where we wait for it
on stop(). This also allows us to test it as a unit test, rather than
needing a jclouds live test.
@bostko can you please review?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server brooklyn-264_refactor
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/266.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #266
----
commit e74bc778cca888716b3d3cfeb27e30b394b8179e
Author: Aled Sage <al...@gmail.com>
Date: 2016-07-18T20:12:59Z
BROOKLYN-264: refactor wait-for-provision
We now mark the internal state as provisioning entirely in
MachineLifecycleEffectorTasks, and that is also where we wait for it
on stop(). This also allows us to test it as a unit test, rather than
needing a jclouds live test.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71251768
--- Diff: software/base/src/test/java/org/apache/brooklyn/entity/ExpungingJcloudsLocationLiveTest.java ---
@@ -1,186 +0,0 @@
-/*
- * 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.
- */
-package org.apache.brooklyn.entity;
-
-import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
-import org.apache.brooklyn.api.entity.EntitySpec;
-import org.apache.brooklyn.core.entity.Attributes;
-import org.apache.brooklyn.core.entity.Entities;
-import org.apache.brooklyn.core.entity.EntityAsserts;
-import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
-import org.apache.brooklyn.core.internal.BrooklynProperties;
-import org.apache.brooklyn.core.location.LocationConfigKeys;
-import org.apache.brooklyn.core.location.cloud.CloudLocationConfig;
-import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
-import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport;
-import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
-import org.apache.brooklyn.core.test.entity.TestApplication;
-import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess;
-import org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess;
-import org.apache.brooklyn.location.jclouds.JcloudsLocation;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.jclouds.aws.ec2.compute.AWSEC2ComputeService;
-import org.jclouds.compute.domain.ComputeMetadata;
-import org.jclouds.compute.domain.NodeMetadata;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-
-import javax.annotation.Nullable;
-import java.util.Map;
-import java.util.concurrent.Callable;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.regex.Pattern;
-
-import static org.apache.brooklyn.test.Asserts.*;
-
-public class ExpungingJcloudsLocationLiveTest extends BrooklynAppLiveTestSupport {
- private static final Logger LOG = LoggerFactory.getLogger(ExpungingJcloudsLocationLiveTest.class);
-
- protected BrooklynProperties brooklynProperties;
-
- @BeforeMethod(alwaysRun=true)
- @Override
- public void setUp() throws Exception {
- // Don't let any defaults from brooklyn.properties (except credentials) interfere with test
- brooklynProperties = BrooklynProperties.Factory.newDefault();
-
- // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty")
- brooklynProperties.remove("brooklyn.ssh.config.scriptHeader");
-
- mgmt = new LocalManagementContextForTests(brooklynProperties);
- super.setUp();
- }
-
- @Test
- public void verifyExpungingMockedEntityIsQuick() throws Exception {
- final EmptySoftwareProcess emptySoftwareProcess = app.createAndManageChild(EntitySpec.create(EmptySoftwareProcess.class));
--- End diff --
This is still a valid test which verifies that there is no bug in the stop functionality.
If it the waitForSshMachine location takes 5 minutes or more, then the stop code can make executing unit tests to last for hours.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71313422
--- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
@@ -379,20 +418,30 @@ public MachineLocation call() throws Exception {
if (!(location instanceof LocalhostMachineProvisioningLocation))
log.info("Starting {}, obtaining a new location instance in {} with ports {}", new Object[]{entity(), location, flags.get("inboundPorts")});
entity().sensors().set(SoftwareProcess.PROVISIONING_LOCATION, location);
+ Transition expectedState = entity().sensors().get(Attributes.SERVICE_STATE_EXPECTED);
+
+ // BROOKLYN-263: see corresponding code in doStop()
+ if (expectedState != null && (expectedState.getState() == Lifecycle.STOPPING || expectedState.getState() == Lifecycle.STOPPED)) {
--- End diff --
I think it's fine to be in this PR. It relates to concurrent calls to start and stop.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71247868
--- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
@@ -379,20 +418,30 @@ public MachineLocation call() throws Exception {
if (!(location instanceof LocalhostMachineProvisioningLocation))
log.info("Starting {}, obtaining a new location instance in {} with ports {}", new Object[]{entity(), location, flags.get("inboundPorts")});
entity().sensors().set(SoftwareProcess.PROVISIONING_LOCATION, location);
+ Transition expectedState = entity().sensors().get(Attributes.SERVICE_STATE_EXPECTED);
+
+ // BROOKLYN-263: see corresponding code in doStop()
+ if (expectedState != null && (expectedState.getState() == Lifecycle.STOPPING || expectedState.getState() == Lifecycle.STOPPED)) {
--- End diff --
Good check, but should it be in this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71316462
--- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
@@ -444,9 +493,6 @@ public void run() {
}
}
entity().addLocations(ImmutableList.of((Location) machine));
- if (entity().getAttribute(Attributes.JCLOUDS_PROVISIONING_RUNNING) != null) {
--- End diff --
One big reason that someone might call stop() while it's still starting is they realise their provisioning parameters were wrong, so want to abort. Therefore continuing to support that use-case seems important.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71313969
--- Diff: software/base/src/test/java/org/apache/brooklyn/entity/ExpungingJcloudsLocationLiveTest.java ---
@@ -1,186 +0,0 @@
-/*
- * 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.
- */
-package org.apache.brooklyn.entity;
-
-import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
-import org.apache.brooklyn.api.entity.EntitySpec;
-import org.apache.brooklyn.core.entity.Attributes;
-import org.apache.brooklyn.core.entity.Entities;
-import org.apache.brooklyn.core.entity.EntityAsserts;
-import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
-import org.apache.brooklyn.core.internal.BrooklynProperties;
-import org.apache.brooklyn.core.location.LocationConfigKeys;
-import org.apache.brooklyn.core.location.cloud.CloudLocationConfig;
-import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
-import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport;
-import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
-import org.apache.brooklyn.core.test.entity.TestApplication;
-import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess;
-import org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess;
-import org.apache.brooklyn.location.jclouds.JcloudsLocation;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.jclouds.aws.ec2.compute.AWSEC2ComputeService;
-import org.jclouds.compute.domain.ComputeMetadata;
-import org.jclouds.compute.domain.NodeMetadata;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-
-import javax.annotation.Nullable;
-import java.util.Map;
-import java.util.concurrent.Callable;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.regex.Pattern;
-
-import static org.apache.brooklyn.test.Asserts.*;
-
-public class ExpungingJcloudsLocationLiveTest extends BrooklynAppLiveTestSupport {
- private static final Logger LOG = LoggerFactory.getLogger(ExpungingJcloudsLocationLiveTest.class);
-
- protected BrooklynProperties brooklynProperties;
-
- @BeforeMethod(alwaysRun=true)
- @Override
- public void setUp() throws Exception {
- // Don't let any defaults from brooklyn.properties (except credentials) interfere with test
- brooklynProperties = BrooklynProperties.Factory.newDefault();
-
- // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty")
- brooklynProperties.remove("brooklyn.ssh.config.scriptHeader");
-
- mgmt = new LocalManagementContextForTests(brooklynProperties);
- super.setUp();
- }
-
- @Test
- public void verifyExpungingMockedEntityIsQuick() throws Exception {
- final EmptySoftwareProcess emptySoftwareProcess = app.createAndManageChild(EntitySpec.create(EmptySoftwareProcess.class));
--- End diff --
Good point - my SoftwareProcessStopsDuringStartTest.testSequentialStartThenStop doesn't time how long the call took.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71248547
--- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
@@ -444,9 +493,6 @@ public void run() {
}
}
entity().addLocations(ImmutableList.of((Location) machine));
- if (entity().getAttribute(Attributes.JCLOUDS_PROVISIONING_RUNNING) != null) {
--- End diff --
Why you do not call
```
entity().sensors().remove(PROVISIONING_TASK_STATE)
entity().sensors().remove(PROVISIONED_MACHINE)
```
Otherwise these sensors will stay after machine provisioning completes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71472214
--- Diff: software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java ---
@@ -0,0 +1,231 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.entity.software.base;
+
+import static org.apache.brooklyn.test.Asserts.assertEquals;
+import static org.apache.brooklyn.test.Asserts.assertNotNull;
+import static org.apache.brooklyn.test.Asserts.assertTrue;
+import static org.apache.brooklyn.test.Asserts.fail;
+
+import java.util.Map;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.api.location.ProvisioningLocation;
+import org.apache.brooklyn.api.mgmt.Task;
+import org.apache.brooklyn.core.entity.Attributes;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.entity.EntityAsserts;
+import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
+import org.apache.brooklyn.core.entity.trait.Startable;
+import org.apache.brooklyn.core.internal.BrooklynProperties;
+import org.apache.brooklyn.core.location.LocationConfigKeys;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
+import org.apache.brooklyn.entity.AbstractEc2LiveTest;
+import org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks;
+import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.apache.brooklyn.util.time.Duration;
+import org.jclouds.aws.ec2.compute.AWSEC2ComputeService;
+import org.jclouds.compute.domain.ComputeMetadata;
+import org.jclouds.compute.domain.NodeMetadata;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Predicate;
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+
+public class SoftwareProcessStopsDuringStartJcloudsLiveTest extends BrooklynAppLiveTestSupport {
+ private static final Logger LOG = LoggerFactory.getLogger(SoftwareProcessStopsDuringStartJcloudsLiveTest.class);
+
+ // same image as in AbstractEc2LiveTest.test_CentOS_6_3
+ // Image: {id=us-east-1/ami-a96b01c0, providerId=ami-a96b01c0, name=CentOS-6.3-x86_64-GA-EBS-02-85586466-5b6c-4495-b580-14f72b4bcf51-ami-bb9af1d2.1, location={scope=REGION, id=us-east-1, description=us-east-1, parent=aws-ec2, iso3166Codes=[US-VA]}, os={family=centos, arch=paravirtual, version=6.3, description=aws-marketplace/CentOS-6.3-x86_64-GA-EBS-02-85586466-5b6c-4495-b580-14f72b4bcf51-ami-bb9af1d2.1, is64Bit=true}, description=CentOS-6.3-x86_64-GA-EBS-02 on EBS x86_64 20130527:1219, version=bb9af1d2.1, status=AVAILABLE[available], loginUser=root, userMetadata={owner=679593333241, rootDeviceType=ebs, virtualizationType=paravirtual, hypervisor=xen}})
+ public static final String PROVIDER = "aws-ec2";
+ public static final String REGION_NAME = "us-east-1";
+ public static final String IMAGE_ID = "us-east-1/ami-a96b01c0";
+ public static final String HARDWARE_ID = AbstractEc2LiveTest.SMALL_HARDWARE_ID;
+ public static final String LOCATION_SPEC = PROVIDER + (REGION_NAME == null ? "" : ":" + REGION_NAME);
+
+ protected BrooklynProperties brooklynProperties;
+
+ protected ExecutorService executor;
+
+ @BeforeMethod(alwaysRun=true)
+ @Override
+ public void setUp() throws Exception {
+ // Don't let any defaults from brooklyn.properties (except credentials) interfere with test
+ brooklynProperties = BrooklynProperties.Factory.newDefault();
+
+ // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty")
+ brooklynProperties.remove("brooklyn.ssh.config.scriptHeader");
+
+ mgmt = new LocalManagementContextForTests(brooklynProperties);
+ super.setUp();
+
+ executor = Executors.newCachedThreadPool();
+ }
+
+ @Override
+ public void tearDown() throws Exception {
+ if (executor != null) {
+ executor.shutdownNow();
+ }
+ super.tearDown();
+ }
+
+ // Integration because takes approx 1 seconds
+ @Test(groups = "Integration")
+ public void testStartStopSequentiallyIsQuickInLocalhost() throws Exception {
+ LocalhostMachineProvisioningLocation localLoc = mgmt.getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class)
+ .configure(SshMachineLocation.SSH_TOOL_CLASS, RecordingSshTool.class.getName()));
+ runStartStopSequentiallyIsQuick(localLoc);
+ }
+
+ // Integration because takes approx 1 seconds
+ @Test(groups = "Integration")
+ public void testStartStopSequentiallyIsQuickInByon() throws Exception {
+ FixedListMachineProvisioningLocation<?> byonLoc = mgmt.getLocationManager().createLocation(LocationSpec.create(FixedListMachineProvisioningLocation.class)
+ .configure(FixedListMachineProvisioningLocation.MACHINE_SPECS, ImmutableList.<LocationSpec<? extends MachineLocation>>of(
+ LocationSpec.create(SshMachineLocation.class)
+ .configure("address", "1.2.3.4")
+ .configure(SshMachineLocation.SSH_TOOL_CLASS, RecordingSshTool.class.getName()))));
+ runStartStopSequentiallyIsQuick(byonLoc);
+ }
+
+ protected void runStartStopSequentiallyIsQuick(final ProvisioningLocation<?> loc) throws Exception {
+ final EmptySoftwareProcess entity = app.createAndManageChild(EntitySpec.create(EmptySoftwareProcess.class)
+ .configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, true));
+
+ executeInLimitedTime(new Callable<Void>() {
+ public Void call() {
+ app.start(ImmutableList.of(loc));
+ return null;
+ }
+ }, Asserts.DEFAULT_LONG_TIMEOUT.toMilliseconds(), TimeUnit.MILLISECONDS);
+ EntityAsserts.assertEntityHealthy(entity);
+ assertEquals(entity.getAttribute(MachineLifecycleEffectorTasks.INTERNAL_PROVISIONING_TASK_STATE), MachineLifecycleEffectorTasks.ProvisioningTaskState.DONE);
+ assertEquals(entity.getAttribute(MachineLifecycleEffectorTasks.INTERNAL_PROVISIONED_MACHINE), Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class).get());
+
+ executeInLimitedTime(new Callable<Void>() {
+ public Void call() {
+ Entities.destroy(app);
+ return null;
+ }
+ }, Asserts.DEFAULT_LONG_TIMEOUT.toMilliseconds(), TimeUnit.MILLISECONDS);
--- End diff --
Isn't this too big value?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/266
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71313855
--- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
@@ -379,20 +418,30 @@ public MachineLocation call() throws Exception {
if (!(location instanceof LocalhostMachineProvisioningLocation))
log.info("Starting {}, obtaining a new location instance in {} with ports {}", new Object[]{entity(), location, flags.get("inboundPorts")});
entity().sensors().set(SoftwareProcess.PROVISIONING_LOCATION, location);
+ Transition expectedState = entity().sensors().get(Attributes.SERVICE_STATE_EXPECTED);
+
+ // BROOKLYN-263: see corresponding code in doStop()
+ if (expectedState != null && (expectedState.getState() == Lifecycle.STOPPING || expectedState.getState() == Lifecycle.STOPPED)) {
+ throw new IllegalStateException("Provisioning aborted before even begun for "+entity()+" in "+location+" (presumably by a concurrent call to stop");
+ }
+ entity().sensors().set(PROVISIONING_TASK_STATE, ProvisioningTaskState.RUNNING);
+
MachineLocation machine;
try {
machine = Tasks.withBlockingDetails("Provisioning machine in " + location, new ObtainLocationTask(location, flags));
- if (machine == null)
- throw new NoMachinesAvailableException("Failed to obtain machine in " + location.toString());
- } catch (Exception e) {
- throw Exceptions.propagate(e);
+ entity().sensors().set(PROVISIONED_MACHINE, machine);
--- End diff --
I'm willing to take that risk - the description says "internal" etc. The problem is that otherwise there is a big gap between when provisioning returns and when we add the machine to the location. I worry that start() will not always go through all those steps if there has been a concurrent call to stop. It might skip executing those subsequent tasks. With the change I made, we set DONE and the machine in a finally block so it removes that risk.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71314400
--- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
@@ -444,9 +493,6 @@ public void run() {
}
}
entity().addLocations(ImmutableList.of((Location) machine));
- if (entity().getAttribute(Attributes.JCLOUDS_PROVISIONING_RUNNING) != null) {
--- End diff --
I'll rename the sensors to include "internal" in their names, as well as in the description.
The reason I didn't like this approach was that if `obtain()` failed, we would wait for the max time before stop continued - there was no way for it to terminate early. I'll add a test for that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #266: BROOKLYN-264: refactor wait-for-provision
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/266#discussion_r71279013
--- Diff: software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java ---
@@ -379,20 +418,30 @@ public MachineLocation call() throws Exception {
if (!(location instanceof LocalhostMachineProvisioningLocation))
log.info("Starting {}, obtaining a new location instance in {} with ports {}", new Object[]{entity(), location, flags.get("inboundPorts")});
entity().sensors().set(SoftwareProcess.PROVISIONING_LOCATION, location);
+ Transition expectedState = entity().sensors().get(Attributes.SERVICE_STATE_EXPECTED);
+
+ // BROOKLYN-263: see corresponding code in doStop()
+ if (expectedState != null && (expectedState.getState() == Lifecycle.STOPPING || expectedState.getState() == Lifecycle.STOPPED)) {
+ throw new IllegalStateException("Provisioning aborted before even begun for "+entity()+" in "+location+" (presumably by a concurrent call to stop");
+ }
+ entity().sensors().set(PROVISIONING_TASK_STATE, ProvisioningTaskState.RUNNING);
+
MachineLocation machine;
try {
machine = Tasks.withBlockingDetails("Provisioning machine in " + location, new ObtainLocationTask(location, flags));
- if (machine == null)
- throw new NoMachinesAvailableException("Failed to obtain machine in " + location.toString());
- } catch (Exception e) {
- throw Exceptions.propagate(e);
+ entity().sensors().set(PROVISIONED_MACHINE, machine);
--- End diff --
I was thinking about this option but doesn't feel like the best option.
The downside I see is that someone could start using this sensor instead of `getLocations()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---