Use more informative container names for spawned Lambda containers#7667
Use more informative container names for spawned Lambda containers#7667
Conversation
joe4dev
left a comment
There was a problem hiding this comment.
Usability improvement makes a lot of sense 👍
There are different options for container naming (see comment).
| self.function_version.id.function_name.lower(), | ||
| ] | ||
| ).replace("_", "-") |
There was a problem hiding this comment.
Do we want to standardize the container names (e.g., lowercase and - only) or make it easier to match for external lookups?
_and capital letters would be valid for container names._cannot be used in URLs and might lead to limitations with the DNS re-work (see Simon's post in #sig-localstack)
Docker container names
- Validation regex:
[a-zA-Z0-9][a-zA-Z0-9_.-](Docker source) - Length constraint: There seem to be no explicit length constraints (also see this SO discussion). There might be implicit limits (e.g., 63 chars in GitLab runners due to hitting max DNS hostname length mentioned here)
Input restrictions
- A Docker container name without length restriction should by definition be a valid input
- All AWS Lambda functions
[a-zA-Z0-9-_]+should be valid container names (AWS FunctionName docs)
There was a problem hiding this comment.
Replacing _ with - was also just partly because we do this in the k8s executor and partly for the DNS reason you mentioned.
As you said, ideally we'd have names that can be used as valid DNS names, although the use case for this is pretty much non-existent for lambda at least and 63 Character is unfortunately also extremely limiting in practice, so we can't encode a lot of information this way. :/
| """ | ||
| container_name = "-".join( | ||
| [ | ||
| get_main_container_name() or "localstack", |
There was a problem hiding this comment.
nice to handle fallback for host mode 👍
| CONTAINER_CLIENT.copy_into_container(self.container_name, source, target) | ||
|
|
||
| CONTAINER_CLIENT.start_container(self.id) | ||
| CONTAINER_CLIENT.start_container(self.container_name) |
There was a problem hiding this comment.
Sidenote: If we want to protect ourselves from external renaming in the future, we could consider using the Container.id / CLI container id instead of the name but the name seems to work fine for now.
Motivation
It's quite hard to find the container you're looking for when just looking at the list of running/exited containers right now since we just assign the randomly generated executor ID as a container name and one has to actually check the container metadata/config to find out which lambda is being executed in it.
Resolves #7635
Changes
ffa532cb281182ee92052ff97e1f37c4) to the following pattern:<main-container-name>-lambda-<function-name>-<excecutor-id>localstack-main-lambda-processing-ffa532cb281182ee92052ff97e1f37c4Discussion
We could make this more generic in the future by providing a config variable we'll use to format this
e.g.
LAMBDA_DOCKER_CONTAINER_NAME_GENERATOR="<main-container-name>-lambda-<region>-<function-name>-<runtime>"or
LAMBDA_DOCKER_CONTAINER_NAME_GENERATOR="<main-container-name>-lambda-<function-arn>"Still, we'll always add the executor/runtime ID as a suffix
Also open to suggestion of other defaults 👍