You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2020/01/07 03:18:35 UTC
[GitHub] [openwhisk] ningyougang opened a new pull request #4790:
[WIP]Config runtime
ningyougang opened a new pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790
Sometimes, admin may want to reinitalize the runtime config, e.g. nodejs:10 prewarm container number is less, lead to `cold start`, in order to handle user's request as soon as possible, admin may want to reinitalize the runtime configuration to increase the nodejs:10 prewarm containers.
And admin may want to reinitalize the runtime config on some limited invokers, this patch includes below changes
- [x] config runtime via controller to all managed invokers(or some limited inovkers which included in managed inovkers)
- [x] config runtime via invoker directly
- [x] get runtime info via invoker directly
- [x] add documents
- [ ] test cases
<!--- Provide a concise summary of your changes in the Title -->
## Description
<!--- Provide a detailed description of your changes. -->
<!--- Include details of what problem you are solving and how your changes are tested. -->
## Related issue and scope
<!--- Please include a link to a related issue if there is one. -->
- [ ] I opened an issue to propose and discuss this change (#????)
## My changes affect the following components
<!--- Select below all system components are affected by your change. -->
<!--- Enter an `x` in all applicable boxes. -->
- [ ] API
- [x] Controller
- [ ] Message Bus (e.g., Kafka)
- [x] Loadbalancer
- [x] Invoker
- [ ] Intrinsic actions (e.g., sequences, conductors)
- [ ] Data stores (e.g., CouchDB)
- [x] Tests
- [ ] Deployment
- [ ] CLI
- [ ] General tooling
- [ ] Documentation
## Types of changes
<!--- What types of changes does your code introduce? Use `x` in all the boxes that apply: -->
- [ ] Bug fix (generally a non-breaking change which closes an issue).
- [x] Enhancement or new feature (adds new functionality).
- [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
## Checklist:
<!--- Please review the points below which help you make sure you've covered all aspects of the change you're making. -->
- [x] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
- [ ] I reviewed the [style guides](https://github.com/apache/openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :).
- [ ] I added tests to cover my changes.
- [ ] My changes require further changes to the documentation.
- [x] I updated the documentation where necessary.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385019331
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,61 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.info(this, s"received invalid runtimes manifest")
+ complete(StatusCodes.BadRequest)
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = """\d+:\d"""
+ if (targetValue.matches(pattern)) {
+ val invokerArray = targetValue.split(":")
+ val beginIndex = invokerArray(0).toInt
+ val finishIndex = invokerArray(1).toInt
+ if (finishIndex < beginIndex) {
+ logging.info(this, "finishIndex can't be less than beginIndex")
+ complete(StatusCodes.BadRequest)
+ } else {
+ val targetInvokers = (beginIndex to finishIndex).toList
+ loadBalancer.sendRuntimeToInvokers(runtime, Some(targetInvokers))
+ logging.info(this, "config runtime request is already sent to target invokers")
+ complete(StatusCodes.BadRequest)
+ }
+ } else {
+ logging.info(this, "limit value can't match [beginIndex:finishIndex]")
+ complete(StatusCodes.BadRequest)
+ }
+ case None =>
+ loadBalancer.sendRuntimeToInvokers(runtime, None)
+ logging.info(this, "config runtime request is already sent to all managed invokers")
+ complete(StatusCodes.Accepted)
Review comment:
I changed to `complete(StatusCodes.Accepted)`, because it is a async operation.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385010928
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
Review comment:
I just follow other codes using `execManifest.isFailure` to judge whether success or fail
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang opened a new pull request #4790:
[WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang opened a new pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790
Sometimes, admin may want to reinitalize the runtime config, e.g. nodejs:10 prewarm container number is less, lead to `cold start`, in order to handle user's request as soon as possible, admin may want to reinitalize the runtime configuration to increase the nodejs:10 prewarm containers.
And admin may want to reinitalize the runtime config on some limited invokers, this patch includes below changes
- [x] config runtime via controller to all managed invokers(or some limited inovkers which included in managed inovkers)
- [x] config runtime via invoker directly
- [x] get runtime info via invoker directly
- [x] add documents
- [ ] test cases
<!--- Provide a concise summary of your changes in the Title -->
## Description
<!--- Provide a detailed description of your changes. -->
<!--- Include details of what problem you are solving and how your changes are tested. -->
## Related issue and scope
<!--- Please include a link to a related issue if there is one. -->
- [ ] I opened an issue to propose and discuss this change (#????)
## My changes affect the following components
<!--- Select below all system components are affected by your change. -->
<!--- Enter an `x` in all applicable boxes. -->
- [ ] API
- [x] Controller
- [ ] Message Bus (e.g., Kafka)
- [x] Loadbalancer
- [x] Invoker
- [ ] Intrinsic actions (e.g., sequences, conductors)
- [ ] Data stores (e.g., CouchDB)
- [x] Tests
- [ ] Deployment
- [ ] CLI
- [ ] General tooling
- [ ] Documentation
## Types of changes
<!--- What types of changes does your code introduce? Use `x` in all the boxes that apply: -->
- [ ] Bug fix (generally a non-breaking change which closes an issue).
- [x] Enhancement or new feature (adds new functionality).
- [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
## Checklist:
<!--- Please review the points below which help you make sure you've covered all aspects of the change you're making. -->
- [x] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
- [ ] I reviewed the [style guides](https://github.com/apache/openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :).
- [ ] I added tests to cover my changes.
- [ ] My changes require further changes to the documentation.
- [x] I updated the documentation where necessary.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371009470
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.core.invoker
+
+import akka.actor.ActorSystem
+import akka.http.scaladsl.model.StatusCodes
+import akka.http.scaladsl.model.headers.BasicHttpCredentials
+import akka.http.scaladsl.server.Route
+import org.apache.openwhisk.common.{InvokerCredentials, Logging, TransactionId}
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.containerpool.PrewarmingConfig
+import org.apache.openwhisk.core.entity.{CodeExecAsString, ExecManifest}
+import org.apache.openwhisk.http.BasicRasService
+
+import pureconfig._
+import pureconfig.generic.auto._
+
+import scala.concurrent.ExecutionContext
+
+/**
+ * Implements web server to handle certain REST API calls.
+ */
+class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionContext,
+ val actorSystem: ActorSystem,
+ val logger: Logging)
+ extends BasicRasService {
+
+ private val invokerCredentials = loadConfigOrThrow[InvokerCredentials](ConfigKeys.invokerCredentials)
+
+ override def routes(implicit transid: TransactionId): Route = {
Review comment:
how will new runtime images get pulled to the invoker nodes? are you assuming they get pulled either via stem-cell "run" or when an action actually runs in the future?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r365698400
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +164,65 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerUsername = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.username");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+ private val controllerPassword = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.password");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerUsername && password == controllerPassword) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
+ if (targetValue.matches(pattern)) {
+ val invokerArray = targetValue.split(":")
+ val beginIndex = invokerArray(0).toInt
+ val finishIndex = invokerArray(1).toInt
+ if (finishIndex < beginIndex) {
Review comment:
Yes, already support `0:0 case`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385012173
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
+ if (targetValue.matches(pattern)) {
Review comment:
hm.. can you show a example? my codes like below
```
if (targetValue.matches(pattern)) {
val invokerArray = targetValue.split(":")
val beginIndex = invokerArray(0).toInt
val finishIndex = invokerArray(1).toInt
if (finishIndex < beginIndex) {
logging.info(this, "finishIndex can't be less than beginIndex")
complete(StatusCodes.BadRequest)
} else {
val targetInvokers = (beginIndex to finishIndex).toList
loadBalancer.sendRuntimeToInvokers(runtime, Some(targetInvokers))
logging.info(this, "config runtime request is already sent to target invokers")
complete(StatusCodes.BadRequest)
}
} else {
logging.info(this, "limit value can't match [beginIndex:finishIndex]")
complete(StatusCodes.BadRequest)
}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385013929
##########
File path: tests/src/test/scala/common/WhiskProperties.java
##########
@@ -258,10 +258,40 @@ public static int getControllerBasePort() {
return Integer.parseInt(whiskProperties.getProperty("controller.host.basePort"));
}
+ public static String getControllerProtocol() {
Review comment:
Yes, already deleted.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385008599
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
Review comment:
Hi, almost forgot this patch :(
I changed like below
```scala
logging.info(this, s"received invalid runtimes manifest")
complete(StatusCodes.BadRequest)
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-596277222
Rebased.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] codecov-io edited a comment on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-573566159
# [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=h1) Report
> Merging [#4790](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/394e9f6cb3ec09dd4a5ce158afaf2268d64fec12?src=pr&el=desc) will **decrease** coverage by `6.86%`.
> The diff coverage is `41.78%`.
[](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4790 +/- ##
==========================================
- Coverage 82.35% 75.49% -6.87%
==========================================
Files 198 199 +1
Lines 9021 9123 +102
Branches 353 379 +26
==========================================
- Hits 7429 6887 -542
- Misses 1592 2236 +644
```
| [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...la/org/apache/openwhisk/core/invoker/Invoker.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyLnNjYWxh) | `71.66% <ø> (-0.47%)` | :arrow_down: |
| [.../apache/openwhisk/core/controller/Controller.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9Db250cm9sbGVyLnNjYWxh) | `0% <0%> (ø)` | :arrow_up: |
| [...che/openwhisk/core/loadBalancer/LoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0xvYWRCYWxhbmNlci5zY2FsYQ==) | `15.38% <0%> (-1.29%)` | :arrow_down: |
| [...e/loadBalancer/ShardingContainerPoolBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL1NoYXJkaW5nQ29udGFpbmVyUG9vbEJhbGFuY2VyLnNjYWxh) | `73.91% <0%> (-2.93%)` | :arrow_down: |
| [...rg/apache/openwhisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `97.36% <100%> (+0.14%)` | :arrow_up: |
| [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `94.85% <100%> (+0.03%)` | :arrow_up: |
| [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `77.61% <37.5%> (-2.55%)` | :arrow_down: |
| [...e/openwhisk/core/containerpool/ContainerPool.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQb29sLnNjYWxh) | `89.04% <44.44%> (-6.61%)` | :arrow_down: |
| [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `78.62% <64.4%> (-1.03%)` | :arrow_down: |
| [.../openwhisk/core/invoker/DefaultInvokerServer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9EZWZhdWx0SW52b2tlclNlcnZlci5zY2FsYQ==) | `75% <75%> (ø)` | |
| ... and [21 more](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=footer). Last update [394e9f6...12201bb](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385013712
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.core.invoker
+
+import akka.actor.ActorSystem
+import akka.http.scaladsl.model.StatusCodes
+import akka.http.scaladsl.model.headers.BasicHttpCredentials
+import akka.http.scaladsl.server.Route
+import org.apache.openwhisk.common.{InvokerCredentials, Logging, TransactionId}
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.containerpool.PrewarmingConfig
+import org.apache.openwhisk.core.entity.{CodeExecAsString, ExecManifest}
+import org.apache.openwhisk.http.BasicRasService
+
+import pureconfig._
+import pureconfig.generic.auto._
+
+import scala.concurrent.ExecutionContext
+
+/**
+ * Implements web server to handle certain REST API calls.
+ */
+class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionContext,
+ val actorSystem: ActorSystem,
+ val logger: Logging)
+ extends BasicRasService {
+
+ private val invokerCredentials = loadConfigOrThrow[InvokerCredentials](ConfigKeys.invokerCredentials)
+
+ override def routes(implicit transid: TransactionId): Route = {
Review comment:
When send to reqeust to backend (e.g. `curl -u xxx:xxx-X POST http://xxx:xxx:10001/config/runtime`), it will pull new runtime image immediately and create prewarm container if the stem-cell > 0.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385009108
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
Review comment:
Changed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang edited a comment on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang edited a comment on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-591880231
@rabbah
> The limiting of a runtime to specific invokers is related to how invokers are partitioned for black box vs non-black box invokers (based on a % of active invokers in the system). I suggest splitting out the "limit" feature into a separate PR and having a mechanism for specifying distributions of runtimes to invokers and letting the load balancer calculate the partition.
hm.. this patch just reconfigure the runtime.json(recreate prewarm containers according to passed runtime string), i don't understand what's mean ` I suggest splitting out the "limit" feature into a separate PR`
> That isn't powerful enough to when it is desirable to route to a specific invoker - is that a desired feature (vs limiting to a range of invokers active in the system)?
Yes, because i use `limit`, so can deleted the codes of `route to a specific invoker`.
why i added it, just for extra method.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r365701699
##########
File path: ansible/roles/controller/tasks/deploy.yml
##########
@@ -203,6 +203,9 @@
"CONFIG_whisk_db_activationsFilterDdoc": "{{ db_whisk_activations_filter_ddoc | default() }}"
"CONFIG_whisk_userEvents_enabled": "{{ user_events | default(false) | lower }}"
+ "CONFIG_whisk_credentials_controller_username": "{{ controller.username }}"
+ "CONFIG_whisk_credentials_controller_password": "{{ controller.password }}"
+
Review comment:
If generate controller/invoker credentials to container's /conf/, `the Standalone Tests` run failed due to `lack /conf/ under the build machine`
On the other hand, `couchdb credentials is passed via environment variable`, so controller/invoker credentials can be passed via environment variable as well.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang edited a comment on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang edited a comment on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-591880231
@rabbah
> The limiting of a runtime to specific invokers is related to how invokers are partitioned for black box vs non-black box invokers (based on a % of active invokers in the system). I suggest splitting out the "limit" feature into a separate PR and having a mechanism for specifying distributions of runtimes to invokers and letting the load balancer calculate the partition.
hm.. this patch just reconfigure the runtime.json(recreate prewarm containers according to passed runtime string), i don't understand what's mean ` I suggest splitting out the "limit" feature into a separate PR`
> That isn't powerful enough to when it is desirable to route to a specific invoker - is that a desired feature (vs limiting to a range of invokers active in the system)?
Yes, it is not powerful, so i deleted `route to a specific invoker` because already have `limiting to a range of invokers `
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371008900
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
Review comment:
use
```suggestion
val pattern = """\d+:\d"""
```
using triple quotes allows you to use a regex without escape characters.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385014637
##########
File path: tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfiguration.scala
##########
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.operation
+
+import akka.http.scaladsl.Http
+import akka.http.scaladsl.model.headers.{Authorization, BasicHttpCredentials}
+import akka.http.scaladsl.model.{ContentTypes, HttpEntity, HttpMethods, HttpRequest, StatusCodes}
+import akka.http.scaladsl.unmarshalling.Unmarshal
+import akka.stream.ActorMaterializer
+import common._
+import common.rest.HttpConnection
+import org.apache.openwhisk.core.connector.PrewarmContainerDataList
+import org.apache.openwhisk.core.connector.PrewarmContainerDataProtocol._
+import org.junit.runner.RunWith
+import org.scalatest.Matchers
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.junit.JUnitRunner
+import spray.json._
+import system.rest.RestUtil
+
+import scala.concurrent.duration._
+import scala.util.Random
+
+@RunWith(classOf[JUnitRunner])
+class RuntimeConfigurationTests
+ extends TestHelpers
+ with RestUtil
+ with Matchers
+ with ScalaFutures
+ with WskActorSystem
+ with StreamLogging {
+
+ implicit val materializer = ActorMaterializer()
+
+ val kind = "nodejs:10"
+ val memory = 128
+ var count = new Random().nextInt(3) + 1
+
+ def getRuntimes = {
+ s""" {
+ "runtimes": {
+ "nodejs": [{
+ "kind": "${kind}",
+ "default": true,
+ "image": {
+ "prefix": "openwhisk",
+ "name": "action-nodejs-v10",
+ "tag": "nightly"
+ },
+ "deprecated": false,
+ "attached": {
+ "attachmentName": "codefile",
+ "attachmentType": "text/plain"
+ },
+ "stemCells": [{
+ "count": ${count},
+ "memory": "${memory} MB"
+ }]
+ }]
+ }
+ }"""
+ }
+
+ val invokerProtocol = WhiskProperties.getInvokerProtocol
+ val invokerAddress = WhiskProperties.getBaseInvokerAddress
+ val invokerUsername = WhiskProperties.getInvokerUsername
+ val invokerPassword = WhiskProperties.getInvokerPassword
+ val invokerAuthHeader = Authorization(BasicHttpCredentials(invokerUsername, invokerPassword))
+
+ val controllerProtocol = WhiskProperties.getControllerProtocol
+ val controllerAddress = WhiskProperties.getBaseControllerAddress
+ val controllerUsername = WhiskProperties.getControllerUsername
+ val controllerPassword = WhiskProperties.getControllerPassword
+ val controllerAuthHeader = Authorization(BasicHttpCredentials(controllerUsername, controllerPassword))
+
+ val getRuntimeUrl = s"${invokerProtocol}://${invokerAddress}/getRuntime"
+ val invokerChangeRuntimeUrl = s"${invokerProtocol}://${invokerAddress}/config/runtime"
+ val controllerChangeRuntimeUrl =
+ s"${controllerProtocol}://${controllerAddress}/config/runtime"
+
+ it should "change assigned invoker node's runtime config directly" in {
+ //Change config
+ Http()
+ .singleRequest(
+ HttpRequest(
+ method = HttpMethods.POST,
+ uri = s"${invokerChangeRuntimeUrl}",
+ headers = List(invokerAuthHeader),
+ entity = HttpEntity(ContentTypes.`text/plain(UTF-8)`, getRuntimes)),
+ connectionContext = HttpConnection.getContext(invokerProtocol))
+ .map { response =>
+ response.status shouldBe StatusCodes.OK
+ }
+
+ Thread.sleep(5.seconds.toMillis)
Review comment:
Just make previous's request handled successfully, because previous is a ansyc operation.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385012173
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
+ if (targetValue.matches(pattern)) {
Review comment:
hm.. can you show a example? my codes like below
```
if (targetValue.matches(pattern)) {
val invokerArray = targetValue.split(":")
val beginIndex = invokerArray(0).toInt
val finishIndex = invokerArray(1).toInt
if (finishIndex < beginIndex) {
logging.info(this, "finishIndex can't be less than beginIndex")
complete(StatusCodes.BadRequest)
} else {
val targetInvokers = (beginIndex to finishIndex).toList
loadBalancer.sendRuntimeToInvokers(runtime, Some(targetInvokers))
logging.info(this, "config runtime request is already sent to target invokers")
complete(StatusCodes.BadRequest)
}
} else {
logging.info(this, "limit value can't match [beginIndex:finishIndex]")
complete(StatusCodes.BadRequest)
}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r363580358
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +164,65 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerUsername = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.username");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+ private val controllerPassword = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.password");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerUsername && password == controllerPassword) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
Review comment:
Support passed limit invokers, e.g. `?limit=0:1` ( sent to invoker0, invoker1 only)
And the config runtime request can be sent to some limited invokers which included in managed invokers as well.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385013712
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.core.invoker
+
+import akka.actor.ActorSystem
+import akka.http.scaladsl.model.StatusCodes
+import akka.http.scaladsl.model.headers.BasicHttpCredentials
+import akka.http.scaladsl.server.Route
+import org.apache.openwhisk.common.{InvokerCredentials, Logging, TransactionId}
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.containerpool.PrewarmingConfig
+import org.apache.openwhisk.core.entity.{CodeExecAsString, ExecManifest}
+import org.apache.openwhisk.http.BasicRasService
+
+import pureconfig._
+import pureconfig.generic.auto._
+
+import scala.concurrent.ExecutionContext
+
+/**
+ * Implements web server to handle certain REST API calls.
+ */
+class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionContext,
+ val actorSystem: ActorSystem,
+ val logger: Logging)
+ extends BasicRasService {
+
+ private val invokerCredentials = loadConfigOrThrow[InvokerCredentials](ConfigKeys.invokerCredentials)
+
+ override def routes(implicit transid: TransactionId): Route = {
Review comment:
When send to reqeust to backend (e.g. curl -u xxx:xxx-X POST http://xxx:xxx:10001/config/runtime), it will pull new runtime image immediately and create prewarm container.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] codecov-io edited a comment on issue #4790: [WIP]Config
runtime
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-573566159
# [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=h1) Report
> Merging [#4790](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/0fcbe814457d593b6350eb1a5559443e1db23dc0?src=pr&el=desc) will **decrease** coverage by `7.5%`.
> The diff coverage is `27.45%`.
[](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4790 +/- ##
==========================================
- Coverage 85.19% 77.69% -7.51%
==========================================
Files 197 198 +1
Lines 8843 8953 +110
Branches 610 626 +16
==========================================
- Hits 7534 6956 -578
- Misses 1309 1997 +688
```
| [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...la/org/apache/openwhisk/core/invoker/Invoker.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyLnNjYWxh) | `71.66% <ø> (-0.47%)` | :arrow_down: |
| [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `72.22% <0%> (-5.75%)` | :arrow_down: |
| [...che/openwhisk/core/loadBalancer/LoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0xvYWRCYWxhbmNlci5zY2FsYQ==) | `76.92% <0%> (-6.42%)` | :arrow_down: |
| [...rg/apache/openwhisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `92.1% <0%> (-5.12%)` | :arrow_down: |
| [...e/loadBalancer/ShardingContainerPoolBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL1NoYXJkaW5nQ29udGFpbmVyUG9vbEJhbGFuY2VyLnNjYWxh) | `85.32% <0%> (-3.38%)` | :arrow_down: |
| [...e/openwhisk/core/containerpool/ContainerPool.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQb29sLnNjYWxh) | `85.31% <0%> (-11.51%)` | :arrow_down: |
| [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `94.85% <100%> (+0.07%)` | :arrow_up: |
| [.../apache/openwhisk/core/controller/Controller.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9Db250cm9sbGVyLnNjYWxh) | `68.03% <14.28%> (-16.18%)` | :arrow_down: |
| [.../openwhisk/core/invoker/DefaultInvokerServer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9EZWZhdWx0SW52b2tlclNlcnZlci5zY2FsYQ==) | `27.27% <27.27%> (ø)` | |
| [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `69.4% <49.18%> (-11.3%)` | :arrow_down: |
| ... and [22 more](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=footer). Last update [0fcbe81...8573912](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] codecov-io edited a comment on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-573566159
# [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=h1) Report
> Merging [#4790](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/08efb58cf5eef3475bdb41e45db448250bcf9c17?src=pr&el=desc) will **decrease** coverage by `6.85%`.
> The diff coverage is `41.78%`.
[](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4790 +/- ##
==========================================
- Coverage 82.36% 75.51% -6.86%
==========================================
Files 198 199 +1
Lines 9021 9123 +102
Branches 353 379 +26
==========================================
- Hits 7430 6889 -541
- Misses 1591 2234 +643
```
| [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...la/org/apache/openwhisk/core/invoker/Invoker.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyLnNjYWxh) | `71.66% <ø> (-0.47%)` | :arrow_down: |
| [.../apache/openwhisk/core/controller/Controller.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9Db250cm9sbGVyLnNjYWxh) | `0% <0%> (ø)` | :arrow_up: |
| [...che/openwhisk/core/loadBalancer/LoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0xvYWRCYWxhbmNlci5zY2FsYQ==) | `15.38% <0%> (-1.29%)` | :arrow_down: |
| [...e/loadBalancer/ShardingContainerPoolBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL1NoYXJkaW5nQ29udGFpbmVyUG9vbEJhbGFuY2VyLnNjYWxh) | `73.91% <0%> (-2.93%)` | :arrow_down: |
| [...rg/apache/openwhisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `97.36% <100%> (+0.14%)` | :arrow_up: |
| [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `94.85% <100%> (+0.03%)` | :arrow_up: |
| [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `76.86% <37.5%> (-3.3%)` | :arrow_down: |
| [...e/openwhisk/core/containerpool/ContainerPool.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQb29sLnNjYWxh) | `89.52% <44.44%> (-6.13%)` | :arrow_down: |
| [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `78.62% <64.4%> (-1.03%)` | :arrow_down: |
| [.../openwhisk/core/invoker/DefaultInvokerServer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9EZWZhdWx0SW52b2tlclNlcnZlci5zY2FsYQ==) | `75% <75%> (ø)` | |
| ... and [23 more](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=footer). Last update [08efb58...fc63c07](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang edited a comment on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang edited a comment on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-591880231
@rabbah
> The limiting of a runtime to specific invokers is related to how invokers are partitioned for black box vs non-black box invokers (based on a % of active invokers in the system). I suggest splitting out the "limit" feature into a separate PR and having a mechanism for specifying distributions of runtimes to invokers and letting the load balancer calculate the partition.
hm.. this patch just reconfigure the runtime.json(recreate prewarm containers according to passed runtime string), i don't understand what's mean ` I suggest splitting out the "limit" feature into a separate PR`
> That isn't powerful enough to when it is desirable to route to a specific invoker - is that a desired feature (vs limiting to a range of invokers active in the system)?
Yes, because we use `limit`, can deleted the `route to a specific invoker`, i added it, just for exteral method of specific invoker
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang closed pull request #4790: [WIP]Config
runtime
Posted by GitBox <gi...@apache.org>.
ningyougang closed pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371009268
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
+ if (targetValue.matches(pattern)) {
+ val invokerArray = targetValue.split(":")
+ val beginIndex = invokerArray(0).toInt
+ val finishIndex = invokerArray(1).toInt
+ if (finishIndex < beginIndex) {
+ complete(s"finishIndex can't be less than beginIndex")
+ } else {
+ val targetInvokers = (beginIndex to finishIndex).toList
+ loadBalancer.sendRuntimeToInvokers(runtime, Some(targetInvokers))
Review comment:
is there a later validation to check that the invoker indexing is in range?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4790:
[WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r363722721
##########
File path: common/scala/src/main/resources/reference.conf
##########
@@ -27,7 +27,7 @@ whisk.spi {
EntitlementSpiProvider = org.apache.openwhisk.core.entitlement.LocalEntitlementProvider
AuthenticationDirectiveProvider = org.apache.openwhisk.core.controller.BasicAuthenticationDirective
InvokerProvider = org.apache.openwhisk.core.invoker.InvokerReactive
- InvokerServerProvider = org.apache.openwhisk.core.invoker.DefaultInvokerServer
+ InvokerServerProvider = org.apache.openwhisk.core.invoker.InvokerServer
Review comment:
This should be reverted.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-591880231
@rabbah
> The limiting of a runtime to specific invokers is related to how invokers are partitioned for black box vs non-black box invokers (based on a % of active invokers in the system). I suggest splitting out the "limit" feature into a separate PR and having a mechanism for specifying distributions of runtimes to invokers and letting the load balancer calculate the partition.
hm.. this patch just reconfigure the runtime.json(recreate prewarm containers according to passed runtime string), i don't understand what's mean ` I suggest splitting out the "limit" feature into a separate PR`
> That isn't powerful enough to when it is desirable to route to a specific invoker - is that a desired feature (vs limiting to a range of invokers active in the system)?
Yes, because we use `limit`, can deleted the `route to a specific invoker`, i added it, just for exteral method of specific invoker
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] codecov-io commented on issue #4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-573566159
# [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=h1) Report
> Merging [#4790](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/0fcbe814457d593b6350eb1a5559443e1db23dc0?src=pr&el=desc) will **decrease** coverage by `7.5%`.
> The diff coverage is `27.45%`.
[](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4790 +/- ##
==========================================
- Coverage 85.19% 77.69% -7.51%
==========================================
Files 197 198 +1
Lines 8843 8953 +110
Branches 610 626 +16
==========================================
- Hits 7534 6956 -578
- Misses 1309 1997 +688
```
| [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...la/org/apache/openwhisk/core/invoker/Invoker.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyLnNjYWxh) | `71.66% <ø> (-0.47%)` | :arrow_down: |
| [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `72.22% <0%> (-5.75%)` | :arrow_down: |
| [...che/openwhisk/core/loadBalancer/LoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0xvYWRCYWxhbmNlci5zY2FsYQ==) | `76.92% <0%> (-6.42%)` | :arrow_down: |
| [...rg/apache/openwhisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `92.1% <0%> (-5.12%)` | :arrow_down: |
| [...e/loadBalancer/ShardingContainerPoolBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL1NoYXJkaW5nQ29udGFpbmVyUG9vbEJhbGFuY2VyLnNjYWxh) | `85.32% <0%> (-3.38%)` | :arrow_down: |
| [...e/openwhisk/core/containerpool/ContainerPool.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQb29sLnNjYWxh) | `85.31% <0%> (-11.51%)` | :arrow_down: |
| [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `94.85% <100%> (+0.07%)` | :arrow_up: |
| [.../apache/openwhisk/core/controller/Controller.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9Db250cm9sbGVyLnNjYWxh) | `68.03% <14.28%> (-16.18%)` | :arrow_down: |
| [.../openwhisk/core/invoker/DefaultInvokerServer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9EZWZhdWx0SW52b2tlclNlcnZlci5zY2FsYQ==) | `27.27% <27.27%> (ø)` | |
| [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `69.4% <49.18%> (-11.3%)` | :arrow_down: |
| ... and [22 more](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=footer). Last update [0fcbe81...8573912](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r363580086
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +164,65 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerUsername = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.username");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+ private val controllerPassword = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.password");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerUsername && password == controllerPassword) {
+ entity(as[String]) { runtime =>
Review comment:
Usually, entity(as[Runtimes]) may be better, but if we apply this, need to change `Runtimes` to support convert json to entity Runtimes, and on the other hand, runtime.json's format doesn't match Runtimes entity, need to change a lot if use `entity(as[Runtimes]) ` to receive the data.
Fortunately，we can reuse openwhisk itself's initialize method, just convert the runtime string to Runtimes.
So here, i think pass runtime string would be ok.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371009731
##########
File path: tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfiguration.scala
##########
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.operation
+
+import akka.http.scaladsl.Http
+import akka.http.scaladsl.model.headers.{Authorization, BasicHttpCredentials}
+import akka.http.scaladsl.model.{ContentTypes, HttpEntity, HttpMethods, HttpRequest, StatusCodes}
+import akka.http.scaladsl.unmarshalling.Unmarshal
+import akka.stream.ActorMaterializer
+import common._
+import common.rest.HttpConnection
+import org.apache.openwhisk.core.connector.PrewarmContainerDataList
+import org.apache.openwhisk.core.connector.PrewarmContainerDataProtocol._
+import org.junit.runner.RunWith
+import org.scalatest.Matchers
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.junit.JUnitRunner
+import spray.json._
+import system.rest.RestUtil
+
+import scala.concurrent.duration._
+import scala.util.Random
+
+@RunWith(classOf[JUnitRunner])
+class RuntimeConfigurationTests
+ extends TestHelpers
+ with RestUtil
+ with Matchers
+ with ScalaFutures
+ with WskActorSystem
+ with StreamLogging {
+
+ implicit val materializer = ActorMaterializer()
+
+ val kind = "nodejs:10"
+ val memory = 128
+ var count = new Random().nextInt(3) + 1
+
+ def getRuntimes = {
+ s""" {
+ "runtimes": {
+ "nodejs": [{
+ "kind": "${kind}",
+ "default": true,
+ "image": {
+ "prefix": "openwhisk",
+ "name": "action-nodejs-v10",
+ "tag": "nightly"
+ },
+ "deprecated": false,
+ "attached": {
+ "attachmentName": "codefile",
+ "attachmentType": "text/plain"
+ },
+ "stemCells": [{
+ "count": ${count},
+ "memory": "${memory} MB"
+ }]
+ }]
+ }
+ }"""
+ }
+
+ val invokerProtocol = WhiskProperties.getInvokerProtocol
+ val invokerAddress = WhiskProperties.getBaseInvokerAddress
+ val invokerUsername = WhiskProperties.getInvokerUsername
+ val invokerPassword = WhiskProperties.getInvokerPassword
+ val invokerAuthHeader = Authorization(BasicHttpCredentials(invokerUsername, invokerPassword))
+
+ val controllerProtocol = WhiskProperties.getControllerProtocol
+ val controllerAddress = WhiskProperties.getBaseControllerAddress
+ val controllerUsername = WhiskProperties.getControllerUsername
+ val controllerPassword = WhiskProperties.getControllerPassword
+ val controllerAuthHeader = Authorization(BasicHttpCredentials(controllerUsername, controllerPassword))
+
+ val getRuntimeUrl = s"${invokerProtocol}://${invokerAddress}/getRuntime"
+ val invokerChangeRuntimeUrl = s"${invokerProtocol}://${invokerAddress}/config/runtime"
+ val controllerChangeRuntimeUrl =
+ s"${controllerProtocol}://${controllerAddress}/config/runtime"
+
+ it should "change assigned invoker node's runtime config directly" in {
+ //Change config
+ Http()
+ .singleRequest(
+ HttpRequest(
+ method = HttpMethods.POST,
+ uri = s"${invokerChangeRuntimeUrl}",
+ headers = List(invokerAuthHeader),
+ entity = HttpEntity(ContentTypes.`text/plain(UTF-8)`, getRuntimes)),
+ connectionContext = HttpConnection.getContext(invokerProtocol))
+ .map { response =>
+ response.status shouldBe StatusCodes.OK
+ }
+
+ Thread.sleep(5.seconds.toMillis)
Review comment:
why?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r365698617
##########
File path: common/scala/src/main/resources/reference.conf
##########
@@ -27,7 +27,7 @@ whisk.spi {
EntitlementSpiProvider = org.apache.openwhisk.core.entitlement.LocalEntitlementProvider
AuthenticationDirectiveProvider = org.apache.openwhisk.core.controller.BasicAuthenticationDirective
InvokerProvider = org.apache.openwhisk.core.invoker.InvokerReactive
- InvokerServerProvider = org.apache.openwhisk.core.invoker.DefaultInvokerServer
+ InvokerServerProvider = org.apache.openwhisk.core.invoker.InvokerServer
Review comment:
`DefaultInvokerServer` is already reverted
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371008732
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
Review comment:
`.error` is for internal errors, where here this is user input error, i think `.info` is better.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385011585
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
+ if (targetValue.matches(pattern)) {
+ val invokerArray = targetValue.split(":")
+ val beginIndex = invokerArray(0).toInt
+ val finishIndex = invokerArray(1).toInt
+ if (finishIndex < beginIndex) {
+ complete(s"finishIndex can't be less than beginIndex")
+ } else {
+ val targetInvokers = (beginIndex to finishIndex).toList
+ loadBalancer.sendRuntimeToInvokers(runtime, Some(targetInvokers))
Review comment:
Yes, validate it in ShardingContainerPoolLoadbalacer.scala
```
/** send runtime to invokers*/
override def sendRuntimeToInvokers(runtime: String, targetInvokers: Option[List[Int]]): Unit = {
val runtimeMessage = RuntimeMessage(runtime)
schedulingState.managedInvokers.filter { manageInvoker =>
targetInvokers.getOrElse(schedulingState.managedInvokers.map(_.id.instance)).contains(manageInvoker.id.instance)
} foreach { invokerHealth =>
val topic = s"invoker${invokerHealth.id.toInt}"
messageProducer.send(topic, runtimeMessage).andThen {
case Success(_) =>
logging.info(this, s"Successfully posted runtime to topic $topic")
case Failure(_) =>
logging.error(this, s"Failed posted runtime to topic $topic")
}
}
}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] codecov-io edited a comment on issue #4790: Add
administrative interface to invoker and controller to reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-573566159
# [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=h1) Report
> Merging [#4790](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/08efb58cf5eef3475bdb41e45db448250bcf9c17?src=pr&el=desc) will **decrease** coverage by `6.85%`.
> The diff coverage is `41.78%`.
[](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4790 +/- ##
==========================================
- Coverage 82.36% 75.51% -6.86%
==========================================
Files 198 199 +1
Lines 9021 9123 +102
Branches 353 379 +26
==========================================
- Hits 7430 6889 -541
- Misses 1591 2234 +643
```
| [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...la/org/apache/openwhisk/core/invoker/Invoker.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyLnNjYWxh) | `71.66% <ø> (-0.47%)` | :arrow_down: |
| [.../apache/openwhisk/core/controller/Controller.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9Db250cm9sbGVyLnNjYWxh) | `0% <0%> (ø)` | :arrow_up: |
| [...che/openwhisk/core/loadBalancer/LoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0xvYWRCYWxhbmNlci5zY2FsYQ==) | `15.38% <0%> (-1.29%)` | :arrow_down: |
| [...e/loadBalancer/ShardingContainerPoolBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL1NoYXJkaW5nQ29udGFpbmVyUG9vbEJhbGFuY2VyLnNjYWxh) | `73.91% <0%> (-2.93%)` | :arrow_down: |
| [...rg/apache/openwhisk/core/entity/ExecManifest.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWNNYW5pZmVzdC5zY2FsYQ==) | `97.36% <100%> (+0.14%)` | :arrow_up: |
| [.../scala/org/apache/openwhisk/core/WhiskConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvV2hpc2tDb25maWcuc2NhbGE=) | `94.85% <100%> (+0.03%)` | :arrow_up: |
| [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `76.86% <37.5%> (-3.3%)` | :arrow_down: |
| [...e/openwhisk/core/containerpool/ContainerPool.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQb29sLnNjYWxh) | `89.52% <44.44%> (-6.13%)` | :arrow_down: |
| [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `78.62% <64.4%> (-1.03%)` | :arrow_down: |
| [.../openwhisk/core/invoker/DefaultInvokerServer.scala](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9EZWZhdWx0SW52b2tlclNlcnZlci5zY2FsYQ==) | `75% <75%> (ø)` | |
| ... and [23 more](https://codecov.io/gh/apache/openwhisk/pull/4790/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=footer). Last update [08efb58...fc63c07](https://codecov.io/gh/apache/openwhisk/pull/4790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385008961
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
Review comment:
Already changed to `.info`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4790:
[WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r363723237
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +164,65 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerUsername = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.username");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+ private val controllerPassword = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.password");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerUsername && password == controllerPassword) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
+ if (targetValue.matches(pattern)) {
+ val invokerArray = targetValue.split(":")
+ val beginIndex = invokerArray(0).toInt
+ val finishIndex = invokerArray(1).toInt
+ if (finishIndex < beginIndex) {
Review comment:
Would be great to support `0:0` case as well.
Ansible is using the same way.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371009313
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
+ limit match {
+ case Some(targetValue) =>
+ val pattern = "\\d+:\\d"
+ if (targetValue.matches(pattern)) {
Review comment:
scala nit: you can rewrite this either if/else and nested clauses using case matching on regex.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r363580358
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +164,65 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerUsername = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.username");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+ private val controllerPassword = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.password");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerUsername && password == controllerPassword) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
Review comment:
Support passed limit invokers, e.g. ?limit=0:1
And config runtime action will be done on limited invokers which included in managed invokers
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385013712
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.core.invoker
+
+import akka.actor.ActorSystem
+import akka.http.scaladsl.model.StatusCodes
+import akka.http.scaladsl.model.headers.BasicHttpCredentials
+import akka.http.scaladsl.server.Route
+import org.apache.openwhisk.common.{InvokerCredentials, Logging, TransactionId}
+import org.apache.openwhisk.core.ConfigKeys
+import org.apache.openwhisk.core.containerpool.PrewarmingConfig
+import org.apache.openwhisk.core.entity.{CodeExecAsString, ExecManifest}
+import org.apache.openwhisk.http.BasicRasService
+
+import pureconfig._
+import pureconfig.generic.auto._
+
+import scala.concurrent.ExecutionContext
+
+/**
+ * Implements web server to handle certain REST API calls.
+ */
+class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionContext,
+ val actorSystem: ActorSystem,
+ val logger: Logging)
+ extends BasicRasService {
+
+ private val invokerCredentials = loadConfigOrThrow[InvokerCredentials](ConfigKeys.invokerCredentials)
+
+ override def routes(implicit transid: TransactionId): Route = {
Review comment:
When send to reqeust to backend (e.g. `curl -u xxx:xxx-X POST http://xxx:xxx:10001/config/runtime`), it will pull new runtime image immediately and create prewarm container.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on issue #4790: Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on issue #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#issuecomment-574063268
Already added test 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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: Add administrative interface to invoker and controller to
reconfigure runtimes
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: Add administrative interface to invoker and controller to reconfigure runtimes
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r385021269
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -279,6 +285,23 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
case RescheduleJob =>
freePool = freePool - sender()
busyPool = busyPool - sender()
+ case prewarmConfigList: PreWarmConfigList =>
+ laststPrewarmConfig = prewarmConfigList.list
Review comment:
Add a inner variable `laststPrewarmConfig`, just make it pointed to the passed config runtime.
Because in this patch: https://github.com/apache/openwhisk/pull/4698
when received `ContainerRemoved`(may be prewarm container crashed), it will backfill the prewarm.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371008969
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
Review comment:
scala nit: you can use a case match here instead.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371008660
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +171,56 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerCredentials = loadConfigOrThrow[ControllerCredentials](ConfigKeys.controllerCredentials)
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerCredentials.username && password == controllerCredentials.password) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
Review comment:
this completes the https request with status code 200 - is that what was intended?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4790:
Config runtime
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4790: Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r371009654
##########
File path: tests/src/test/scala/common/WhiskProperties.java
##########
@@ -258,10 +258,40 @@ public static int getControllerBasePort() {
return Integer.parseInt(whiskProperties.getProperty("controller.host.basePort"));
}
+ public static String getControllerProtocol() {
Review comment:
i dont think any of these changes are necessary if you read configs through pureconfig.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4790: [WIP]Config runtime
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4790: [WIP]Config runtime
URL: https://github.com/apache/openwhisk/pull/4790#discussion_r363580358
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -163,6 +164,65 @@ class Controller(val instance: ControllerInstanceId,
LogLimit.config,
runtimes,
List(apiV1.basepath()))
+
+ private val controllerUsername = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.username");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+ private val controllerPassword = {
+ val source = scala.io.Source.fromFile("/conf/controllerauth.password");
+ try source.mkString.replaceAll("\r|\n", "")
+ finally source.close()
+ }
+
+ /**
+ * config runtime
+ */
+ private val configRuntime = {
+ implicit val executionContext = actorSystem.dispatcher
+ (path("config" / "runtime") & post) {
+ extractCredentials {
+ case Some(BasicHttpCredentials(username, password)) =>
+ if (username == controllerUsername && password == controllerPassword) {
+ entity(as[String]) { runtime =>
+ val execManifest = ExecManifest.initialize(runtime)
+ if (execManifest.isFailure) {
+ logging.error(this, s"Received invalid runtimes manifest")
+ complete(s"Received invalid runtimes manifest")
+ } else {
+ parameter('limit.?) { limit =>
Review comment:
Support passed limit invokers, e.g. `?limit=0:1` (invoker0, invoker1)
And the config runtime request can be sent to some limited invokers which included in managed invokers as well.
----------------------------------------------------------------
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
With regards,
Apache Git Services