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

Add shimdiag flag to find shim process ID for shims #912

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Dec 16, 2020

  • Adds a new -pids flag to the shimdiag list command that will print out the
    process ID of the shim executable.

Use case would be to forcefully kill a shim if a pod was ever in a "stuck" state.

Sample output:

PS C:> shimdiag.exe list -pids
k8s.io-3719754aab39925924b0beb27d2a195a4a6784c6ebe3690b81b2e7274a7021af ==> 65892
k8s.io-908ea3697874891fd5814b1967515d3214257b65fc1974bdafd17276ee8ba3e5 ==> 69444

Signed-off-by: Daniel Canter [email protected]

@dcantah dcantah requested a review from a team as a code owner December 16, 2020 06:09
@dcantah
Copy link
Contributor Author

dcantah commented Dec 16, 2020

Remembered I had a branch with a bunch of shimdiag additions and saw that there was an item in the backlog that had one of the things in the branch so shrugs haha. Not sure if this was what was envisioned for it or if it'd be preferred as its own command.

@katiewasnothere
Copy link
Contributor

Could you add a use case/motivation case to the description?

@dcantah
Copy link
Contributor Author

dcantah commented Dec 16, 2020

@katiewasnothere done

}
fmt.Fprintf(w, "%s \t %d\n", shim, pid)
} else {
fmt.Println(shim)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to use the tabwriter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, here's the output of both:

PS C:\ContainerPlat> .\shimdiag.exe list
k8s.io-10ca615f457902b0a31252ad299f64bd22e0a09b309eb36e0bf5b61376f563b1
k8s.io-128d6078c12b8df8a219ef3569b7542f4d19cc3615f7ca7f130724a911e92d21
k8s.io-f5b13b1e43e3223f3591a5a4e20938643453a9fd88ad21eec0274e4b600abcc5

PS C:\ContainerPlat> .\shimdiag.exe list -pids
Shim Pid
k8s.io-10ca615f457902b0a31252ad299f64bd22e0a09b309eb36e0bf5b61376f563b1 48420
k8s.io-128d6078c12b8df8a219ef3569b7542f4d19cc3615f7ca7f130724a911e92d21 30272
k8s.io-f5b13b1e43e3223f3591a5a4e20938643453a9fd88ad21eec0274e4b600abcc5 17992

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lovely formatting github, here's a pic instead.
listpids

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of making the tab writer then when pids is not specified? Might as well use it if it doesn't cause fformat issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason other than we have to make it before because we have to flush it after the loop so it can't be made inside.

* Adds a new -pids flag to the `shimdiag list` command that will print out the
process ID of the shim executable.

Sample output:

PS C:\> shimdiag.exe list -pids
Shim                                                                     Pid
k8s.io-3719754aab39925924b0beb27d2a195a4a6784c6ebe3690b81b2e7274a7021af  65892
k8s.io-908ea3697874891fd5814b1967515d3214257b65fc1974bdafd17276ee8ba3e5  69444

Signed-off-by: Daniel Canter <[email protected]>
Copy link
Contributor

@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

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.

2 participants