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: ensure we use the snapshot for the highest image layer #346

Merged
merged 2 commits into from
Jan 6, 2022
Merged

fix: ensure we use the snapshot for the highest image layer #346

merged 2 commits into from
Jan 6, 2022

Conversation

richardcase
Copy link
Member

@richardcase richardcase commented Jan 6, 2022

What this PR does / why we need it:

This fixes a bug where we where using the snapshot of the first layer of
a container image always. This wasn't a problem previously as the image
we where using for the root filesystem had a single layer. However, when
we started using an image with multiple layers the microvm wasn't
seeing the contents of any additional layers and instead it just saw the
first layer, which in our case meant that the additional apt packages
(like systemd) weren't available to the microvm.

For example, when using docker.io/richardcase/capmvm-ubuntu-base:20.04
which has 2 layers, it creates 2 snapshots when the image is unpacked:

KEY                                                                     PARENT                                                                  KIND
sha256:6bee76bc089d04f0afe71e48cf0f385a20b5f251380fb48364106e50101a7fbd sha256:9f54eef412758095c8079ac465d494a2872e02e90bf1fb5f12a1641c0d1bb78b Committed
sha256:9f54eef412758095c8079ac465d494a2872e02e90bf1fb5f12a1641c0d1bb78b                                                                         Committed

We where using sha256:9f54eef412758095c8079ac465d494a2872e02e90bf1fb5f12a1641c0d1bb78b
but we should be using sha256:6bee76bc089d04f0afe71e48cf0f385a20b5f251380fb48364106e50101a7fbd
when creating the active snapshot for the microvm. Notice the parent column.

This change makes sure we use the snapshot for the highest layer.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

This fixes a bug where we where using the snapshot of the first layer of
a container image always. This wasn't a problem previously as the image
we where using for the root filesystem had a single layer. However, when
we started using an image with multiple layers  the microvm wasn't
seeing the contents of any additional layers and instead it just saw the
first layer, which in our case meant that the additional apt packages
(like systemd) weren't available to the microvm.

For example, when using `docker.io/richardcase/capmvm-ubuntu-base:20.04`
which has 2 layers, it creates 2 snapshots when the image is unpacked:

```
KEY                                                                     PARENT                                                                  KIND
sha256:6bee76bc089d04f0afe71e48cf0f385a20b5f251380fb48364106e50101a7fbd sha256:9f54eef412758095c8079ac465d494a2872e02e90bf1fb5f12a1641c0d1bb78b Committed
sha256:9f54eef412758095c8079ac465d494a2872e02e90bf1fb5f12a1641c0d1bb78b                                                                         Committed
```

We where using `sha:2569f54eef412758095c8079ac465d494a2872e02e90bf1fb5f12a1641c0d1bb78b`
but we should be using `sha256:6bee76bc089d04f0afe71e48cf0f385a20b5f251380fb48364106e50101a7fbd`
when creating the active snapshot for the microvm.

This change makes sure we use the snapshot for the highest layer.

Signed-off-by: Richard Case <richard@weave.works>
@richardcase richardcase added the kind/bug Something isn't working label Jan 6, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2022

Codecov Report

Merging #346 (cde6555) into main (e5aa3b8) will increase coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #346      +/-   ##
==========================================
+ Coverage   58.12%   58.32%   +0.19%     
==========================================
  Files          51       51              
  Lines        2505     2505              
==========================================
+ Hits         1456     1461       +5     
+ Misses        932      929       -3     
+ Partials      117      115       -2     
Impacted Files Coverage Δ
infrastructure/containerd/image_service.go 90.75% <100.00%> (ø)
pkg/process/process.go 39.62% <0.00%> (+1.88%) ⬆️
infrastructure/containerd/event_service.go 59.64% <0.00%> (+3.50%) ⬆️
pkg/queue/queue.go 100.00% <0.00%> (+6.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5aa3b8...cde6555. Read the comment docs.

@richardcase richardcase merged commit 7a52fa5 into liquidmetal-dev:main Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants