Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: support update hostname #2230

Conversation

wangforthinker
Copy link
Contributor

@wangforthinker wangforthinker commented Sep 11, 2018

Signed-off-by: allen.wang allen.wq@alipay.com

Ⅰ. Describe what this PR did

Consider to update container's hostname in a running container, It should join container namespace. So it imports package github.com/opencontainers/runc/libcontainer/nsenter to meet the requirement(about more implementation details refferring to Ⅳ. Describe how to verify it). Otherwise, It provides more register functions which can do more in container namespace.

Ⅱ. Does this pull request fix one issue?

fixes #2179

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

add test TestRunningContainerUpdateHostname and TestStoppedContainerUpdateHostname.

Ⅳ. Describe how to verify it

after doing update hostname, we can exec into container, and run command "hostname" to verify it.

Ⅴ. Special notes for reviews

It adds a package pkg/nsexec, which defines a function NsExec to join container namespace. It provides register functions map to allow do something in container namespace such as update hostname. It registers a reexec command called nsexec, which imports github.com/opencontainers/runc/libcontainer/nsenter to join container namespace by calling C code. Package pkg/nsexec executes new command to call pouchd nsexec.

@wangforthinker wangforthinker force-pushed the feature/support-update-hostname branch 2 times, most recently from e8c7a3e to f16afaa Compare September 11, 2018 11:51
@allencloud allencloud changed the title support update hostname feature: support update hostname Sep 11, 2018
@allencloud
Copy link
Collaborator

I think this pr has updated or added too many codes, so I am afraid we have to add more pr description s to make the demand and implementation clearly. @wangforthinker
Thanks a lot. 😄

@codecov-io
Copy link

codecov-io commented Sep 11, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@3470d89). Click here to learn what that means.
The diff coverage is 45.55%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2230   +/-   ##
=========================================
  Coverage          ?   67.44%           
=========================================
  Files             ?      281           
  Lines             ?    18866           
  Branches          ?        0           
=========================================
  Hits              ?    12724           
  Misses            ?     4626           
  Partials          ?     1516
Flag Coverage Δ
#criv1alpha1test 31.17% <2.22%> (?)
#criv1alpha2test 35.27% <2.22%> (?)
#integrationtest 40.74% <45.55%> (?)
#unittest 26.59% <4.08%> (?)
Impacted Files Coverage Δ
pkg/nsexec/nsexec_child_linux.go 0% <0%> (ø)
daemon/mgr/container_update_hostname.go 14.63% <14.63%> (ø)
daemon/mgr/container.go 58.71% <50%> (ø)
pkg/nsexec/nsexec_linux.go 61.53% <61.53%> (ø)

@fuweid fuweid added this to the v1.1 milestone Oct 8, 2018
@pouchrobot
Copy link
Collaborator

ping @wangforthinker
Conflict happens after merging a previous commit.
Please rebase the branch against master and push it back again. Thanks a lot.

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.

[proposal] pouch supports to update hostname for rich container
7 participants