Skip to content

Conversation

@syedahsn
Copy link
Contributor

Currently, credentials are loaded once at the initialization of the ECS Executor, and those credentials are used to authenticate to the ECS API. If those credentials expire, the Executor will not able to schedule new tasks, or update existing ones. There is no way to reload the credentials except by restarting the scheduler.

The purpose of this PR is to allow the Executor to refresh its credentials if they expire. It includes an exponential backoff retry method that retries the credentials based on the previous attempt so that the API has a chance to work through any transient issues, and to not unnecessarily spam the API if the credentials are genuinely expired.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@Taragolis
Copy link
Contributor

Currently, credentials are loaded once at the initialization of the ECS Executor, and those credentials are used to authenticate to the ECS API. If those credentials expire, the Executor will not able to schedule new tasks, or update existing ones.

Maybe it is some bug in refresh credentials into the BaseSessionFactory or either into the botocore / boto3?

I've asked it because refresh credentials it is internals of the Credential Providers into the botocore, that part not well documented as well as not well implemented into the AWS Provider, to be honest the solution which works now seems to be copy-pasted from the SO

session._credentials = credentials
session.set_config_variable("region", self.basic_session.region_name)
return boto3.session.Session(botocore_session=session, **session_kwargs)

However if it failed into the ECS Executor then seems like it should failed also in regular usage

@eladkal eladkal changed the title Syedahsn/aws executors ecs credentials refresh Refresh credentials in AwsEcsExecutor Dec 15, 2023
Copy link
Contributor

@eladkal eladkal left a comment

Choose a reason for hiding this comment

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

My 2 comments are just thoughts/observations.
LGTM in any case so feel free to move forward

@syedahsn syedahsn force-pushed the syedahsn/aws_executors_ecs_credentials_refresh branch from 16fb263 to 0632fdd Compare January 5, 2024 10:02
@syedahsn syedahsn force-pushed the syedahsn/aws_executors_ecs_credentials_refresh branch from 8a545cb to 6f2d6f9 Compare January 9, 2024 08:28
@o-nikolas o-nikolas merged commit cb1182e into apache:main Jan 9, 2024
@o-nikolas o-nikolas deleted the syedahsn/aws_executors_ecs_credentials_refresh branch January 9, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants