ferruzzi commented on code in PR #71207:
URL: https://github.com/apache/airflow/pull/71207#discussion_r3732607088
##########
providers/amazon/tests/system/amazon/aws/example_emr_eks.py:
##########
@@ -332,7 +281,7 @@ def delete_virtual_cluster(virtual_cluster_id):
task_id="start_job",
virtual_cluster_id=str(create_emr_eks_cluster.output),
execution_role_arn=job_role_arn,
- release_label="emr-7.0.0-latest",
+ release_label="emr-7.13.0-latest",
Review Comment:
Just curious if there is any particular reason for this version as opposed
to 7.3.0? Not blocking, just curious if this is just the current latest as
opposed to using 7.3 which is the minver you stated above.
##########
providers/amazon/tests/system/amazon/aws/example_emr_eks.py:
##########
@@ -127,43 +123,37 @@ def run_eksctl_commands(cluster_name, ns):
raise RuntimeError(err)
-@task(trigger_rule=TriggerRule.ALL_DONE)
-def delete_iam_oidc_identity_provider(cluster_name):
- oidc_provider_issuer_url = boto3.client("eks").describe_cluster(
- name=cluster_name,
- )["cluster"]["identity"]["oidc"]["issuer"]
- oidc_provider_issuer_endpoint =
oidc_provider_issuer_url.replace("https://";, "")
-
- account_id = boto3.client("sts").get_caller_identity()["Account"]
- boto3.client("iam").delete_open_id_connect_provider(
-
OpenIDConnectProviderArn=f"arn:aws:iam::{account_id}:oidc-provider/{oidc_provider_issuer_endpoint}"
- )
+@task
+def install_pod_identity_agent(cluster_name):
+ """Install and wait for the EKS Pod Identity Agent add-on.
+ Each run creates Pod Identity associations scoped to its own cluster. The
+ role's trust policy stays static.
+ """
+ log = logging.getLogger(__name__)
+ eks_client = boto3.client("eks")
+ addon_name = "eks-pod-identity-agent"
-@task
-def update_trust_policy_execution_role(cluster_name, cluster_namespace,
role_name):
- # Remove any already existing trusted entities added with
"update-role-trust-policy"
- # Prevent getting an error "Cannot exceed quota for ACLSizePerRole"
- client = boto3.client("iam")
- role_trust_policy =
client.get_role(RoleName=role_name)["Role"]["AssumeRolePolicyDocument"]
- # We assume if the action is sts:AssumeRoleWithWebIdentity, the statement
had been added with
- # "update-role-trust-policy". Removing it to not exceed the quota
- role_trust_policy["Statement"] = [
- statement
- for statement in role_trust_policy["Statement"]
- if statement["Action"] != "sts:AssumeRoleWithWebIdentity"
- ]
-
- client.update_assume_role_policy(
- RoleName=role_name,
- PolicyDocument=json.dumps(role_trust_policy),
+ eks_client.create_addon(clusterName=cluster_name, addonName=addon_name)
+
+ waiter = eks_client.get_waiter("addon_active")
+ waiter.wait(
+ clusterName=cluster_name,
+ addonName=addon_name,
+ WaiterConfig={"Delay": 10, "MaxAttempts": 60},
)
+ log.info("Pod Identity Agent addon is ACTIVE on cluster %s", cluster_name)
+
- # See
https://docs.aws.amazon.com/emr/latest/EMR-on-EKS-DevelopmentGuide/setting-up-trust-policy.html
- # The action "update-role-trust-policy" is not available in boto3, thus we
need to do it using AWS CLI
+@task
+def create_pod_identity_role_associations(cluster_name, namespace, role_name):
+ # `emr-containers create-role-associations` is a CLI-only helper — it
creates
+ # the three Pod Identity associations (client, driver, executor) that the
EMR
+ # job pods use. There is no boto3 equivalent. Requires AWS CLI > 2.24.0.
Review Comment:
If we have a minver here, we should likely add a check to enforce it like we
did in example_sagemaker
##########
providers/amazon/tests/system/amazon/aws/example_emr_eks.py:
##########
@@ -385,15 +334,15 @@ def delete_virtual_cluster(virtual_cluster_id):
create_cluster_and_nodegroup,
await_create_nodegroup,
run_eksctl_commands(eks_cluster_name, eks_namespace),
- update_trust_policy_execution_role(eks_cluster_name, eks_namespace,
job_role_name),
- wait_for_trust_policy_propagation(eks_cluster_name, job_role_name),
+ install_pod_identity_agent(eks_cluster_name),
+ create_pod_identity_role_associations(eks_cluster_name, eks_namespace,
job_role_name),
Review Comment:
Have you checked if there is propagation delay on the `create identity`
here? Just making sure we aren't trading one race for another without a waiter
or sensor here to check it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
