Add comment around default network of docker on getIpAddr of docker.go#511
Add comment around default network of docker on getIpAddr of docker.go#5110405ysj wants to merge 1 commit intogoogle:mainfrom
Conversation
0405ysj
requested review from
Databean,
adelva1984,
jemoreira,
jmacnak,
rmuthiah and
ser-io
as code owners
0405ysj
requested review from
3405691582 and
ikicha
and removed request for
Databean,
adelva1984,
jmacnak and
rmuthiah
pkg/app/instances/docker.go
Outdated
| } | ||
|
|
||
| func (m *DockerInstanceManager) getIpAddr(container *types.Container) (string, error) { | ||
| // When creating docker instances as default, docker instance gets belonged |
There was a problem hiding this comment.
I would actually attach the comment on createDockerContainer.
There was a problem hiding this comment.
Agree, we should add this comment to an Instance Manger public method implementation. I think GetHostClient is more relevant here, CreateHost could be used as well.
There was a problem hiding this comment.
Rather than CreateHost, I think GetHostClient is suitable place as it deals with Host Orchestrator URL. In fact, CreateHost doesn't do special things around docker instance's IP address.
pkg/app/instances/docker.go
Outdated
| } | ||
|
|
||
| func (m *DockerInstanceManager) getIpAddr(container *types.Container) (string, error) { | ||
| // When creating docker instances as default, docker instance gets belonged |
There was a problem hiding this comment.
Agree, we should add this comment to an Instance Manger public method implementation. I think GetHostClient is more relevant here, CreateHost could be used as well.
0405ysj
force-pushed
the
co_doc
branch
2 times, most recently
from
e9d516c to
807de99
Compare
| return nil | ||
| } | ||
|
|
||
| func (m *DockerInstanceManager) createDockerContainer(ctx context.Context, user accounts.User) (string, error) { |
There was a problem hiding this comment.
| // createDockerContainer creates the cuttlefish host orchestrator container. | |
| // The container is expected to have: | |
| // - an entrypoint set that starts the host orchestrator, | |
| // - network access through the default docker0 bridge interface, and | |
| // - a preconfigured volume created for sharing artifacts (see `uaVolumeName`). | |
| // While the container image name is configurable, we do not intend or want | |
| // to support other containers or alternative command execution in config. | |
| func (m *DockerInstanceManager) createDockerContainer(ctx context.Context, user accounts.User) (string, error) { |
There was a problem hiding this comment.
Applied your suggestion, but I placed some in the different place.
0405ysj
force-pushed
the
co_doc
branch
2 times, most recently
from
5e913a2 to
d8e5449
Compare
| const DockerIMType IMType = "docker" | ||
|
|
||
| type DockerIMConfig struct { | ||
| // We don't intend to support any customized docker images except |
There was a problem hiding this comment.
This should probably be made into a proper godoc comment for the struct.
3 participants
No description provided.