bugfix: yurthub can not exit when SIGINT/SIGTERM happened #1143
Conversation
|
@rambohe-ch: GitHub didn't allow me to assign the following users: your_reviewer. Note that only openyurtio members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rambohe-ch The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report
@@ Coverage Diff @@
## master #1143 +/- ##
==========================================
+ Coverage 54.97% 55.59% +0.61%
==========================================
Files 96 98 +2
Lines 12582 12881 +299
==========================================
+ Hits 6917 7161 +244
- Misses 5070 5091 +21
- Partials 595 629 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
rambohe-ch
force-pushed
the
fix-server-graceful-stop
branch
3 times, most recently
from
5daed7e
to
edc0d1f
Compare
rambohe-ch
force-pushed
the
fix-server-graceful-stop
branch
from
edc0d1f
to
01878a3
Compare
| // accept traffic to localhost:10261 | ||
| {iptables.Prepend, iptables.TableFilter, iptables.ChainInput, []string{"-p", "tcp", "--dport", ifPort, "--destination", "localhost", "-j", "ACCEPT"}}, | ||
| // accept traffic from localhost:10261 | ||
| {iptables.Prepend, iptables.TableFilter, iptables.ChainOutput, []string{"-p", "tcp", "--sport", ifPort, "-s", "localhost", "-j", "ACCEPT"}}, |
There was a problem hiding this comment.
Could you please give some details about why we change this?
There was a problem hiding this comment.
traffic to localhost can be accepted in default status, so these iptables settings are dummy setting, so remove them.
| @@ -84,30 +81,10 @@ func NewCmdStartYurtHub(stopCh <-chan struct{}) *cobra.Command { | |||
|
|
|||
| // Run runs the YurtHubConfiguration. This should never exit | |||
| func Run(cfg *config.YurtHubConfiguration, stopCh <-chan struct{}) error { | |||
| defer cfg.CertManager.Stop() | |||
There was a problem hiding this comment.
Could we also use stopCh to stop CertManager?
There was a problem hiding this comment.
no, CertManager has stopCh that is created by itself.
|
Another question:
If it was intentional，could you please also give some explanation? |
@Congrool originally, we have two https server: secure proxy server and secure dummyProxyServer, but in real scenario, only one secure server is used. in order to reduce the overhead of Yurthub, so we reduce the unused secure server. btw: two http server: proxy server and dummyProxyServer(if |
|
/lgtm |
What type of PR is this?
/kind bug
What this PR does / why we need it:
if you send SIGINT signal(kill -SIGINT pid) to yurthub component, yurthub can not exit and restart.
so i improved the yurthub http/https server by adding graceful stop feature.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
other Note