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

Bump up containerd to v1.7.6 #4301

Closed
wants to merge 3 commits into from
Closed

Conversation

ktock
Copy link
Collaborator

@ktock ktock commented Oct 3, 2023

ktock added 2 commits October 3, 2023 20:29
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
@jedevc
Copy link
Member

jedevc commented Oct 3, 2023

Windows failure looks to be genuine, seems related to containerd/containerd@38d4e50.

@ktock
Copy link
Collaborator Author

ktock commented Oct 4, 2023

@jedevc It seems that platforms.Parse() on windows now fills OSVersion field with the underlying OS's version information. : https://github.com/containerd/containerd/blob/v1.7.6/platforms/platforms.go#L200

So maybe we need to explicitly clear that field in the purl package to pass the test. Another option is to fix the test to allow OSVersion field being filled. 2f5e841 implements the former but please let me know if we should take another approach.

@ktock ktock marked this pull request as draft October 4, 2023 02:47
@thaJeztah
Copy link
Member

@jedevc
Copy link
Member

jedevc commented Oct 11, 2023

@jedevc It seems that platforms.Parse() on windows now fills OSVersion field with the underlying OS's version information. : containerd/containerd@v1.7.6/platforms/platforms.go#L200

So maybe we need to explicitly clear that field in the purl package to pass the test. Another option is to fix the test to allow OSVersion field being filled. 2f5e841 implements the former but please let me know if we should take another approach.

I think the PURL should probably contain the OSVersion if it's set - I think we can handle this in the test.

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
@ktock
Copy link
Collaborator Author

ktock commented Oct 12, 2023

Thanks for the comment.

I think we can handle this in the test.

Fixed the test to handle OSVersion.

I think the PURL should probably contain the OSVersion if it's set

purl pkg seems to use platforms.Format and this always drops OSVersion.

Value: platforms.Format(p),

@jedevc
Copy link
Member

jedevc commented Oct 13, 2023

@ktock I'm going to close this, since it's succeeded by #4315 (and so we avoid having the same discussions in both places).

Feel free to re-open if we should keep it 🎉

@jedevc jedevc closed this Oct 13, 2023
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