You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@heron.apache.org by GitBox <gi...@apache.org> on 2020/12/13 02:01:11 UTC
[GitHub] [incubator-heron] thinker0 opened a new pull request #3650: Allow localhost graceful shutdown
thinker0 opened a new pull request #3650:
URL: https://github.com/apache/incubator-heron/pull/3650
#3581 Add gracefully shutdown
In Apache Aurora, it is necessary to allow a graceful shutdown on localhost.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] joshfischer1108 merged pull request #3650: Allow localhost graceful shutdown
Posted by GitBox <gi...@apache.org>.
joshfischer1108 merged pull request #3650:
URL: https://github.com/apache/incubator-heron/pull/3650
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] thinker0 commented on a change in pull request #3650: Allow localhost graceful shutdown
Posted by GitBox <gi...@apache.org>.
thinker0 commented on a change in pull request #3650:
URL: https://github.com/apache/incubator-heron/pull/3650#discussion_r541850288
##########
File path: heron/shell/src/python/handlers/killexecutorhandler.py
##########
@@ -44,13 +44,20 @@ def kill_parent():
logger.info("Killing parent executor")
os.killpg(os.getppid(), signal.SIGTERM)
+ def is_local():
+ remote_ip = self.request.remote_ip
+ if 'localhost' == remote_ip or '127.0.0.1' == remote_ip or '::1' == remote_ip:
+ return True
+ else:
+ return False
+
logger = logging.getLogger(__file__)
logger.info("Received 'Killing process' request")
data = dict(parse_qsl(self.request.body))
# check shared secret
sharedSecret = data.get('secret')
- if sharedSecret != options.secret:
+ if not is_local() and sharedSecret != options.secret:
Review comment:
# Aurora proceeds in the following order.
https://aurora-scheduler.readthedocs.io/en/latest/reference/task-lifecycle/
```
1. POST localhost/quitquitquit
sleep 10
2. POST localhost/aboutabout
sleep 10
3. SIGTERM pid
sleep 10
4. SIGKILL pid
```
admin port 31000 to 32000, Random port,
# current 403
1. Currently 403 and the next SIGTERM, SIGKILL proceeded.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] joshfischer1108 commented on pull request #3650: Allow localhost graceful shutdown
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on pull request #3650:
URL: https://github.com/apache/incubator-heron/pull/3650#issuecomment-752823961
@thinker0 or @Code0x58 Is this PR ok to be further reviewed and merged?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on a change in pull request #3650: Allow localhost graceful shutdown
Posted by GitBox <gi...@apache.org>.
Code0x58 commented on a change in pull request #3650:
URL: https://github.com/apache/incubator-heron/pull/3650#discussion_r541828521
##########
File path: heron/shell/src/python/handlers/killexecutorhandler.py
##########
@@ -44,13 +44,20 @@ def kill_parent():
logger.info("Killing parent executor")
os.killpg(os.getppid(), signal.SIGTERM)
+ def is_local():
+ remote_ip = self.request.remote_ip
+ if 'localhost' == remote_ip or '127.0.0.1' == remote_ip or '::1' == remote_ip:
+ return True
+ else:
+ return False
Review comment:
```suggestion
return self.request.remote_ip in ('localhost', '127.0.0.1', '::1')
```
##########
File path: heron/shell/src/python/handlers/killexecutorhandler.py
##########
@@ -44,13 +44,20 @@ def kill_parent():
logger.info("Killing parent executor")
os.killpg(os.getppid(), signal.SIGTERM)
+ def is_local():
+ remote_ip = self.request.remote_ip
+ if 'localhost' == remote_ip or '127.0.0.1' == remote_ip or '::1' == remote_ip:
+ return True
+ else:
+ return False
+
logger = logging.getLogger(__file__)
logger.info("Received 'Killing process' request")
data = dict(parse_qsl(self.request.body))
# check shared secret
sharedSecret = data.get('secret')
- if sharedSecret != options.secret:
+ if not is_local() and sharedSecret != options.secret:
Review comment:
This change is really that the secret is disregarded if the last hop of the request comes from a local connection (which could happen if you use a reverse proxy like nginx).
If this change is to allow graceful shutown, does mean aurora always calls this endpoint without the secret and from a loopback address? So before this patch, aurora would call this endpoint and get a 403 and no action, instead of getting to send a SIGTERM to the executor process group? If so, I'm wondering if you could just make Aurora send the SIGTERM to avoid having to worry about secrets being ignored in certain cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on a change in pull request #3650: Allow localhost graceful shutdown
Posted by GitBox <gi...@apache.org>.
Code0x58 commented on a change in pull request #3650:
URL: https://github.com/apache/incubator-heron/pull/3650#discussion_r541828521
##########
File path: heron/shell/src/python/handlers/killexecutorhandler.py
##########
@@ -44,13 +44,20 @@ def kill_parent():
logger.info("Killing parent executor")
os.killpg(os.getppid(), signal.SIGTERM)
+ def is_local():
+ remote_ip = self.request.remote_ip
+ if 'localhost' == remote_ip or '127.0.0.1' == remote_ip or '::1' == remote_ip:
+ return True
+ else:
+ return False
Review comment:
:golf:
```suggestion
return self.request.remote_ip in ('localhost', '127.0.0.1', '::1')
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] thinker0 commented on pull request #3650: Allow localhost graceful shutdown
Posted by GitBox <gi...@apache.org>.
thinker0 commented on pull request #3650:
URL: https://github.com/apache/incubator-heron/pull/3650#issuecomment-744185827
Nice work.. ^^;
```
[I 201214 14:47:11 web:1811] 200 GET /health (127.0.0.1) 0.64ms
[I 201214 14:47:31 web:1811] 200 GET /health (127.0.0.1) 0.53ms
[I 201214 14:47:46 killexecutorhandler:51] Received 'Killing process' request
[I 201214 14:47:46 web:1811] 200 POST /quitquitquit (127.0.0.1) 1.08ms
[I 201214 14:47:46 killexecutorhandler:44] Killing parent executor
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org