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: adding config option for SOCI installation on VM #506

Merged
merged 23 commits into from
Aug 11, 2023

Conversation

CodeChanning
Copy link
Contributor

@CodeChanning CodeChanning commented Jul 28, 2023

Description of changes:
Adding snapshotter option in config file and integrating SOCI with Finch

Setting Snapshotters

  • Users can set the snapshotters they'd like to use by listing them in the config file as the snapshotters value.
  • All listed snapshotters will be installed if they are currently supported with Finch. As of this PR the two that are supported are OverlayFS (the default snapshotter) and SOCI.
  • The first snapshotter listed will be made the default snapshotter used. Other snapshotters can be used by specifying when running commands (i.e. finch --snapshotter={exampleSnapshotter} pull ...)

Example snapshotters setting in finch.yaml:

snapshotters:
    -overlayfs
    -soci

This would install SOCI on the user's VM and allow for it to be used when specified in commands, but would keep OverlayFS as the default for commands

To Install SOCI

  • SOCI can be setup with minimal configuration by adding "- soci" to the snapshotters list in finch.yaml .

  • Once the option has been set SOCI will be installed on either finch vm init or finch vm start. The binary will be downloaded onto the user's VM and the needed settings for SOCI containerd configuration will be appended to etc/containerd/config.toml in the VM. If SOCI is the first snapshotter listed it will also be set as the default nerdctl snapshotter in the user's VM which would allow the user to pull images with SOCI simply by doing finch pull ... .

To Stop Using SOCI by default

  • "- soci" should be removed or not be the first snapshotter of the snapshotters list in finch.yaml . The user can also make the first snapshotter listed "- overlayfs" to revert back to the original default used by nerdctl/containerd.

Note: removing a snapshotter from snapshotters list will not uninstall the snapshotter from the user's VM. In order to fully uninstall the snapshotter the user must shell into the VM and remove the binaries from /usr/local/bin.

Testing done:

[ x] 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.

@ningziwen
Copy link
Member

ningziwen commented Jul 28, 2023

The commit message should mention config. (We are not packaging SOCI by default)

@CodeChanning CodeChanning changed the title feat: packaging SOCI with Finch feat: adding config option for SOCI VM installation Jul 28, 2023
@CodeChanning CodeChanning changed the title feat: adding config option for SOCI VM installation feat: adding config option for SOCI installation on VM Jul 28, 2023
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/lima_config_applier.go Outdated Show resolved Hide resolved
e2e/vm/soci_test.go Outdated Show resolved Hide resolved
e2e/vm/vm_test.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
e2e/vm/soci_test.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/lima_config_applier.go Show resolved Hide resolved
@CodeChanning CodeChanning marked this pull request as ready for review August 8, 2023 22:36
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
… bool

Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
ginglis13
ginglis13 previously approved these changes Aug 9, 2023
Copy link
Contributor

@ginglis13 ginglis13 left a comment

Choose a reason for hiding this comment

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

LGTM, a few nits and CI failure tracked in #527

Also can you update this section of the PR description:

SOCI can be setup with minimal configuration by setting the soci_snapshotter option in finch.yaml with the value true.

e2e/vm/soci_test.go Outdated Show resolved Hide resolved
e2e/vm/soci_test.go Show resolved Hide resolved
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
e2e/vm/soci_test.go Outdated Show resolved Hide resolved
e2e/vm/soci_test.go Outdated Show resolved Hide resolved
e2e/vm/soci_test.go Outdated Show resolved Hide resolved
e2e/vm/soci_test.go Outdated Show resolved Hide resolved
e2e/vm/soci_test.go Outdated Show resolved Hide resolved
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
ginglis13
ginglis13 previously approved these changes Aug 10, 2023
Copy link
Contributor

@ginglis13 ginglis13 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

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.

Looks good overall, left a few comments.

pkg/config/lima_config_applier.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sam-berning sam-berning left a comment

Choose a reason for hiding this comment

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

LGTM overall, just a few minor comments

README.md Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/lima_config_applier.go Outdated Show resolved Hide resolved
pkg/config/lima_config_applier.go Show resolved Hide resolved
pkg/config/lima_config_applier.go Outdated Show resolved Hide resolved
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
…ue to make it easier for users to switch back to overlayfs from soci

Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
ningziwen
ningziwen previously approved these changes Aug 11, 2023
e2e/vm/soci_test.go Show resolved Hide resolved
pendo324
pendo324 previously approved these changes Aug 11, 2023
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.

Thanks, LGTM 👍

pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/lima_config_applier.go Outdated Show resolved Hide resolved
Signed-off-by: Channing Gaddy <chxgaddy@amazon.com>
@CodeChanning CodeChanning dismissed stale reviews from pendo324 and ningziwen via bb21ba3 August 11, 2023 18:29
Copy link
Contributor

@sam-berning sam-berning left a comment

Choose a reason for hiding this comment

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

LGTM, great work!

@ginglis13 ginglis13 merged commit a2e077b into runfinch:main Aug 11, 2023
26 checks passed
KevinLiAWS pushed a commit that referenced this pull request Aug 16, 2023
🤖 I have created a release *beep* *boop*
---


## [0.8.0](v0.7.0...v0.8.0)
(2023-08-16)


### Features

* adding config option for SOCI installation on VM
([#506](#506))
([a2e077b](a2e077b))


### Bug Fixes

* configure aws creds in sync submodules/deps action
([#518](#518))
([b67452e](b67452e))
* give pull request write permissions to sync job
([#520](#520))
([55b5235](55b5235))
* give token write perms to sync-submodules
([#519](#519))
([8b639ea](8b639ea))
* Mount /var/folders to finch vm
([#525](#525))
([c97d2e9](c97d2e9))
* option to use installed lima for SOCI e2e tests
([#533](#533))
([8b66659](8b66659))
* quote recursive calls to make
([#515](#515))
([d603096](d603096))
* Restart buildkit after containerd when provisioning
([#461](#461))
([fca1828](fca1828))


### Build System or External Dependencies

* **deps:** Bump github.com/docker/cli from 24.0.4+incompatible to
24.0.5+incompatible
([#495](#495))
([e9e8617](e9e8617))
* **deps:** Bump github.com/docker/docker from 24.0.4+incompatible to
24.0.5+incompatible
([#497](#497))
([6f1afbb](6f1afbb))
* **deps:** Bump github.com/lima-vm/lima from 0.16.0 to 0.17.2
([#531](#531))
([6e33d15](6e33d15))
* **deps:** Bump github.com/onsi/gomega from 1.27.8 to 1.27.10
([#496](#496))
([d08d102](d08d102))
* **deps:** Bump github.com/pkg/sftp from 1.13.5 to 1.13.6
([#530](#530))
([09b3846](09b3846))
* **deps:** Bump github.com/shirou/gopsutil/v3 from 3.23.6 to 3.23.7
([#513](#513))
([83bd718](83bd718))
* **deps:** Bump golang.org/x/tools from 0.11.0 to 0.11.1
([#509](#509))
([e826bcf](e826bcf))
* **deps:** Bump golang.org/x/tools from 0.11.1 to 0.12.0
([#523](#523))
([09d6514](09d6514))
* **deps:** Bump k8s.io/apimachinery from 0.27.3 to 0.27.4
([#487](#487))
([444bbc0](444bbc0))
* **deps:** Bump k8s.io/apimachinery from 0.27.4 to 0.28.0
([#535](#535))
([8df84cf](8df84cf))
* **deps:** Bump submodules and dependencies
([#521](#521))
([1b3ad94](1b3ad94))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

5 participants