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

CDK native Hook to upload zipped code (assets) #7513

Closed
2 tasks
Cloudrage opened this issue Apr 22, 2020 · 11 comments
Closed
2 tasks

CDK native Hook to upload zipped code (assets) #7513

Cloudrage opened this issue Apr 22, 2020 · 11 comments
Assignees
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package closing-soon This issue will automatically close in 4 days unless further comments are made. effort/large Large work item – several weeks of effort feature-request A feature should be added or improved.

Comments

@Cloudrage
Copy link

Use Case

When you're creating a Lambda with CDK, you have multiple way to provide the Code, fromAsset, fromBucket, fromInline...

But for example, if you have a Lambda Function in Python with Boto3 needed (but every imports needed are concerned); I don't want to store in my CodeCommit xXxMo of datas useless in every ZIP on each Lambda I have.
And after all, don't store any ZIP files on a Git...

Proposed Solution

So, actually using Sceptre (Troposphere), we have made a hook to check a requirements.txt file under the Lambda directory to download every modules needed with pip, but in local (also in .gitignore).
After that, we create the ZIP file & compare the hash with the one on S3, if existing.
If any changes appears, we upload the code/assets on S3 for the Lambda.

That way, we have a clean Repository with only the code of the Lambda; and costs optimized too.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@Cloudrage Cloudrage added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 22, 2020
@SomayaB SomayaB added @aws-cdk/assets Related to the @aws-cdk/assets package @aws-cdk/aws-lambda Related to AWS Lambda labels Apr 23, 2020
@nija-at
Copy link
Contributor

nija-at commented Jun 4, 2020

Hello @Cloudrage -

Would something like this #7898 satisfy your needs?
You could specify any arbitrary commands around packaging your code as docker commands.

@nija-at nija-at added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jun 4, 2020
@Cloudrage
Copy link
Author

Hi @nija-at !

You mean something like that ?

new lambda.Function(this, 'Function', {
  code: lambda.Code.fromAsset(path.join(__dirname, 'python-lambda-handler'), {
    bundle: {
      image: lambda.BundleImage.fromImage('python:3.6'),
      command: [ 'pip', 'install', '-r ./requirements.txt', '-t .' ],
    }
  })
});

So :

  • Target the dir of the code (.fromAsset) with, for example, the .py & requirements.txt files
  • Specify the command, for example, to install pip dependencies needed by .py
  • "image" not needed in our case

That's it ?

In that case, I think it can be more easy to set for our needs with .fromAsset; maybe just by setting an option if you're trying to build your Lambda from a ZIP file or not.

For example, a boolean of a bundle prop (or whatever) set to true/false :

  • If true, the bundle (ZIP) is built from files under the dir (and if a requirements.txt file exist, doing a "pip install -r" before)
  • If false, it means that the ZIP file is under the dir and it can be use as is.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 6, 2020
@nija-at
Copy link
Contributor

nija-at commented Jun 8, 2020

We could do that by creating a new PythonAsset class that understands the existence of a requirements.txt file. Once we're confident that the pattern in #7898 works well, we'll look into building something like this.

At the moment, we don't have plans to do this but #7898 should give you the first steps to supporting this, once it is merged and released.

Let me know if there is something here that doesn't satisfy your original use case.

@Cloudrage
Copy link
Author

Yes that's it !
I'm ok with your proposition.

Also, I've reported that issue too, to our TAM and we need now to make a call with the CDK Service Team to see what we could do about some of them in the Roadmap.
Like this one, it could be interesting to plan supporting it; It's important to our Prod needs.

@nija-at nija-at added effort/large Large work item – several weeks of effort and removed @aws-cdk/aws-lambda Related to AWS Lambda labels Jun 10, 2020
@nija-at nija-at assigned eladb and nija-at and unassigned nija-at and eladb Aug 17, 2020
@nija-at
Copy link
Contributor

nija-at commented Aug 17, 2020

@Cloudrage - I believe the new aws-lambda-python module satisfies your ask. We now do pip install automatically.

@nija-at nija-at added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Aug 17, 2020
@eladb
Copy link
Contributor

eladb commented Aug 17, 2020

Closing for now, let us know if lambda-python works for you.

@eladb eladb closed this as completed Aug 17, 2020
@Cloudrage
Copy link
Author

Aha, I've seen that new module and planned to use it this week ^^
I'll give you a feedback as soon as possible; thanks guys.

@Cloudrage
Copy link
Author

The construct will handle installing all required modules in a Lambda compatible Docker container (?)
I'm using WSL and I can't run Docker on it (working on a C9 when needed) :
iptables v1.6.1: can't initialize iptables table 'filter': Table does not exist (do you need to insmod?)

According to the Microsoft WSL page on github.com, iptables isn't supported :
microsoft/WSL#767

It seems to be possible with WSL 2 : https://docs.docker.com/docker-for-windows/wsl/
Need to try that...

@jogold
Copy link
Contributor

jogold commented Aug 19, 2020

WSL2 together with the WSL 2 Docker engine is the way to go under Windows.

(it is possible with WSL via daemon but Docker is really slow then)

@Cloudrage
Copy link
Author

Ok, so it's working with WSL2 & Docker Desktop.

Create & Build Lambda with modules from requirements.txt.
But !
Everytime I make a cdk ls it rebuild the asset...
I've test a Lambda with a huge module, boto3 for example, every cdk ls takes about 3-5min (and just for 1 Lambda) -_-'

The solution below make it the same way, too long (build with a Container without checking a hash if the Build is needed or not) :

        bundling: {
          image: lambda.Runtime.PYTHON_3_6.bundlingDockerImage,
          command: [
            'bash', '-c', `
            pip install -r requirements.txt -t /asset-output &&
            cp -au . /asset-output
            `,
          ],
        },
      }),

Like when we create a Lambda with code as ZIP file, if we update the ZIP, CDK don't know if it has been updated.

At the begining of the Request, I've said that actually, using Sceptre, we have made a hook to check the requirements.txt file under the Lambda directory to download every modules needed with pip, but in local (also in .gitignore).
After that, we create the ZIP file & compare the hash with the one on S3 (head-object), if existing.
If any changes appears, we upload the code/assets on S3 for the Lambda.

Maybe something can be done with CDK to improve these Lambda mechanism ?

@jogold
Copy link
Contributor

jogold commented Aug 20, 2020

See #9582, #9576, #9424 and #9540 where all this is tracked or being addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package closing-soon This issue will automatically close in 4 days unless further comments are made. effort/large Large work item – several weeks of effort feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

5 participants