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 Linux support #1023

Merged
merged 39 commits into from
Sep 16, 2024
Merged

feat: add Linux support #1023

merged 39 commits into from
Sep 16, 2024

Conversation

pendo324
Copy link
Member

@pendo324 pendo324 commented Jul 9, 2024

Issue #, if available:

Description of changes:
Adds Linux support to Finch by creating a new "native" code path that doesn't use Lima and instead relies on the system packages for nerdctl/containerd/etc. being properly configured.

TODO in this PR:

  • Fix e2e tests
  • Add environment variable overrides for nerdctl config and buildkit socket paths

Because this diff is already huge, TODO in followup PR(s):

  • Add e2e tests for Linux
    • Right now, they're all excluded from building to avoid fixing lint errors and refactoring
  • Increase Linux/native specific unit test coverage: test(linux): add unit tests for files which were refactored for Linux support #1058
  • Fix semantics around what toggling the soci snapshotter or ecr-credential-helper actually means on Linux (e.g. toggling environment variables or config files)
  • Fix what logs a support bundle should include on Linux (e.g. dumping journalctl)

Testing done:

  • 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.

Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
@pendo324 pendo324 marked this pull request as ready for review July 26, 2024 18:30
@pendo324 pendo324 self-assigned this Jul 26, 2024
Makefile Outdated Show resolved Hide resolved
@pendo324
Copy link
Member Author

pendo324 commented Jul 29, 2024

squash the commits may be unless they have actual feat that are separable

We do that automatically on merge

will finch run all its command as sudo?

Finch itself doesn't have to run commands as sudo, but right now since we don't support rootless, user's will need to either be root, use setuid, or run sudo finch for Finch to read it's config file and execute nerdctl commands successfully

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
cmd/finch/main_native.go Show resolved Hide resolved
cmd/finch/support_bundle_native.go Show resolved Hide resolved
cmd/finch/version_native.go Outdated Show resolved Hide resolved
pkg/command/nerdctl.go Show resolved Hide resolved
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
@henry118
Copy link
Member

LGTM in general, can you address the test failures?

@pendo324
Copy link
Member Author

LGTM in general, can you address the test failures?

Re-ran the tests

@pendo324 pendo324 merged commit 82f698b into runfinch:main Sep 16, 2024
22 checks passed
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.

4 participants