Skip to content

When fetching the pid counts for the container the state can be invalid sometimes#1807

Merged
katiewasnothere merged 1 commit intomicrosoft:mainfrom
jsturtevant:fix-pids-query
Jun 15, 2023
Merged

When fetching the pid counts for the container the state can be invalid sometimes#1807
katiewasnothere merged 1 commit intomicrosoft:mainfrom
jsturtevant:fix-pids-query

Conversation

@jsturtevant
Copy link
Copy Markdown
Contributor

@jsturtevant jsturtevant commented Jun 10, 2023

This uses the same logic for pods that are in invalid states when the properties are queried. Found this while testing kubernetes/kubernetes#116968 and the "not found error" will allow to handle gracefully in containerd/containerd#8671.

@jsturtevant
When fetch pid counts the contianer can be in an invalid state sometimes
2095093 
Signed-off-by: James Sturtevant <jstur@microsoft.com>
@jsturtevant jsturtevant requested a review from a team as a code owner June 10, 2023 00:19
@jsturtevant jsturtevant mentioned this pull request Jun 10, 2023
Merged
@jsturtevant
Copy link
Copy Markdown
Contributor Author

jsturtevant commented Jun 12, 2023

I don't think the integration test failures are related:

        	Error:      	Received unexpected error:
        	            	rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: hcs::CreateComputeSystem 3dcbdde33c007e0da6090e918bc3b398d1bbff9b2e1c40575e0c1225b42f606f: The parameter is incorrect.: unknown
        	Test:       	TestContainerSymlinkVolumes/symlinkedFileInSymlinkedFolder
--- FAIL: TestContainerSymlinkVolumes (21.17s)
    --- FAIL: TestContainerSymlinkVolumes/file_in_symlinked_folder (7.05s)
    --- FAIL: TestContainerSymlinkVolumes/symlinked_file (7.04s)
    --- FAIL: TestContainerSymlinkVolumes/symlinkedFileInSymlinkedFolder (7.08s)

@jsturtevant jsturtevant changed the title When fetch pid counts the contianer can be in an invalid state sometimes When fetching the pid counts for the container the state can be invalid sometimes Jun 12, 2023
@jsturtevant
Copy link
Copy Markdown
Contributor Author

jsturtevant commented Jun 12, 2023

@katiewasnothere as you've helped me in this area before

@helsaawy
Copy link
Copy Markdown
Contributor

helsaawy commented Jun 12, 2023

I don't think the integration test failures are related:

Those tests have been failing consistently, so the failures are safe (ish?) to ignore

katiewasnothere
katiewasnothere approved these changes Jun 15, 2023
View reviewed changes
Copy link
Copy Markdown

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

LGTM, thanks :)

@katiewasnothere katiewasnothere merged commit d8cf194 into microsoft:main Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kiashok kiashok kiashok approved these changes

+1 more reviewer

@katiewasnothere katiewasnothere katiewasnothere approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jsturtevant @helsaawy @katiewasnothere @kiashok