Skip to content

[SPARK-18232] [Mesos] Support CNI #15740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
mgummelt wants to merge 1 commit into from
Closed

[SPARK-18232] [Mesos] Support CNI #15740

mgummelt wants to merge 1 commit into from

Conversation

@mgummelt
Copy link

@mgummelt mgummelt commented Nov 2, 2016

What changes were proposed in this pull request?

Adds support for CNI-isolated containers

How was this patch tested?

I launched SparkPi both with and without spark.mesos.network.name, and verified the job completed successfully.

@SparkQA
Copy link

SparkQA commented Nov 2, 2016

Test build #68020 has finished for PR 15740 at commit 7caaa84.

  • This patch fails Scala style tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@mgummelt mgummelt force-pushed the spark-342-cni branch 2 times, most recently from 4a9b206 to ac1d4a0 Compare November 2, 2016 22:44
@SparkQA
Copy link

SparkQA commented Nov 2, 2016

Test build #68021 has finished for PR 15740 at commit 4a9b206.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

mgummelt
mgummelt commented Nov 2, 2016
View reviewed changes
...scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala Outdated
Copy link
Author

@mgummelt mgummelt Nov 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

portMappings were never supported, since we don't support bridge networking, so I removed this feature and the docs for it.

@mgummelt
Copy link
Author

mgummelt commented Nov 2, 2016

cc @skonto @susanxhuynh

@SparkQA
Copy link

SparkQA commented Nov 2, 2016

Test build #68022 has finished for PR 15740 at commit ac1d4a0.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 2, 2016

Test build #68032 has finished for PR 15740 at commit b1b8b22.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

susanxhuynh
susanxhuynh reviewed Nov 4, 2016
View reviewed changes
mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala Outdated
Copy link
Contributor

@susanxhuynh susanxhuynh Nov 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the parsePortMappingsSpec method now (defined above)?

Copy link
Author

@mgummelt mgummelt Nov 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually just re-added the ports stuff, in case we want to support it in the future. It's just removed from the docs.

susanxhuynh
susanxhuynh reviewed Nov 4, 2016
View reviewed changes
Copy link
Contributor

@susanxhuynh susanxhuynh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few comments. In general, LGTM.

...main/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackend.scala Outdated
Copy link
Contributor

@susanxhuynh susanxhuynh Nov 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the hostname set to 0.0.0.0?

@SparkQA
Copy link

SparkQA commented Nov 4, 2016

Test build #68144 has finished for PR 15740 at commit 5789d6b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

susanxhuynh
susanxhuynh approved these changes Nov 4, 2016
View reviewed changes
Copy link
Contributor

@susanxhuynh susanxhuynh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mgummelt
Copy link
Author

mgummelt commented Nov 4, 2016

@srowen Can I get a merge?

@skonto
Copy link
Contributor

skonto commented Nov 4, 2016

LGTM. @mgummelt do you have integration tests for that?

@mgummelt
Copy link
Author

mgummelt commented Nov 4, 2016

I've written one. I'll merge it after this goes through.

@skonto
Copy link
Contributor

skonto commented Nov 4, 2016

Great!

@mgummelt
Copy link
Author

mgummelt commented Nov 9, 2016

cc @srowen @rxin Can we merge this please? We'd like to backport this to our internal 2.0.2 release, so I'd like to get it in soon.

@mgummelt
Copy link
Author

mgummelt commented Nov 14, 2016

@rxin @srowen What will it take to merge this?

@rxin
Copy link
Contributor

rxin commented Nov 15, 2016

Merging in master! Thanks.

@asfgit asfgit closed this in d89bfc9 Nov 15, 2016
@sumitchawla
Copy link

sumitchawla commented Nov 15, 2016

is there any documentation about this feature?

@mgummelt
Copy link
Author

mgummelt commented Nov 15, 2016

@sumitchawla Check out the changes to running-on-mesos.md. Most of the info on CNI is in the linked Mesos docs.

@sumitchawla
Copy link

sumitchawla commented Nov 15, 2016

thanks @mgummelt . Which release version would contain this change?

@mgummelt
Copy link
Author

mgummelt commented Nov 15, 2016

I was hoping 2.1, but it looks like 2.1 was cut before this, so now it's looking to be 2.2. We've backported this into 2.0.2 for DC/OS Spark, though, and we'll do so for 2.1 as well.

@rxin
Copy link
Contributor

rxin commented Nov 15, 2016

Sounds like a great solution!

uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
@uzadude
[SPARK-18232][MESOS] Support CNI
22e95af 
## What changes were proposed in this pull request?

Adds support for CNI-isolated containers

## How was this patch tested?

I launched SparkPi both with and without `spark.mesos.network.name`, and verified the job completed successfully.

Author: Michael Gummelt <mgummelt@mesosphere.io>

Closes apache#15740 from mgummelt/spark-342-cni.
mgummelt pushed a commit to d2iq-archive/spark that referenced this pull request Feb 23, 2017
[SPARK-18232][MESOS] Support CNI
8aeef01 
## What changes were proposed in this pull request?

Adds support for CNI-isolated containers

## How was this patch tested?

I launched SparkPi both with and without `spark.mesos.network.name`, and verified the job completed successfully.

Author: Michael Gummelt <mgummelt@mesosphere.io>

Closes apache#15740 from mgummelt/spark-342-cni.
ammolitor pushed a commit to splicemachine/spark that referenced this pull request Apr 26, 2017
[SPARK-18232][MESOS] Support CNI
9e997b3 
Adds support for CNI-isolated containers

I launched SparkPi both with and without `spark.mesos.network.name`, and verified the job completed successfully.

Author: Michael Gummelt <mgummelt@mesosphere.io>

Closes apache#15740 from mgummelt/spark-342-cni.

Conflicts:
	mesos/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala
mgummelt pushed a commit to d2iq-archive/spark that referenced this pull request Jun 8, 2017
[SPARK-18232][MESOS] Support CNI
fdbd555 
## What changes were proposed in this pull request?

Adds support for CNI-isolated containers

## How was this patch tested?

I launched SparkPi both with and without `spark.mesos.network.name`, and verified the job completed successfully.

Author: Michael Gummelt <mgummelt@mesosphere.io>

Closes apache#15740 from mgummelt/spark-342-cni.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@susanxhuynh susanxhuynh susanxhuynh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mgummelt @SparkQA @skonto @rxin @sumitchawla @susanxhuynh