Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

Add Stop command on the gRPC side. #99

Merged
merged 3 commits into from
May 18, 2020
Merged

Add Stop command on the gRPC side. #99

merged 3 commits into from
May 18, 2020

Conversation

rumpl
Copy link
Contributor

@rumpl rumpl commented May 16, 2020

This is a mixed PR and I am really sorry about it, it contains:

  • add the --all flag to docker ps
  • added the all parameter in the interfaces and in the gRPC layer
  • added the Stop command to stop a running container (only implemented it in moby, still needs to be implemented for ACI).

I just realized there is an inconsistency in the naming, docker rm calls the Delete method, we would need to create a new method Remove for this and not use Delete on rm.

azure/backend.go Outdated Show resolved Hide resolved
azure/backend.go Outdated Show resolved Hide resolved
@@ -42,16 +42,25 @@ service Containers {
rpc Exec(ExecRequest) returns (ExecResponse);
}

message Port {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is on top of #98, otherwise not sure why we have ports, etc. added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This snuck in this PR, it's kinda related to #98 but it can stay here, they are independent.

moby/backend.go Outdated
@@ -84,6 +90,11 @@ func (ms *mobyService) Run(ctx context.Context, r containers.ContainerConfig) er
return ms.apiClient.ContainerStart(ctx, create.ID, types.ContainerStartOptions{})
}

func (ms *mobyService) Stop(ctx context.Context, containerName string) error {
timeout := 1 * time.Second
Copy link
Contributor

Choose a reason for hiding this comment

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

1 second might be too short. What does docker/cli and docker/compose use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

timeout is a parameter to stop now

cli/cmd/ps.go Outdated Show resolved Hide resolved
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

Just a fix on the options and a test to add, otherwise LGTM

@rumpl rumpl force-pushed the feat-stop branch 2 times, most recently from a428ab2 to c4a04ca Compare May 18, 2020 13:12
@rumpl rumpl merged commit cf1be96 into master May 18, 2020
@rumpl rumpl deleted the feat-stop branch May 18, 2020 14:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants