-
Notifications
You must be signed in to change notification settings - Fork 949
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: complete the details of StartPodSandbox #2242
feature: complete the details of StartPodSandbox #2242
Conversation
df6301d
to
c763955
Compare
cri/v1alpha2/cri_utils.go
Outdated
var netnsPath string | ||
hostNet := networkNamespaceMode == runtime.NamespaceMode_NODE | ||
// If it is in host network, no need to configure the network of sandbox. | ||
if !hostNet { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
if hostNet {
return netnsPath, nil
}
Use return fast
to reduce ident and make code more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly, and has been updated. ^_^
c763955
to
612602b
Compare
@rudyfly @YaoZengzeng PTAL. Thanks. ^_^ |
cri/v1alpha2/cri.go
Outdated
sandboxMeta := res.(*SandboxMeta) | ||
|
||
// Setup networking for the sandbox. | ||
netnsPath, setupNeterr := c.setupPodNetwork(ctx, podSandboxID, sandboxMeta.Config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetNetwork(), sandboxMeta.Config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/setupNeterr/setupNetErr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has been updated.
612602b
to
63bc378
Compare
cri/v1alpha2/cri.go
Outdated
if setupNetErr != nil { | ||
return nil, setupNetErr | ||
} | ||
defer func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If setupNetErr
is not nil, the defer function will never being invoked.
cri/v1alpha2/cri.go
Outdated
|
||
podSandboxID := r.GetPodSandboxId() | ||
|
||
// start Podsandbox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Podsandbox -> PodSandbox
cri/v1alpha2/cri.go
Outdated
} | ||
sandboxMeta := res.(*SandboxMeta) | ||
|
||
// Setup networking for the sandbox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setup -> setup
72d3738
to
95b82d5
Compare
3cd2a30
to
9d1fb49
Compare
9d1fb49
to
758c818
Compare
LGTM |
758c818
to
0e10bff
Compare
LGTM, and waiting for the ci passed. |
0e10bff
to
f00fd04
Compare
f00fd04
to
397d830
Compare
397d830
to
5ee45e7
Compare
Signed-off-by: Starnop <[email protected]>
5ee45e7
to
1d46085
Compare
Signed-off-by: Starnop [email protected]
Ⅰ. Describe what this PR did
Provide an interface to start a sandbox pod which was stopped by accident and setup the network with network plugin.
Note: Only if the CNI-plugin supports returning the same IP with the same parameters of
containdID
,pod name
,pod namespace
, we will make sure it reacquire the original network configuration of sandbox, like IP address.Ⅱ. Does this pull request fix one issue?
None.
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)
None.
Ⅳ. Describe how to verify it
use cri-tools
Ⅴ. Special notes for reviews
Related Pull Request: #2237