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

feat: Add Dockercompat Mode & DevContainer Support #1100

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

chews93319
Copy link
Contributor

@chews93319 chews93319 commented Sep 19, 2024

Issue #, if available: #387, #745

Description of changes:
This PR will introduce parsing changes to support docker compatibility for various DevContainer invocations from VsCode.
The introduction of dockercompat: true within Finch configuration will specifically activate certain Finch behavior to map Docker cli arguments to compatible nerdctl arguments.

Testing done:

  • Manual testing on Apple M1 (arm64) launching devContainers via VsCode

  • Unit Tests

    • Pre-existing
    • New within nerdctl_darwin_test.go
  • Pre-existing E2E Tests

    • Possible flakiness related to "container/cosign_test.go" for macos-e2e-tests (13, test-e2e-vm-serial, X64, test) and windows-e2e-tests (test-e2e-container, amd64, test)
  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

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

@chews93319 chews93319 changed the title Dockercompat feat: Add Dockercompat Mode & DevContainer Support Sep 20, 2024
@chews93319 chews93319 force-pushed the dockercompat branch 3 times, most recently from 9a7ecd8 to 999371e Compare September 21, 2024 05:26
@chews93319
Copy link
Contributor Author

With commit 999371e, I resolved the buildx bug and confirmed that the handleCache is actually working correctly and extracting the "consistency" strings from the args.

Unfortunately, my devContainers experiment was now able to build an image, but unable to open it due to the length of identifier.

FATA[0000] invalid name "vsc-arbtest_devcontainers_features-f31e83368f02f49b07b37f8b815ef72280fdebe4e5bb5de366af337b5e70ae4a-features-62cc1": identifier "vsc-arbtest_devcontainers_features-f31e83368f02f49b07b37f8b815ef72280fdebe4e5bb5de366af337b5e70ae4a-features-62cc1" greater than maximum length (76 characters): invalid argument 

@chews93319 chews93319 force-pushed the dockercompat branch 2 times, most recently from de74f5b to 9672022 Compare September 27, 2024 07:25
@chews93319
Copy link
Contributor Author

The update to commit 9672022 is an incremental improvement:

  • repaired the compile error for linux e2e tests
  • refactored handleBuildx for use in all 3 platforms (Darwin, Windows, Native)
  • refactored Docker build --load handler for use in all 3 platforms (Darwin, Windows, Native)

cmd/finch/nerdctl_remote.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
@chews93319 chews93319 force-pushed the dockercompat branch 3 times, most recently from 4afe739 to 8cfbc1f Compare October 1, 2024 21:20
Signed-off-by: Sam Chew <stchew@amazon.com>
pendo324
pendo324 previously approved these changes Oct 1, 2024
Copy link
Member

@pendo324 pendo324 left a comment

Choose a reason for hiding this comment

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

There are still linting errors, but LGTM as long as all of the automated approval workflows pass

Shubhranshu153
Shubhranshu153 previously approved these changes Oct 2, 2024
Signed-off-by: Sam Chew <stchew@amazon.com>
@Shubhranshu153 Shubhranshu153 marked this pull request as ready for review October 2, 2024 08:15
@Shubhranshu153 Shubhranshu153 merged commit c004516 into runfinch:main Oct 2, 2024
28 checks passed
@chews93319 chews93319 deleted the dockercompat branch October 2, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants