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

Issues related to missing layer (likely caused by https://github.com/containerd/containerd/issues/8973 ) #3425

Open
apostasie opened this issue Sep 10, 2024 · 7 comments · May be fixed by #3435
Labels
bug Something isn't working
Milestone

Comments

@apostasie
Copy link
Contributor

apostasie commented Sep 10, 2024

Description

This here is to collect issues that may be caused by containerd/containerd#8973

How to solve it is unclear at this point.

Steps to reproduce the issue

na

Describe the results you received and expected

na

What version of nerdctl are you using?

main

Are you using a variant of nerdctl? (e.g., Rancher Desktop)

None

Host information

No response

@apostasie apostasie added the kind/unconfirmed-bug-claim Unconfirmed bug claim label Sep 10, 2024
@apostasie
Copy link
Contributor Author

Tagging @lingdie who likely has a good understanding of whats going on.

@apostasie
Copy link
Contributor Author

@AkihiroSuda while this is still murky, I feel this is bad enough given the number of issues.
Do you think this could be added to the v2 milestone?

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Sep 10, 2024
@AkihiroSuda AkihiroSuda added bug Something isn't working and removed kind/unconfirmed-bug-claim Unconfirmed bug claim labels Sep 10, 2024
@apostasie
Copy link
Contributor Author

@AkihiroSuda containerd/containerd#8973 is stale, despite pings.

What is your opinion?

  • do you think this should be a blocker for containerd v2 release? Or otherwise any chance to get eyeballs on it?
  • or should we try harder to work around it here (not sure this is even feasible honestly)?

The suggestion over there ("Changing the default pull behavior should be up to the client") is a bit asinine TBH, as it would have wide ranging impact on performance...

@apostasie
Copy link
Contributor Author

apostasie commented Sep 18, 2024

@lingdie your patch was definitely going in the right direction, but I guess if the layers are missing, it will not be enough?

What if we check if there are missing layers, then force a fetch?

Something in the line of:

_, _, _, missing, err := images.Check(ctx, client.ContentStore(), im.Target, platform)
if len(missing) > 0 {
    img, err = client.Fetch(ctx, ref, opts...)
}

// unpack the image to snapshotter
cimg := containerd.NewImage(client, img)
if err := cimg.Unpack(ctx, snName); err != nil {
    return emptyDigest, err
}

Since this is a general problem across our codebase (and not just on commit), we need a strategy to address this.
It seems like once the image has been tagged, it is too late? (eg: can't fetch anymore) - so, we might need to enforce fetch:

  • at tagging time
  • at commit time

wdyt?

@apostasie
Copy link
Contributor Author

Here is a repro involving tagging:

nerdctl rm -f $(nerdctl ps -aq) 2>&1 >/dev/null
nerdctl rmi -f $(nerdctl images -q) 2>&1 >/dev/null
nerdctl image pull busybox 2>&1 >/dev/null
nerdctl run -d busybox 2>&1 >/dev/null
nerdctl image rm -f busybox 2>&1 >/dev/null
nerdctl pull busybox
nerdctl tag busybox myregistry:5000/test
nerdctl push myregistry:5000/test

This will fail when we try to convert to a temp single platform image (because we tagged an image with missing layers).

@AkihiroSuda
Copy link
Member

a blocker for containerd v2 release

I'd prefer to limit the blockers to:

  • regressions in v2
  • new features in v2, merged with a wrong design

@apostasie
Copy link
Contributor Author

a blocker for containerd v2 release

I'd prefer to limit the blockers to:

  • regressions in v2
  • new features in v2, merged with a wrong design

Fair enough.

I might have a workable fix in the linked PR.

apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 19, 2024
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 19, 2024
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 21, 2024
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 24, 2024
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 24, 2024
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 24, 2024
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 24, 2024
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants