Skip to content
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

Lambda enhancements -- Docker configuration, including networking #567

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
10 participants
@gadgetjunkie
Copy link
Contributor

gadgetjunkie commented Jan 21, 2018

  • Added support for identifying the docker network when running lambda functions in docker containers
The docker network used when running lambda functions in a container may be specified with the LAMBDA_DEFAULT_DOCKER_NETWORK and LAMBDA_SUBNET_AS_DOCKERNET environment variables along with the subnet specified in the lambda's vpc config
  • Added environment variables specifying the function arn and function name
    for use in docker command options -- useful when combined with LAMBDA_DOCKER_OPTIONS

  • fixed issue where environment variables were not returned in get_function_configuration

  • Added support for custom docker actions when running lambda functions in docker containers
    custom options to be passed to docker can be specified via the LAMBDA_DOCKER_OPTIONS
    environment variables

  • Added test configurations for lambda docker network config

  • Added lambda networking test to travis build

While this pull request is active, I am maintaining a docker image with these changes in the docker registry by the name of gadgetjunkie/localstack. Feel free to use it as you see fit.

@gadgetjunkie gadgetjunkie changed the title Lambda enhancements Lambda enhancements -- Docker configuration, including networking Jan 21, 2018

@gadgetjunkie

This comment has been minimized.

Copy link
Contributor Author

gadgetjunkie commented Jan 31, 2018

Is there a reason this PR hasn't been merged? Anything I can do to help?

@whummer

This comment has been minimized.

Copy link
Collaborator

whummer commented Feb 4, 2018

First of all, thanks for this comprehensive pull request @gadgetjunkie .

The proposed changes are quite involved and touch on some of the key functionalities of LocalStack. I had a quick look at the PR but didn't have a chance to look in detail. I'll do my best to review it in more detail today or in the next couple of days.

Meanwhile, can you please rebase onto latest master and resolve the merge conflicts. Thanks

@mcosta91 mcosta91 referenced this pull request Feb 5, 2018

Closed

Unable to run lambda #377

@gadgetjunkie gadgetjunkie force-pushed the gadgetjunkie:feat/lambda_docker_options-pr branch from 2a5a7f6 to d9c5cce Feb 7, 2018

@gadgetjunkie

This comment has been minimized.

Copy link
Contributor Author

gadgetjunkie commented Feb 7, 2018

I understand that there's a lot in this pr. I usually try to make things smaller, but there was just too much interconnected in this one. The tests added a lot as well.

There were two primary goals for these changes that these changes needed to meet to allow using localstack in our application's development/testing:

  1. Ability to specify the docker network -- our environment runs in a specific docker-compose stack/network and uses python3 lambdas, so the docker containers launched by localstack for the lambdas need to be able to talk to the other services (including localstack) that are running in the docker compose stack.

  2. Ability to view logs from lambdas. This is not unique to our environment, and seems to be a big piece that was missing from localstack for lambda development. While synchronous lambda calls can have their output logged to the localstack docker log, logs from async logs are lost. This change allows us to set the logging options for the docker containers as they are launched by localstack. In my case, I have a syslog server running which collects the logs and places the output into their own file based on the name of the lambda.

Thanks,

Mike Williams aka GadgetJunkie

@gadgetjunkie gadgetjunkie force-pushed the gadgetjunkie:feat/lambda_docker_options-pr branch from d9c5cce to aaf7500 Feb 7, 2018

@gadgetjunkie

This comment has been minimized.

Copy link
Contributor Author

gadgetjunkie commented Feb 7, 2018

Rebased against master, conflicts resolved.

@Afsoon

This comment has been minimized.

Copy link

Afsoon commented Apr 23, 2018

Some news about this PR?. In my company, we are using this image right now because we need attach the lambdas with the docker compose network. Plus the lambdas' logs

@gadgetjunkie

This comment has been minimized.

Copy link
Contributor Author

gadgetjunkie commented Apr 24, 2018

@Afsoon -- based on the fact that this PR has been here since 1/20, it would appear that there is not any interest from the repo admin to merge this. Sadly, there have been no statements as to why this PR has not been merged.

@duhseekoh

This comment has been minimized.

Copy link

duhseekoh commented May 10, 2018

Would love to see this functionality. Right now, can call from lambda -> other localstack services, but can't call from lambda to my other docker-compose services (without this).

@wesselvdv

This comment has been minimized.

Copy link

wesselvdv commented May 13, 2018

@gadgetjunkie Could you rebase on master again? There's a fix for permissions in there that some (including me) need.

Mike Williams
Lambda enhancements
   - Added support for identifying the docker network when running lambda functions in docker containers

    The docker network used when running lambda functions in a container may be specified with the LAMBDA_DEFAULT_DOCKER_NETWORK and LAMBDA_SUBNET_AS_DOCKERNET environment variables along with the subnet specified in the lambda's vpc config

   - Added environment variables specifying the function arn and function name
     for use in docker command options -- useful when combined with LAMBDA_DOCKER_OPTIONS
   - fixed issue where environment variables were not returned in get_function_configuration
   - Added support for custom docker actions when running lambda functions in docker containers
   custom options to be passed to docker can be specified via the LAMBDA_DOCKER_OPTIONS
   environment variables

   - Added test configurations for lambda docker network config
   - Added lambda networking test to travis build

@gadgetjunkie gadgetjunkie force-pushed the gadgetjunkie:feat/lambda_docker_options-pr branch from aaf7500 to 85ca833 May 24, 2018

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 24, 2018

Pull Request Test Coverage Report for Build 955

  • 12 of 34 (35.29%) changed or added relevant lines in 5 files are covered.
  • 160 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-1.9%) to 74.461%

Changes Missing Coverage Covered Lines Changed/Added Lines %
localstack/services/awslambda/lambda_api.py 1 3 33.33%
localstack/services/awslambda/lambda_executors.py 6 26 23.08%
Files with Coverage Reduction New Missed Lines %
localstack/services/infra.py 1 73.89%
localstack/config.py 2 70.83%
localstack/utils/testutil.py 4 89.36%
localstack/services/awslambda/lambda_api.py 7 76.1%
localstack/utils/common.py 19 75.1%
localstack/services/awslambda/lambda_executors.py 127 41.39%
Totals Coverage Status
Change from base Build 954: -1.9%
Covered Lines: 4353
Relevant Lines: 5846

💛 - Coveralls
@niklaskors

This comment has been minimized.

Copy link

niklaskors commented Jul 24, 2018

@duhseekoh I had the exact same issue right now. I was planning to implement this feature myself until I saw this PR, great stuff @gadgetjunkie . Any news @whummer ?

def lambda_docker_cmd_networksection(self, func_arn, func_details):
network = self.lambda_docker_network(func_arn, func_details)
if network:
return ' --network="%s" ' % network

This comment has been minimized.

@shrishinde

shrishinde Aug 3, 2018

Contributor

Nitpick: can we change this to return ' --network="%s" ' % network if network else ""

This comment has been minimized.

@gadgetjunkie

gadgetjunkie Aug 8, 2018

Author Contributor

You can change whatever you want here.

This PR is over 6 months old, with no signs of it ever getting merged.

I no longer use local stack as it was too brittle for my needs, and have moved on.

I will no longer be devoting any time or attention to this PR, or to local stack in general.

Sorry it didn't work out.

This comment has been minimized.

@cabdesigns

cabdesigns Nov 19, 2018

@gadgetjunkie what did you end up moving to?

This comment has been minimized.

@gadgetjunkie

gadgetjunkie Feb 19, 2019

Author Contributor

@cabdesigns I am simply using separate CloudFormation stacks in AWS for each environment/developer. It costs more, but it eliminates the many performance and inconsistency issues we were seeing when we tried to run our stuff in localstack.

@calbertts

This comment has been minimized.

Copy link

calbertts commented Sep 15, 2018

What a shame, it looks like there's no interest to receive help from open source community.

@cabdesigns

This comment has been minimized.

Copy link

cabdesigns commented Nov 19, 2018

How is this not a massive issue for people? We've managed to sidestep this for a while as it seems to just work on Mac (possibly because of the use of the docker.for.mac.localhost alias rather than the default bridge network ip).

However, now linux users have joined the team - no one can use their dev env effectively as the bridge network ip (172.17.0.1) fails to resolve. Presumably because the lambda is spun up on the default bridge network and localstack and co on the docker compose default network.

There seems to be no easy way around this without changes to localstack itself.

There's now a second PR around this topic: #898

Is anyone from the core team able to comment on this?

@whummer

This comment has been minimized.

Copy link
Collaborator

whummer commented Dec 8, 2018

First of all, apologies to @gadgetjunkie and everyone that this PR never got the attention it would have deserved to receive.

There was some reluctance initially because the PR introduced some major changes to the core of Lambda execution, and it also introduced some fairly intrusive changes that were either (1) diverging from the coding style we've been following in this project (e.g., comprehensive test setup code in Makefile, which we would have preferred to go directly into python), or (2) unrelated to the functionality in the PR (e.g., changes to branch builds in .travis.yml), or (3) would have substantially increased the CI build time (three additional rounds of make test added to the Travis config). Ultimately we never got to review this PR properly, which admittedly is a shame.

Meanwhile, PR #898 got merged, which is a somewhat lightweight implementation and partially covers the features in this PR. Can we try to identify which features are still missing (if any), and how we could rebase them onto latest master. I'm specifically looking at the LAMBDA_SUBNET_AS_DOCKERNET and LAMBDA_DOCKER_OPTIONS configurations. Would also be great to get help from the folks involved in the discussion thread here @Afsoon @duhseekoh @wesselvdv @niklaskors @calbertts @cabdesigns

If there is no longer a need and/or interest in updating/maintaining this PR - that is understandable as well. In that case, we can close this one and create follow-up PRs as needed. Many thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.