Skip to content

refactor: Updated EKS integration to use integration_pattern #177

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

Merged
merged 8 commits into from
Nov 8, 2021

Conversation

ca-nguyen
Copy link
Contributor

@ca-nguyen ca-nguyen commented Nov 1, 2021

Description

Update Amazon EKS service integration to use integration_pattern input instead or wait_for_completion flag.

Fixes #(issue) - N/A

Why is the change necessary?

This change is necessary for consistency with the new service integration implementation pattern introduced in commit (Add support for Nested Step Functions) that uses the integration_pattern arg in the step constructor to build the resource.

Support for Amazon EKS service integration was added in this commit, but not released yet.
A later commit (Add support for Nested Step Functions) introduced a new implementation pattern using the IntegrationPattern enum as input to construct the step instead of the wait_for_completion flag. (See PR for more detail on rationale behind the implementation).

Solution

Replace the wait_for_completion flag with integration_pattern arg for the step construction:

The IntegrationPattern is used to build the Resource arn as follow:

IntegrationPattern Resource Doc
WaitForCompletion "arn:aws:states:::states:eks:createCluster.sync" Run A job
CallAndContinue "arn:aws:states:::states:eks:createCluster" Request Response

See Service Integration Patterns for more details

Apply changes to the following steps:

  • EksRunJob to create a Task state that allows you to run a job on your Amazon EKS cluster
    • CallAndContinue: arn:aws:states:::eks:runJob
    • WaitForCompletion: arn:aws:states:::eks:runJob.sync
  • EksCall to create a Task state that allows you to use the Kubernetes API to read and write Kubernetes resource objects via a Kubernetes API endpoint
    • CallAndContinue: arn: arn:aws:states:::eks:call
  • EksCreateClusterStep to create a Task state that creates an Amazon EKS cluster
    • CallAndContinue: arn:aws:states:::eks:createCluster
    • WaitForCompletion: arn:aws:states:::eks:createCluster.sync
  • EksDeleteClusterStep to create a Task state that deletes an Amazon EKS cluster
    • CallAndContinue: arn:aws:states:::eks:deleteCluster
    • WaitForCompletion: arn:aws:states:::eks:deleteCluster.sync
  • EksCreateFargateProfileStep to create a Task state that creates a Fargate profile
    • CallAndContinue: arn:aws:states:::eks:createFargateProfile
    • WaitForCompletion: arn:aws:states:::eks:createFargateProfile.sync
  • EksDeleteFargateProfileStep to create a Task state that deletes a Fargate profile
    • CallAndContinue: arn:aws:states:::eks:deleteFargateProfile
    • WaitForCompletion: arn:aws:states:::eks:deleteFargateProfile.sync
  • EksCreateNodeGroupStep to create a Task state that creates a node group
    • CallAndContinue: arn:aws:states:::eks:createNodegroup
    • WaitForCompletion: arn:aws:states:::eks:createNodegroup.sync
  • EksDeleteNodeGroupStep to create a Task state that deletes a node group
    • CallAndContinue: arn:aws:states:::eks:deleteNodegroup
    • WaitForCompletion: arn:aws:states:::eks:deleteNodegroup.sync

Normally, replacing a constructor argument would be a breaking change, but since we have not released support for Amazon EKS service integration yet, it is acceptable to do so. After next release, it making such changes will be considered as not being backward compatible.

Testing

  • Updated unit tests for all resources
  • No integ tests added as external resources were required for testing
  • Manual test done for all resources

Manual tests

EKS cluster management

Created step machine to test several eks steps (based on the Manage EKS cluster sample project):

create_cluster_step = EksCreateClusterStep(
    "Create Eks cluster",
    integration_pattern=IntegrationPattern.WaitForCompletion, parameters={
        'Name': 'ExampleCluster',
        'ResourcesVpcConfig': {
          'SubnetIds': [
            'subnet-XXXXXXXXXXXXXXXXX',
            'subnet-XXXXXXXXXXXXXXXXX'
          ]
        },
        'RoleArn': <execution_role_arn>
    },
    result_path="$.eks"
)

create_node_group_step = EksCreateNodegroupStep(
    "Create Node Group", integration_pattern=IntegrationPattern.WaitForCompletion, parameters={
        'ClusterName': 'ExampleCluster',
        'NodegroupName': 'ExampleNodegroup',
        'NodeRole': <node_role_arn>,
        'Subnets': [
            'subnet-XXXXXXXXXXXXXXXXX',
            'subnet-XXXXXXXXXXXXXXXXX'
        ]
    },
    result_path="$.nodegroup"
)

run_job_step = EksRunJobStep("Run Job", integration_pattern=IntegrationPattern.WaitForCompletion, parameters={
        'ClusterName': 'ExampleCluster',
        'CertificateAuthority.$': '$.eks.Cluster.CertificateAuthority.Data',
        'Endpoint.$': '$.eks.Cluster.Endpoint',
        'LogOptions': {
            'RetrieveLogs': True
        },
        'Job': {
            'apiVersion': 'batch/v1',
            'kind': 'Job',
            'metadata': {
                'name': 'example-job'
            },
            'spec': {
                'backoffLimit': 0,
                'template': {
                    'metadata': {
                        'name': 'example-job'
                    },
                    'spec': {
                        'containers': [
                            {
                                'name': 'pi-20',
                                'image': 'perl',
                                'command': ['perl'],
                                'args': [
                                    '-Mbignum=bpi',
                                    '-wle',
                                    'print bpi(20)'
                                ]
                            }
                        ],
                        'restartPolicy': 'Never'
                    }
                }
            }
        }
    },
    result_path="$.RunJobResult"
)


call_delete_job_step = EksCallStep("Call", parameters={
    'ClusterName': 'ExampleCluster',
    'CertificateAuthority.$': '$.eks.Cluster.CertificateAuthority.Data',
    'Endpoint.$': '$.eks.Cluster.Endpoint',
    'Method': 'DELETE',
    'Path': '/apis/batch/v1/namespaces/default/jobs/example-job',
    
})


delete_node_group = EksDeleteNodegroupStep(
    "Delete Node Group", integration_pattern=IntegrationPattern.WaitForCompletion, parameters={
        'ClusterName': 'ExampleCluster',
        'NodegroupName': 'ExampleNodegroup'
    })


delete_cluster_step = EksDeleteClusterStep(
    "Delete Eks cluster", integration_pattern=IntegrationPattern.WaitForCompletion, parameters={
        'Name': 'ExampleCluster'
    })

# Chain the steps
path=Chain([create_cluster_step, create_node_group_step, run_job_step, call_delete_job_step, delete_node_group, delete_cluster_step])

# Define workflow
workflow = Workflow(
    name="EKSStateMachineExample",
    definition=path,
    role=<workflow_execution_arn>
)

workflow.create()
workflow.execute()
Fargate profile creation/deletion test

Using an existing cluster <cluster_name>, create Fargate profile and delete it once profile creation succeeds

create_fargate_profile_step = EksCreateFargateProfileStep(
    "Create Fargate profile", integration_pattern=IntegrationPattern.WaitForCompletion, parameters={
        'ClusterName': <cluster_name>,
        'FargateProfileName': 'ExampleFargateProfile',
        'PodExecutionRoleArn': <pod_execution_role_arn>,
        'Selectors': [{
            'Namespace': 'my-namespace',
            'Labels': {'my-label': 'my-value'}
          }],
          'Subnets': [
            'subnet-XXXXXXXX'
          ]
    }
)

delete_fargate_profile_step = EksDeleteFargateProfileStep(
    "Delete Fargate profile", integration_pattern=IntegrationPattern.CallAndContinue, parameters={
        'ClusterName': <cluster_name>,
        'FargateProfileName': 'ExampleFargateProfile'
    }
)

# Chain the steps
path=Chain([create_fargate_profile_step, delete_fargate_profile_step])

# Define the workflow
workflow = Workflow(
    name="EKSFargateProfileExample",
    definition=path,
    role=<workflow_execution_role>
)

workflow.create()
workflow.execute()

Pull Request Checklist

Please check all boxes (including N/A items)

Testing

  • Unit tests added
  • Integration test added - N/A No integ tests added since external resources required for testing
  • Manual testing - No integ tests added since external resources required for testing

Documentation

  • docs: All relevant docs updated
  • docstrings: All public APIs documented

Title and description

  • Change type: Title is prefixed with change type: and follows conventional commits
  • References: Indicate issues fixed via: Fixes #xxx - N/A

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

@ca-nguyen ca-nguyen marked this pull request as ready for review November 1, 2021 18:50
Copy link
Contributor

@shivlaks shivlaks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice change! all the locations where a service integration pattern is now accepted also need a test case exercising an "unsupported" integration pattern.

@ca-nguyen
Copy link
Contributor Author

Thanks for the review @shivlaks !

all the locations where a service integration pattern is now accepted also need a test case exercising an "unsupported" integration pattern.

Added the unite tests for unsupported integration_patterns in the latest commit

I also added the integration_pattern arg in the EksCallStep constructor for consistency with the other steps (it was the only one that was missing - omitted previously since it only supports one of the integration patterns)

@ca-nguyen ca-nguyen requested a review from shivlaks November 2, 2021 22:46
@ca-nguyen ca-nguyen dismissed shivlaks’s stale review November 2, 2021 22:46

Comments were addressed and suggested changes were added with last commits

shivlaks
shivlaks previously approved these changes Nov 2, 2021

@patch.object(boto3.session.Session, 'region_name', 'us-east-1')
def test_eks_create_cluster_step_creation_wait_for_task_token_raises_error():
error_message = re.escape(f"Integration Pattern ({IntegrationPattern.WaitForTaskToken.name}) is not supported for this step - "
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was debating on adding a method in test/unit/utils to build the error message to avoid code repetition, but found that building the error message in each test improves readability. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like having it in the test for readability so don't have any objections to what you've gone with.

@ca-nguyen ca-nguyen requested a review from shivlaks November 4, 2021 19:49
@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-sEHrOdk7acJc
  • Commit ID: 1677185
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link

@evscott evscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🚀

@ca-nguyen ca-nguyen merged commit aafdb76 into aws:main Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants