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

Update image used for image volume task #50158

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nojnhuh
Copy link
Contributor

@nojnhuh nojnhuh commented Mar 19, 2025

Description

This PR resolves issues I ran into following Use an Image Volume With a Pod with containerd:

  1. The pod gets stuck in ErrImagePull because of the following error, also observed in CI
    Failed to pull image "quay.io/crio/artifact:v1": failed to pull and unpack image "quay.io/crio/artifact:v1": number of layers and diffIDs don't match: 1 != 0
    
  2. The syntax for the kubectl attach command was invalid. I updated it to use kubectl exec instead.

Issue

Closes: #

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 19, 2025
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Mar 19, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nate-double-u for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 19, 2025
@nojnhuh
Copy link
Contributor Author

nojnhuh commented Mar 19, 2025

/cc @saschagrunert

Copy link

netlify bot commented Mar 20, 2025

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 2495311
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/67db597cafebe200083e160e
😎 Deploy Preview https://deploy-preview-50158--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@saschagrunert
Copy link
Member

We probably have to incorporate that into #49936

@@ -13,5 +13,5 @@ spec:
volumes:
- name: volume
image:
reference: quay.io/crio/artifact:v1
reference: alpine
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
reference: alpine
reference: docker.io/library/alpine:3

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Ideally, we pick an image that isn't executable the way a traditional container would be.

@network-charles
Copy link
Contributor

Testing this PR locally on Killercoda has been difficult. This part in the documentation isn't clear.

- The container runtime needs to support the image volumes feature
- You need to exec commands in the host
- You need to be able to exec into pods
- You need to enable the `ImageVolume` [feature gate](/docs/reference/command-line-tools-reference/feature-gates/)

Which container runtime supports it? I am using io.containerd.runc.v2
Which commands are executing on the host? This page doesn't include it.

@sftim
Copy link
Contributor

sftim commented Mar 22, 2025

You may not be able to use Killercoda to test this alpha feature @network-charles

@network-charles
Copy link
Contributor

Alright

We may need to remove this line, it says we can test it on Killercoda.

{{< include "task-tutorial-prereqs.md" >}} {{< version-check >}}

@sftim
Copy link
Contributor

sftim commented Mar 22, 2025

I don't think https://kubernetes.io/docs/tasks/configure-pod-container/image-volumes/#before-you-begin is outright wrong. In a separate PR, we could clarify that the playgrounds (that are external) may not support all alpha / beta features.

This PR is about changing the example volume.

@network-charles
Copy link
Contributor

Alright

Since it's a minor fix, it’d be great if @nojnhuh would resolve it here rather than opening a new PR.

@sftim
Copy link
Contributor

sftim commented Mar 22, 2025

(writing as an approver for English)
@nojnhuh you are welcome to keep this PR fixed on the one issue. We prefer to open separate issues when we spot unrelated concerns; @network-charles if you're willing to, you could open.

@network-charles
Copy link
Contributor

Alright, I will.

Can you tell me how to test the example locally?

@sftim
Copy link
Contributor

sftim commented Mar 24, 2025

Maybe we could add a hint like this to the page! Anyway, try:

minikube start --feature-gates=ImageVolume=true

@network-charles this is the wrong change though so as things stand it is not useful to put in a lot of effort to test it.

@network-charles
Copy link
Contributor

Sorry, what do you mean by “this is the wrong change”?

@sftim
Copy link
Contributor

sftim commented Mar 24, 2025

Sorry, what do you mean by “this is the wrong change”?

#50158 (review)

The image @nojnhuh is suggesting isn't a good example of an image to use as an image volume, because it's an executable image.

@nojnhuh
Copy link
Contributor Author

nojnhuh commented Mar 24, 2025

Ideally, we pick an image that isn't executable the way a traditional container would be.

My main motivation for this PR was to modify the example to also work with containerd, and that runtime doesn't support mounting non-executable OCI artifacts yet: containerd/containerd#11381

I understand mounting a "regular" container image is the less interesting use case, but that happens to be the lowest common denominator among what works with both CRI-O and containerd at the moment AFAIK.

@network-charles
Copy link
Contributor

Hi @sftim, the Alpine image (docker.io/library/alpine:3) you suggested didn't work for me on my minikube cluster.

kubectl get pod

NAME           READY   STATUS                 RESTARTS   AGE
image-volume   0/1     CreateContainerError   0          5m41s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants