Skip to content

Conversation

@haircommander
Copy link
Collaborator

Added Pause() and Unpause() to libpod/pod.go

Added man pages, tests and completions

Signed-off-by: haircommander pehunt@redhat.com

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 35d536c has been approved by baude

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 35d536c with merge 743350a...

rh-atomic-bot pushed a commit that referenced this pull request Jul 21, 2018
Added Pause() and Unpause() to libpod/pod.go

Added man pages, tests and completions

Signed-off-by: haircommander <pehunt@redhat.com>

Closes: #1126
Approved by: baude
@baude
Copy link
Member

baude commented Jul 21, 2018

bot, retest this please

@rh-atomic-bot
Copy link
Collaborator

💡 This pull request was already approved, no need to approve it again.

  • This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 35d536c has been approved by baude

@rh-atomic-bot
Copy link
Collaborator

💔 Test failed - status-papr

Copy link
Member

Choose a reason for hiding this comment

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

No need for else

Copy link
Member

Choose a reason for hiding this comment

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

No need for else.

Copy link
Member

Choose a reason for hiding this comment

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

Alphabetize

Copy link
Member

Choose a reason for hiding this comment

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

The logic here is wrong - err will always be set if there's an error, even if ctr_errs is not nil

Copy link
Member

Choose a reason for hiding this comment

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

Alphabetize

@haircommander haircommander force-pushed the pod-pause-unpause branch 4 times, most recently from d4d6b5b to e40ae0a Compare July 24, 2018 18:22
@mheon
Copy link
Member

mheon commented Jul 24, 2018

LGTM
@rhatdan @TomSweeneyRedHat @umohnani8 PTAL

@TomSweeneyRedHat
Copy link
Member

All kinds of test unhappiness, including travis @haircommander

Copy link
Member

Choose a reason for hiding this comment

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

Can you pause a running pod? Should this be "unpause all paused pods"?

Copy link
Member

Choose a reason for hiding this comment

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

Could be Name or ID no? I think we've other usage's that show you could do one or the other, emulate those.

Copy link
Member

Choose a reason for hiding this comment

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

Name or ID no?

@TomSweeneyRedHat
Copy link
Member

Couple nit things.
Education question for me. is there a Pod state? If so, if we have containers in the pod that are paused and others that are started in the pod, what is the Pod state?

@mheon
Copy link
Member

mheon commented Jul 24, 2018

@TomSweeneyRedHat We don't really have a solid pod state - we display one in ps but generally speaking it's just based on the states its containers are in. I think we would consider a pod Running in your given scenario, given that not all containers are paused?

@haircommander
Copy link
Collaborator Author

@TomSweeneyRedHat Yeah, there's documentation on what container state constitutes what pod state in the pod ps man page. @mheon is correct in your scenario, if there are any running containers the pod is considered running.

@mheon
Copy link
Member

mheon commented Jul 25, 2018

Should be good to merge once travis goes green

@TomSweeneyRedHat
Copy link
Member

Travis isn't hip @haircommander

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably 8ce0e0b) made this pull request unmergeable. Please resolve the merge conflicts.

@mheon
Copy link
Member

mheon commented Jul 25, 2018

@haircommander Can you squash the commits down?

@haircommander
Copy link
Collaborator Author

yeah I'm trying to figure out how without having to redo the merge again.

@mheon
Copy link
Member

mheon commented Jul 25, 2018

@haircommander git rebase -i HEAD~3 and fixup the merge commits so they squash

@haircommander
Copy link
Collaborator Author

There's some issue with the ginkgo suite and this code. It just stalls after finishing. I reproduce locally and am trying to figure out what is making it happen.

@mheon
Copy link
Member

mheon commented Jul 26, 2018

@haircommander Can you rebase against master to fix the tests?

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably f9152d0) made this pull request unmergeable. Please resolve the merge conflicts.

@rhatdan
Copy link
Member

rhatdan commented Jul 27, 2018

@haircommander Needs a rebase.

Added Pause() and Unpause() to libpod/pod.go

Added man pages, tests and completions

Signed-off-by: haircommander <pehunt@redhat.com>
@mheon
Copy link
Member

mheon commented Jul 27, 2018

I think this is the last thing I'd like in for today's release.

@rhatdan
Copy link
Member

rhatdan commented Jul 27, 2018

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit f9b853d has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit f9b853d with merge f258e43...

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr
Approved by: rhatdan
Pushing f258e43 to master...

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants