You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by susanxhuynh <gi...@git.apache.org> on 2017/08/10 18:38:10 UTC

[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

GitHub user susanxhuynh opened a pull request:

    https://github.com/apache/spark/pull/18910

    [SPARK-21694][MESOS] Support Mesos CNI network labels

    JIRA ticket: https://issues.apache.org/jira/browse/SPARK-21694
    
    ## What changes were proposed in this pull request?
    
    Spark already supports launching containers attached to a given CNI network by specifying it via the config `spark.mesos.network.name`.
    
    This PR adds support to pass in network labels to CNI plugins via a new config option `spark.mesos.network.labels`. These network labels are key-value pairs that are set in the `NetworkInfo` of both the driver and executor tasks. More details in the related Mesos documentation:  http://mesos.apache.org/documentation/latest/cni/#mesos-meta-data-to-cni-plugins
    
    ## How was this patch tested?
    
    Unit tests, for both driver and executor tasks.
    Manual integration test to submit a job with the `spark.mesos.network.labels` option, hit the mesos/state.json endpoint, and check that the labels are set in the driver and executor tasks.
    
    @ArtRand @skonto 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mesosphere/spark sh-mesos-cni-labels

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/18910.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #18910
    
----
commit 255e5195a414f0f9e609cb579a2e8fdfea200be1
Author: Susan X. Huynh <xh...@mesosphere.com>
Date:   2017-08-10T18:19:51Z

    [SPARK-21694][MESOS] Allow the user to pass network labels to CNI plugins via

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    @srowen Thanks for reviewing. I've removed those spaces; please take a look.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r132766509
  
    --- Diff: resource-managers/mesos/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala ---
    @@ -582,6 +583,10 @@ class MesosCoarseGrainedSchedulerBackendSuite extends SparkFunSuite
         val networkInfos = launchedTasks.head.getContainer.getNetworkInfosList
         assert(networkInfos.size == 1)
         assert(networkInfos.get(0).getName == "test-network-name")
    +    assert(networkInfos.get(0).getLabels.getLabels(0).getKey == "key1")
    +    assert(networkInfos.get(0).getLabels.getLabels(0).getValue == "val1")
    +    assert(networkInfos.get(0).getLabels.getLabels(1).getKey == "key2")
    +    assert(networkInfos.get(0).getLabels.getLabels(1).getValue == "val2")
    --- End diff --
    
    @ArtRand I believe this _is_ testing the worker tasks: (L.572) the `setBackend()` method creates a MesosCoarseGrainedSchedulerBackend, L.577-579 creates the requirements for an Executor, (L. 580) backend.resourceOffers() looks for an offer that matches the requirements and launches an Executor task. 
    There is a separate unit test above, in MesosClusterSchedulerSuite.scala, which checks for the labels in the driver itself.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by ArtRand <gi...@git.apache.org>.
Github user ArtRand commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    LGTM.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81051/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    @skonto @ArtRand Thanks for the feedback. I have fixed the documentation and added NETWORK_NAME to the config object. Please let me know what you think.
    
    @skonto I have not tested this particular change on a real CNI network. I think the only difference is that a Spark job runs on a different network, but there's no change in the Spark functionality.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80555/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80498/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80556/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #80555 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80555/testReport)** for PR 18910 at commit [`d261593`](https://github.com/apache/spark/commit/d261593a68fd5bd9d2527118eca7d2665570bb4e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #80555 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80555/testReport)** for PR 18910 at commit [`d261593`](https://github.com/apache/spark/commit/d261593a68fd5bd9d2527118eca7d2665570bb4e).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by ArtRand <gi...@git.apache.org>.
Github user ArtRand commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r132715831
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala ---
    @@ -21,6 +21,7 @@ import org.apache.mesos.Protos.{ContainerInfo, Image, NetworkInfo, Parameter, Vo
     import org.apache.mesos.Protos.ContainerInfo.{DockerInfo, MesosInfo}
     
     import org.apache.spark.{SparkConf, SparkException}
    +import org.apache.spark.deploy.mesos.config._
    --- End diff --
    
    +1, this makes it more consistent with MesosClusterScheduler


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    @susanxhuynh have you tested how spark jobs behave with a real cni network, just curious should be no difference right? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #81051 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81051/testReport)** for PR 18910 at commit [`38e3b4d`](https://github.com/apache/spark/commit/38e3b4df33ac37c5e75224b607d3255ea8296f7b).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by ArtRand <gi...@git.apache.org>.
Github user ArtRand commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r132715684
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/deploy/mesos/config.scala ---
    @@ -70,4 +70,12 @@ package object config {
               "during a temporary disconnection, before tearing down all the executors.")
           .doubleConf
           .createWithDefault(0.0)
    +
    +  private [spark] val NETWORK_LABELS =
    +    ConfigBuilder("spark.mesos.network.labels")
    +      .doc("Network labels to pass to CNI plugins.  This is a comma-separated list " +
    +        "of key-value pairs, where each key-value pair has the format key:value. " +
    +        "Example: key1=val1,key2=val2")
    --- End diff --
    
    I thought we use `:` to separate the key-value pairs? So it would say:
    `Example: key1:val1,key2:val2


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Merged to master


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #80556 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80556/testReport)** for PR 18910 at commit [`dc09312`](https://github.com/apache/spark/commit/dc09312a9d011e7d2d6c62c5b0ac7982284ab6aa).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r134795818
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/deploy/mesos/config.scala ---
    @@ -70,4 +70,19 @@ package object config {
               "during a temporary disconnection, before tearing down all the executors.")
           .doubleConf
           .createWithDefault(0.0)
    +
    +  private [spark] val NETWORK_NAME =
    --- End diff --
    
    Tiny nit, could you remove the space after `private`? that would make it consistent with the rest of the code.
    
    I trust your judgment as to whether this is the right functional change for Mesos and the details and style look OK to me otherwise.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    @vanzin Would you mind helping review this PR? Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by ArtRand <gi...@git.apache.org>.
Github user ArtRand commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r132716461
  
    --- Diff: resource-managers/mesos/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala ---
    @@ -582,6 +583,10 @@ class MesosCoarseGrainedSchedulerBackendSuite extends SparkFunSuite
         val networkInfos = launchedTasks.head.getContainer.getNetworkInfosList
         assert(networkInfos.size == 1)
         assert(networkInfos.get(0).getName == "test-network-name")
    +    assert(networkInfos.get(0).getLabels.getLabels(0).getKey == "key1")
    +    assert(networkInfos.get(0).getLabels.getLabels(0).getValue == "val1")
    +    assert(networkInfos.get(0).getLabels.getLabels(1).getKey == "key2")
    +    assert(networkInfos.get(0).getLabels.getLabels(1).getValue == "val2")
    --- End diff --
    
    Is there a way to test that the driver's worker tasks also get the labels? It's important that all of the `ContainerInfo`s associated with a job (not jus the driver scheduler) get the labels for CNI to work. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/18910


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r132580807
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala ---
    @@ -21,6 +21,7 @@ import org.apache.mesos.Protos.{ContainerInfo, Image, NetworkInfo, Parameter, Vo
     import org.apache.mesos.Protos.ContainerInfo.{DockerInfo, MesosInfo}
     
     import org.apache.spark.{SparkConf, SparkException}
    +import org.apache.spark.deploy.mesos.config._
    --- End diff --
    
    Might be better to avoid wildcard import.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by ArtRand <gi...@git.apache.org>.
Github user ArtRand commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r132715924
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala ---
    @@ -162,7 +163,11 @@ private[mesos] object MesosSchedulerBackendUtil extends Logging {
         }
     
         conf.getOption("spark.mesos.network.name").map { name =>
    -      val info = NetworkInfo.newBuilder().setName(name).build()
    +      val networkLabels = MesosProtoUtils.mesosLabels(conf.get(NETWORK_LABELS).getOrElse(""))
    --- End diff --
    
    +1 for making the `NETWORK_LABELS` alias. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #81051 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81051/testReport)** for PR 18910 at commit [`38e3b4d`](https://github.com/apache/spark/commit/38e3b4df33ac37c5e75224b607d3255ea8296f7b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18910: [SPARK-21694][MESOS] Support Mesos CNI network la...

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18910#discussion_r132581612
  
    --- Diff: resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala ---
    @@ -162,7 +163,11 @@ private[mesos] object MesosSchedulerBackendUtil extends Logging {
         }
     
         conf.getOption("spark.mesos.network.name").map { name =>
    -      val info = NetworkInfo.newBuilder().setName(name).build()
    +      val networkLabels = MesosProtoUtils.mesosLabels(conf.get(NETWORK_LABELS).getOrElse(""))
    --- End diff --
    
    Shouldn't spark.mesos.network.name have a config name like labels have: NETWORK_LABELS?
    At some point all config options should be placed in one file to be easily manipulated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #80498 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80498/testReport)** for PR 18910 at commit [`255e519`](https://github.com/apache/spark/commit/255e5195a414f0f9e609cb579a2e8fdfea200be1).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #80556 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80556/testReport)** for PR 18910 at commit [`dc09312`](https://github.com/apache/spark/commit/dc09312a9d011e7d2d6c62c5b0ac7982284ab6aa).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    **[Test build #80498 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80498/testReport)** for PR 18910 at commit [`255e519`](https://github.com/apache/spark/commit/255e5195a414f0f9e609cb579a2e8fdfea200be1).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18910: [SPARK-21694][MESOS] Support Mesos CNI network labels

Posted by susanxhuynh <gi...@git.apache.org>.
Github user susanxhuynh commented on the issue:

    https://github.com/apache/spark/pull/18910
  
    @srowen Sean, would you like to review this PR? Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org