Contributor Guidelines

Contributing Documentation Changes

You were confused about our documentation? You ran into a pitfall that others also might run into? Help us making the Marathon documentation great.

The documentation that is published here actually gets generated from what is found in the docs directory.

If you simply want to correct a spelling mistake or improve the wording of a sentence, you can browse the mark down files here and use the edit button above the markup. That will make it easy to create a pull request that will be reviewed by us.

If want to contribute a larger improvement to our documentation:

  • Edit the files in the docs directory.
  • Check the rendered result as described in the docs/README.md.
  • If you are feeling perfectionistic, check if there is already an issue about this documentation deficiency here. Prefix your commit message with with “Fixes #1234 - “ where #1234 is the number of the issue.
  • Create a pull request against master.

Please rebase your pull requests on top of the current master using git fetch origin && git rebase origin/master, and squash your changes to a single commit as described here. Yes, we want you to rewrite history - the branch on which you are implementing your changes is only meant for this pull request. You can either rebase before or after you squash your commits, depending on how you’d like to resolve potential merge conflicts. The idea behind is that we don’t want an arbitrary number of commits for one pull request, but exactly one commit. This commit should be easy to merge onto master, therefore we ask you to rebase to master.

After you pull request has been accepted, there is still some manual work that we need to do to publish that documentation. But don’t worry, we will do that for you.

Getting Started with Code Changes

Maybe you already have a bugfix or enhancement in mind. If not, there are a number of relatively approachable issues with the label “good first issue”.

JIRA Issues (Suggesting Bugs and Improvements)

As a general rule, before any change is made to Marathon, a JIRA ticket is filed for bugs, or for proposed improvements. As a guideline, an improvement ticket should include the following:

  • Include a background and motivation
  • An overview of the proposed change
  • Some acceptance criteria (how would you manually verify that this change worked?)

As the Marathon product team, major changes and new features go through a process of proposal and discussion in order to help us understand the impact and how it aligns with our longer term goals. Creating a JIRA before submitting a code change sets the stage for this necessary discussion to happen, consensus on direction to be achieved, ideally before too much effort is spent coding it.

If a behavioral change isn’t discussed before code is submitted, there is a higher chance there will be some unconsidered side-effect of the change, or that the change is not in alignment with the long-term vision of Marathon.

Sometimes, we are overwhelmed by the never ending sea of JIRA. If you submit an issue, and don’t feel it is getting the level of attention that it should, please find us in the #marathon channel on the Mesos community Slack.

Backporting Policy

All development (new features and bug fixes) are made, first, to the master branch.

If code is heavily refactored but the behavior has not been changed, these patches are generally not backported unless it will help reduce merge conflicts in the future.

Bugs are backported to the current supported versions of Marathon. Historically, this usually tends to be the current stable release, plus the past 2 major releases. (IE, if 1.5.6 is the last stable release, then 1.4.x and 1.3.x would also receive the fix).

New features are rarely backported; when they are, they are backported to the latest stable version of Marathon. Major new features are definitely not backported.

Submitting Code Changes to Marathon

  1. A GitHub pull request is the preferred way of submitting code changes.

  2. If a JIRA issue doesn’t already exist, please open one for the proposed change or bug-fix (see the section above, “JIRA Issues”).

  3. Except for simple bug fixes, start a discussion in the JIRA ticket created in step 2 (ideally, in the comments section, but, also in the #marathon channel). Seek to get validation and consensus before making the change.

  4. Please rebase your pull requests on top of the current master, as needed, to integrate upstream changes and resolve conflicts. You can squash your commits if it helps resolve potential merge conflicts. When your PR is merged, it will be squashed to a single commit.

  5. If you are targeting to get something fixed for an older release of Marathon, fix it on master, first. If it is already fixed in master, first, find out why. See our “Backporting Policy” above.

  6. Please include in your commit message the line “JIRA Issues: MARATHON-1234”, where MARATHON-1234 is the JIRA issue mentioned in item 2 above. This helps reviewers better understand the context of the change.

  7. If you change modifies the public API or behavior, then the project documentation must be updated (in the same pull request). Further, notes about the change should be specified in changelog.md.

  8. Pull requests should include appropriate additions to the unit test suite (see “Test Guidelines”, below). If the change is a bugfix, then one of the added tests should fail without the fix.

  9. Compile your code prior to committing. We would like the result of our automatic code formatters to be included in the commit as to not produce a dirty work tree after fresh checkout and first compile.

  10. Run, at the very least, all unit tests (sbt test). Integration tests can also be run with sbt integration/test (requires docker).

Test Guidelines

General guidelines

  • Tests should extend mesosphere.UnitTest
  • Tests should avoid testing more behavior than necessary; IE - don’t test other libs or services if a mock/stub suffices.
  • Tests must be deterministic. Whenever possible, the system clock should not be an input to the test.

Fixtures

Our testing guidelines regarding fixtures are as follows:

  • DO NOT use var thing: Type = _ with before / after.
  • Fixture instantiation and tear down should be defined and handled in the same module.
  • When testing actor systems, we instantiate an actor system for the entire suite. It’s acceptable to let the actors get cleaned up when the suite finishes. Use either unique actor names or use the loan-fixtures technique to get a unique ActorRefFactory per test.
  • When teardown-per-test is desired, use the loan-fixtures methods. Otherwise, prefer parameterized case classes or traits.

Running Integration Tests

Prerequisites

Running integration tests requires that several components are installed and available locally:

  • Mesos (master and agent)
  • Docker
  • Mesos native lib (with MESOS_NATIVE_JAVA_LIBRARY set)

Note that the integration test suites launch and use a local, embedded, version of Zookeeper.

Installing Mesos

Marathon integration tests require an installation of Mesos in order to run. The Mesos executables, including mesos, mesos-master, and mesos-agent must be included in the PATH. Further, the environment variable MESOS_NATIVE_JAVA_LIBRARY should be set to the path of libmesos.dylib / libmesos.so.

Installing from source, manually

You can install Mesos from source, manually, by following the instructions here

Installing from source using mvm.sh

To install Mesos from source, you can use the provided scripts/mvm.sh script. Please read the header of the script for pre-requisite steps.

scripts/mvm.sh --latest

This will take 30 minutes or so to install on a modern processor.

Installing from Homebrew (OS X, fastest)

You can install a version of Mesos from Homebrew. The Homebrew Mesos version often lags behind the latest version of Mesos. Most of the time, however, this is not a problem.

Running Tests

Some integration tests are disabled by default (references WhenEnvSet). Setting these environment variables will enable them.

export RUN_DOCKER_INTEGRATION_TESTS=false
export RUN_MESOS_INTEGRATION_TESTS=false

To run all of the integration tests using sbt:

sbt integration/test

To run a single integration test:

$ sbt

marathon(...)> set testOptions in Test := Nil

...


marathon(...)> testOnly mesosphere.marathon.integration.AppDeployIntegrationTest -- -oF

Source Files

  • Public classes should be defined in a file of the same name, except for trivial subtypes of a sealed trait. In that case, the file name must be the plural form of the trait, and it must begin with a lowercase letter.

  • Some of the source files have diagrams as part of the comments using the PlantUML language.
    To see a rendered diagram use the online version or install locally.

Style

Style Checker

Executing the test task in SBT also invokes the style checker. Some basic style issues will cause the build to fail: While you should fix all of these, you can disable them if it is a false positive with // linter:ignore Arg as listed here: Linter.

Type Annotations

Scala has a powerful type inference engine. The reason for including more type annotations than are required to make the program compile is to increase readability.

  • Methods with public or protected scope must have an explicit return type annotation. Without it, an implementaiton change later on could accidentally change the public API if a new type is inferred.

  • Nontrivial expressions must be explicitly annotated. Of course, “nontrivial” is subjective. As a rule of thumb, if an expression contains more than one higher-order function, annotate the result type and/or consider breaking the expression down into simpler intermediate values.

Null

Assigning the null value should be avoided. It’s usually only necessary when calling into a Java library that ascribes special semantics to null. Prefer scala.Option in all other cases.

Higher Order Functions

GOOD:

xs.map(f)
xs.map(_.size)
xs.map(_ * 2)
xs.map { item =>
  item -> f(item)
}
xs.map {
  case a: A if a.isEmpty => f(a)
  case a: A              => g(a)
  case b: B              => h(b)
  case other: _          => e(other)
}

Note: match expressions must be exhaustive unless the higher-order function signature explicitly expects a PartialFunction[_, _] as in collect, and collectFirst.

for {
  x <- xs
  y <- x
  z <- y
} yield z

BAD:

xs map f // dangerous if more infix operators follow
xs.map(item => f(item)) // use curlies
xs.flatMap(_.flatMap(_.map(_.z))) // replace with a for comprehension

Code Commenting

Multi-line comments should use /*, as follows:

/*
 * This variable is so important that it warrants multiple lines worth of
 * lengthy documentation.
 */
val important = 42

Method or class description documentation should use /**. Ideally parameter and returns are documented, also, as follows:

/**
  * Guide Charlie towards a happier future
  *
  * @param sad Whether Charlie is sad
  * @param frowning Whether Charlie has a big fat frown
  *
  * @return False if Charlie would rather keep his ear clear
  */
def insertBananaIntoEar(sad: Boolean, frowning: Boolean): Boolean = {
  ...
}