ecs.py _cleanup_stale_resources() leaves behind Security Groups

[ptocca]

The issue occurs the AWS cloud provider.
It seems to me that when, for any reason, Security Groups are not garbage collected, the _cleanup_stale_resources() function fails to clean them up. That happens because _cleanup_stale_resources() enumerates only Security Groups with the tag 'createdBy' appropriately set to "dask-cloudprovider". In the AWS web console I can see that the Security Groups created by ECSCluster() do not have any tags.

I noticed the code that sets the tag when creating a Security Group has been commented out. https://github.com/dask/dask-cloudprovider/blob/027bc1ae98210c1f3f7f080c32a6190c2990e931/dask_cloudprovider/providers/aws/ecs.py#L1074-L1076

Is there any reason for refraining from setting the tag?

[jacobtomlinson]

Is there any reason for refraining from setting the tag?

Hmm it looks like this was commented out when switching to aiobotocore 1.0. @mrocklin do you remember why this was done?

[mrocklin]

I don't have a clear memory of this, but my guess is that these were causing some error that I didn't have time to track down at the time. If I recall correctly the aiobotocore 1.0 transition was done in haste (dask-cloudprovider was suddenly broken for all new installs). I'm +1 to anyone adding back in tags.

On Mon, Nov 2, 2020 at 6:22 AM Jacob Tomlinson [email protected] wrote:

Is there any reason for refraining from setting the tag?

Hmm it looks like this was commented out when switching to aiobotocore 1.0. @mrocklin https://github.com/mrocklin do you remember why this was done?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dask/dask-cloudprovider/issues/151#issuecomment-720501714, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKZTE2PTEC2SH7Y5RPBO3SN26DPANCNFSM4THN25MQ .

[ptocca]

Thanks for the quick answers. I will go ahead and uncomment those lines locally, and see what happens.

[jacobtomlinson]

Thanks @ptocca!