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

ci: Add workflow to sync finch submodules #271

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

vsiravar
Copy link
Contributor

@vsiravar vsiravar commented Mar 7, 2023

Issue #, if available: N/A

Description of changes:
Add workflow to update submodules in finch.

Details

  1. Workflow runs daily at 9am UTC.
  2. Has a workflow_dispatch trigger for manually running the workflow in addition to the regular daily cadence.
    Testing done:
    Yes. See build(deps): Bump submodules vsiravar/finch-public#2
    Test with updating FINCH_OS_BASENAME : https://github.com/vsiravar/finch-public/pull/4/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52
  • 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.

submodules

Signed-off-by: Vishwas Siravara <[email protected]>
@vsiravar vsiravar requested review from AnqiPang and KevinLiAWS March 7, 2023 21:40
@vsiravar vsiravar changed the title ci: Add workflow to sync finch ci: Add workflow to sync finch submodules Mar 8, 2023
Copy link
Contributor

@davidhsingyuchen davidhsingyuchen left a comment

Choose a reason for hiding this comment

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

From #255, it seems that FINCH_OS_BASENAME has to be in sync with what's under finch-core/hashes. I'm not very sure about this, but maybe it makes sense to also update FINCH_OS_BASENAME when updating submodules? /cc @pendo324

@vsiravar
Copy link
Contributor Author

vsiravar commented Mar 8, 2023

From #255, it seems that FINCH_OS_BASENAME has to be in sync with what's under finch-core/hashes. I'm not very sure about this, but maybe it makes sense to also update FINCH_OS_BASENAME when updating submodules? /cc @pendo324

Makes sense, finch-core uses sha512 to check os base image file integrity while in the finch.yaml file we use sha256. I guess that's the reason why we need FINCH_OS_BASENAME in finch's Makefile. Added script to update FINCH_OS_BASENAME in the Makefile along with updating submodules. Added test in the description. cc @davidhsingyuchen @pendo324

pendo324
pendo324 previously approved these changes Mar 9, 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.

Yeah, the comments in the PR are correct; the name has to be in sync. I'm actually wondering if its possible to remove this condition, but for now, this action looks good 👍

.github/bin/update-os-image.sh Outdated Show resolved Hide resolved
.github/workflows/sync-submodules.yaml Outdated Show resolved Hide resolved
Signed-off-by: Vishwas Siravara <[email protected]>
@vsiravar vsiravar force-pushed the vsiravar/sync-submodules branch from e7a732d to e72182f Compare March 9, 2023 22:33
@vsiravar vsiravar merged commit 14db6ec into runfinch:main Mar 9, 2023
vsiravar added a commit to vsiravar/finch-public that referenced this pull request Mar 19, 2023
* Add workflow to sync finch
submodules

Signed-off-by: Vishwas Siravara <[email protected]>

* fix: print debug logs when lima disk command fails (runfinch#270)

## Before

```sh
➜  finch git:(main) ✗ ./_output/bin/finch vm init
FATA[0000] exit status 1
```

## After

```sh
➜  finch git:(disk-combined-output) ✗ ./_output/bin/finch vm init
FATA[0000] failed to create disk, debug logs:
time="2023-03-07T13:35:02-08:00" level=fatal msg="disk \"finch\" already exists (\"/Users/davidhyc/dev/runfinch/finch/_output/lima/data/_disks/finch\")"
```

## Notes

The concept behind this PR is from:
https://github.com/runfinch/finch/blob/674b3794fe5f5902c264d5327024fb92e147e60f/cmd/finch/virtual_machine_init.go#L97-L101

## 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: Hsing-Yu (David) Chen <[email protected]>

* fix: print debug logs after newline (runfinch#273)

## Why

It increases readability because debug logs can be multi-line.

## Before

```sh
➜  finch git:(main) ./_output/bin/finch vm init
INFO[0000] Initializing and starting Finch virtual machine...
ERRO[0000] Finch virtual machine failed to start, debug logs: time="2023-03-07T14:50:40-08:00" level=info msg="Terminal is not available, proceeding without opening an editor"
time="2023-03-07T14:50:40-08:00" level=fatal msg="field `images[0].digest` is invalid: sha256:156de3fd8a0c7e80dea9054aa9a0873e111efc16e5d8519929f913a1ca5ae9: invalid checksum digest length"
FATA[0000] exit status 1
```

## After

```sh
➜  finch git:(debug-logs-newline) ✗ ./_output/bin/finch vm init
INFO[0000] Initializing and starting Finch virtual machine...
ERRO[0000] Finch virtual machine failed to start, debug logs:
time="2023-03-07T14:49:45-08:00" level=info msg="Terminal is not available, proceeding without opening an editor"
time="2023-03-07T14:49:45-08:00" level=fatal msg="field `images[0].digest` is invalid: sha256:156de3fd8a0c7e80dea9054aa9a0873e111efc16e5d8519929f913a1ca5ae9: invalid checksum digest length"
FATA[0000] exit status 1
```

## 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: Hsing-Yu (David) Chen <[email protected]>

* ci: use the Go version in go.mod instead of a hard-coded one (runfinch#276)

## Summary

PR is a follow-up of runfinch#257.

## 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: Hsing-Yu (David) Chen <[email protected]>

* docs: make 'git pull' automatically update submodules (runfinch#274)

## Why

Before this PR, if the pinned commit of any submodule is updated in the
remote tracking branch, folks need to explicitly add `--recurse` when
running `git pull` to also update the submodule to the updated commit.
Regarding our usage of submodules, since we only consume upstream
updates (i.e., not modifying them), and there're not many submodules,
which means that recursively updating them won't take a lot of time,
it's likely that we always want to recursively update all the submodules
when running `git pull`.

## Steps to Reproduce

Setup:

```sh
git clone --recurse-submodules --branch v0.4.0 https://github.com/runfinch/finch.git
cd finch
git checkout -b demo
git branch --set-upstream-to origin/main
git pull
```

Submodules are not updated:

```sh
➜  finch git:(demo) ✗ git --no-pager diff
diff --git a/deps/finch-core b/deps/finch-core
index eef2102..01e6162 160000
--- a/deps/finch-core
+++ b/deps/finch-core
@@ -1 +1 @@
-Subproject commit eef21029b89d7db00bddc6e426e4405c920b13ed
+Subproject commit 01e6162d6fd76fddb9d8ef59845c782a0b6ebafd
```

Fix it:

```sh
➜  finch git:(demo) ✗ git config submodule.recurse true
➜  finch git:(demo) ✗ git pull
Fetching submodule deps/finch-core
Fetching submodule deps/finch-core/src/lima
Fetching submodule deps/finch-core/src/socket_vmnet
Already up to date.
Submodule path 'deps/finch-core': checked out 'eef21029b89d7db00bddc6e426e4405c920b13ed'
Submodule path 'deps/finch-core/src/socket_vmnet': checked out 'ee27d206872fc861c2993264be93d2ccc2740f9c'
➜  finch git:(demo) git --no-pager diff
➜  finch git:(demo) echo $?
0
```

## Notes

If `submodule.stickyRecursiveClone` is set, and the repository is cloned
with `--recurse-submodules`, it'd also work, but it does not account for
the case when the repository is already cloned without
`--recurse-submodules`, so setting `git config submodule.recurse true`
seems to be the simplest way that works for all scenarios.

## 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: Hsing-Yu (David) Chen <[email protected]>

* ci: Add workflow to sync finch submodules (runfinch#271)

Issue #, if available: N/A

*Description of changes:*
Add workflow to update submodules in `finch`. 

#### Details
1. Workflow runs daily at 9am UTC. 
2. Has a `workflow_dispatch` trigger for manually running the workflow
in addition to the regular daily cadence.
*Testing done:*
Yes. See #2
Test with updating `FINCH_OS_BASENAME` :
https://github.com/vsiravar/finch-public/pull/4/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52


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

---------

Signed-off-by: Vishwas Siravara <[email protected]>
Signed-off-by: Vishwas Siravara <[email protected]>

* PR 1 with a new interface for testing reverting a PR

* PR 2 which uses previoulsy created interface in PR1

* build(deps): Bump submodules (runfinch#281)

Automated changes by
[create-pull-request](https://github.com/peter-evans/create-pull-request)
GitHub action

Signed-off-by: GitHub <[email protected]>
Co-authored-by: vsiravar <[email protected]>

* test: Improve test coverage for additional disk feature (runfinch#280)

Issue #, if available:
Improving the e2e test coverage for additional disk.
*Description of changes:*
Adding test to retaining volume, network, and restart the container
after the VM is removed
*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.

---------

Signed-off-by: Ang Zhou <[email protected]>

* ci: add sanity test for running multiple VM instances in release test (runfinch#278)

Issue #, if available:

*Description of changes:*

*Testing done:*
https://github.com/runfinch/finch/actions/runs/4368693657/workflow
https://github.com/runfinch/finch/actions/runs/4367945800


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

---------

Signed-off-by: Anqi Pang <[email protected]>

* ci: Add schedule to update-deps workflow (runfinch#292)

Issue #, if available:

*Description of changes:*
Schedule [update
dependencies](https://github.com/runfinch/finch/blob/main/.github/workflows/update-deps.yaml)
workflow which is currently triggered only by `workflow_dispatch`. This
workflow will run after dependencies are built in `finch-core` by
https://github.com/runfinch/finch-core/blob/main/.github/workflows/release.yaml(runs
at 9 am UTC on Tuesday) at 11 am UTC on Tuesday.
*Testing done:*
Yes. 
runfinch#291

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

Signed-off-by: Vishwas Siravara <[email protected]>

* build(deps): bump github.com/onsi/ginkgo/v2 from 2.9.0 to 2.9.1 (runfinch#285)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump github.com/runfinch/common-tests from 0.6.1 to 0.6.2 (runfinch#300)

Bumps
[github.com/runfinch/common-tests](https://github.com/runfinch/common-tests)
from 0.6.1 to 0.6.2.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/runfinch/common-tests/releases">github.com/runfinch/common-tests's
releases</a>.</em></p>
<blockquote>
<h2>v0.6.2</h2>
<h2><a
href="https://github.com/runfinch/common-tests/compare/v0.6.1...v0.6.2">0.6.2</a>
(2023-03-16)</h2>
<h3>Bug Fixes</h3>
<ul>
<li>Fix tests to match nerdctl 1.2.1 outputs (<a
href="https://redirect.github.com/runfinch/common-tests/issues/50">#50</a>)
(<a
href="https://github.com/runfinch/common-tests/commit/3d9b4f4794d8df965dd2d611b2bed59aabff7dc2">3d9b4f4</a>)</li>
</ul>
<h3>Build System or External Dependencies</h3>
<ul>
<li><strong>deps:</strong> bump github.com/onsi/ginkgo/v2 from 2.8.3 to
2.8.4 (<a
href="https://redirect.github.com/runfinch/common-tests/issues/41">#41</a>)
(<a
href="https://github.com/runfinch/common-tests/commit/a9476c13bc4febd40a4f98cc8e6f8eebc04cfb5e">a9476c1</a>)</li>
<li><strong>deps:</strong> bump github.com/onsi/gomega from 1.27.1 to
1.27.2 (<a
href="https://redirect.github.com/runfinch/common-tests/issues/40">#40</a>)
(<a
href="https://github.com/runfinch/common-tests/commit/e8fc71a9c94afe2084bfdb129de5f5828adfa8b8">e8fc71a</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/runfinch/common-tests/blob/main/CHANGELOG.md">github.com/runfinch/common-tests's
changelog</a>.</em></p>
<blockquote>
<h2><a
href="https://github.com/runfinch/common-tests/compare/v0.6.1...v0.6.2">0.6.2</a>
(2023-03-16)</h2>
<h3>Bug Fixes</h3>
<ul>
<li>Fix tests to match nerdctl 1.2.1 outputs (<a
href="https://redirect.github.com/runfinch/common-tests/issues/50">#50</a>)
(<a
href="https://github.com/runfinch/common-tests/commit/3d9b4f4794d8df965dd2d611b2bed59aabff7dc2">3d9b4f4</a>)</li>
</ul>
<h3>Build System or External Dependencies</h3>
<ul>
<li><strong>deps:</strong> bump github.com/onsi/ginkgo/v2 from 2.8.3 to
2.8.4 (<a
href="https://redirect.github.com/runfinch/common-tests/issues/41">#41</a>)
(<a
href="https://github.com/runfinch/common-tests/commit/a9476c13bc4febd40a4f98cc8e6f8eebc04cfb5e">a9476c1</a>)</li>
<li><strong>deps:</strong> bump github.com/onsi/gomega from 1.27.1 to
1.27.2 (<a
href="https://redirect.github.com/runfinch/common-tests/issues/40">#40</a>)
(<a
href="https://github.com/runfinch/common-tests/commit/e8fc71a9c94afe2084bfdb129de5f5828adfa8b8">e8fc71a</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/runfinch/common-tests/commit/da6f643de4f9b9e1e2345c8195d968da9281a936"><code>da6f643</code></a>
chore(main): release 0.6.2 (<a
href="https://redirect.github.com/runfinch/common-tests/issues/44">#44</a>)</li>
<li><a
href="https://github.com/runfinch/common-tests/commit/3d9b4f4794d8df965dd2d611b2bed59aabff7dc2"><code>3d9b4f4</code></a>
fix: Fix tests to match nerdctl 1.2.1 outputs (<a
href="https://redirect.github.com/runfinch/common-tests/issues/50">#50</a>)</li>
<li><a
href="https://github.com/runfinch/common-tests/commit/e8fc71a9c94afe2084bfdb129de5f5828adfa8b8"><code>e8fc71a</code></a>
build(deps): bump github.com/onsi/gomega from 1.27.1 to 1.27.2 (<a
href="https://redirect.github.com/runfinch/common-tests/issues/40">#40</a>)</li>
<li><a
href="https://github.com/runfinch/common-tests/commit/a9476c13bc4febd40a4f98cc8e6f8eebc04cfb5e"><code>a9476c1</code></a>
build(deps): bump github.com/onsi/ginkgo/v2 from 2.8.3 to 2.8.4 (<a
href="https://redirect.github.com/runfinch/common-tests/issues/41">#41</a>)</li>
<li>See full diff in <a
href="https://github.com/runfinch/common-tests/compare/v0.6.1...v0.6.2">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/runfinch/common-tests&package-manager=go_modules&previous-version=0.6.1&new-version=0.6.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): Bump lima version (runfinch#302)

Automated changes by
[create-pull-request](https://github.com/peter-evans/create-pull-request)
GitHub action

Signed-off-by: GitHub <[email protected]>
Co-authored-by: vsiravar <[email protected]>

* build(deps): Bump submodules (runfinch#304)

Automated changes by
[create-pull-request](https://github.com/peter-evans/create-pull-request)
GitHub action

Signed-off-by: GitHub <[email protected]>
Co-authored-by: ahsan-z-khan <[email protected]>

---------

Signed-off-by: Vishwas Siravara <[email protected]>
Signed-off-by: Hsing-Yu (David) Chen <[email protected]>
Signed-off-by: Vishwas Siravara <[email protected]>
Signed-off-by: GitHub <[email protected]>
Signed-off-by: Ang Zhou <[email protected]>
Signed-off-by: Anqi Pang <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Hsing-Yu (David) Chen <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: vsiravar <[email protected]>
Co-authored-by: Ang Zhou <[email protected]>
Co-authored-by: Anqi Pang <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: ahsan-z-khan <[email protected]>
@vsiravar vsiravar deleted the vsiravar/sync-submodules branch August 28, 2023 17:19
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