-
Notifications
You must be signed in to change notification settings - Fork 17.6k
Deferrable mode for EKS Create/Delete Operator #32355
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
o-nikolas
merged 9 commits into
apache:main
from
aws-mwaa:syedahsn/deferrable-eks-cluster
Jul 17, 2023
Merged
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e206854
EKS Create Cluster deferrable
syedahsn 20e0b67
Add unit tests
syedahsn fe183e6
D205 Doc style changes
syedahsn bc03637
Merge remote-tracking branch 'origin/main' into syedahsn/deferrable-e…
syedahsn a79c229
Rebase to use AwsBaseWaiterTrigger
syedahsn 9660412
Add type hints to execute_* functions
syedahsn 0befff8
Change function definition of execute_* method to set default value o…
syedahsn 0cd3069
Minor Refactor
syedahsn ea6b966
Merge branch 'main' into syedahsn/deferrable-eks-cluster
syedahsn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ | |
| import warnings | ||
| from ast import literal_eval | ||
| from datetime import timedelta | ||
| from typing import TYPE_CHECKING, List, Sequence, cast | ||
| from typing import TYPE_CHECKING, Any, List, Sequence, cast | ||
|
|
||
| from botocore.exceptions import ClientError, WaiterError | ||
|
|
||
|
|
@@ -30,8 +30,10 @@ | |
| from airflow.models import BaseOperator | ||
| from airflow.providers.amazon.aws.hooks.eks import EksHook | ||
| from airflow.providers.amazon.aws.triggers.eks import ( | ||
| EksCreateClusterTrigger, | ||
| EksCreateFargateProfileTrigger, | ||
| EksCreateNodegroupTrigger, | ||
| EksDeleteClusterTrigger, | ||
| EksDeleteFargateProfileTrigger, | ||
| EksDeleteNodegroupTrigger, | ||
| ) | ||
|
|
@@ -187,6 +189,9 @@ class EksCreateClusterOperator(BaseOperator): | |
| (templated) | ||
| :param waiter_delay: Time (in seconds) to wait between two consecutive calls to check cluster state | ||
| :param waiter_max_attempts: The maximum number of attempts to check cluster state | ||
| :param deferrable: If True, the operator will wait asynchronously for the job to complete. | ||
| This implies waiting for completion. This mode requires aiobotocore module to be installed. | ||
| (default: False) | ||
|
|
||
| """ | ||
|
|
||
|
|
@@ -225,6 +230,7 @@ def __init__( | |
| wait_for_completion: bool = False, | ||
| aws_conn_id: str = DEFAULT_CONN_ID, | ||
| region: str | None = None, | ||
| deferrable: bool = conf.getboolean("operators", "default_deferrable", fallback=False), | ||
| waiter_delay: int = 30, | ||
| waiter_max_attempts: int = 40, | ||
| **kwargs, | ||
|
|
@@ -237,7 +243,7 @@ def __init__( | |
| self.nodegroup_role_arn = nodegroup_role_arn | ||
| self.fargate_pod_execution_role_arn = fargate_pod_execution_role_arn | ||
| self.create_fargate_profile_kwargs = create_fargate_profile_kwargs or {} | ||
| self.wait_for_completion = wait_for_completion | ||
| self.wait_for_completion = False if deferrable else wait_for_completion | ||
| self.waiter_delay = waiter_delay | ||
| self.waiter_max_attempts = waiter_max_attempts | ||
| self.aws_conn_id = aws_conn_id | ||
|
|
@@ -246,6 +252,7 @@ def __init__( | |
| self.create_nodegroup_kwargs = create_nodegroup_kwargs or {} | ||
| self.fargate_selectors = fargate_selectors or [{"namespace": DEFAULT_NAMESPACE_NAME}] | ||
| self.fargate_profile_name = fargate_profile_name | ||
| self.deferrable = deferrable | ||
| super().__init__( | ||
| **kwargs, | ||
| ) | ||
|
|
@@ -274,12 +281,25 @@ def execute(self, context: Context): | |
|
|
||
| # Short circuit early if we don't need to wait to attach compute | ||
| # and the caller hasn't requested to wait for the cluster either. | ||
| if not self.compute and not self.wait_for_completion: | ||
| if not any([self.compute, self.wait_for_completion, self.deferrable]): | ||
| return None | ||
|
|
||
| self.log.info("Waiting for EKS Cluster to provision. This will take some time.") | ||
| self.log.info("Waiting for EKS Cluster to provision. This will take some time.") | ||
| client = self.eks_hook.conn | ||
|
|
||
| if self.deferrable: | ||
| self.defer( | ||
| trigger=EksCreateClusterTrigger( | ||
| cluster_name=self.cluster_name, | ||
| aws_conn_id=self.aws_conn_id, | ||
| region_name=self.region, | ||
| waiter_delay=self.waiter_delay, | ||
| waiter_max_attempts=self.waiter_max_attempts, | ||
| ), | ||
| method_name="deferrable_create_cluster_next", | ||
| timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay), | ||
| ) | ||
|
|
||
| try: | ||
| client.get_waiter("cluster_active").wait( | ||
| name=self.cluster_name, | ||
|
|
@@ -311,6 +331,80 @@ def execute(self, context: Context): | |
| subnets=cast(List[str], self.resources_vpc_config.get("subnetIds")), | ||
| ) | ||
|
|
||
| def deferrable_create_cluster_next(self, context: Context, event: dict[str, Any] = {}) -> None: | ||
| if event["status"] == "failed": | ||
| self.log.error("Cluster failed to start and will be torn down.") | ||
| self.eks_hook.delete_cluster(name=self.cluster_name) | ||
| self.defer( | ||
| trigger=EksDeleteClusterTrigger( | ||
| cluster_name=self.cluster_name, | ||
| waiter_delay=self.waiter_delay, | ||
| waiter_max_attempts=self.waiter_max_attempts, | ||
| aws_conn_id=self.aws_conn_id, | ||
| region_name=self.region, | ||
| force_delete_compute=False, | ||
| ), | ||
| method_name="execute_failed", | ||
| timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay), | ||
| ) | ||
| elif event["status"] == "success": | ||
| self.log.info("Cluster is ready to provision compute.") | ||
| _create_compute( | ||
| compute=self.compute, | ||
| cluster_name=self.cluster_name, | ||
| aws_conn_id=self.aws_conn_id, | ||
| region=self.region, | ||
| wait_for_completion=self.wait_for_completion, | ||
| waiter_delay=self.waiter_delay, | ||
| waiter_max_attempts=self.waiter_max_attempts, | ||
| nodegroup_name=self.nodegroup_name, | ||
| nodegroup_role_arn=self.nodegroup_role_arn, | ||
| create_nodegroup_kwargs=self.create_nodegroup_kwargs, | ||
| fargate_profile_name=self.fargate_profile_name, | ||
| fargate_pod_execution_role_arn=self.fargate_pod_execution_role_arn, | ||
| fargate_selectors=self.fargate_selectors, | ||
| create_fargate_profile_kwargs=self.create_fargate_profile_kwargs, | ||
| subnets=cast(List[str], self.resources_vpc_config.get("subnetIds")), | ||
| ) | ||
| if self.compute == "fargate": | ||
| self.defer( | ||
| trigger=EksCreateFargateProfileTrigger( | ||
| cluster_name=self.cluster_name, | ||
| fargate_profile_name=self.fargate_profile_name, | ||
| waiter_delay=self.waiter_delay, | ||
| waiter_max_attempts=self.waiter_max_attempts, | ||
| aws_conn_id=self.aws_conn_id, | ||
| region=self.region, | ||
| ), | ||
| method_name="execute_complete", | ||
| timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay), | ||
| ) | ||
| else: | ||
| self.defer( | ||
| trigger=EksCreateNodegroupTrigger( | ||
| nodegroup_name=self.nodegroup_name, | ||
| cluster_name=self.cluster_name, | ||
| aws_conn_id=self.aws_conn_id, | ||
| region_name=self.region, | ||
| waiter_delay=self.waiter_delay, | ||
| waiter_max_attempts=self.waiter_max_attempts, | ||
| ), | ||
| method_name="execute_complete", | ||
| timeout=timedelta(seconds=self.waiter_max_attempts * self.waiter_delay), | ||
| ) | ||
|
|
||
| def execute_failed(self, context: Context, event: dict[str, Any] = {}) -> None: | ||
| if event["status"] == "delteted": | ||
|
syedahsn marked this conversation as resolved.
Outdated
|
||
| self.log.info("Cluster deleted") | ||
| raise event["exception"] | ||
|
|
||
| def execute_complete(self, context: Context, event: dict[str, Any] = {}) -> None: | ||
| resource = "fargate profile" if self.compute == "fargate" else self.compute | ||
| if event["status"] != "success": | ||
| raise AirflowException(f"Error creating {resource}: {event}") | ||
|
|
||
| self.log.info("%s created successfully", resource) | ||
|
|
||
|
|
||
| class EksCreateNodegroupOperator(BaseOperator): | ||
| """ | ||
|
|
@@ -564,6 +658,11 @@ class EksDeleteClusterOperator(BaseOperator): | |
| maintained on each worker node). | ||
| :param region: Which AWS region the connection should use. (templated) | ||
| If this is None or empty then the default boto3 behaviour is used. | ||
| :param waiter_delay: Time (in seconds) to wait between two consecutive calls to check cluster state | ||
| :param waiter_max_attempts: The maximum number of attempts to check cluster state | ||
| :param deferrable: If True, the operator will wait asynchronously for the cluster to be deleted. | ||
| This implies waiting for completion. This mode requires aiobotocore module to be installed. | ||
| (default: False) | ||
|
|
||
| """ | ||
|
|
||
|
|
@@ -582,22 +681,40 @@ def __init__( | |
| wait_for_completion: bool = False, | ||
| aws_conn_id: str = DEFAULT_CONN_ID, | ||
| region: str | None = None, | ||
| deferrable: bool = conf.getboolean("operators", "default_deferrable", fallback=False), | ||
| waiter_delay: int = 30, | ||
| waiter_max_attempts: int = 40, | ||
| **kwargs, | ||
| ) -> None: | ||
| self.cluster_name = cluster_name | ||
| self.force_delete_compute = force_delete_compute | ||
| self.wait_for_completion = wait_for_completion | ||
| self.wait_for_completion = False if deferrable else wait_for_completion | ||
| self.aws_conn_id = aws_conn_id | ||
| self.region = region | ||
| self.deferrable = deferrable | ||
| self.waiter_delay = waiter_delay | ||
| self.waiter_max_attempts = waiter_max_attempts | ||
| super().__init__(**kwargs) | ||
|
|
||
| def execute(self, context: Context): | ||
| eks_hook = EksHook( | ||
| aws_conn_id=self.aws_conn_id, | ||
| region_name=self.region, | ||
| ) | ||
|
|
||
| if self.force_delete_compute: | ||
| if self.deferrable: | ||
| self.defer( | ||
| trigger=EksDeleteClusterTrigger( | ||
| cluster_name=self.cluster_name, | ||
| waiter_delay=self.waiter_delay, | ||
| waiter_max_attempts=self.waiter_max_attempts, | ||
| aws_conn_id=self.aws_conn_id, | ||
| region_name=self.region, | ||
| force_delete_compute=self.force_delete_compute, | ||
| ), | ||
| method_name="execute_complete", | ||
| timeout=timedelta(seconds=self.waiter_delay * self.waiter_max_attempts), | ||
| ) | ||
| elif self.force_delete_compute: | ||
| self.delete_any_nodegroups(eks_hook) | ||
| self.delete_any_fargate_profiles(eks_hook) | ||
|
|
||
|
|
@@ -645,6 +762,10 @@ def delete_any_fargate_profiles(self, eks_hook) -> None: | |
| ) | ||
| self.log.info(SUCCESS_MSG.format(compute=FARGATE_FULL_NAME)) | ||
|
|
||
| def execute_complete(self, context: Context, event: dict[str, Any] = {}) -> None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as #32355 (comment) |
||
| if event["status"] == "success": | ||
| self.log.info("Cluster deleted successfully.") | ||
|
|
||
|
|
||
| class EksDeleteNodegroupOperator(BaseOperator): | ||
| """ | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure whether we should use
{}as the default value. I believe it's generally not a good idea to use mutable object as defaultThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm that's a good point. I guess I can set it to
Noneand do a check forNonebefore indexing it i.e.Its going to be tedious to do it everywhere, but I don't see a better option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I think this is the best option we have as of now.