StackStorm / orquestaconvert

Converts Mistral workflows into Orchestra workflows
Apache License 2.0
13 stars 7 forks source link

Betterize publish conversion #18

Closed blag closed 5 years ago

blag commented 5 years ago

Overall, this PR corrects the output of this conversion script to avoid producing Orquesta workflows that do not have the same semantics as Mistral workflows.

5fc72f8 Rename variable to not override vars builtin

Simply rename variable from vars to expression_vars, since vars is a Python built-in

a8a4f29 Publish variables in transitions with expressions

Before this commit, the script would convert the following:

  get_webui_server:
    action: core.noop
    publish:
      webui_base_url: https://st2cicd.uswest2.stackstorm.net
    on-success:
      - notify_start
      - set_github_status_pending: <% $.initial_commit != null %>
      - create_vm: <% $.initial_commit = null %>

to this:

  get_webui_server:
    action: core.noop
    next:
      - when: <% succeeded() %>
        publish:
          - webui_base_url: https://st2cicd.uswest2.stackstorm.net
        do:
          - notify_start
      - when: <% succeeded() and (ctx().initial_commit != null) %>
        do:
          - set_github_status_pending
      - when: <% succeeded() and (ctx().initial_commit = null) %>
        do:
          - create_vm

But due to the way Orquesta contexts work, the webui_base_url variable would not be available in the subsequent set_github_status_pending and create_vm tasks, even though it would be accessible in the Mistral workflow.

This commit adds the publish attribute to all "expression" transitions:

  get_webui_server:
    action: core.noop
    next:
      - when: <% succeeded() %>
        publish:
          - webui_base_url: https://st2cicd.uswest2.stackstorm.net
        do:
          - notify_start
      - when: <% succeeded() and (ctx().initial_commit != null) %>
        publish:
          - webui_base_url: https://st2cicd.uswest2.stackstorm.net
        do:
          - set_github_status_pending
      - when: <% succeeded() and (ctx().initial_commit = null) %>
        publish:
          - webui_base_url: https://st2cicd.uswest2.stackstorm.net
        do:
          - create_vm

which does publish the webui_base_url variable into the contexts consumed by the subsequent set_github_status_pending and create_vm tasks.

d4828ef Use only-publish transitions when they publish to the workflow context

Before this commit, the convert script would convert the following:

output:
  workflow_result: <% ctx().workflow_result %>
tasks:
  create_vm:
    action: st2cd.create_vm_role
    input:
      hostname: <% $.hostname %>
    publish:
      vm_info: <% task(create_vm).result.vm_info %>
      vm_id: <% task(create_vm).result.vm_info.id %>
    on-success:
      - patch_rhel6: <% $.distro = 'RHEL6' %>
      - get_bootstrap_script: <% $.distro != 'RHEL6' %>
  # ...
  nothing:
    action: core.noop
    publish:
      workflow_result: AMBIGUOUS SUCCESS!

to three different transitions from create_vm, only one of which simply published the variables to the workflow "global" context:

output:
  - workflow_result: <% ctx().workflow_result %>
tasks:
  create_vm:
    action: st2cd.create_vm_role
    input:
      hostname: <% ctx().hostname %>
    next:
      # This transition simply publishes the variables to the workflow "global" 
      # context, which is probably unnecessary
      - when: <% succeeded() %>
        publish:
          - vm_info: <% result().vm_info %>
          - vm_id: <% result().vm_info.id %>
      - when: <% succeeded() and (ctx().distro = 'RHEL6') %>
        publish:
          - vm_info: <% result().vm_info %>
          - vm_id: <% result().vm_info.id %>
        do:
          - patch_rhel6
      - when: <% succeeded() and (ctx().distro != 'RHEL6') %>
        publish:
          - vm_info: <% result().vm_info %>
          - vm_id: <% result().vm_info.id %>
        do:
          - get_bootstrap_script
  # ...
  nothing:
    action: core.noop
    next:
      # A publish-only transition
      - when: <% succeeded() %>
        publish:
          - workflow_result: AMBIGUOUS SUCCESS!

This commit only adds a "publish-only" transition if the published variables are used in the workflow context (read: are used in the workflow output attribute):

output:
  - workflow_result: <% ctx().workflow_result %>
tasks:
  create_vm:
    action: st2cd.create_vm_role
    input:
      hostname: <% ctx().hostname %>
    next:
      # No publish-only transition
      - when: <% succeeded() and (ctx().distro = 'RHEL6') %>
        publish:
          - vm_info: <% result().vm_info %>
          - vm_id: <% result().vm_info.id %>
        do:
          - patch_rhel6
      - when: <% succeeded() and (ctx().distro != 'RHEL6') %>
        publish:
          - vm_info: <% result().vm_info %>
          - vm_id: <% result().vm_info.id %>
        do:
          - get_bootstrap_script
  # ...
  nothing:
    action: core.noop
    next:
      # Since workflow_result is used in the workflow output, this publish-only transition is included
      - when: <% succeeded() %>
        publish:
          - workflow_result: SUCCESS!

Due to the way Orquesta task contexts work, publish-only transitions are only useful for publishing variables for consumption in output. The variables that are published in publish-only transitions are not available to subsequent tasks in other transitions.

Notes

codecov-io commented 5 years ago

Codecov Report

Merging #18 into master will increase coverage by 0.05%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
+ Coverage   98.62%   98.67%   +0.05%     
==========================================
  Files          12       12              
  Lines         581      604      +23     
==========================================
+ Hits          573      596      +23     
  Misses          8        8
Impacted Files Coverage Δ
orquestaconvert/workflows/base.py 100% <100%> (ø) :arrow_up:
orquestaconvert/expressions/__init__.py 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update dafe7a7...e42119d. Read the comment docs.

blag commented 5 years ago

@nmaludy Rebased on top of master and added the integration test fixtures as you noted.