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: Delete the removal of sha256: prefix in imageHref #7677

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

Conversation

SoniaSandler
Copy link
Contributor

What does this PR do?

Deletes the removal of sha256: prefix in the imageHref property of the ContainerInfoUI interface
(the sha256: prefix stays in imageHref)

Screenshot / video of UI

What issues does this PR fix or reference?

Fixes #6801

How to test this PR?

  • Tests are covering the bug fix or the new feature

@SoniaSandler SoniaSandler requested review from benoitf and a team as code owners June 17, 2024 18:09
@SoniaSandler SoniaSandler requested review from axel7083 and lstocchi and removed request for a team June 17, 2024 18:10
@deboer-tim
Copy link
Collaborator

Maybe this would be a good time to start switching to the navigation API (#5556)?

@SoniaSandler
Copy link
Contributor Author

@deboer-tim I'll take a look at the navigation API

Copy link
Contributor

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

Globally this does not fix in certain scenario.

demo-error-href.mp4

Specific scenario

I am not really sure why, but in some cases it does not work, let's break the example down.

In my container list, I have a pod, and three container in it. One of the container, as visible in the video is called llamacpp-server-podified.

The link is the following reading from the Inspect.image: sha256:462f6b668ede74b92d4823a309eda3033809dee5d3bd536f78308cd5afd7aaaf

Let's go in the image list, and search this sha256

We do get the right image which is being used by the container

image

Now let's look at the url.

http://localhost:5173/#/images/sha256:462f6b668ede74b92d4823a309eda3033809dee5d3bd536f78308cd5afd7aaaf/podman.Podman%20Machine/cXVheS5pby9haS1sYWIvbGxhbWFjcHBwX3B5dGhvbjpsYXRlc3Q=/summary

Now let's compare with the blank page url.

http://localhost:5173/#/images/sha256:462f6b668ede74b92d4823a309eda3033809dee5d3bd536f78308cd5afd7aaaf/podman.Podman%20Machine/c2hhMjU2OjQ2MmY2YjY2OGVkZTc0YjkyZDQ4MjNhMzA5ZWRhMzAzMzgwOWRlZTVkM2JkNTM2Zjc4MzA4Y2Q1YWZkN2FhYWY=/summary

Let's diff

- http://localhost:5173/#/images/sha256:462f6b668ede74b92d4823a309eda3033809dee5d3bd536f78308cd5afd7aaaf/podman.Podman%20Machine/cXVheS5pby9haS1sYWIvbGxhbWFjcHBwX3B5dGhvbjpsYXRlc3Q=/summary
+ http://localhost:5173/#/images/sha256:462f6b668ede74b92d4823a309eda3033809dee5d3bd536f78308cd5afd7aaaf/podman.Podman%20Machine/c2hhMjU2OjQ2MmY2YjY2OGVkZTc0YjkyZDQ4MjNhMzA5ZWRhMzAzMzgwOWRlZTVkM2JkNTM2Zjc4MzA4Y2Q1YWZkN2FhYWY=/summary

The last part of the url id different, which is the ImageBase64RepoTag, weird...

Let's decode it:

$: atob("cXVheS5pby9haS1sYWIvbGxhbWFjcHBwX3B5dGhvbjpsYXRlc3Q=")
'quay.io/ai-lab/llamacppp_python:latest'
$: atob("c2hhMjU2OjQ2MmY2YjY2OGVkZTc0YjkyZDQ4MjNhMzA5ZWRhMzAzMzgwOWRlZTVkM2JkNTM2Zjc4MzA4Y2Q1YWZkN2FhYWY=")
'sha256:462f6b668ede74b92d4823a309eda3033809dee5d3bd536f78308cd5afd7aaaf'

Okey okey, so the ImageBase64RepoTag is one case is the one of the sha256, and the other of the image tag. This is a problem.

cc @deboer-tim @benoitf the consequence is the following: in image-utils.ts we are trying to find an image based on the base64Tag provided

const images = this.getImagesInfoUI(imageInfo, containersInfo, context, viewContributions);
return images.find(image => image.base64RepoTag === base64RepoTag);

But we construct the image href with a base64 value from the container info

imageHref: `/images/${containerInfo.ImageID}/${containerInfo.engineId}/${containerInfo.ImageBase64RepoTag}/summary`,

which seems to not exists in the images available.

Solution proposal

We are generating a list of imageInfo for the ImageDetails

imageInfo = allImages.find(c => c.Id === imageID && c.engineId === engineId);

Those image are generated by iterating over the RepoTags: string[] property like shown bellow

return imageInfo.RepoTags.map(repoTag => {

As we understood above, one condition that is not generated is the cases where the base64RepoTag is the sha256 itself. So we could add this possiblity and do the following

- return imageInfo.RepoTags.map(repoTag => {
+ return [...imageInfo.RepoTags, imageInfo.Id].map(repoTag => {

This solutions works on my computer in addition of the change made by @SoniaSandler.

Moreover, to cover all cases, we might also generate one more imageInfo for the case where there are not RepoTags

@deboer-tim
Copy link
Collaborator

Excellent digging @axel7083. I think your solution is good, but I also can't help feeling that we need to understand/define what the full unique identifier(s) are for an image, and this code should only exist with the navigation helper and either image details or utils. i.e. places like container utils should have no idea how to build the url for an image or what the unique properties are. Then we should also repeat for all other types. :)

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.

bug(ImageDetails): navigation to container image crashes Podman Desktop
3 participants