Basic Workflow
==============
-The following chart illustrates basic development workflow:
+The following chart illustrates the basic Ceph development workflow:
.. ditaa::
| pull request | git push \-------------/
\--------------/
-Below we present an explanation of this chart. The explanation is written
-with the assumption that you, the reader, are a beginning developer who
-has an idea for a bugfix, but do not know exactly how to proceed. Watch
-the `Getting Started with Ceph Development
-<https://www.youtube.com/watch?v=t5UIehZ1oLs>`_ video for
-a practical summary of the same.
+The below explanation is written with the assumption that you, the reader, are
+a new contributor who has an idea for a bugfix or enhancement, but do not know
+exactly how to proceed. Watch the `Getting Started with Ceph Development
+<https://www.youtube.com/watch?v=t5UIehZ1oLs>`_ video for a practical summary
+of this workflow.
Update the tracker
------------------
-Before you start, you should know the :ref:`issue-tracker` number of the bug
-you intend to fix. If there is no tracker issue, now is the time to create
-one.
+Before you start, you should know the :ref:`issue-tracker` (Redmine) number
+of the bug you intend to fix. If there is no tracker issue, now is the time to
+create one for code changes. Straightforward documentation cleanup does
+not necessarily require a corresponding tracker issue. However, an issue
+(ticket) should be created if one is adding new documentation chapters or
+files, or for other substantial changes.
-The tracker is there to explain the issue (bug) to your fellow Ceph
-developers and keep them informed as you make progress toward resolution.
-To this end, then, provide a descriptive title as well as sufficient
-information and details in the description.
+The tracker ticket serves to explain the issue (bug) to your fellow Ceph
+developers and keep them informed as you make progress toward resolution. To
+this end, please provide a descriptive title and write appropriate information
+and details into the description. When composing the ticket's title, consider "If I
+want to search for this ticket two years from now, what keywords will I search
+for?"
If you have sufficient tracker permissions, assign the bug to yourself by
-changing the ``Assignee`` field. If your tracker permissions have not yet
-been elevated, simply add a comment to the issue with a short message like
-"I am working on this issue".
+setting the ``Assignee`` field. If your tracker permissions have not been
+elevated, simply add a comment with a short message like "I am working on this
+issue".
Upstream code
-------------
-This section, and the ones that follow, correspond to the nodes in the
-above chart.
+This section, and the ones that follow, correspond to nodes in the above chart.
-The upstream code lives in https://github.com/ceph/ceph.git, which is
-sometimes referred to as the "upstream repo", or simply "upstream". As the
-chart illustrates, we will make a local copy of this code, modify it, test
-our modifications, and submit the modifications back to the upstream repo
-for review.
+The upstream code is found at https://github.com/ceph/ceph.git, which is known
+as the "upstream repo", or simply "upstream". As the chart shows, we will make
+a local copy of this repository, modify it, test our modifications, then submit
+the modifications for review and merging.
A local copy of the upstream code is made by
-1. forking the upstream repo on GitHub, and
-2. cloning your fork to make a local working copy
+1. Forking the upstream repo on GitHub, and
+2. Cloning your fork to make a local working copy
-See the `the GitHub documentation
+See the `GitHub documentation
<https://help.github.com/articles/fork-a-repo/#platform-linux>`_ for
detailed instructions on forking. In short, if your GitHub username is
-"mygithubaccount", your fork of the upstream repo will show up at
+"mygithubaccount", your fork of the upstream repo will appear at
https://github.com/mygithubaccount/ceph. Once you have created your fork,
-you clone it by doing:
+clone it by running:
.. prompt:: bash $
git clone https://github.com/mygithubaccount/ceph
-While it is possible to clone the upstream repo directly, in this case you
-must fork it first. Forking is what enables us to open a `GitHub pull
+While it is possible to clone the upstream repo directly, for the Ceph workflow
+you must fork it first. Forking is what enables us to open a `GitHub pull
request`_.
For more information on using GitHub, refer to `GitHub Help
Local environment
-----------------
-In the local environment created in the previous step, you now have a
-copy of the ``master`` branch in ``remotes/origin/master``. Since the fork
+In the local environment created in the previous step, you now have a copy of
+the ``master`` branch in ``remotes/origin/master``. This fork
(https://github.com/mygithubaccount/ceph.git) is frozen in time and the
upstream repo (https://github.com/ceph/ceph.git, typically abbreviated to
-``ceph/ceph.git``) is updated frequently by other developers, you will need
-to sync your fork periodically. To do this, first add the upstream repo as
-a "remote" and fetch it
+``ceph/ceph.git``) is updated frequently by other contributors, you must sync
+your fork periodically. Failure to do so may result in your commits and pull
+requests failing to merge because they refer to file contents that have since
+changed.
+
+First, ensure that you have properly configured your local git environment with
+your name and email address. Skip this step if you have already configured this
+information.
+
+.. prompt:: bash $
+
+ git config user.name "FIRST_NAME LAST_NAME"
+ git config user.email "MY_NAME@example.com"
+
+Now add the upstream repo as a "remote" and fetch it:
.. prompt:: bash $
the last sync. After running these commands, all the branches from
``ceph/ceph.git`` are downloaded to the local git repo as
``remotes/ceph/$BRANCH_NAME`` and can be referenced as
-``ceph/$BRANCH_NAME`` in certain git commands.
+``ceph/$BRANCH_NAME`` in local git commands.
For example, your local ``master`` branch can be reset to the upstream Ceph
-``master`` branch by doing
+``master`` branch by running
.. prompt:: bash $
git checkout master
git reset --hard ceph/master
-Finally, the ``master`` branch of your fork can then be synced to upstream
+Finally, the ``master`` branch of your fork is synced to the upstream
master by
.. prompt:: bash $
Bugfix branch
-------------
-Next, create a branch for the bugfix:
+Next, create a branch for your bugfix:
.. prompt:: bash $
git checkout -b fix_1
git push -u origin fix_1
-This creates a ``fix_1`` branch locally and in our GitHub fork. At this
-point, the ``fix_1`` branch is identical to the ``master`` branch, but not
-for long! You are now ready to modify the code.
+This creates a ``fix_1`` branch locally and in our GitHub fork. At this point,
+the ``fix_1`` branch is identical to the ``master`` branch, but not for long!
+You are now ready to modify the code. Be careful to always run `git checkout
+master` first, otherwise you may find commits from an unrelated branch mixed
+with your new work.
Fix bug locally
---------------
-At this point, change the status of the tracker issue to "In progress" to
-communicate to the other Ceph developers that you have begun working on a
-fix. If you don't have permission to change that field, your comment that
-you are working on the issue is sufficient.
+Now change the status of the tracker issue to "In progress" to communicate to
+other Ceph contributors that you have begun working on a fix. This helps avoid
+duplication of effort. If you don't have permission to change that field, your
+previous comment that you are working on the issue is sufficient.
-Possibly, your fix is very simple and requires only minimal testing.
-More likely, it will be an iterative process involving trial and error, not
-to mention skill. An explanation of how to fix bugs is beyond the
-scope of this document. Instead, we focus on the mechanics of the process
-in the context of the Ceph project.
+Your fix may be very simple and require only minimal testing. More likely,
+this will be an iterative process involving trial and error, not to mention
+skill. An explanation of how to fix bugs is beyond the scope of this
+document. Instead, we focus on the mechanics of the process in the context of
+the Ceph project.
-A detailed discussion of the tools available for validating your bugfixes,
+For a detailed discussion of the tools available for validating bugfixes,
see the chapters on testing.
-For now, let us just assume that you have finished work on the bugfix and
-that you have tested it and believe it works. Commit the changes to your local
+For now, let us just assume that you have finished work on the bugfix, that
+you have tested, and that you believe it works. Commit the changes to your local
branch using the ``--signoff`` option
.. prompt:: bash $
GitHub pull request
-------------------
-The next step is to open a GitHub pull request. The purpose of this step is
-to make your bugfix available to the community of Ceph developers. They
-will review it and may do additional testing on it.
+The next step is to open a GitHub pull request (PR). This makes your bugfix
+visible to the community of Ceph contributors. They will review it and may
+perform additional testing and / or request changes.
-In short, this is the point where you "go public" with your modifications.
-Psychologically, you should be prepared to receive suggestions and
-constructive criticism. Don't worry! In our experience, the Ceph project is
-a friendly place!
+This is the point where you "go public" with your modifications. Be prepared
+to receive suggestions and constructive criticism in the form of comments
+within the PR. Don't worry! The Ceph project is a friendly place!
-If you are uncertain how to use pull requests, you may read
+If you are uncertain how to create and manage pull requests, you may read
`this GitHub pull request tutorial`_.
.. _`this GitHub pull request tutorial`:
https://help.github.com/articles/using-pull-requests/
-For some ideas on what constitutes a "good" pull request, see
+For ideas on what constitutes a "good" pull request, see
the `Git Commit Good Practice`_ article at the `OpenStack Project Wiki`_.
.. _`Git Commit Good Practice`: https://wiki.openstack.org/wiki/GitCommitMessages
.. _`OpenStack Project Wiki`: https://wiki.openstack.org/wiki/Main_Page
+and our own `Submitting Patches <https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst>`_ document.
+
Once your pull request (PR) is opened, update the :ref:`issue-tracker` by
-adding a comment to the bug pointing the other developers to your PR. The
-update can be as simple as::
+adding a comment directing other contributors to your PR. The comment can be
+as simple as::
*PR*: https://github.com/ceph/ceph/pull/$NUMBER_OF_YOUR_PULL_REQUEST
Automated PR validation
-----------------------
-When your PR hits GitHub, the Ceph project's `Continuous Integration (CI)
-<https://en.wikipedia.org/wiki/Continuous_integration>`_
-infrastructure will test it automatically. At the time of this writing
-(September 2020), the automated CI testing included five tests to check that the
-commits in the PR are properly signed (see :ref:`submitting-patches`), to check that the documentation builds, to check that the submodules are unmodified, to check that the API is in order, and a :ref:`make-check` test.
+When your PR is created or updated, the Ceph project's `Continuous Integration
+(CI) <https://en.wikipedia.org/wiki/Continuous_integration>`_ infrastructure
+will test it automatically. At the time of this writing (September 2020), the
+automated CI testing included five tests to check that the commits in the PR
+are properly signed (see :ref:`submitting-patches`), to check that the
+documentation builds, to check that the submodules are unmodified, to check
+that the API is in order, and a :ref:`make-check` test. Additional tests may
+be performed depending on which files are modified by your PR.
The :ref:`make-check`, builds the PR and runs it through a battery of
-tests. These tests run on machines operated by the Ceph Continuous
+tests. These tests run on servers operated by the Ceph Continuous
Integration (CI) team. When the tests complete, the result will be shown
on GitHub in the pull request itself.
The GitHub :ref:`make check<make-check>` test is driven by a Jenkins instance.
Jenkins merges your PR branch into the latest version of the base branch before
-starting the build. This means that you don't have to rebase the PR to pick up any fixes.
+starting tests. This means that you don't have to rebase the PR to pick up any fixes.
-You can trigger the PR tests at any time by adding a comment to the PR - the
+You can trigger PR tests at any time by adding a comment to the PR - the
comment should contain the string "test this please". Since a human subscribed
-to the PR might interpret that as a request for him or her to test the PR,
-we recommend that you address Jenkins directly in the request; for example, write "jenkins retest this please".
-
-If there is a build failure and you aren't sure what caused it, check the :ref:`make check<make-check>` log. To access it, first click on "details" (next
-to the :ref:`make check<make-check>` test in the PR) to enter the Jenkins
-web GUI. Then click on "Console Output" (on the left).
-
-Jenkins is set up to grep the log for strings known to have been associated
-with :ref:`make check<make-check>` failures in the past. However, there is no guarantee that the known strings are associated with any given :ref:`make check<make-check>` failure. You'll have to dig into the log to determine the cause of the failure.
+to the PR might interpret that as a request for him or her to test the PR, we
+recommend that you address Jenkins directly. For example, write "jenkins retest
+this please". For efficiency a single re-test can also be requested with
+e.g. "jenkins test signed". For reference, a list of these requests is
+automatically added to the end of each new PR's description.
+
+If there is a build failure and you aren't sure what caused it, check the
+:ref:`make check<make-check>` log. To access it, click on the "details" (next
+to the :ref:`make check<make-check>` test in the PR) link to enter the Jenkins web
+GUI. Then click on "Console Output" (on the left).
+
+Jenkins is configured to search logs for strings known to have been associated
+with :ref:`make check<make-check>` failures in the past. However, there is no
+guarantee that these known strings are associated with any given
+:ref:`make check<make-check>` failure. You'll have to read through the log to determine the
+cause of your specific failure.
Integration tests AKA ceph-qa-suite
-----------------------------------
-Since Ceph is a complex beast, it may also be necessary to test your fix to
-see how it behaves on real clusters running either on real or virtual
+Since Ceph is complex, it may be necessary to test your fix to
+see how it behaves on real clusters running on physical or virtual
hardware. Tests designed for this purpose live in the `ceph/qa
sub-directory`_ and are run via the `teuthology framework`_.
The Ceph community has access to the `Sepia lab
<https://wiki.sepia.ceph.com/doku.php>`_ where :ref:`testing-integration-tests` can be
-run on real hardware. Other developers may add tags like "needs-qa" to your
+run on physical hardware. Other developers may add tags like "needs-qa" to your
PR. This allows PRs that need testing to be merged into a single branch and
tested all at the same time. Since teuthology suites can take hours (even
days in some cases) to run, this can save a lot of time.
Once your bugfix has been thoroughly tested, or even during this process,
it will be subjected to code review by other developers. This typically
-takes the form of correspondence in the PR itself, but can be supplemented
+takes the form of comments in the PR itself, but can be supplemented
by discussions on :ref:`irc` and the :ref:`mailing-list`.
Amending your PR
While your PR is going through testing and `Code Review`_, you can
modify it at any time by editing files in your local branch.
-After the changes are committed locally (to the ``fix_1`` branch in our
+After updates are committed locally (to the ``fix_1`` branch in our
example), they need to be pushed to GitHub so they appear in the PR.
Modifying the PR is done by adding commits to the ``fix_1`` branch upon
git push --force origin fix_1
+Why do we take these extra steps instead of simply adding additional commits
+the the PR? It is best practice for a PR to consist of a single commit; this
+makes for clean history, eases peer review of your changes, and facilitates
+merges. In rare circumstances it also makes it easier to cleanly revert
+changes.
+
Merge
-----
-The bugfixing process culminates when one of the project leads decides to
-merge your PR.
+The bugfix process completes when a project lead merges your PR.
When this happens, it is a signal for you (or the lead who merged the PR)
to change the :ref:`issue-tracker` status to "Resolved". Some issues may be
This consists of two parts:
-#. The title of the commit of the pull request to be merged.
+#. The title of the commit / PR to be merged.
#. The name and email address of the reviewer. Enclose the reviewer's email
address in angle brackets.
Using .githubmap to Find a Reviewer's Email Address
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-If you cannot find the email address of the reviewer on his or her github
+If you cannot find the email address of the reviewer on his or her GitHub
page, you can look it up in the **.githubmap** file, which can be found in
the repository at **/ceph/.githubmap**.
Using "git log" to find a Reviewer's Email Address
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-If you cannot find a reviewer's email address by using the above methods,
-you can search the git log for their email address. Reviewers are likely
-to have committed something before, and as long as they have committed
-something, the git log will probably contain their email address.
+If you cannot find a reviewer's email address by using the above methods, you
+can search the git log for their email address. Reviewers are likely to have
+committed something before. If they have made previous contributions, the git
+log will probably contain their email address.
Use the following command
client: move client_lock to _unmount()
client: add timer_lock support
Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>
+