areed/ch3130/container inspect all#36
Merged
areed merged 1 commit intoreplicatedcom:masterfrom Nov 7, 2017
areed:areed/ch3130/container-inspect-all
Merged
areed/ch3130/container inspect all#36areed merged 1 commit intoreplicatedcom:masterfrom areed:areed/ch3130/container-inspect-all
areed merged 1 commit intoreplicatedcom:masterfrom
areed:areed/ch3130/container-inspect-all
Conversation
emosbaugh
requested changes
Nov 6, 2017
Member
emosbaugh
left a comment
emosbaugh
left a comment
There was a problem hiding this comment.
you can take or leave some of this feedback
| // convert labels to field args | ||
| args := filters.NewArgs() | ||
| for k, v := range spec.Config.ContainerLabels { | ||
| label := fmt.Sprintf("%s=%s", k, v) |
Member
There was a problem hiding this comment.
i think you should make v optional, see below
vagrant@default:/vagrant$ sudo docker container ls --filter=label=com.docker.compose.project
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
25a46e09d15a vagrant_replicated-ui "sh -c 'make insta..." 6 days ago Up 6 days 0.0.0.0:8800->8800/tcp, 0.0.0.0:32778->6060/tcp replicated-ui
300a0a3b3f2b vagrant_qa-integration-onprem "sh -c 'sleep infi..." 6 days ago Up 6 days 0.0.0.0:8082->8081/tcp qa-integration-onprem
4a4836ce176f vagrant_replicated "sh -c 'make insta..." 6 days ago Up 6 days 0.0.0.0:9874->9874/tcp, 0.0.0.0:9877-9878->9877-9878/tcp, 0.0.0.0:32777->6060/tcp replicated
c89b929f175a vagrant_replicated-operator "sh -c 'make insta..." 6 days ago Up 6 days 0.0.0.0:32775->6060/tcp replicated-operator
b3659ae60a77 selenium/standalone-chrome-debug:3.6 "/opt/bin/entry_po..." 6 days ago Up 6 days 0.0.0.0:4444->4444/tcp, 0.0.0.0:5900->5900/tcp vagrant_selenuim_1
6118cd42d299 nginx:1 "nginx -g 'daemon ..." 6 days ago Up 6 days 80/tcp, 0.0.0.0:32769->8006/tcp saas_market-api_1
22dd7ca60fb5 nginx:1 "nginx -g 'daemon ..." 6 days ago Up 6 days 80/tcp, 0.0.0.0:32768->10443/tcp saas_registry_1
vagrant@default:/vagrant$ sudo docker container ls --filter=label=com.docker.compose.project=
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
vagrant@default:/vagrant$ sudo docker container ls --filter=label=com.docker.compose.project=saas
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
6118cd42d299 nginx:1 "nginx -g 'daemon ..." 6 days ago Up 6 days 80/tcp, 0.0.0.0:32769->8006/tcp saas_market-api_1
22dd7ca60fb5 nginx:1 "nginx -g 'daemon ..." 6 days ago Up 6 days 80/tcp, 0.0.0.0:32768->10443/tcp saas_registry_1
vagrant@default:/vagrant$
| inspectTask := &plans.StructuredSource{ | ||
| Producer: d.producers.Inspect(container.ID), | ||
| } | ||
| logsTask := &plans.StreamSource{ |
Member
There was a problem hiding this comment.
why logs and inspect especially if this task has name InspectLabels? i think this should be two separate tasks
| "daemon": docker.Daemon, | ||
| "logs": docker.Logs, | ||
| "inspect": docker.Inspect, | ||
| "inspect-labels": docker.InspectLabels, |
Member
There was a problem hiding this comment.
how about container-inspect-ls?
container-inspect
container-logs
| EnablePull bool `json:"enable_pull"` | ||
| ContainerID string `json:"container_id"` | ||
| ContainerName string `json:"container_name"` | ||
| ContainerLabels map[string]string `json:"container_labels"` |
Member
There was a problem hiding this comment.
how about the whole ContainerListOptions? similar to DockerRunCommand
emosbaugh
approved these changes
Nov 7, 2017
| container_list_options: | ||
| filters: | ||
| label: | ||
| foo=bar: true`) |
| EnablePull bool `json:"enable_pull"` | ||
| ContainerID string `json:"container_id"` | ||
| ContainerName string `json:"container_name"` | ||
| ContainerLabels map[string]string `json:"container_labels"` |
| "read-file": docker.ReadFile, | ||
| "exec-command": docker.ExecCommand, | ||
| "run-command": docker.RunCommand, | ||
| "container-logs": docker.ContainerLogs, |
Member
There was a problem hiding this comment.
i suppose i meant for logs to be container-logs and inspect to be container-inspect. these could be container-ls-logs and container-ls-inspect
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This allows Replicated to get logs and inspect for all containers in an app version.