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

fix(core): support cache-from and cache-to flags in DockerImage #26337

Merged
merged 14 commits into from Aug 25, 2023

Conversation

alukach
Copy link
Contributor

@alukach alukach commented Jul 12, 2023

In #24024, we added the ability to specify Docker cache flags during ecr-asset builds. However, it was not added to the DockerImage class. This PR adds the ability to specify the --cache-from and --cache-to flag to DockerImage builds.

This logic was primarily lifted directly from #24024.

Fixup of issues from #25925


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Jul 12, 2023

@github-actions github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Jul 12, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team July 12, 2023 16:20
@github-actions github-actions bot added the p2 label Jul 12, 2023
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@mrgrain
Copy link
Contributor

mrgrain commented Jul 12, 2023

aws-cdk-lib: /codebuild/output/src4060025597/src/github.com/aws/aws-cdk/packages/aws-cdk-lib/core/test/bundling.test.ts
aws-cdk-lib:   140:54  error  Strings must use singlequote  quotes
aws-cdk-lib:   142:84  error  Missing trailing comma        comma-dangle
aws-cdk-lib:   145:99  error  Missing trailing comma        comma-dangle

@mrgrain mrgrain added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Jul 29, 2023
mrgrain
mrgrain previously approved these changes Jul 29, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review July 29, 2023 08:53

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@mrgrain
Copy link
Contributor

mrgrain commented Jul 29, 2023

@alukach your test is failing

@alukach
Copy link
Contributor Author

alukach commented Jul 29, 2023

@alukach your test is failing

Yes, thanks. I am struggling to run tests locally, any tips on how you run the test runner for a single package?

@mrgrain
Copy link
Contributor

mrgrain commented Jul 29, 2023

@alukach your test is failing

Yes, thanks. I am struggling to run tests locally, any tips on how you run the test runner for a single package?

cd packages/aws-cdk-lib

// Run a single test suite 
npx jest core/test/bundling.test.js

// Run a test case by name
npx jest -t "cache-to"

I think they can also be combined for more performance.

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@alukach
Copy link
Contributor Author

alukach commented Aug 22, 2023

@mrgrain Thanks for the tips. Sorry to be dragging this out for so long. I'm not having any success with running these tests.

▶ npx jest core/test/bundling.test.js

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib
  1816 files checked.
  testMatch: /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib/**/test/**/?(*.)+(test).js - 1 match
  testPathIgnorePatterns: /node_modules/ - 1816 matches
  testRegex:  - 0 matches
Pattern: core/test/bundling.test.js - 0 matches

▶ npx jest core/test/bundling.test.ts   

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib
  1816 files checked.
  testMatch: /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib/**/test/**/?(*.)+(test).js - 1 match
  testPathIgnorePatterns: /node_modules/ - 1816 matches
  testRegex:  - 0 matches
Pattern: core/test/bundling.test.ts - 0 matches

▶ npx jest core/test/bundling.test.* 

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib
  1816 files checked.
  testMatch: /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib/**/test/**/?(*.)+(test).js - 1 match
  testPathIgnorePatterns: /node_modules/ - 1816 matches
  testRegex:  - 0 matches
Pattern: core/test/bundling.test.ts - 0 matches

▶ npx jest core/test/bundling.test  

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib
  1816 files checked.
  testMatch: /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib/**/test/**/?(*.)+(test).js - 1 match
  testPathIgnorePatterns: /node_modules/ - 1816 matches
  testRegex:  - 0 matches
Pattern: core/test/bundling.test - 0 matches

▶ npx jest "core/test/bundling.test"

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib
  1816 files checked.
  testMatch: /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib/**/test/**/?(*.)+(test).js - 1 match
  testPathIgnorePatterns: /node_modules/ - 1816 matches
  testRegex:  - 0 matches
Pattern: core/test/bundling.test - 0 matches

▶ npx jest "core/test/*"            

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib
  1816 files checked.
  testMatch: /Users/alukach/Projects/personal/aws-cdk/packages/aws-cdk-lib/**/test/**/?(*.)+(test).js - 1 match
  testPathIgnorePatterns: /node_modules/ - 1816 matches
  testRegex:  - 0 matches
Pattern: core/test/* - 0 matches

▶ npx jest -t "cache-to"


=============================== Coverage summary ===============================
Statements   : 13.81% ( 21/152 )
Branches     : 0% ( 0/79 )
Functions    : 0% ( 0/29 )
Lines        : 15.1% ( 21/139 )
================================================================================
Jest: "global" coverage threshold for statements (55%) not met: 13.81%
Jest: "global" coverage threshold for branches (40%) not met: 0%
Snapshot Summary
 › 5 snapshot files obsolete from 5 test suites. To remove them all, run `npm run npx -- -u`.
   ↳   • assertions/test/__snapshots__/render.test.js.snap
       • aws-lambda-nodejs/test/__snapshots__/bundling.test.js.snap
       • cloud-assembly-schema/test/__snapshots__/manifest.test.js.snap
       • cx-api/test/__snapshots__/cloud-assembly.test.js.snap
       • region-info/test/__snapshots__/region-info.test.js.snap

Test Suites: 1 skipped, 0 of 1 total
Tests:       25 skipped, 25 total
Snapshots:   5 files obsolete, 0 total
Time:        3.173 s
Ran all test suites with tests matching "cache-to".

I know that tech support likely isn't in your job description so apologies if this is laborious for you. If you have any tips on what I'm missing here, please let me know. Otherwise, I'm unsure that I can realistically build tests for this.

@mergify mergify bot dismissed mrgrain’s stale review August 22, 2023 17:07

Pull request has been modified.

@mrgrain
Copy link
Contributor

mrgrain commented Aug 22, 2023

Weird! npx jest core/test/bundling.test.ts works for me 🤔

Are you running this from within packages/aws-cdk-lib ?

@mrgrain mrgrain self-assigned this Aug 23, 2023
@mrgrain mrgrain changed the title feat(core): Support cache-from and cache-to flags in DockerImage fix(core): support cache-from and cache-to flags in DockerImage Aug 25, 2023
@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 83884c2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit d152d61 into aws:main Aug 25, 2023
9 checks passed
@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants