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 2022/10/21 13:57:52 UTC

[brooklyn-server] branch master updated (a19f974705 -> b35bab6dcd)

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

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


    from a19f974705 remove stale comments
     add a4da3a3622 persist workflows in sensor
     add 83ec6fc551 better distinguish different classes of interruption
     add 44f4e71e49 replay workflow from specified points or current point
     add a689819231 ensure workflow effector sets the internal workflow state sensor synchronously
     add b3d00859fe can replay workflow after shutdown
     add 2a6fa16a8f allow replay-from-last to descend into nested workflow
     add 5ef6f163ee after rebind, mark workflows that were running as shutdown failed, then replay from failure point
     add 827effe77b better info on test failing in jenkins but passing locally
     add 1c980f02cc REST API for workflows
     add 7473f0519c minor additions to workflow
     add 14f24039fc when logbook is queried for descendants of a task, also include workflow it was part of
     add df2299d8bb store all task IDs for a workflow and subworkflow IDs
     add 4fa37b8b59 better cancellation for tasks, and REST API supports cancelling task
     add ebfa455d01 change workflow api to match activities and others
     add e9b43c734c store more info on workflow replays and other metadata on steps
     add 126c9fd2fa use tags for entity initialization tasks, top-level tasks
     add 5440e64770 support replay in backend, with some refactor
     add 74150e6997 tidy task extended summary
     add 5a1ebfbcca minor fixes for replay
     add 0ae2c7d728 error handler for workflow
     add 64c9153060 more DslPredicates to support errors -- java-instance-of more powerful
     add c52df620cc error-cause is working
     add 33d14867e1 add error field lookup
     add 32402f5299 change error-cause to be a filter, with default
     add 3de540267b timeout and onError working, for steps and workflow
     add 52e0a4bd0c require ... for multimatch, and allow inline before a literal
     add 4012c75093 tidy logging and task naming for error handling
     add 42f443636a make ssh workflow step easier to be reused (eg for ansible ssh workflow step)
     add 397de8c3e1 retry step working, with tests
     add d72df592d7 support workflow for software process entities
     add 500e4ead73 add retry backoff, and test and fix on-error retry behaviour
     new a50296510d address code review comments, improve https support
     new 84876d12fd make step output available on errors, and improve short name metadata for types
     new a33153f6c8 block type instantiation for steps
     new ccf3cc8e65 REST API to run workflow, and tidy workflow submission metadata
     new 286eb0d4a0 fix typo
     new 5b2c5aa310 ensure ssh workflow step fails if non-zero exit code
     new bed78cd487 use constants for special steps -1 -2
     new 89c2662604 fix bug where location tags get set as config on some locations
     new 6d47a190ef merge master - fix location tags
     new 4062c3ab7d misc workflow fixes - more details, including error and rest calls
     new b35bab6dcd workflow basic expiry, and more workflow fixes

The 11 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../brooklyn/api/entity/EntityInitializer.java     |   7 +-
 .../brooklyn/camp/brooklyn/LocationsYamlTest.java  |  15 +-
 .../camp/brooklyn/WorkflowExpressionsYamlTest.java |   4 +-
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   |  88 +-
 .../brooklyn/core/effector/AbstractEffector.java   |   2 +-
 .../brooklyn/core/effector/EffectorTasks.java      |   6 +-
 .../apache/brooklyn/core/effector/Effectors.java   |  30 +-
 .../brooklyn/core/entity/AbstractEntity.java       |   2 +-
 .../org/apache/brooklyn/core/entity/Entities.java  |   1 +
 .../core/location/BasicLocationRegistry.java       |  23 +-
 .../brooklyn/core/mgmt/BrooklynTaskTags.java       | 106 ++-
 .../brooklyn/core/mgmt/internal/EffectorUtils.java |   4 +-
 .../mgmt/internal/EntityManagementSupport.java     | 193 +++--
 .../core/mgmt/internal/LocalManagementContext.java |  10 +-
 .../brooklyn/core/mgmt/rebind/RebindIteration.java |   4 +-
 .../core/mgmt/rebind/RebindManagerImpl.java        |   2 +-
 .../resolve/jackson/AsPropertyIfAmbiguous.java     |  31 +-
 .../resolve/jackson/CommonTypesSerialization.java  |  29 +-
 .../jackson/JsonPassThroughDeserializer.java       |  59 ++
 .../core/workflow/DanglingWorkflowException.java   |  19 +-
 .../brooklyn/core/workflow/ShorthandProcessor.java | 162 ++--
 .../core/workflow/WorkflowCommonConfig.java        |  19 +-
 .../brooklyn/core/workflow/WorkflowEffector.java   |  76 +-
 .../core/workflow/WorkflowErrorHandling.java       | 146 ++++
 .../core/workflow/WorkflowExecutionContext.java    | 919 ++++++++++++++++++---
 .../workflow/WorkflowExpressionResolution.java     |  31 +-
 .../core/workflow/WorkflowReplayUtils.java         | 236 ++++++
 .../brooklyn/core/workflow/WorkflowSensor.java     |   4 +-
 .../core/workflow/WorkflowStepDefinition.java      | 144 +++-
 .../WorkflowStepInstanceExecutionContext.java      | 106 ++-
 .../core/workflow/WorkflowStepResolution.java      |  46 +-
 .../core/workflow/steps/CustomWorkflowStep.java    | 101 ++-
 .../core/workflow/steps/EntityValueToSet.java      |   2 +
 .../core/workflow/steps/HttpWorkflowStep.java      |  14 +-
 .../workflow/steps/InvokeEffectorWorkflowStep.java |  55 +-
 .../core/workflow/steps/LogWorkflowStep.java       |   2 +-
 .../core/workflow/steps/RetryWorkflowStep.java     | 280 +++++++
 .../core/workflow/steps/ReturnWorkflowStep.java    |   2 +-
 .../core/workflow/steps/SetConfigWorkflowStep.java |   9 +-
 .../core/workflow/steps/SetSensorWorkflowStep.java |  24 +-
 .../workflow/steps/SetVariableWorkflowStep.java    |  59 +-
 .../core/workflow/steps/SshWorkflowStep.java       |  35 +-
 .../core/workflow/steps/TypedValueToSet.java       |   2 +
 .../core/workflow/steps/WaitWorkflowStep.java      | 167 +---
 .../store/WorkflowStatePersistenceViaSensors.java  | 123 +++
 .../brooklyn/util/core/config/ConfigBag.java       |   8 +-
 .../util/core/internal/ssh/sshj/SshjTool.java      |   2 +-
 .../util/core/logbook/DelegatingLogStore.java      |   6 +
 .../brooklyn/util/core/logbook/LogStore.java       |  47 +-
 .../util/core/logbook/file/FileLogStore.java       |  21 +-
 .../logbook/opensearch/OpenSearchLogStore.java     |  17 +-
 .../util/core/predicates/DslPredicates.java        | 162 +++-
 .../apache/brooklyn/util/core/task/BasicTask.java  | 360 ++++----
 .../util/core/task/DynamicSequentialTask.java      |   2 +-
 .../brooklyn/util/core/task/ScheduledTask.java     |  38 +-
 .../brooklyn/util/core/task/TaskBuilder.java       |  20 +-
 .../org/apache/brooklyn/util/core/task/Tasks.java  |  11 +-
 .../brooklyn/util/core/text/TemplateProcessor.java |  16 +-
 .../core/effector/SampleManyTasksEffector.java     |  16 +-
 .../core/mgmt/rebind/RebindLocationTest.java       |   3 +-
 .../core/mgmt/rebind/RebindTestFixture.java        |  15 +-
 .../core/workflow/ShorthandProcessorTest.java      |  13 +
 .../brooklyn/core/workflow/WorkflowBasicTest.java  |  31 +-
 .../core/workflow/WorkflowBeefyStepTest.java       |  53 +-
 .../workflow/WorkflowInputOutputExtensionTest.java |  30 +
 .../workflow/WorkflowPersistReplayErrorsTest.java  | 662 +++++++++++++++
 .../brooklyn/core/workflow/WorkflowRetryTest.java  | 324 ++++++++
 .../logbook/opensearch/OpenSearchLogStoreTest.java |  28 +-
 .../util/core/predicates/DslPredicateTest.java     | 101 ++-
 karaf/init/src/main/resources/catalog.bom          |  32 +-
 .../org/apache/brooklyn/rest/api/ActivityApi.java  |  16 +
 .../org/apache/brooklyn/rest/api/EntityApi.java    |  91 ++
 .../brooklyn/rest/resources/ActivityResource.java  |   8 +
 .../brooklyn/rest/resources/EntityResource.java    |  84 +-
 .../brooklyn/rest/resources/ActivityRestTest.java  | 440 ++++++----
 .../rest/resources/EntityWorkflowsRestTest.java    | 127 +++
 .../util/json/BrooklynJacksonSerializerTest.java   |   5 +-
 ...laProcess.java => WorkflowSoftwareProcess.java} |  42 +-
 ...ver.java => WorkflowSoftwareProcessDriver.java} |   2 +-
 ...sImpl.java => WorkflowSoftwareProcessImpl.java} |   4 +-
 .../base/WorkflowSoftwareProcessSshDriver.java     | 150 ++++
 .../tasks/kubectl/ContainerWorkflowStep.java       |   9 +-
 .../software/base/WorkflowSoftwareProcessTest.java | 142 ++++
 .../brooklyn/location/winrm/WinrmWorkflowStep.java |   4 +-
 .../java/org/apache/brooklyn/test/Asserts.java     |  21 +-
 .../brooklyn/util/exceptions/Exceptions.java       |  37 +-
 .../exceptions/RuntimeInterruptedException.java    |  26 +-
 .../brooklyn/util/http/executor/HttpConfig.java    |   5 +
 .../brooklyn/util/http/executor/HttpRequest.java   |   2 +-
 .../executor/apacheclient/HttpExecutorImpl.java    |   3 +-
 .../org/apache/brooklyn/util/time/Duration.java    |  11 +
 91 files changed, 5579 insertions(+), 1067 deletions(-)
 create mode 100644 core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonPassThroughDeserializer.java
 copy rest/rest-server/src/main/java/org/apache/brooklyn/rest/NopSecurityHandler.java => core/src/main/java/org/apache/brooklyn/core/workflow/DanglingWorkflowException.java (63%)
 create mode 100644 core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
 create mode 100644 core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
 create mode 100644 core/src/main/java/org/apache/brooklyn/core/workflow/steps/RetryWorkflowStep.java
 create mode 100644 core/src/main/java/org/apache/brooklyn/core/workflow/store/WorkflowStatePersistenceViaSensors.java
 create mode 100644 core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
 create mode 100644 core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java
 create mode 100644 rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EntityWorkflowsRestTest.java
 copy software/base/src/main/java/org/apache/brooklyn/entity/software/base/{AbstractVanillaProcess.java => WorkflowSoftwareProcess.java} (57%)
 copy software/base/src/main/java/org/apache/brooklyn/entity/software/base/{VanillaSoftwareProcessDriver.java => WorkflowSoftwareProcessDriver.java} (91%)
 copy software/base/src/main/java/org/apache/brooklyn/entity/software/base/{VanillaSoftwareProcessImpl.java => WorkflowSoftwareProcessImpl.java} (94%)
 create mode 100644 software/base/src/main/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessSshDriver.java
 create mode 100644 software/base/src/test/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessTest.java


[brooklyn-server] 06/11: fix bug where location tags get set as config on some locations

Posted by he...@apache.org.
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 <al...@cloudsoft.io>
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));
 


[brooklyn-server] 08/11: use constants for special steps -1 -2

Posted by he...@apache.org.
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 bed78cd4877fbed824d51d6a4f2b782e27ccd502
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Oct 20 17:53:27 2022 +0100

    use constants for special steps -1 -2
---
 .../core/workflow/WorkflowErrorHandling.java       |  2 +-
 .../core/workflow/WorkflowExecutionContext.java    | 37 ++++++++++++----------
 .../core/workflow/WorkflowReplayUtils.java         |  6 ++--
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
index 6d3e966836..f17d25792d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
@@ -97,7 +97,7 @@ public class WorkflowErrorHandling implements Callable<WorkflowErrorHandling.Wor
 
             WorkflowStepDefinition errorStep = errorOptions.get(i);
 
-            WorkflowStepInstanceExecutionContext handlerContext = new WorkflowStepInstanceExecutionContext(stepIndexIfStepErrorHandler!=null ? stepIndexIfStepErrorHandler : -3, errorStep, context);
+            WorkflowStepInstanceExecutionContext handlerContext = new WorkflowStepInstanceExecutionContext(stepIndexIfStepErrorHandler!=null ? stepIndexIfStepErrorHandler : WorkflowExecutionContext.STEP_INDEX_FOR_ERROR_HANDLER, errorStep, context);
             context.errorHandlerContext = handlerContext;
             handlerContext.error = error;
 
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index 388ea5101c..90e32abd10 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -70,6 +70,11 @@ public class WorkflowExecutionContext {
 
     private static final Logger log = LoggerFactory.getLogger(WorkflowExecutionContext.class);
 
+    public static final int STEP_INDEX_FOR_START = -1;
+    public static final int STEP_INDEX_FOR_END = -2;
+    public static final int STEP_INDEX_FOR_ERROR_HANDLER = -3;
+
+
     String name;
     @Nullable BrooklynObject adjunct;
     Entity entity;
@@ -324,7 +329,7 @@ public class WorkflowExecutionContext {
             Set<Integer> considered = MutableSet.of();
             Set<Integer> possibleOthers = MutableSet.of();
             while (true) {
-                if (WorkflowReplayUtils.isReplayable(this, stepIndex) || stepIndex == -1) {
+                if (WorkflowReplayUtils.isReplayable(this, stepIndex) || stepIndex == STEP_INDEX_FOR_START) {
                     break;
                 }
 
@@ -332,14 +337,14 @@ public class WorkflowExecutionContext {
                 OldStepRecord osi = oldStepInfo.get(stepIndex);
                 if (osi == null) {
                     log.warn("Unable to backtrack from step " + (stepIndex) + "; no step information. Replaying overall workflow.");
-                    stepIndex = -1;
+                    stepIndex = STEP_INDEX_FOR_START;
                     break;
                 }
 
                 Set<Integer> prev = osi.previous;
                 if (prev == null || prev.isEmpty()) {
                     log.warn("Unable to backtrack from step " + (stepIndex) + "; no previous step recorded. Replaying overall workflow.");
-                    stepIndex = -1;
+                    stepIndex = STEP_INDEX_FOR_START;
                     break;
                 }
 
@@ -347,7 +352,7 @@ public class WorkflowExecutionContext {
                 if (repeating) {
                     if (possibleOthers.size() != 1) {
                         log.warn("Unable to backtrack from step " + (stepIndex) + "; ambiguous precedents " + prev + " / " + possibleOthers + ". Replaying overall workflow.");
-                        stepIndex = -1;
+                        stepIndex = STEP_INDEX_FOR_START;
                         break;
                     } else {
                         stepIndex = possibleOthers.iterator().next();
@@ -371,7 +376,7 @@ public class WorkflowExecutionContext {
     }
 
     public WorkflowStepDefinition.ReplayContinuationInstructions makeInstructionsForReplayingFromStart(String reason, boolean forced) {
-        return makeInstructionsForReplayingFromStep(-1, reason, forced);
+        return makeInstructionsForReplayingFromStep(STEP_INDEX_FOR_START, reason, forced);
     }
 
     public WorkflowStepDefinition.ReplayContinuationInstructions makeInstructionsForReplayingLast(String reason, boolean forced) {
@@ -386,7 +391,7 @@ public class WorkflowExecutionContext {
         Integer replayFromStep = null;
         if (currentStepIndex == null) {
             // not yet started
-            replayFromStep = -1;
+            replayFromStep = STEP_INDEX_FOR_START;
         } else if (currentStepInstance == null || currentStepInstance.stepIndex != currentStepIndex) {
             // replaying from a different step, or current step which has either not run or completed but didn't save
             log.debug("Replaying workflow '" + name + "', cannot replay within step " + currentStepIndex + " because step instance not known; will reinitialize then replay that step");
@@ -424,7 +429,7 @@ public class WorkflowExecutionContext {
                 .tag(BrooklynTaskTags.tagForWorkflow(this))
                 .tag(BrooklynTaskTags.WORKFLOW_TAG)
                 .body(new Body(continuationInstructions)).build();
-        WorkflowReplayUtils.updateOnWorkflowStartOrReplay(this, task, continuationInstructions.customBehaviourExplanation, continuationInstructions.stepToReplayFrom!=null && continuationInstructions.stepToReplayFrom!=-1);
+        WorkflowReplayUtils.updateOnWorkflowStartOrReplay(this, task, continuationInstructions.customBehaviourExplanation, continuationInstructions.stepToReplayFrom!=null && continuationInstructions.stepToReplayFrom!=STEP_INDEX_FOR_START);
 
         taskId = task.getId();
 
@@ -629,7 +634,7 @@ public class WorkflowExecutionContext {
                         status = WorkflowStatus.RUNNING;
 
                         if (replaying) {
-                            if (replayFromStep != null && replayFromStep == -1) {
+                            if (replayFromStep != null && replayFromStep == STEP_INDEX_FOR_START) {
                                 log.debug("Replaying workflow '" + name + "', from start " +
                                         " (was at " + (currentStepIndex == null ? "<UNSTARTED>" : workflowStepReference(currentStepIndex)) + ")");
                                 currentStepIndex = 0;
@@ -699,13 +704,13 @@ public class WorkflowExecutionContext {
                         // finished -- checkpoint noting previous step and null for current because finished
                         status = WorkflowStatus.SUCCESS;
                         // record how it ended
-                        oldStepInfo.compute(previousStepIndex == null ? -1 : previousStepIndex, (index, old) -> {
+                        oldStepInfo.compute(previousStepIndex == null ? STEP_INDEX_FOR_START : previousStepIndex, (index, old) -> {
                             if (old == null) old = new OldStepRecord();
-                            old.next = MutableSet.<Integer>of(-1).putAll(old.next);
+                            old.next = MutableSet.<Integer>of(STEP_INDEX_FOR_START).putAll(old.next);
                             old.nextTaskId = null;
                             return old;
                         });
-                        oldStepInfo.compute(-1, (index, old) -> {
+                        oldStepInfo.compute(STEP_INDEX_FOR_START, (index, old) -> {
                             if (old == null) old = new OldStepRecord();
                             old.previous = MutableSet.<Integer>of(previousStepIndex).putAll(old.previous);
                             old.previousTaskId = previousStepTaskId;
@@ -879,12 +884,12 @@ public class WorkflowExecutionContext {
                 if (!workflowScratchVariables.isEmpty())
                     old.workflowScratch = MutableMap.copyOf(workflowScratchVariables);
                 else old.workflowScratch = null;
-                old.previous = MutableSet.<Integer>of(previousStepIndex == null ? -1 : previousStepIndex).putAll(old.previous);
+                old.previous = MutableSet.<Integer>of(previousStepIndex == null ? STEP_INDEX_FOR_START : previousStepIndex).putAll(old.previous);
                 old.previousTaskId = previousStepTaskId;
                 old.nextTaskId = null;
                 return old;
             });
-            oldStepInfo.compute(previousStepIndex==null ? -1 : previousStepIndex, (index, old) -> {
+            oldStepInfo.compute(previousStepIndex==null ? STEP_INDEX_FOR_START : previousStepIndex, (index, old) -> {
                 if (old==null) old = new OldStepRecord();
                 old.next = MutableSet.<Integer>of(currentStepIndex).putAll(old.next);
                 old.nextTaskId = t.getId();
@@ -1036,9 +1041,9 @@ public class WorkflowExecutionContext {
 
     private String indexCode(int index) {
         // these numbers shouldn't be used for much, but they are used in a few places :(
-        if (index==-1) return "start";
-        if (index==-2) return "end";
-        if (index==-3) return "error-handler";
+        if (index==STEP_INDEX_FOR_START) return "start";
+        if (index==STEP_INDEX_FOR_END) return "end";
+        if (index==STEP_INDEX_FOR_ERROR_HANDLER) return "error-handler";
         return "neg-"+(index); // unknown
     }
 
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
index 579bc1d96a..30112ea2c3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
@@ -118,7 +118,7 @@ public class WorkflowReplayUtils {
 
     public static void updateOnWorkflowSuccess(WorkflowExecutionContext ctx, Task<?> task, Object result) {
         WorkflowReplayRecord.updateInternal(ctx, task, true, result);
-        ctx.replayableLastStep = -2;
+        ctx.replayableLastStep = WorkflowExecutionContext.STEP_INDEX_FOR_END;
     }
 
     public static void updateOnWorkflowError(WorkflowExecutionContext ctx, Task<?> task, Throwable error) {
@@ -193,8 +193,8 @@ public class WorkflowReplayUtils {
             // forced, eg throwing exception
             return subWorkflow.createTaskReplaying(subWorkflow.makeInstructionsForReplayingLastForcedWithCustom(instructions.customBehaviourExplanation, instructions.customBehaviour));
         } else {
-            if (Objects.equals(subWorkflow.replayableLastStep, -2)) return null;
-            // may throw if not forced and not replayable
+            if (Objects.equals(subWorkflow.replayableLastStep, WorkflowExecutionContext.STEP_INDEX_FOR_END)) return null;
+            // may throw if not forced and not replayable[
             return subWorkflow.createTaskReplaying(subWorkflow.makeInstructionsForReplayingLast(instructions.customBehaviourExplanation, instructions.forced));
         }
     }


[brooklyn-server] 01/11: address code review comments, improve https support

Posted by he...@apache.org.
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 a50296510d3cb950bc158fd69c8875be020f643a
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Wed Oct 19 09:29:32 2022 +0100

    address code review comments, improve https support
---
 .../core/workflow/WorkflowStepDefinition.java      |  8 ++--
 .../core/workflow/steps/HttpWorkflowStep.java      |  6 +++
 .../core/workflow/steps/SshWorkflowStep.java       |  6 +--
 .../core/workflow/WorkflowBeefyStepTest.java       | 53 +++++++++++++++++-----
 .../tasks/kubectl/ContainerWorkflowStep.java       |  9 +---
 .../brooklyn/location/winrm/WinrmWorkflowStep.java |  4 +-
 .../brooklyn/util/http/executor/HttpConfig.java    |  5 ++
 .../brooklyn/util/http/executor/HttpRequest.java   |  2 +-
 .../executor/apacheclient/HttpExecutorImpl.java    |  3 +-
 9 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
index 5d32073a34..92532a0bc9 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
@@ -47,15 +47,12 @@ public abstract class WorkflowStepDefinition {
 
     private static final Logger log = LoggerFactory.getLogger(WorkflowStepDefinition.class);
 
-    //    name:  a name to display in the UI; if omitted it is constructed from the step ID and step type
-    @JsonInclude(JsonInclude.Include.NON_EMPTY)
-    protected Map<String,Object> input = MutableMap.of();
-
     protected String id;
     public String getId() {
         return id;
     }
 
+    //    name:  a name to display in the UI; if omitted it is constructed from the step ID and step type
     protected String name;
     public String getName() {
         return name;
@@ -63,6 +60,9 @@ public abstract class WorkflowStepDefinition {
 
     protected String userSuppliedShorthand;
 
+    @JsonInclude(JsonInclude.Include.NON_EMPTY)
+    protected Map<String,Object> input = MutableMap.of();
+
     //    next:  the next step to go to, assuming the step runs and succeeds; if omitted, or if the condition does not apply, it goes to the next step per the ordering (described below)
     @JsonProperty("next")  //use this field for access, not the getter/setter
     protected String next;
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
index 35501196e0..8a4988029b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
@@ -34,6 +34,7 @@ import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.http.HttpTool;
 import org.apache.brooklyn.util.http.auth.UsernamePassword;
+import org.apache.brooklyn.util.http.executor.HttpConfig;
 import org.apache.brooklyn.util.http.executor.HttpExecutor;
 import org.apache.brooklyn.util.http.executor.HttpRequest;
 import org.apache.brooklyn.util.http.executor.HttpResponse;
@@ -62,6 +63,9 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
     public static final ConfigKey<Map<String, String>> HEADERS = new MapConfigKey<>(String.class, "headers");
     public static final ConfigKey<String> METHOD = ConfigKeys.newStringConfigKey("method");
 
+    /** directly customizable here, otherwise based on entity and brooklyn.properties per {@link BrooklynHttpConfig} */
+    public static final ConfigKey<HttpConfig> HTTPS_CONFIG = ConfigKeys.newConfigKey(HttpConfig.class, "config");
+
     public static final ConfigKey<String> USERNAME = ConfigKeys.newStringConfigKey("username", "Username for HTTP request, if required");
     public static final ConfigKey<String> PASSWORD = ConfigKeys.newStringConfigKey("password", "Password for HTTP request, if required");
 
@@ -116,6 +120,8 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
         Map<String, String> headers = context.getInput(HEADERS);
         if (headers!=null) httpb.headers(headers);
 
+        httpb.config(context.getInput(HTTPS_CONFIG));
+
         String method = context.getInput(METHOD);
         if (Strings.isBlank(method)) method = "get";
         httpb.method(method);
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
index 0d14f70345..5b084c8bd9 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
@@ -19,7 +19,6 @@
 package org.apache.brooklyn.core.workflow.steps;
 
 import com.google.common.reflect.TypeToken;
-import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.config.MapConfigKey;
@@ -34,7 +33,6 @@ import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.ssh.SshTasks;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
-import org.apache.brooklyn.util.core.units.Range;
 import org.apache.brooklyn.util.text.Strings;
 
 import java.util.Map;
@@ -45,7 +43,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
 
     public static final ConfigKey<String> ENDPOINT = ConfigKeys.newStringConfigKey("endpoint");
     public static final ConfigKey<String> COMMAND = ConfigKeys.newStringConfigKey("command");
-    public static final ConfigKey<Map<String,Object>> ENv = new MapConfigKey.Builder(Object.class, "env").build();
+    public static final ConfigKey<Map<String,Object>> ENV = new MapConfigKey.Builder(Object.class, "env").build();
     public static final ConfigKey<DslPredicates.DslPredicate<Integer>> EXIT_CODE = ConfigKeys.newConfigKey(new TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit-code");
 
     @Override
@@ -74,7 +72,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
     public static <U, T extends ProcessTaskFactory<U>> ProcessTaskFactory<Map<?,?>> customizeProcessTaskFactory(WorkflowStepInstanceExecutionContext context, T tf) {
         DslPredicates.DslPredicate<Integer> exitcode = context.getInput(EXIT_CODE);
         if (exitcode!=null) tf.allowingNonZeroExitCode();
-        Map<String, Object> env = context.getInput(ENv);
+        Map<String, Object> env = context.getInput(ENV);
         if (env!=null) tf.environmentVariables(new ShellEnvironmentSerializer(context.getWorkflowExectionContext().getManagementContext()).serialize(env));
         return tf.returning(ptw -> {
             checkExitCode(ptw, exitcode);
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
index e382b4cfe4..996bacf3f1 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
@@ -24,35 +24,25 @@ import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.location.LocationSpec;
 import org.apache.brooklyn.api.location.NoMachinesAvailableException;
 import org.apache.brooklyn.api.mgmt.Task;
-import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.entity.EntityAsserts;
 import org.apache.brooklyn.core.entity.EntityInternal;
-import org.apache.brooklyn.core.resolve.jackson.BeanWithTypePlanTransformer;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
-import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
-import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
-import org.apache.brooklyn.core.typereg.RegisteredTypes;
-import org.apache.brooklyn.core.workflow.steps.LogWorkflowStep;
 import org.apache.brooklyn.entity.stock.BasicApplication;
 import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
-import org.apache.brooklyn.location.ssh.SshMachineLocationReuseIntegrationTest;
 import org.apache.brooklyn.test.Asserts;
-import org.apache.brooklyn.test.ClassLogWatcher;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.http.BetterMockWebServer;
 import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.apache.brooklyn.util.http.executor.HttpConfig;
 import org.apache.brooklyn.util.net.Networking;
-import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
-import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
@@ -69,6 +59,9 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
         return runSteps(MutableList.<Object>of(step), appFunction);
     }
     Object runSteps(List<Object> steps, Consumer<BasicApplication> appFunction) {
+        return runSteps(steps, appFunction, null);
+    }
+    Object runSteps(List<Object> steps, Consumer<BasicApplication> appFunction, ConfigBag defaultConfig) {
         loadTypes();
         BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
         this.lastApp = app;
@@ -76,6 +69,7 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
                 .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
                 .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
                 .configure(WorkflowEffector.STEPS, steps)
+                .putAll(defaultConfig)
         );
         if (appFunction!=null) appFunction.accept(app);
         eff.apply((EntityLocal)app);
@@ -85,7 +79,7 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
     }
 
     @Test
-    public void testEffector() throws IOException {
+    public void testEffector() {
         Object result = runSteps(MutableList.of(
                 "let x = ${entity.sensor.x} + 1 ?? 0",
                 "set-sensor x = ${x}",
@@ -130,6 +124,41 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
         Asserts.assertThat(result.get("duration"), x -> Duration.nanos(1).isShorterThan(Duration.of(x)));
     }
 
+    @Test(groups="Integration") //requires internet
+    public void testHttps() throws IOException {
+        doTestHttpsGoogle("https://www.google.com", null, true);
+        doTestHttpsGoogle("www.google.com", null, true);
+        // IP of google won't work unless we trust it
+        doTestHttpsGoogle("172.217.169.68", null, false);
+        doTestHttpsGoogle("172.217.169.68", MutableMap.of("config", HttpConfig.builder().trustAll(true).build()), true);
+        doTestHttpsGoogle("172.217.169.68", MutableMap.of("config", MutableMap.of("trustAll", true)), true);
+        doTestHttpsGoogle("172.217.169.68", MutableMap.of("config", MutableMap.of("trustAll", false)), false);
+    }
+
+    public Map doTestHttpsGoogle(String url, Map<String, Object> extraConfig, Boolean shouldWork) {
+        Map result = null;
+        try {
+            result = (Map) runStep(MutableMap.<String, Object>of("s", "http " + url).add(extraConfig), null);
+            if (shouldWork == null) {
+                // no op, just return result
+            } else if (shouldWork) {
+                Asserts.assertEquals(result.get("status_code"), 200);
+                MutableList.of("" + result.get("content"), "" + new String((byte[]) result.get("content_bytes"))).forEach(s ->
+                        Asserts.assertStringContains(s, "<html", "google.timers.load"));
+            } else {
+                Asserts.shouldHaveFailedPreviously("Instead got: " + result);
+            }
+        } catch (Exception e) {
+            if (Boolean.FALSE.equals(shouldWork)) {
+                // expected, just make sure it isn't the "should have failed" exception
+                Asserts.expectedFailure(e);
+            } else {
+                Asserts.fail(e);
+            }
+        }
+        return result;
+    }
+
     // container, winrm defined in downstream projects and tested in those projects and/or workflow yaml
 
     /*
diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
index 9bd0c57d3c..0a41d5dbb8 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
@@ -19,7 +19,6 @@
 package org.apache.brooklyn.tasks.kubectl;
 
 import com.google.common.reflect.TypeToken;
-import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.config.MapConfigKey;
@@ -30,11 +29,7 @@ import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
 import org.apache.brooklyn.util.core.predicates.DslPredicates;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
-import org.apache.brooklyn.util.core.task.ssh.SshTasks;
-import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
-import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.text.Strings;
-import org.apache.brooklyn.util.time.Duration;
 
 import java.util.Arrays;
 import java.util.List;
@@ -49,7 +44,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition {
     public static final ConfigKey<List<String>> COMMANDS = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "commands");
     public static final ConfigKey<List<String>> RAW_COMMAND = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "raw-command");
     public static final ConfigKey<PullPolicy> PULL_POLICY = ConfigKeys.newConfigKey(PullPolicy.class, "pull-policy", ContainerCommons.CONTAINER_IMAGE_PULL_POLICY.getDescription(), ContainerCommons.CONTAINER_IMAGE_PULL_POLICY.getDefaultValue());
-    public static final ConfigKey<Map<String,Object>> ENv = new MapConfigKey.Builder(Object.class, "env").build();
+    public static final ConfigKey<Map<String,Object>> ENV = new MapConfigKey.Builder(Object.class, "env").build();
     public static final ConfigKey<DslPredicates.DslPredicate<Integer>> EXIT_CODE = ConfigKeys.newConfigKey(new TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit-code");
 
     @Override
@@ -92,7 +87,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition {
             throw new IllegalStateException("Incompatible command specification, max 1, received: "+commandTypesSet);
         }
 
-        Map<String, Object> env = context.getInput(ENv);
+        Map<String, Object> env = context.getInput(ENV);
         if (env != null)
             tf.environmentVariables(new ShellEnvironmentSerializer(context.getWorkflowExectionContext().getManagementContext()).serialize(env));
 
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
index 60241c4db5..bbb8ab4e65 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
@@ -41,7 +41,7 @@ public class WinrmWorkflowStep extends WorkflowStepDefinition {
 
     public static final ConfigKey<String> ENDPOINT = ConfigKeys.newStringConfigKey("endpoint");
     public static final ConfigKey<String> COMMAND = ConfigKeys.newStringConfigKey("command");
-    public static final ConfigKey<Map<String,Object>> ENv = new MapConfigKey.Builder(Object.class, "env").build();
+    public static final ConfigKey<Map<String,Object>> ENV = new MapConfigKey.Builder(Object.class, "env").build();
     public static final ConfigKey<DslPredicates.DslPredicate<Integer>> EXIT_CODE = ConfigKeys.newConfigKey(new TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit-code");
 
     @Override
@@ -67,7 +67,7 @@ public class WinrmWorkflowStep extends WorkflowStepDefinition {
         DslPredicates.DslPredicate<Integer> exitcode = context.getInput(EXIT_CODE);
         ProcessTaskFactory<?> tf = WinRmTasks.newWinrmExecTaskFactory(machine, command);
         if (exitcode!=null) tf.allowingNonZeroExitCode();
-        Map<String, Object> env = context.getInput(ENv);
+        Map<String, Object> env = context.getInput(ENV);
         if (env!=null) tf.environmentVariables(new ShellEnvironmentSerializer(context.getWorkflowExectionContext().getManagementContext()).serialize(env));
         tf.returning(ptw -> {
             checkExitCode(ptw, exitcode);
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java
index e9ac8eb67a..39a1294ec4 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java
@@ -57,6 +57,11 @@ public class HttpConfig {
     private final boolean trustAll;
     private final boolean trustSelfSigned;
 
+    /** jackson provider */
+    protected HttpConfig() {
+        this(builder());
+    }
+
     protected HttpConfig(Builder builder) {
         laxRedirect = builder.laxRedirect;
         trustAll = builder.trustAll;
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java
index 9db28392db..5cf36057d5 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java
@@ -125,7 +125,7 @@ public interface HttpRequest {
     Credentials credentials();
 
     /**
-     * Additional optional configuration to customize how the call is done.
+     * Redirect and trust/cert settings. If supplied, overrides the executor's. If blank, takes from the exector.
      */
     @Nullable
     HttpConfig config();
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java
index 79cd722366..50c248bc82 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java
@@ -61,6 +61,7 @@ public class HttpExecutorImpl implements HttpExecutor {
 
     HttpConfig config = DEFAULT_CONFIG;
 
+    /** config to use if none is specified on the request */
     public HttpExecutorImpl withConfig(HttpConfig config) {
         this.config = config;
         return this;
@@ -68,7 +69,7 @@ public class HttpExecutorImpl implements HttpExecutor {
 
     @Override
     public HttpResponse execute(HttpRequest request) throws IOException {
-        HttpConfig config = (request.config() != null) ? request.config() : DEFAULT_CONFIG;
+        HttpConfig config = (request.config() != null) ? request.config() : this.config!=null ? this.config : DEFAULT_CONFIG;
         Credentials creds = (request.credentials() != null) ? new UsernamePasswordCredentials(request.credentials().getUser(), request.credentials().getPassword()) : null;
         HttpClient httpClient = HttpTool.httpClientBuilder()
                 .uri(request.uri())


[brooklyn-server] 07/11: ensure ssh workflow step fails if non-zero exit code

Posted by he...@apache.org.
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 5b2c5aa31093a58c3c41155b07c86b78d339454d
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Oct 20 17:41:49 2022 +0100

    ensure ssh workflow step fails if non-zero exit code
---
 .../org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java  | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
index 8db28cbd51..ba03791962 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
@@ -86,7 +86,11 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
     }
 
     protected static void checkExitCode(ProcessTaskWrapper<?> ptw, DslPredicates.DslPredicate<Integer> exitcode) {
-        if (exitcode==null) return;
+        if (exitcode==null) {
+            if (ptw.getExitCode()!=0) throw new IllegalStateException("Invalid exit code '"+ptw.getExitCode()+"'");
+            return;
+        }
+
         if (exitcode instanceof DslPredicates.DslPredicateBase) {
             Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals;
             if (implicit!=null) {
@@ -99,7 +103,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
             // ranges still require `exit-code: { range: [0, 4] }`, same with `exit-code: { less-than: 5 }`.
         }
         if (!exitcode.apply(ptw.getExitCode())) {
-            throw new IllegalStateException("Invalid exit code '"+ptw.getExitCode()+"'");
+            throw new IllegalStateException("Invalid exit code '"+ptw.getExitCode()+"'; does not match explicit exit-code requirement");
         }
     }
 


[brooklyn-server] 10/11: misc workflow fixes - more details, including error and rest calls

Posted by he...@apache.org.
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 4062c3ab7deb7a06718991e64a95f5e12b735b9f
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Oct 21 11:42:04 2022 +0100

    misc workflow fixes - more details, including error and rest calls
---
 .../brooklyn/core/workflow/WorkflowExecutionContext.java  | 13 +++++++++++--
 .../brooklyn/core/workflow/WorkflowReplayUtils.java       | 14 +++++++++++++-
 .../workflow/WorkflowStepInstanceExecutionContext.java    |  3 +++
 .../brooklyn/core/workflow/steps/CustomWorkflowStep.java  | 14 ++++++++++++--
 .../brooklyn/core/workflow/steps/SshWorkflowStep.java     |  4 ++--
 .../org/apache/brooklyn/util/core/task/TaskBuilder.java   | 15 ++++++++++-----
 .../apache/brooklyn/rest/resources/EntityResource.java    |  5 ++++-
 .../software/base/WorkflowSoftwareProcessSshDriver.java   |  2 +-
 8 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index 90e32abd10..76752c672f 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -134,6 +134,10 @@ public class WorkflowExecutionContext {
     String previousStepTaskId;
 
     WorkflowStepInstanceExecutionContext currentStepInstance;
+
+    /** set if an error handler is the last thing which ran */
+    String errorHandlerTaskId;
+    /** set for the last _step_ inside the error handler */
     WorkflowStepInstanceExecutionContext errorHandlerContext;
 
     Map<Integer, OldStepRecord> oldStepInfo = MutableMap.of();
@@ -773,7 +777,9 @@ public class WorkflowExecutionContext {
                             WorkflowErrorHandling.WorkflowErrorHandlingResult result = null;
                             try {
                                 log.debug("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + " '" + task.getDisplayName() + "', running error handler");
-                                result = DynamicTasks.queue(WorkflowErrorHandling.createWorkflowErrorHandlerTask(WorkflowExecutionContext.this, task, e)).getUnchecked();
+                                Task<WorkflowErrorHandling.WorkflowErrorHandlingResult> workflowErrorHandlerTask = WorkflowErrorHandling.createWorkflowErrorHandlerTask(WorkflowExecutionContext.this, task, e);
+                                errorHandlerTaskId = workflowErrorHandlerTask.getId();
+                                result = DynamicTasks.queue(workflowErrorHandlerTask).getUnchecked();
                                 if (result != null) {
                                     errorHandled = true;
 
@@ -899,6 +905,7 @@ public class WorkflowExecutionContext {
             // and update replayable info
             WorkflowReplayUtils.updateOnWorkflowStepChange(currentStepRecord, currentStepInstance, step);
             errorHandlerContext = null;
+            errorHandlerTaskId = null;
 
             persist();
 
@@ -929,7 +936,9 @@ public class WorkflowExecutionContext {
                 if (!step.onError.isEmpty()) {
                     WorkflowErrorHandling.WorkflowErrorHandlingResult result;
                     try {
-                        result = DynamicTasks.queue(WorkflowErrorHandling.createStepErrorHandlerTask(step, currentStepInstance, t, e)).getUnchecked();
+                        Task<WorkflowErrorHandling.WorkflowErrorHandlingResult> stepErrorHandlerTask = WorkflowErrorHandling.createStepErrorHandlerTask(step, currentStepInstance, t, e);
+                        currentStepInstance.errorHandlerTaskId = stepErrorHandlerTask.getId();
+                        result = DynamicTasks.queue(stepErrorHandlerTask).getUnchecked();
                         if (result!=null) {
                             if (Strings.isNonBlank(result.next)) customNext.set(result.next);
                             saveOutput.accept(result.output);
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
index 30112ea2c3..d1449cff01 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
@@ -22,10 +22,12 @@ import com.fasterxml.jackson.annotation.JsonInclude;
 import com.google.common.collect.Iterables;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.time.Instant;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Supplier;
@@ -75,8 +77,18 @@ public class WorkflowReplayUtils {
                 log.warn("Mismatch in workflow replays for "+ctx+": "+ctx.replayCurrent +" vs "+task);
                 return;
             }
-            ctx.replayCurrent.submittedByTaskId = task.getSubmittedByTaskId();
+
+            // try hard to get submitter data in case tasks go awol before execution
+            if (task.getSubmittedByTaskId()!=null) {
+                ctx.replayCurrent.submittedByTaskId = task.getSubmittedByTaskId();
+            } else if (ctx.replayCurrent.submittedByTaskId==null && Tasks.current()!=null && !Tasks.current().equals(task)) {
+                ctx.replayCurrent.submittedByTaskId = Tasks.current().getId();
+            }
             ctx.replayCurrent.submitTimeUtc = task.getSubmitTimeUtc();
+            // fake this because we won't see the real value until we also see the start value.
+            // however we need to ensure any workflow that is created is intended to be run.
+            if (ctx.replayCurrent.submitTimeUtc<=0) ctx.replayCurrent.submitTimeUtc = Instant.now().toEpochMilli();
+
             ctx.replayCurrent.startTimeUtc = task.getStartTimeUtc();
             ctx.replayCurrent.endTimeUtc = task.getEndTimeUtc();
             ctx.replayCurrent.status = task.getStatusSummary();
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
index b3bc5065f5..d9ea83f774 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
@@ -61,6 +61,9 @@ public class WorkflowStepInstanceExecutionContext {
     /** set if the step is in an error handler context, containing the error being handled */
     Throwable error;
 
+    /** set if there was an error handled locally */
+    String errorHandlerTaskId;
+
     public String getTaskId() {
         return taskId;
     }
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
index 4f36809a5e..c1e313f78c 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
@@ -28,6 +28,7 @@ import com.google.common.reflect.TypeToken;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
+import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.resolve.jackson.JsonPassThroughDeserializer;
@@ -168,14 +169,23 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
         return result;
     }
 
+    /** Returns a top-level workflow running the workflow defined here */
     public WorkflowExecutionContext newWorkflowExecution(Entity entity, String name, ConfigBag extraConfig) {
+        return newWorkflowExecution(entity, name, extraConfig, null);
+    }
+    public WorkflowExecutionContext newWorkflowExecution(Entity entity, String name, ConfigBag extraConfig, Map extraTaskFlags) {
         return WorkflowExecutionContext.newInstancePersisted(entity, name,
                 ConfigBag.newInstance()
                         .configure(WorkflowCommonConfig.PARAMETER_DEFS, parameters)
                         .configure(WorkflowCommonConfig.STEPS, steps)
-                        .configure(WorkflowCommonConfig.OUTPUT, workflowOutput),
+                        .configure(WorkflowCommonConfig.INPUT, input)
+                        .configure(WorkflowCommonConfig.OUTPUT, workflowOutput)
+                        .configure(WorkflowCommonConfig.REPLAYABLE, replayable)
+                        .configure(WorkflowCommonConfig.ON_ERROR, onError)
+                        .configure(WorkflowCommonConfig.TIMEOUT, timeout)
+                        .configure((ConfigKey) WorkflowCommonConfig.CONDITION, condition),
                 null,
-                ConfigBag.newInstance(getInput()).putAll(extraConfig), null);
+                ConfigBag.newInstance(getInput()).putAll(extraConfig), extraTaskFlags);
     }
 
     @VisibleForTesting
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
index ba03791962..e16b4559b0 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
@@ -87,7 +87,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
 
     protected static void checkExitCode(ProcessTaskWrapper<?> ptw, DslPredicates.DslPredicate<Integer> exitcode) {
         if (exitcode==null) {
-            if (ptw.getExitCode()!=0) throw new IllegalStateException("Invalid exit code '"+ptw.getExitCode()+"'");
+            if (ptw.getExitCode()!=0) throw new IllegalStateException("Invalid exit code "+ptw.getExitCode());
             return;
         }
 
@@ -103,7 +103,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
             // ranges still require `exit-code: { range: [0, 4] }`, same with `exit-code: { less-than: 5 }`.
         }
         if (!exitcode.apply(ptw.getExitCode())) {
-            throw new IllegalStateException("Invalid exit code '"+ptw.getExitCode()+"'; does not match explicit exit-code requirement");
+            throw new IllegalStateException("Invalid exit code "+ptw.getExitCode()+"; does not match explicit exit-code requirement");
         }
     }
 
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java b/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java
index c6177999bb..79624125d0 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java
@@ -18,10 +18,7 @@
  */
 package org.apache.brooklyn.util.core.task;
 
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.Callable;
 
 import org.apache.brooklyn.api.mgmt.Task;
@@ -34,10 +31,14 @@ import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
 
 import com.google.common.collect.Iterables;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** Convenience for creating tasks; note that DynamicSequentialTask is the default */
 public class TaskBuilder<T> {
 
+    private static final Logger log = LoggerFactory.getLogger(TaskBuilder.class);
+
     String displayName = null;
     String description = null;
     Callable<T> body = null;
@@ -152,7 +153,11 @@ public class TaskBuilder<T> {
         MutableMap<String, Object> taskFlags = MutableMap.copyOf(flags);
         if (displayName!=null) taskFlags.put("displayName", displayName);
         if (description!=null) taskFlags.put("description", description);
-        if (!tags.isEmpty()) taskFlags.put("tags", tags);
+        if (!tags.isEmpty()) {
+            Object otherTags = taskFlags.put("tags", tags);
+            if (otherTags instanceof Collection) tags.addAll((Collection)otherTags);
+            else log.warn("Ignoring unexpected 'tags' flag in task: "+otherTags);
+        }
         
         if (Boolean.FALSE.equals(dynamic) && children.isEmpty()) {
             if (swallowChildrenFailures!=null)
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
index 84495ea37b..81cacb01fd 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
@@ -64,6 +64,7 @@ import org.apache.brooklyn.rest.transform.TaskTransformer;
 import org.apache.brooklyn.rest.util.EntityRelationUtils;
 import org.apache.brooklyn.rest.util.WebResourceUtils;
 import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.ClassLoaderUtils;
 import org.apache.brooklyn.util.core.ResourceUtils;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
@@ -425,7 +426,9 @@ public class EntityResource extends AbstractBrooklynRestResource implements Enti
         }
 
         WorkflowExecutionContext execution = workflow.newWorkflowExecution(target,
-                Strings.firstNonBlank(workflow.getName(), workflow.getId(), "API workflow invocation"), null);
+                Strings.firstNonBlank(workflow.getName(), workflow.getId(), "API workflow invocation"),
+                null,
+                MutableMap.of("tags", MutableMap.of("workflow_yaml", yaml)));
 
         Task<Object> task = Entities.submit(target, execution.getTask(true).get());
         task.blockUntilEnded(timeoutS==null ? Duration.millis(20) : Duration.of(timeoutS));
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessSshDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessSshDriver.java
index c8bb664ea2..6a7108eb11 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessSshDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessSshDriver.java
@@ -93,7 +93,7 @@ public class WorkflowSoftwareProcessSshDriver extends AbstractSoftwareProcessSsh
         WorkflowExecutionContext workflowContext = workflow.newWorkflowExecution(entity, key.getName().toLowerCase(),
                 null /* could getInput from workflow, and merge shell environment here */);
 
-        return Maybe.of(DynamicTasks.queue( workflowContext.getTask(true).get() ).getUnchecked());
+        return Maybe.of(DynamicTasks.queueIfPossible( workflowContext.getTask(true).get() ).orSubmitAsync(entity).getTask().getUnchecked());
     }
 
     @Override


[brooklyn-server] 11/11: workflow basic expiry, and more workflow fixes

Posted by he...@apache.org.
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 b35bab6dcd8f74ea727f9a1ea0fe9da90a260802
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Oct 21 14:55:06 2022 +0100

    workflow basic expiry, and more workflow fixes
---
 .../core/workflow/WorkflowErrorHandling.java       |  2 +
 .../core/workflow/WorkflowExecutionContext.java    | 59 ++++++++++++++++++++--
 .../core/workflow/WorkflowStepDefinition.java      |  3 ++
 .../core/workflow/steps/RetryWorkflowStep.java     |  6 ++-
 .../store/WorkflowStatePersistenceViaSensors.java  | 29 +++++++++--
 .../brooklyn/util/core/task/TaskBuilder.java       |  5 +-
 .../workflow/WorkflowPersistReplayErrorsTest.java  |  4 +-
 7 files changed, 96 insertions(+), 12 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
index f17d25792d..532ab13ee4 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
@@ -52,6 +52,7 @@ public class WorkflowErrorHandling implements Callable<WorkflowErrorHandling.Wor
         log.debug("Encountered error in step "+context.getWorkflowStepReference()+" '" + stepTask.getDisplayName() + "' (handler present): " + Exceptions.collapseText(error));
         Task<WorkflowErrorHandlingResult> task = Tasks.<WorkflowErrorHandlingResult>builder().dynamic(true).displayName(context.getWorkflowStepReference()+"-error-handler")
                 .tag(BrooklynTaskTags.tagForWorkflowStepErrorHandler(context, null, context.getTaskId()))
+                .tag(BrooklynTaskTags.WORKFLOW_TAG)
                 .body(new WorkflowErrorHandling(step.getOnError(), context.getWorkflowExectionContext(), context.getWorkflowExectionContext().currentStepIndex, stepTask, error))
                 .build();
         TaskTags.addTagDynamically(stepTask, BrooklynTaskTags.tagForErrorHandledBy(task));
@@ -64,6 +65,7 @@ public class WorkflowErrorHandling implements Callable<WorkflowErrorHandling.Wor
         log.debug("Encountered error in workflow "+context.getWorkflowId()+"/"+context.getTaskId()+" '" + workflowTask.getDisplayName() + "' (handler present): " + Exceptions.collapseText(error));
         Task<WorkflowErrorHandlingResult> task = Tasks.<WorkflowErrorHandlingResult>builder().dynamic(true).displayName(context.getWorkflowId()+"-error-handler")
                 .tag(BrooklynTaskTags.tagForWorkflowStepErrorHandler(context))
+                .tag(BrooklynTaskTags.WORKFLOW_TAG)
                 .body(new WorkflowErrorHandling(context.onError, context, null, workflowTask, error))
                 .build();
         TaskTags.addTagDynamically(workflowTask, BrooklynTaskTags.tagForErrorHandledBy(task));
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index 76752c672f..2d2eb1688e 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -20,7 +20,9 @@ package org.apache.brooklyn.core.workflow;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.google.common.collect.Iterables;
 import com.google.common.reflect.TypeToken;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
@@ -60,6 +62,7 @@ import java.util.*;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -121,6 +124,9 @@ public class WorkflowExecutionContext {
     String taskId;
     transient Task<Object> task;
 
+    @JsonProperty("expiryKey")
+    String expiryKey;
+
     /** all tasks created for this workflow */
     Set<WorkflowReplayUtils.WorkflowReplayRecord> replays = MutableSet.of();
     transient WorkflowReplayUtils.WorkflowReplayRecord replayCurrent = null;
@@ -239,7 +245,7 @@ public class WorkflowExecutionContext {
 
         TaskBuilder<Object> tb = Tasks.builder().dynamic(true);
         if (optionalTaskFlags!=null) tb.flags(optionalTaskFlags);
-        else tb.displayName(name);
+        if (Strings.isBlank(tb.getDisplayName())) tb.displayName(name);
         task = tb.body(new Body()).build();
         WorkflowReplayUtils.updateOnWorkflowStartOrReplay(this, task, "initial run", false);
         workflowId = taskId = task.getId();
@@ -564,6 +570,49 @@ public class WorkflowExecutionContext {
         return errorHandlerContext;
     }
 
+    @JsonIgnore
+    public String getExpiryKey() {
+        if (Strings.isNonBlank(expiryKey)) return expiryKey;
+        if (Strings.isNonBlank(getName())) return getName();
+        return "anonymous-workflow";
+    }
+
+    /** look in tasks, steps, and replays to find most recent activity */
+    public long getMostRecentActivityTime() {
+        AtomicLong result = new AtomicLong(-1);
+
+        Consumer<Long> consider = l -> {
+            if (l!=null && l>result.get()) result.set(l);
+        };
+        Consumer<Task> considerTask = task -> {
+            if (task!=null) {
+                consider.accept(task.getEndTimeUtc());
+                consider.accept(task.getStartTimeUtc());
+                consider.accept(task.getSubmitTimeUtc());
+            }
+        };
+        considerTask.accept(getTask(false).orNull());
+
+        Consumer<WorkflowReplayUtils.WorkflowReplayRecord> considerReplay = replay -> {
+            if (replay!=null) {
+                consider.accept(replay.endTimeUtc);
+                consider.accept(replay.startTimeUtc);
+                consider.accept(replay.submitTimeUtc);
+            }
+        };
+        if (replayCurrent!=null) {
+            considerReplay.accept(replayCurrent);
+        } else if (!replays.isEmpty()) {
+            considerReplay.accept(Iterables.getLast(replays));
+        }
+
+        if (currentStepInstance!=null) {
+            considerTask.accept(getManagementContext().getExecutionManager().getTask(currentStepInstance.getTaskId()));
+        }
+
+        return result.get();
+    }
+
     public List<Object> getStepsDefinition() {
         return MutableList.copyOf(stepsDefinition).asUnmodifiable();
     }
@@ -771,12 +820,12 @@ public class WorkflowExecutionContext {
                         boolean errorHandled = false;
                         if (isTimeout) {
                             // don't run error handler
-                            log.debug("Timeout in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + " '" + task.getDisplayName() + "', throwing: " + Exceptions.collapseText(e));
+                            log.debug("Timeout in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + ", throwing: " + Exceptions.collapseText(e));
 
                         } else if (onError != null && !onError.isEmpty() && provisionalStatus.persistable) {
                             WorkflowErrorHandling.WorkflowErrorHandlingResult result = null;
                             try {
-                                log.debug("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + " '" + task.getDisplayName() + "', running error handler");
+                                log.debug("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + ", running error handler");
                                 Task<WorkflowErrorHandling.WorkflowErrorHandlingResult> workflowErrorHandlerTask = WorkflowErrorHandling.createWorkflowErrorHandlerTask(WorkflowExecutionContext.this, task, e);
                                 errorHandlerTaskId = workflowErrorHandlerTask.getId();
                                 result = DynamicTasks.queue(workflowErrorHandlerTask).getUnchecked();
@@ -797,13 +846,13 @@ public class WorkflowExecutionContext {
                                 }
 
                             } catch (Exception e2) {
-                                log.warn("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + " '" + task.getDisplayName() + "' error handler for -- " + Exceptions.collapseText(e) + " -- threw another error (rethrowing): " + Exceptions.collapseText(e2));
+                                log.warn("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + " error handler for -- " + Exceptions.collapseText(e) + " -- threw another error (rethrowing): " + Exceptions.collapseText(e2));
                                 log.debug("Full trace of original error was: " + e, e);
                                 e = e2;
                             }
 
                         } else {
-                            log.debug("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + " '" + task.getDisplayName() + "', no error handler so rethrowing: " + Exceptions.collapseText(e));
+                            log.debug("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + ", no error handler so rethrowing: " + Exceptions.collapseText(e));
                         }
 
                         if (!errorHandled) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
index 9a2d5b2d05..2026721992 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
@@ -58,6 +58,9 @@ public abstract class WorkflowStepDefinition {
     public String getName() {
         return name;
     }
+    public void setName(String name) {
+        this.name = name;
+    }
 
     protected String userSuppliedShorthand;
     protected String shorthandTypeName;
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/RetryWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/RetryWorkflowStep.java
index 62746d2806..444aba31b4 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/RetryWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/RetryWorkflowStep.java
@@ -53,6 +53,10 @@ public class RetryWorkflowStep extends WorkflowStepDefinition {
     public static final ConfigKey<List<RetryLimit>> LIMIT = ConfigKeys.newConfigKey(new TypeToken<List<RetryLimit>>() {}, "limit");
     public static final ConfigKey<RetryBackoff> BACKOFF = ConfigKeys.newConfigKey(RetryBackoff.class, "backoff");
 
+    // if multiple retry steps declare the same key, their counts will be combined; used if the same error might be handled in different ways
+    // note that the limits and backoff _instructions_ apply only at the step where they are defing, so they may need to be defined at each step
+    public static final ConfigKey<String> KEY = ConfigKeys.newStringConfigKey("key");
+
     public enum RetryReplayOption { TRUE, FALSE, FORCE }
 
     public static class RetryLimit {
@@ -188,7 +192,7 @@ public class RetryWorkflowStep extends WorkflowStepDefinition {
     @Override
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
 
-        String key = context.getWorkflowExectionContext().getWorkflowStepReference(Tasks.current());
+        String key = Strings.firstNonBlank(context.getInput(KEY), context.getWorkflowExectionContext().getWorkflowStepReference(Tasks.current()));
         List<Instant> retries = context.getWorkflowExectionContext().getRetryRecords().compute(key, (k, v) -> v != null ? v : MutableList.of());
 
         List<RetryLimit> limit = context.getInput(LIMIT);
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/store/WorkflowStatePersistenceViaSensors.java b/core/src/main/java/org/apache/brooklyn/core/workflow/store/WorkflowStatePersistenceViaSensors.java
index 6874ea357e..47e4390f17 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/store/WorkflowStatePersistenceViaSensors.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/store/WorkflowStatePersistenceViaSensors.java
@@ -26,17 +26,22 @@ import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.core.workflow.WorkflowErrorHandling;
 import org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
+import java.util.stream.Collectors;
 
 public class WorkflowStatePersistenceViaSensors {
 
+    private static final Logger log = LoggerFactory.getLogger(WorkflowStatePersistenceViaSensors.class);
+
     public static final AttributeSensor<Map<String,WorkflowExecutionContext>> INTERNAL_WORKFLOWS = Sensors.newSensor(new TypeToken<Map<String, WorkflowExecutionContext>>() {}, "internals.brooklyn.workflow");
 
 
@@ -54,6 +59,24 @@ public class WorkflowStatePersistenceViaSensors {
             entity.sensors().modify(INTERNAL_WORKFLOWS, v -> {
                 if (v == null) v = MutableMap.of();
                 v.put(context.getWorkflowId(), context);
+                String k = Strings.firstNonBlank(context.getExpiryKey(), "empty-expiry-key");  //should always be set
+                // TODO follow expiry instructions; for now, just keep 3 latest, apart from this one
+                List<WorkflowExecutionContext> finishedTwins = v.values().stream()
+                        .filter(c -> k.equals(c.getExpiryKey()))
+                        .filter(c -> c.getStatus()!=null && c.getStatus().ended)
+                        .filter(c -> !c.equals(context))
+                        .collect(Collectors.toList());
+                if (finishedTwins.size()>3) {
+                    finishedTwins = MutableList.copyOf(finishedTwins);
+                    Collections.sort(finishedTwins, (t1,t2) -> Long.compare(t2.getMostRecentActivityTime(), t1.getMostRecentActivityTime()));
+                    Iterator<WorkflowExecutionContext> ti = finishedTwins.iterator();
+                    for (int i=0; i<3; i++) ti.next();
+                    while (ti.hasNext()) {
+                        WorkflowExecutionContext w = ti.next();
+                        log.debug("Expiring old workflow "+w+" because it is finished and there are newer ones");
+                        v.remove(w.getWorkflowId());
+                    }
+                }
                 return Maybe.of(v);
             });
             mgmt.getRebindManager().forcePersistNow(false, null);
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java b/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java
index 79624125d0..069903f728 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/TaskBuilder.java
@@ -71,7 +71,10 @@ public class TaskBuilder<T> {
         this.displayName = displayName;
         return this;
     }
-    
+    public String getDisplayName() {
+        return displayName;
+    }
+
     public TaskBuilder<T> description(String description) {
         this.description = description;
         return this;
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
index 049a69235d..b68d8bbb83 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
@@ -573,8 +573,8 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl
                     m -> m.matches("Starting workflow 'Workflow for effector myWorkflow', moving to first step .*-1"),
                     m -> m.matches("Starting step .*-1 in task .*"),
                     m -> m.matches("Error in step '1 - invoke-effector does-not-exist', no error handler so rethrowing: No effector matching 'does-not-exist'"),
-                    m -> m.matches("Error in workflow 'Workflow for effector myWorkflow' around step .*-1 'myWorkflow', running error handler"),
-                    m -> m.matches("Encountered error in workflow .*/.* 'myWorkflow' .handler present.: No effector matching 'does-not-exist'"),
+                    m -> m.matches("Error in workflow 'Workflow for effector myWorkflow' around step .*-1, running error handler"),
+                    m -> m.matches("Encountered error in workflow .*/.* 'Workflow for effector myWorkflow' .handler present.: No effector matching 'does-not-exist'"),
                     m -> m.matches("Creating workflow .* error handler .*-error-handler in task .*"),
                     m -> m.matches("Starting .*-error-handler with 1 handler in task .*"),
                     m -> m.matches("Creating handler .*-error-handler-1 'no-op' in task .*"),


[brooklyn-server] 02/11: make step output available on errors, and improve short name metadata for types

Posted by he...@apache.org.
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 84876d12fde500c4054228a7fa22516e98fafb2f
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Wed Oct 19 15:55:18 2022 +0100

    make step output available on errors, and improve short name metadata for types
---
 .../brooklyn/core/workflow/WorkflowStepDefinition.java       |  8 +++++++-
 .../core/workflow/WorkflowStepInstanceExecutionContext.java  |  7 +++++++
 .../brooklyn/core/workflow/WorkflowStepResolution.java       |  3 +++
 .../brooklyn/core/workflow/steps/HttpWorkflowStep.java       |  4 +++-
 .../apache/brooklyn/core/workflow/steps/SshWorkflowStep.java |  9 ++++++---
 .../org/apache/brooklyn/core/workflow/WorkflowBasicTest.java | 12 ++++++++++--
 .../core/workflow/WorkflowPersistReplayErrorsTest.java       |  2 +-
 7 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
index 92532a0bc9..9a2d5b2d05 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
@@ -43,6 +43,7 @@ import javax.annotation.Nonnull;
 import java.util.List;
 import java.util.Map;
 
+@JsonInclude(JsonInclude.Include.NON_EMPTY)
 public abstract class WorkflowStepDefinition {
 
     private static final Logger log = LoggerFactory.getLogger(WorkflowStepDefinition.class);
@@ -59,6 +60,7 @@ public abstract class WorkflowStepDefinition {
     }
 
     protected String userSuppliedShorthand;
+    protected String shorthandTypeName;
 
     @JsonInclude(JsonInclude.Include.NON_EMPTY)
     protected Map<String,Object> input = MutableMap.of();
@@ -229,7 +231,11 @@ public abstract class WorkflowStepDefinition {
         return Strings.join(parts, " - ");
     }
 
-    protected String getShorthandTypeName() {
+    public void setShorthandTypeName(String shorthandTypeDefinition) { this.shorthandTypeName = shorthandTypeDefinition; }
+    @JsonProperty("shorthandTypeName")  // REST API should prefer this accessor
+    public String getShorthandTypeName() {
+        if (Strings.isNonBlank(shorthandTypeName)) return shorthandTypeName;
+
         String name = getClass().getSimpleName();
         if (Strings.isBlank(name)) return getClass().getCanonicalName();
         name = Strings.removeFromEnd(name, "WorkflowStep");
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
index 85d238e552..b3bc5065f5 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
@@ -136,6 +136,13 @@ public class WorkflowStepInstanceExecutionContext {
         return context;
     }
 
+    public Object getOutput() {
+        return output;
+    }
+    public void setOutput(Object output) {
+        this.output = output;
+    }
+
     @JsonIgnore
     public Object getPreviousStepOutput() {
         return getWorkflowExectionContext().getPreviousStepOutput();
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java
index 439e0ffe78..464f7f217f 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java
@@ -105,6 +105,9 @@ public class WorkflowStepResolution {
             if (userSuppliedShorthand!=null) {
                 defW.userSuppliedShorthand = userSuppliedShorthand;
             }
+            if (typeBestGuess!=null) {
+                defW.shorthandTypeName = typeBestGuess;
+            }
 
             List<Object> onError = defW.getOnError();
             if (onError!=null && !onError.isEmpty()) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
index 8a4988029b..d003510cc5 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
@@ -160,8 +160,10 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
         Predicate<Integer> exitcode = context.getInput(STATUS_CODE);
         if (exitcode==null) exitcode = code -> HttpTool.isStatusCodeHealthy(code);
 
+        context.setOutput(MutableMap.of("status_code", response.code(), "headers", response.headers(), "content", contentString, "content_bytes", contentBytes, "duration", Duration.millis(endTime - startTime)));
+        // make sure the output is set even if there is an error
         checkExitCode(response.code(), exitcode);
-        return MutableMap.of("status_code", response.code(), "headers", response.headers(), "content", contentString, "content_bytes", contentBytes, "duration", Duration.millis(endTime - startTime));
+        return context.getOutput();
     }
 
     protected void checkExitCode(Integer code, Predicate<Integer> exitcode) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
index 5b084c8bd9..8db28cbd51 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
@@ -34,6 +34,7 @@ import org.apache.brooklyn.util.core.task.ssh.SshTasks;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.time.Duration;
 
 import java.util.Map;
 
@@ -75,10 +76,12 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
         Map<String, Object> env = context.getInput(ENV);
         if (env!=null) tf.environmentVariables(new ShellEnvironmentSerializer(context.getWorkflowExectionContext().getManagementContext()).serialize(env));
         return tf.returning(ptw -> {
-            checkExitCode(ptw, exitcode);
-            return MutableMap.of("stdout", ptw.getStdout(),
+            context.setOutput(MutableMap.of("stdout", ptw.getStdout(),
                     "stderr", ptw.getStderr(),
-                    "exit_code", ptw.getExitCode());
+                    "exit_code", ptw.getExitCode()));
+            // make sure the output is set even if there is an error
+            checkExitCode(ptw, exitcode);
+            return (Map<?,?>) context.getOutput();
         });
     }
 
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
index e1cf23932a..08c5f7f467 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
@@ -46,8 +46,10 @@ import org.apache.brooklyn.test.ClassLogWatcher;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.json.BrooklynObjectsJsonMapper;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.time.Duration;
+import org.apache.brooklyn.util.yaml.Yamls;
 import org.testng.annotations.Test;
 
 import java.io.IOException;
@@ -122,14 +124,20 @@ public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport {
     }
 
     @Test
-    public void testShorthandStepResolution() {
+    public void testShorthandStepResolution() throws JsonProcessingException {
         loadTypes();
         String input = "sleep 1s";
 
-        // only util will work for shorthand
+        // jackson doesn't handle shorthand; our custom method does that
         WorkflowStepDefinition s = WorkflowStepResolution.resolveStep(mgmt, input);
         Asserts.assertInstanceOf(s, SleepWorkflowStep.class);
         Asserts.assertEquals( Duration.of(s.getInput().get(SleepWorkflowStep.DURATION.getName())), Duration.ONE_SECOND);
+
+        String output1 = BrooklynObjectsJsonMapper.newDslToStringSerializingMapper(mgmt).writeValueAsString(s);
+        String output2 = BeanWithTypeUtils.newYamlMapper(mgmt, false, null, false).writerFor(Object.class).writeValueAsString(s);
+
+        Asserts.assertStringContains(output1, "\"shorthandTypeName\":\"sleep\"");
+        Asserts.assertStringContains(output2, "shorthandTypeName: \"sleep\"");
     }
 
     @Test
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
index 3987e3cfdb..049a69235d 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
@@ -577,7 +577,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl
                     m -> m.matches("Encountered error in workflow .*/.* 'myWorkflow' .handler present.: No effector matching 'does-not-exist'"),
                     m -> m.matches("Creating workflow .* error handler .*-error-handler in task .*"),
                     m -> m.matches("Starting .*-error-handler with 1 handler in task .*"),
-                    m -> m.matches("Creating handler .*-error-handler-1 'NoOp' in task .*"),
+                    m -> m.matches("Creating handler .*-error-handler-1 'no-op' in task .*"),
                     m -> m.matches("Starting .*-error-handler-1 in task .*"),
                     m -> m.matches("Completed handler .*-error-handler-1; proceeding to default next step"),
                     m -> m.matches("Handled error in workflow around step .*-1; explicit next 'end': Workflow completed")));


[brooklyn-server] 05/11: fix typo

Posted by he...@apache.org.
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 286eb0d4a02a3141763e85305b7de1f4fc15d51c
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Oct 20 17:32:56 2022 +0100

    fix typo
---
 core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
index bffe5599f3..ff23055de4 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
@@ -84,7 +84,7 @@ public class BrooklynTaskTags extends TaskTags {
     /** Tag for a task which should be treated as a top-level task, for the purpose of listing */
     public static final Object TOP_LEVEL_TASK = "TOP-LEVEL";
     /** Tag for a task which represents entity initialization */
-    public static final Object ENTITY_INITIALIZATION = "INITALIZATION";
+    public static final Object ENTITY_INITIALIZATION = "INITIALIZATION";
     /** Tag for a task which represents an effector */
     public static final String EFFECTOR_TAG = "EFFECTOR";
     /** Tag for a task which represents a sensor being published */


[brooklyn-server] 03/11: block type instantiation for steps

Posted by he...@apache.org.
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 a33153f6c81aa9c092022115b94e2f96a91b1d7c
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Oct 20 09:28:33 2022 +0100

    block type instantiation for steps
    
    they are instantiated specially later, and the field `type` should be ignored
---
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   | 88 +++++++++++++++++++++-
 .../resolve/jackson/AsPropertyIfAmbiguous.java     | 31 +++++++-
 .../jackson/JsonPassThroughDeserializer.java       | 59 +++++++++++++++
 .../core/workflow/WorkflowExecutionContext.java    |  9 +++
 .../core/workflow/steps/CustomWorkflowStep.java    | 15 +++-
 .../brooklyn/core/workflow/WorkflowBasicTest.java  | 18 ++++-
 .../software/base/WorkflowSoftwareProcessTest.java |  9 +++
 7 files changed, 220 insertions(+), 9 deletions(-)

diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
index 4559d9bcde..728283f24a 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
@@ -19,33 +19,48 @@
 package org.apache.brooklyn.camp.brooklyn;
 
 import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import org.apache.brooklyn.api.effector.Effector;
+import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.location.Location;
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.location.MachineLocation;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.policy.Policy;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.entity.Attributes;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.entity.Dumper;
 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.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
 import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
-import org.apache.brooklyn.core.workflow.WorkflowBasicTest;
-import org.apache.brooklyn.core.workflow.WorkflowEffector;
-import org.apache.brooklyn.core.workflow.WorkflowPolicy;
-import org.apache.brooklyn.core.workflow.WorkflowSensor;
+import org.apache.brooklyn.core.workflow.*;
 import org.apache.brooklyn.core.workflow.steps.LogWorkflowStep;
+import org.apache.brooklyn.core.workflow.store.WorkflowStatePersistenceViaSensors;
+import org.apache.brooklyn.entity.software.base.WorkflowSoftwareProcess;
 import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.location.winrm.WinrmWorkflowStep;
 import org.apache.brooklyn.tasks.kubectl.ContainerWorkflowStep;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.test.ClassLogWatcher;
+import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.apache.brooklyn.util.core.json.BrooklynObjectsJsonMapper;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.testng.Assert;
@@ -53,9 +68,13 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import java.util.Arrays;
+import java.util.List;
 import java.util.Optional;
 import java.util.function.Predicate;
 
+import static org.apache.brooklyn.util.core.internal.ssh.ExecCmdAsserts.assertExecContains;
+import static org.apache.brooklyn.util.core.internal.ssh.ExecCmdAsserts.assertExecsContain;
+
 public class WorkflowYamlTest extends AbstractYamlTest {
 
     static final String VERSION = "0.1.0-SNAPSHOT";
@@ -212,6 +231,11 @@ public class WorkflowYamlTest extends AbstractYamlTest {
         } else {
             EntityAsserts.assertAttributeEqualsContinually(entity, s, null);
         }
+
+        WorkflowExecutionContext lastWorkflowContext = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(entity).values().iterator().next();
+        List<Object> defs = lastWorkflowContext.getStepsDefinition();
+        // step definitions should not be resolved by jackson
+        defs.forEach(def -> Asserts.assertThat(def, d -> !(d instanceof WorkflowStepDefinition)));
     }
 
     public void doTestWorkflowPolicy(String triggers, Predicate<Duration> timeCheckOrNullIfShouldFail) throws Exception {
@@ -460,4 +484,60 @@ public class WorkflowYamlTest extends AbstractYamlTest {
             Asserts.expectedFailureContainsIgnoreCase(e, "resolve step", "unsupported-type");
         }
     }
+
+    @Test
+    public void testWorkflowSoftwareProcessAsYaml() throws Exception {
+        RecordingSshTool.clear();
+
+        FixedListMachineProvisioningLocation loc = 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()))));
+
+        Application app = createApplicationUnstarted(
+                "services:",
+                "- type: " + WorkflowSoftwareProcess.class.getName(),
+                "  brooklyn.config:",
+                "    "+BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION.getName()+": true",
+                "    install.workflow:",
+                "      steps:",
+                "        - ssh installWorkflow",
+                "        - set-sensor boolean installed = true",
+                "        - type: no-op",
+                "    stop.workflow:",
+                "      steps:",
+                "        - ssh stopWorkflow",
+                "        - set-sensor boolean stopped = true"
+        );
+
+        Entity child = app.getChildren().iterator().next();
+        List<Object> steps = child.config().get(WorkflowSoftwareProcess.INSTALL_WORKFLOW).peekSteps();
+        // should not be resolved yet
+        steps.forEach(def -> Asserts.assertThat(def, d -> !(d instanceof WorkflowStepDefinition)));
+
+        ((Startable)app).start(MutableList.of(loc));
+
+        assertExecsContain(RecordingSshTool.getExecCmds(), ImmutableList.of(
+                "installWorkflow"));
+
+        EntityAsserts.assertAttributeEquals(child, Sensors.newSensor(Boolean.class, "installed"), true);
+        EntityAsserts.assertAttributeEquals(child, Sensors.newSensor(Boolean.class, "stopped"), null);
+
+        EntityAsserts.assertAttributeEqualsEventually(child, Attributes.SERVICE_UP, true);
+        EntityAsserts.assertAttributeEqualsEventually(child, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING);
+
+        WorkflowExecutionContext lastWorkflowContext = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(child).values().iterator().next();
+        List<Object> defs = lastWorkflowContext.getStepsDefinition();
+        // step definitions should not be resolved by jackson
+        defs.forEach(def -> Asserts.assertThat(def, d -> !(d instanceof WorkflowStepDefinition)));
+
+        ((Startable)app).stop();
+
+        EntityAsserts.assertAttributeEquals(child, Sensors.newSensor(Boolean.class, "stopped"), true);
+        assertExecContains(RecordingSshTool.getLastExecCmd(), "stopWorkflow");
+
+        EntityAsserts.assertAttributeEqualsEventually(child, Attributes.SERVICE_UP, false);
+        EntityAsserts.assertAttributeEqualsEventually(child, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STOPPED);
+    }
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java
index fc9b041a80..819721baba 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java
@@ -44,6 +44,7 @@ import java.lang.reflect.AccessibleObject;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.atomic.AtomicInteger;
 
 public class AsPropertyIfAmbiguous {
 
@@ -116,7 +117,31 @@ public class AsPropertyIfAmbiguous {
         }
     }
 
-    /** Type deserializer which undersrtands a '@type' property if 'type' conflicts with a field on the class and which uses the base type if no type is specified */
+    static ThreadLocal<AtomicInteger> suppressingTypeFieldDeserialization = new ThreadLocal<>();
+    static boolean isSuppressingTypeFieldDeserialization() {
+        AtomicInteger count = suppressingTypeFieldDeserialization.get();
+        if (count==null) return false;
+        return count.get() > 0;
+    }
+    static void startSuppressingTypeFieldDeserialization() {
+        AtomicInteger count = suppressingTypeFieldDeserialization.get();
+        if (count==null) {
+            count = new AtomicInteger();
+            suppressingTypeFieldDeserialization.set(count);
+        }
+        count.incrementAndGet();
+    }
+    static void stopSuppressingTypeFieldDeserialization() {
+        AtomicInteger count = suppressingTypeFieldDeserialization.get();
+        if (count==null) {
+            throw new IllegalStateException("Count mismatch starting/stopping type field deserialization");
+        }
+        if (count.decrementAndGet()==0) {
+            suppressingTypeFieldDeserialization.remove();
+        }
+    }
+
+    /** Type deserializer which understands a '@type' property if 'type' conflicts with a field on the class and which uses the base type if no type is specified */
     public static class AsPropertyButNotIfFieldConflictTypeDeserializer extends AsPropertyTypeDeserializer {
         public AsPropertyButNotIfFieldConflictTypeDeserializer(JavaType bt, TypeIdResolver idRes, String typePropertyName, boolean typeIdVisible, JavaType defaultImpl, As inclusion) {
             super(bt, idRes, typePropertyName, typeIdVisible, defaultImpl, inclusion);
@@ -171,6 +196,10 @@ public class AsPropertyIfAmbiguous {
 
         // copied from super class
         private Object deserializeTypedFromObjectSuper(JsonParser p, DeserializationContext ctxt, boolean mustUseConflictingTypePrefix) throws IOException {
+            if (isSuppressingTypeFieldDeserialization()) {
+                return _deserializeTypedUsingDefaultImpl(p, ctxt, null, "typed deserialization is suppressed");
+            }
+
 //            return super.deserializeTypedFromObject(p, ctxt);
 
             // 02-Aug-2013, tatu: May need to use native type ids
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonPassThroughDeserializer.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonPassThroughDeserializer.java
new file mode 100644
index 0000000000..58ae276cd1
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonPassThroughDeserializer.java
@@ -0,0 +1,59 @@
+/*
+ * 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.core.resolve.jackson;
+
+import com.fasterxml.jackson.core.JacksonException;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
+import com.google.common.annotations.Beta;
+
+import java.io.IOException;
+
+/** deserializer intended for use via contentUsing (not content), to prevent type expansion */
+@Beta
+public class JsonPassThroughDeserializer extends JsonDeserializer {
+
+    @Override
+    public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JacksonException {
+        try {
+            AsPropertyIfAmbiguous.startSuppressingTypeFieldDeserialization();
+            return ctxt.readValue(p, Object.class);
+        } finally {
+            AsPropertyIfAmbiguous.stopSuppressingTypeFieldDeserialization();
+        }
+    }
+
+    @Override
+    public Object deserialize(JsonParser p, DeserializationContext ctxt, Object intoValue) throws IOException {
+        throw new IllegalStateException("Unsupported to deserialize into an object");
+    }
+
+    @Override
+    public Object deserializeWithType(JsonParser p, DeserializationContext ctxt, TypeDeserializer typeDeserializer) throws IOException {
+        return deserialize(p, ctxt);
+    }
+
+    @Override
+    public Object deserializeWithType(JsonParser p, DeserializationContext ctxt, TypeDeserializer typeDeserializer, Object intoValue) throws IOException {
+        return deserialize(p, ctxt, intoValue);
+    }
+
+}
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index 6679e3c494..388ea5101c 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -20,6 +20,7 @@ package org.apache.brooklyn.core.workflow;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.google.common.reflect.TypeToken;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
@@ -32,6 +33,7 @@ import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityAdjuncts;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.core.resolve.jackson.JsonPassThroughDeserializer;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.core.workflow.store.WorkflowStatePersistenceViaSensors;
 import org.apache.brooklyn.util.collections.MutableList;
@@ -94,7 +96,10 @@ public class WorkflowExecutionContext {
     transient WorkflowExecutionContext parent;
     String parentId;
 
+    // should be treated as raw json
+    @JsonDeserialize(contentUsing = JsonPassThroughDeserializer.class)
     List<Object> stepsDefinition;
+
     DslPredicates.DslPredicate condition;
 
     @JsonInclude(JsonInclude.Include.NON_EMPTY)
@@ -550,6 +555,10 @@ public class WorkflowExecutionContext {
         return errorHandlerContext;
     }
 
+    public List<Object> getStepsDefinition() {
+        return MutableList.copyOf(stepsDefinition).asUnmodifiable();
+    }
+
     transient Map<String,Pair<Integer,WorkflowStepDefinition>> stepsWithExplicitId;
     transient List<WorkflowStepDefinition> stepsResolved;
     @JsonIgnore
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
index ef1cbe7766..4f36809a5e 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
@@ -20,14 +20,17 @@ package org.apache.brooklyn.core.workflow.steps;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.google.common.collect.Iterables;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.reflect.TypeToken;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
-import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
+import org.apache.brooklyn.core.resolve.jackson.JsonPassThroughDeserializer;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.core.workflow.*;
 import org.apache.brooklyn.core.workflow.store.WorkflowStatePersistenceViaSensors;
@@ -58,6 +61,9 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
     }
 
     Map<String,Object> parameters;
+
+    // should be treated as raw json
+    @JsonDeserialize(contentUsing = JsonPassThroughDeserializer.class)
     List<Object> steps;
 
     Object workflowOutput;
@@ -171,4 +177,9 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
                 null,
                 ConfigBag.newInstance(getInput()).putAll(extraConfig), null);
     }
+
+    @VisibleForTesting
+    public List<Object> peekSteps() {
+        return steps;
+    }
 }
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
index 08c5f7f467..073b477c2f 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
@@ -40,6 +40,7 @@ import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
 import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.core.workflow.steps.*;
+import org.apache.brooklyn.core.workflow.store.WorkflowStatePersistenceViaSensors;
 import org.apache.brooklyn.entity.stock.BasicApplication;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.test.ClassLogWatcher;
@@ -110,7 +111,7 @@ public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport {
     }
 
     @Test
-    public void testStepResolution() {
+    public void testStepResolution() throws JsonProcessingException {
         loadTypes();
         Map<String,Object> input = MutableMap.of("type", "no-op");
 
@@ -121,6 +122,12 @@ public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport {
         // util
         s = WorkflowStepResolution.resolveStep(mgmt, input);
         Asserts.assertInstanceOf(s, NoOpWorkflowStep.class);
+
+        String output1 = BrooklynObjectsJsonMapper.newDslToStringSerializingMapper(mgmt).writeValueAsString(s);
+        String output2 = BeanWithTypeUtils.newYamlMapper(mgmt, false, null, false).writerFor(Object.class).writeValueAsString(s);
+
+        Asserts.assertStringContains(output1, "\"shorthandTypeName\":\"no-op\"");
+        Asserts.assertStringContains(output2, "shorthandTypeName: \"no-op\"");
     }
 
     @Test
@@ -157,7 +164,7 @@ public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport {
     }
 
     @Test
-    public void testCommonStepsInEffector() {
+    public void testCommonStepsInEffector() throws JsonProcessingException {
         loadTypes();
         BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
 
@@ -217,6 +224,13 @@ public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport {
         Asserts.assertNull(badSensor);
         Asserts.assertEquals(app.sensors().get(Sensors.newSensor(Object.class, "bad")), null);
         Asserts.assertThat(app.sensors().getAll().keySet().stream().map(Sensor::getName).collect(Collectors.toSet()), s -> !s.contains("bad"));
+
+        WorkflowExecutionContext lastWorkflowContext = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(app).values().iterator().next();
+        String output1 = BrooklynObjectsJsonMapper.newDslToStringSerializingMapper(mgmt).writeValueAsString(lastWorkflowContext);
+        String output2 = BeanWithTypeUtils.newYamlMapper(mgmt, false, null, false).writerFor(Object.class).writeValueAsString(lastWorkflowContext);
+
+        Asserts.assertStringContains(output1, "\"type\":\"no-op\"");
+        Asserts.assertStringContains(output2, "type: \"no-op\"");
     }
 
     public static class WorkflowTestStep extends WorkflowStepDefinition {
diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessTest.java
index 1c49bd516f..9e3e939756 100644
--- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessTest.java
+++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/WorkflowSoftwareProcessTest.java
@@ -35,7 +35,10 @@ import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.sensor.function.FunctionSensor;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.workflow.WorkflowBasicTest;
+import org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
+import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.steps.CustomWorkflowStep;
+import org.apache.brooklyn.core.workflow.store.WorkflowStatePersistenceViaSensors;
 import org.apache.brooklyn.enricher.stock.UpdatingMap;
 import org.apache.brooklyn.entity.software.base.SoftwareProcess.ChildStartableMode;
 import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
@@ -56,6 +59,7 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -122,6 +126,11 @@ public class WorkflowSoftwareProcessTest extends BrooklynAppUnitTestSupport {
         EntityAsserts.assertAttributeEqualsEventually(child, Attributes.SERVICE_UP, true);
         EntityAsserts.assertAttributeEqualsEventually(child, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING);
 
+        WorkflowExecutionContext lastWorkflowContext = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(child).values().iterator().next();
+        List<Object> defs = lastWorkflowContext.getStepsDefinition();
+        // step definitions should not be resolved by jackson
+        defs.forEach(def -> Asserts.assertThat(def, d -> !(d instanceof WorkflowStepDefinition)));
+
         app.stop();
 
         EntityAsserts.assertAttributeEquals(child, Sensors.newSensor(Boolean.class, "stopped"), true);


[brooklyn-server] 04/11: REST API to run workflow, and tidy workflow submission metadata

Posted by he...@apache.org.
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 ccf3cc8e65b19611702dbfc780b1b0a441ae7686
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Oct 20 15:51:37 2022 +0100

    REST API to run workflow, and tidy workflow submission metadata
---
 .../core/workflow/WorkflowReplayUtils.java         | 10 ++++++-
 .../org/apache/brooklyn/rest/api/EntityApi.java    | 31 +++++++++++++++++++
 .../brooklyn/rest/resources/EntityResource.java    | 35 ++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
index dade3d3bdf..579bc1d96a 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowReplayUtils.java
@@ -18,6 +18,7 @@
  */
 package org.apache.brooklyn.core.workflow;
 
+import com.fasterxml.jackson.annotation.JsonInclude;
 import com.google.common.collect.Iterables;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
@@ -48,9 +49,11 @@ public class WorkflowReplayUtils {
         //   - replays others
     }
 
+    @JsonInclude(JsonInclude.Include.NON_NULL)
     public static class WorkflowReplayRecord {
         String taskId;
         String reasonForReplay;
+        String submittedByTaskId;
         long submitTimeUtc;
         long startTimeUtc;
         long endTimeUtc;
@@ -72,6 +75,7 @@ public class WorkflowReplayUtils {
                 log.warn("Mismatch in workflow replays for "+ctx+": "+ctx.replayCurrent +" vs "+task);
                 return;
             }
+            ctx.replayCurrent.submittedByTaskId = task.getSubmittedByTaskId();
             ctx.replayCurrent.submitTimeUtc = task.getSubmitTimeUtc();
             ctx.replayCurrent.startTimeUtc = task.getStartTimeUtc();
             ctx.replayCurrent.endTimeUtc = task.getEndTimeUtc();
@@ -85,7 +89,11 @@ public class WorkflowReplayUtils {
                     ctx.replayCurrent.result = Exceptions.collapseTextInContext(t, task);
                 }
             } else {
-                if (ctx.replayCurrent.endTimeUtc <= 0) ctx.replayCurrent.endTimeUtc = System.currentTimeMillis();
+                // when forcing end, we are invoked _by_ the task so we fake the completion information
+                if (ctx.replayCurrent.endTimeUtc <= 0) {
+                    ctx.replayCurrent.endTimeUtc = System.currentTimeMillis();
+                    ctx.replayCurrent.status = forceEndSuccessOrError ? "Completed" : "Failed";
+                }
                 ctx.replayCurrent.isError = !forceEndSuccessOrError;
                 ctx.replayCurrent.result = result;
             }
diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java
index fe05711937..9ad2abfb0a 100644
--- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java
+++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java
@@ -453,6 +453,37 @@ public interface EntityApi {
             @ApiParam(value = "Workflow ID", required = true)
             @PathParam("workflowId") String workflowId);
 
+    @POST
+    @ApiOperation(value = "Run a workflow on this entity from a YAML workflow spec",
+            response = org.apache.brooklyn.rest.domain.TaskSummary.class)
+    @Consumes({"application/x-yaml",
+            // per addChildren
+            "text/yaml", "text/x-yaml", "application/yaml", MediaType.APPLICATION_JSON})
+    @Path("/{entity}/workflows")
+    @ApiResponses(value = {
+            @ApiResponse(code = 201, message = "Accepted"),
+            @ApiResponse(code = 400, message = "Bad Request"),
+            @ApiResponse(code = 401, message = "Unauthorized"),
+            @ApiResponse(code = 404, message = "Application or entity missing"),
+            @ApiResponse(code = 500, message = "Internal Server Error")
+    })
+    public Response runWorkflow(
+            @PathParam("application") final String application,
+            @PathParam("entity") final String entity,
+
+            @ApiParam(name = "timeout", value = "Delay before server should respond with incomplete activity task, rather than completed task: " +
+                    "'never' means block until complete; " +
+                    "'0' means return task immediately; " +
+                    "and e.g. '20ms' (the default) will wait 20ms for completed task information to be available",
+                    required = false, defaultValue = "20ms")
+            @QueryParam("timeout") final String timeout,
+
+            @ApiParam(
+                    name = "workflowSpec",
+                    value = "Workflow spec in YAML (including 'steps' root element with a list of steps)",
+                    required = true)
+                    String yaml);
+
     @POST
     @Path("/{entity}/workflows/{workflowId}/replay/from/{step}")
     @ApiOperation(value = "Replays a workflow from the given step, or 'start' to restart or 'end' to resume from last replayable point; the workflow will rollback to the previous replay point unless forced; returns the task ID of the replay")
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
index fb6f5e73de..84495ea37b 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
@@ -22,6 +22,9 @@ import static javax.ws.rs.core.Response.created;
 import static javax.ws.rs.core.Response.status;
 import static javax.ws.rs.core.Response.Status.ACCEPTED;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
+import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.mgmt.BrooklynTags.SpecSummary;
 import static org.apache.brooklyn.rest.util.WebResourceUtils.serviceAbsoluteUriBuilder;
 
@@ -46,8 +49,10 @@ import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
 import org.apache.brooklyn.core.mgmt.EntityManagementUtils.CreationResult;
 import org.apache.brooklyn.core.mgmt.entitlement.EntitlementPredicates;
 import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
+import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
+import org.apache.brooklyn.core.workflow.steps.CustomWorkflowStep;
 import org.apache.brooklyn.core.workflow.store.WorkflowStatePersistenceViaSensors;
 import org.apache.brooklyn.rest.api.EntityApi;
 import org.apache.brooklyn.rest.domain.*;
@@ -59,10 +64,13 @@ import org.apache.brooklyn.rest.transform.TaskTransformer;
 import org.apache.brooklyn.rest.util.EntityRelationUtils;
 import org.apache.brooklyn.rest.util.WebResourceUtils;
 import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.core.ClassLoaderUtils;
 import org.apache.brooklyn.util.core.ResourceUtils;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.javalang.ClassLoadingContext;
+import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -400,4 +408,31 @@ public class EntityResource extends AbstractBrooklynRestResource implements Enti
         return (String) WebResourceUtils.getValueForDisplay(mapper(), t.getId(), true, true);
     }
 
+    @Override
+    public Response runWorkflow(String applicationToken, String entityToken, String timeoutS, String yaml) {
+        final Entity target = brooklyn().getEntity(applicationToken, entityToken);
+        // TODO new entitlement, here and above
+        if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.MODIFY_ENTITY, target)) {
+            throw WebResourceUtils.forbidden("User '%s' is not authorized to modify entity '%s'",
+                    Entitlements.getEntitlementContext().user(), entityToken);
+        }
+        CustomWorkflowStep workflow;
+        try {
+            workflow = BeanWithTypeUtils.newYamlMapper(mgmt(), true, RegisteredTypes.getClassLoadingContext(target), true)
+                    .readerFor(CustomWorkflowStep.class).readValue(yaml);
+        } catch (JsonProcessingException e) {
+            return ApiError.of(e).asBadRequestResponseJson();
+        }
+
+        WorkflowExecutionContext execution = workflow.newWorkflowExecution(target,
+                Strings.firstNonBlank(workflow.getName(), workflow.getId(), "API workflow invocation"), null);
+
+        Task<Object> task = Entities.submit(target, execution.getTask(true).get());
+        task.blockUntilEnded(timeoutS==null ? Duration.millis(20) : Duration.of(timeoutS));
+
+        URI ref = serviceAbsoluteUriBuilder(uriInfo.getBaseUriBuilder(), EntityApi.class, "getWorkflow")
+                .build(target.getApplicationId(), target.getId(), execution.getWorkflowId());
+        ResponseBuilder response = created(ref);
+        return response.entity(TaskTransformer.taskSummary(task, ui.getBaseUriBuilder())).build();
+    }
 }


[brooklyn-server] 09/11: merge master - fix location tags

Posted by he...@apache.org.
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 6d47a190efdd1bc89e8dd971285f2b4640fcf6c9
Merge: bed78cd487 89c2662604
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Oct 20 17:54:02 2022 +0100

    merge master - fix location tags

 .../brooklyn/camp/brooklyn/LocationsYamlTest.java  | 15 ++++++++++----
 .../core/location/BasicLocationRegistry.java       | 23 ++++++++++++++++++----
 .../core/mgmt/rebind/RebindLocationTest.java       |  3 +--
 3 files changed, 31 insertions(+), 10 deletions(-)