Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AWS Step Functions support #3024

Closed
lielran opened this issue Dec 25, 2016 · 35 comments
Closed

AWS Step Functions support #3024

lielran opened this issue Dec 25, 2016 · 35 comments

Comments

@lielran
Copy link

lielran commented Dec 25, 2016

aws step functions support

Feature proposals:

  • the new aws step function is based on aws lambda and seem like perfect for lambda orchestration(state machine style).
  • serverless integration can be very handy.
@rowanu
Copy link
Member

rowanu commented Dec 25, 2016

How would this work? Would people have to write their own FSM using AWSSL?

@lielran
Copy link
Author

lielran commented Dec 26, 2016

IMO,since the Json format is very simple(unlike cloudformation),It will be a good idea to write FSM using AWSSL and let serverless handle that for you.
One of the issues is that you need to provide ARN for your lambdas function. In that case we will need a place holder or a pointer to your function.

@lielran
Copy link
Author

lielran commented Dec 26, 2016

it's also possible to define a Task("Activity") without ARN that will ping an HTTP endpoint.

@pmuens
Copy link
Contributor

pmuens commented Jan 4, 2017

@horike37 just released the very cool "Step Functions" plugin so you can use them now in Serverless 👍 : https://github.com/horike37/serverless-step-functions

@rowanu
Copy link
Member

rowanu commented Jan 4, 2017

Can this issue be closed then?

@pmuens
Copy link
Contributor

pmuens commented Jan 5, 2017

Not sure if we should put this into core some time (based on @horike37 plugin)...

Any thoughts @eahefnawy @nikgraf @mthenw ?

@eahefnawy
Copy link
Member

We may add it, but I'm guessing the design won't exactly match @horike37 plugin...mainly for cross-provider compatibility reasons. I feel that the config used with that plugin is too AWS specific.

@horike37
Copy link
Member

horike37 commented Jan 5, 2017

If you consider multi-providers, implementing provider-specific feature on the core may make maintenance difficult.
And you might wait for support of CloudFormation. Step Functions SDK can not update a statemachine. My plugin updates a statemachine once by deleting and creating a new one. But Invocation logs are deleted.

If CF supports, it may become to update correctly.

@lielran
Copy link
Author

lielran commented Jan 15, 2017

I think it should be in the core since it's the really one way to support flows and keep the function small(and not mini-application).I like the idea that @horike37 plugin support the aws step file.
also, I'm not sure about multi-providers since it may not be any time soon.

@williamgueiros
Copy link
Contributor

@lielran i think @horike37 is correct, i can't found information, how add information on aws step functions, without interface .

Do you saw that information?

@rowanu
Copy link
Member

rowanu commented Feb 12, 2017

https://aws.amazon.com/about-aws/whats-new/2017/02/aws-cloudformation-adds-support-for-aws-step-functions

@horike37
Copy link
Member

horike37 commented Feb 13, 2017

How would this work finally?
If including in the core, I would love to do this work.
I think the first step is that we should decide command design and structure of serverless.yml for Step functions.

@horike37
Copy link
Member

horike37 commented Apr 1, 2017

Hey everyone here.
I will begin to implement this feature. So I'm considering serverless.yml setting for this as follow and will include this in sls deploy without adding new commands.
I would like to get your opinion.

provider:
  name: aws
  runtime: nodejs4.3

custom: 
  accountId: xxxxxx

functions:
  hellofunc:
    handler: handler.hello

stepFunctions:
  role: arn:aws:iam:::role/InsertYourFavoriteRoleNameHere #Optional, you can chose the existing IAM Role
  stateMachines:
    myStateMachine1:
      events: #Here is events for stepfunctions. You can define http, schedule and cloudwatchEvent.
        - http:
          method: GET
          path: executeMyStateMachine
        - schedule: rate(2 hours)
        - cloudwatchEvent:
          event:
            source:
              - "aws.ec2"
            detail-type:
              - "EC2 Instance State-change Notification"
            detail:
              state:
                - pending
      definition: #Required. Here is the Amazon States Language definition.
        Comment: "A Hello World example of the Amazon States Language using an AWS Lambda Function"
        StartAt: HelloWorld1
        States: 
          HelloWorld1:
            Type: Task
            Resource: arn:aws:lambda:${opt:region}:${self:custom.accountId}:function:${self:service}-${opt:stage}-hello
            End: true
    hellostepfunc2:
      definition:
        StartAt: HelloWorld2
        States: 
          HelloWorld2: 
            Type: Task
            Resource: arn:aws:states:${opt:region}:${self:custom.accountId}:activity:${self:service}-${opt:stage}-myTask
            End: true
  activities: #Here is Task activites
    - myTask

@horike37
Copy link
Member

horike37 commented Apr 2, 2017

Additionally, we should extend sls invoke command so that a stepfunction can be invoked.
I consider that add sls invoke --stepfunction <statemachin name> option to this.

@pmuens
Copy link
Contributor

pmuens commented Apr 3, 2017

Thanks for jumping on this @horike37 👍

Not 100% sure about the syntax yet. But TBH I haven't thought that much about it yet to provide a better proposal.

Is it possible to build this more around the Lamdba functions rather than as a separate kind of resource / section in the serverless.yml file?

Curious what others who've used step functions think how this should look like... /cc @serverless/vip

@lielran
Copy link
Author

lielran commented Apr 3, 2017

@horike37 don't you think we need some kind of link between hellofunc and the Resource section?

@HyperBrain
Copy link
Member

HyperBrain commented Apr 3, 2017

@pmuens @horike37 This function reference seems a bit too hard-coded for me:

          HelloWorld1:
            Type: Task
            Resource: arn:aws:lambda:${opt:region}:${self:custom.accountId}:function:${self:service}-${opt:stage}-hello
            End: true

The same for the activity reference. Woudn't the reference be easier by referencing the function via "Ref" or "GetAtt"? Function naming is a integral part of the framework itself (provider.naming module) and having the need to assemble the arns manually would increase the risk of errors.

That's what you meant with the previous comment, @lielran ?

@horike37
Copy link
Member

horike37 commented Apr 3, 2017

@pmuens

Is it possible to build this more around the Lamdba functions rather than as a separate kind of >resource / section in the serverless.yml file?

Does it mean to define stepfunction setting in functions section?
Like this:

functions:
  hellofunc:
    handler: handler.hello
    stepfunction: .....

@lielran @HyperBrain
Yes, I agree. It would be nice to link it. However, there is one problem. To use Ref or GetAtt you need to get logicalName of Resource, but there is no way how you get this in functions section in serverless.yml.

@HyperBrain
Copy link
Member

@horike37 For lambda functions you can get the logical id with provider.naming.getLambdaLogicalId(functionName). So, if you would allow:

HelloWorld1:
  Type: Task
  Function: hellofunc
  End: true

You can easily deduct the resource name with the given function from naming. Internally you can then set Resource: { Fn::GetAtt: [ 'deref'd function resource', 'Arn' ] } in the compiled CF template.

In general, I would allow the definition to have multiple ways of linking a function into the step resources. Also manually specifying an Arn parameter that you also copy to Resource.

@erikerikson
Copy link
Member

@pmuens IMO, breaking the step functions apart (so as to define pieces under function definitions) would be a nightmare. To reason about a step function you need it collected in a single reviewable chunk. Additionally, it might prohibit or at least make awkward defining Passed, Fail, Succeed, Type: Activity, and Parallel (others?) state types (where would you attach them).
@horike37 I agree with @pmuens that the formatting needs iteration but need a real keyboard to give feedback properly. I also agree with @HyperBrain about allowing the use of the raw function name. I'd suggest also allowing full ARN, or objects such as Fn::GetAtt and Fn:: ImportValue. We should probably have a reusable "functionReference" module to make this handling easier and more consistent. Not that you're obligated in this context, it just occurrs to me that this has shown up multiple times in multiple places so the framework would win if someone did this.

@erikerikson
Copy link
Member

Additionally, I strongly support the integration of StepFunction support into the existing deploy command. Same with regard to the invoke command but with -sf as an additional short version of the flag.

@HyperBrain
Copy link
Member

@erikerikson Indeed. #3212 is the proposal for that. It should provide a universal Arn paser including the detection of refs, getatts and - additionally - the function name case should be added.

@erikerikson
Copy link
Member

Thanks @HyperBrain

@erikerikson
Copy link
Member

@horike37 some feedback and then a modified example desired target follow.

  1. the role is an attribute belonging to a specific step function and should probably not be declared at the top level, implying it should be used for all step functions. As such, moved it under machine.
  2. I think from a user perspective, the user population will expect event declarations to use the same declaration syntax and offer the same features as the corresponding Lambda event declarations. As such, the same code should probably be used for processing both. Some abstraction may be necessary relative to the current state.
  3. the role for a SF should allow multiple value types.
  4. the value of Resource under a Task should allow multiple value types.
stepFunctions:
  activities:
    - myActivity0
    - myActivity1
  stateMachines:
    myStateMachine0:
      role: [ARN | Fn::GetAtt | Fn::ImportValue | etc.]
      events:
        - http: # I would expect this to mirror the corresponding http declaration for lambdas, including support for the compacted form (`- http: GET executeMyStateMachine`), etc. and even use the same codebase for processing.  See https://serverless.com/framework/docs/providers/aws/events/apigateway/ for examples
        - schedule: # should also mirror declaration capability of Lambda
        - cloudwatch: # should also mirror declaration capability of Lambda
      definition:
        Comment: "A comment..."
        StartAt: HelloWorld
        States: 
          HelloWorld:
            Type: Task
            Resource: [lambda name | activity name | lambda ARN | activity ARN | Fn::GetAtt (lambda) | Fn::Ref (activity) | Fn::ImportValue]
            End: true
    myStateMachine1: [...]

Thank you!

@horike37
Copy link
Member

horike37 commented Apr 4, 2017

Thanks for the proposal @erikerikson and @HyperBrain !
I agree 100% with the previous comment. I will begin to implement with these specifications.

@pmuens, do you have any opinion?

@pmuens
Copy link
Contributor

pmuens commented Apr 4, 2017

Thank you very much for all the really helpful feedback!

Really appreciate this as it's always super nice to have different opinions and approaches which are derived from different use-cases. So thanks everyone 🙌 🥇

We also discussed this exact issue internally yesterday.

The conclusion was that we're about to postpone the integration of AWS specific step functions into the Framework core for now. The main reason is that we're looking into ways how we can do this in a way where we support multiple providers.

We'll open up a separate issue for this in the future and take all the feedback from here and incorporate it into a proposal we can then discuss in the new issue.

Furthermore we came up with a "general rule" that everything provider dependent should be prefixed in the future (like aws-foo) to indicate that it's a property which cannot be used by other providers (The Azure Functions provider plugin already uses this pattern).

@horike37 Thank you very much for getting this started again and pushing hard to get step functions support into core.

Could you continue the work on AWS step function support based on the feedback / proposals above in the step-functions plugin for now? We can promote it as the standard way to use AWS step functions with Serverless. This way we can still take the plugin or parts from it and include it into the Framework core if we decide to add native support for AWS step functions.

@horike37
Copy link
Member

horike37 commented Apr 4, 2017

@pmuens Thank you for the repley 😄
I have understood what you commented the above. And I can also understand the concern of putting provider specific feature into the Framework core.

Could continue the work on step function support with AWS based on the feedback / proposals >above in the step-functions plugin for now?

Of cource! Based on the feedback I got here, I will release the plugin as v1.0 😆

@pmuens
Copy link
Contributor

pmuens commented Apr 4, 2017

@horike37

I have understood what you commented the above. And I can also understand the concern of putting provider specific feature into the Framework core.

Thank you very much for your understanding! It always feels bad to postpone / reject feature requests since real people invest their valuable time 🕐 ...

Of cource! Based on the feedback I got here, I will release the plugin as v1.0

Glad to hear that. I hope that the feedback provided above helps with the new implementation. Please keep us posted in this issue and let us know if you need any further feedback or need any help! 👍

@erikerikson
Copy link
Member

erikerikson commented Apr 4, 2017

@pmuens, can you expand "how we can do this in a way where we support multiple providers"?

Are you talking about something like the following?

aws:
  lambdas:
    <name>: [...]
    [...]
  stepFunctions:
    <name>: [...]
    [...]
  [...]
azure:
  [...]
[pluginId]: [...]
[etc...]

@pmuens
Copy link
Contributor

pmuens commented Apr 4, 2017

@erikerikson

@pmuens, can you expand "how we can do this in a way where we support multiple providers"?

🤔 we don't have any specific implementation ideas about that yet so any kind of feedback on this is highly welcomed!

The first, initial idea is to define the functions and then compose them so that they replicate the AWS step-functions implementation in a more abstracted way. But nothing set in 🗿 yet...

@erikerikson
Copy link
Member

@pmuens I'd be wary of abstracting an already rather abstract service. That said, if there are clean yet correct and complete ways to simplify the engine definition, I wouldn't mind. They're a bit of a bear to define and formulate in their full statement. Mostly, in my very limited experience, due to error handling. Variables could be used to avoid that problem though, so I'm not sure how much of a win there would be, especially as there were attempts to stretch the concept beyond AWS. Perhaps, a template that would be merged on top of for each declared state would allow for the avoidance of re-declaring various fields over and over unless you really wanted a special case or behavior. Perhaps even a default template that would be merged over by an optionally declared template that would be merged over by any declared concrete state. A naive though, almost surely.

An example of the verbosity of some state definitions given responsible error handling and whatnot follows. It declares "run a lambda, retrying failures three times before giving up and proceeding upon success" (I should probably notify the myself via SNS for any terminal error and then work on automating them all away but... priorities and all).

  AssignPhotographer:
    Type: Task
    Resource: arn:aws:lambda:${self:custom.private.region}:${self:custom.private.accountId}:function:${self:custom.productPhotos.assignName}-${self:custom.stage}-assign
    Retry:
      - ErrorEquals:
          - States.ALL
        IntervalSeconds: 1
        MaxAttempts: 2
        BackoffRate: 2
    Catch:
      - ErrorEquals:
          - States.ALL
        Next: AssignPhotographerError
    Next: SendAssignmentNotice
  AssignPhotographerError:
    Type: Fail
    Cause: Repeated Failure Assigning Photographer
    Error: RepeatedFailure

@horike37
Copy link
Member

Just released Serverless Step Functions Plugin v1.0 :bowtie:
https://github.com/horike37/serverless-step-functions/releases/tag/1.0.0

@pmuens pmuens changed the title aws step functions support AWS Step Functions support May 30, 2017
@kaiser14
Copy link

I need some help troubleshooting an issue with the plugin. I'm following @horike37 's example. When i try to deploy, only the lambda gets deployed, not the step function. No errors are thrown. When i check cloudformation, there is no json code that describes step function nor state machines. Appreciate any help!

@horike37
Copy link
Member

@kaiser14
Thank you for using the plugin! Could you please open up an another issue on the plugin repo and show me your described serverless.yml?
https://github.com/horike37/serverless-step-functions/issues

I would provide you with some of help on there.

@kaiser14
Copy link

@horike37 Thanks for looking into this. I've opened a new issue and attached the serverless.yml: serverless-operations/serverless-step-functions#70

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants