Skip to content

Adding status command to container - #246

Closed
rajasec wants to merge 1 commit into
opencontainers:masterfrom
rajasec:containerstatus
Closed

Adding status command to container#246
rajasec wants to merge 1 commit into
opencontainers:masterfrom
rajasec:containerstatus

Conversation

@rajasec

@rajasec rajasec commented Sep 4, 2015

Copy link
Copy Markdown
Contributor

@crosbymichael @mrunalp
Quick way to check the container status from CLI, It provides the message to the user whether container is running checkpointed destroyed or paused ( frozen)
I've checked with Running,Checkpointed container, but frozen container, currently it will give the status as Running ( will give the right status once after #218 closure)

Usage:
runc status

Signed-off-by: Rajasekaran rajasec79@gmail.com

Comment thread status.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not handle all status? We have pausing left.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is what I would do and then just printf the value as a %s

diff --git a/libcontainer/container.go b/libcontainer/container.go
index a24b71b..f0ca79c 100644
--- a/libcontainer/container.go
+++ b/libcontainer/container.go
@@ -13,6 +13,21 @@ import (
 // The status of a container.
 type Status int

+func (s Status) String() string {
+   switch s {
+   case Running:
+       return "running"
+   case Pausing:
+       return "pausing"
+   case Paused:
+       return "paused"
+   case Checkpointed:
+       return "checkpointed"
+   case Destroyed:
+       return "destroyed"
+   }
+   return ""
+}
+
 const (
    // The container exists and is running.
    Running Status = iota + 1

@hqhq

hqhq commented Sep 7, 2015

Copy link
Copy Markdown
Contributor

Personally I think we have too many state related APIs in libcontainer

Status() (Status, error)
State() (*State, error)
Stats() (*Stats, error)

They have overlaps and the naming is confusing. We should reconsider it when we design runC commands, and I doubt a signal status worth being a separate command.

@thaJeztah

Copy link
Copy Markdown
Member

I think the last one is for statistics, not status;

Stats() (*Stats, error)

@rajasec

rajasec commented Sep 7, 2015

Copy link
Copy Markdown
Contributor Author

@hqhq
Stats is for statistics not for the status
About pausing, Is frozen(paused) state is more critical than giving the message as Pausing ( freezing) ? Moreover I did not test much on to the pausing status.

@hqhq

hqhq commented Sep 7, 2015

Copy link
Copy Markdown
Contributor

@thaJeztah @rajasec You are right yes it's about statistics, just think the name is so alike as state :)
Maybe we can combine state and status?

@hqhq

hqhq commented Sep 7, 2015

Copy link
Copy Markdown
Contributor

@rajasec Never mind, I found we have pausing defined, but never used.

Comment thread status.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You do not need explicit breaks in Go for switch statements.

Signed-off-by: Rajasekaran <rajasec79@gmail.com>

Fixed the review comments

Signed-off-by: rajasec <rajasec79@gmail.com>
@rajasec

rajasec commented Sep 16, 2015

Copy link
Copy Markdown
Contributor Author

@crosbymichael
Thanks for the comment, I have modified the container.go as per your previous comment to have the customer type declaration string for status. Eliminated the complete switch cases CLI status.go file

@LK4D4

LK4D4 commented Oct 12, 2015

Copy link
Copy Markdown
Contributor

@rajasec I think it'll be redundant after merging #311. You can get that info from state and state will be same for all OCI implementations, so depending on feature of runc(not OCI) isn't good thing.

@LK4D4 LK4D4 closed this Oct 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants