]>
Commit | Line | Data |
---|---|---|
9f95a23c TL |
1 | Basic Workflow |
2 | ============== | |
3 | ||
f67539c2 | 4 | The following chart illustrates the basic Ceph development workflow: |
9f95a23c TL |
5 | |
6 | .. ditaa:: | |
7 | ||
8 | Upstream Code Your Local Environment | |
9 | ||
10 | /----------\ git clone /-------------\ | |
11 | | Ceph | -------------------------> | ceph/master | | |
12 | \----------/ \-------------/ | |
13 | ^ | | |
14 | | | git branch fix_1 | |
15 | | git merge | | |
16 | | v | |
17 | /----------------\ git commit --amend /-------------\ | |
20effc67 | 18 | | ninja check |---------------------> | ceph/fix_1 | |
9f95a23c TL |
19 | | ceph--qa--suite| \-------------/ |
20 | \----------------/ | | |
21 | ^ | fix changes | |
22 | | | test changes | |
23 | | review | git commit | |
24 | | | | |
25 | | v | |
26 | /--------------\ /-------------\ | |
27 | | github |<---------------------- | ceph/fix_1 | | |
28 | | pull request | git push \-------------/ | |
29 | \--------------/ | |
30 | ||
f67539c2 TL |
31 | This page assumes that you are a new contributor with an idea for a bugfix or |
32 | enhancement, but do not know how to proceed. Watch the `Getting Started with | |
33 | Ceph Development <https://www.youtube.com/watch?v=t5UIehZ1oLs>`_ video for a | |
34 | practical summary of this workflow. | |
9f95a23c | 35 | |
f67539c2 TL |
36 | Updating the tracker |
37 | -------------------- | |
9f95a23c | 38 | |
f67539c2 TL |
39 | Before you start, you should know the :ref:`issue-tracker` (Redmine) number |
40 | of the bug you intend to fix. If there is no tracker issue, now is the time to | |
41 | create one for code changes. Straightforward documentation cleanup does | |
42 | not necessarily require a corresponding tracker issue. However, an issue | |
43 | (ticket) should be created if one is adding new documentation chapters or | |
44 | files, or for other substantial changes. | |
9f95a23c | 45 | |
f67539c2 TL |
46 | The tracker ticket serves to explain the issue (bug) to your fellow Ceph |
47 | developers and keep them informed as you make progress toward resolution. To | |
48 | this end, please provide a descriptive title and write appropriate information | |
49 | and details into the description. When composing the ticket's title, consider "If I | |
50 | want to search for this ticket two years from now, what keywords will I search | |
51 | for?" | |
9f95a23c TL |
52 | |
53 | If you have sufficient tracker permissions, assign the bug to yourself by | |
f67539c2 TL |
54 | setting the ``Assignee`` field. If your tracker permissions have not been |
55 | elevated, simply add a comment with a short message like "I am working on this | |
56 | issue". | |
9f95a23c | 57 | |
20effc67 TL |
58 | Forking and Cloning the Ceph Repository |
59 | --------------------------------------- | |
9f95a23c | 60 | |
f67539c2 | 61 | This section, and the ones that follow, correspond to nodes in the above chart. |
9f95a23c | 62 | |
f67539c2 TL |
63 | The upstream code is found at https://github.com/ceph/ceph.git, which is known |
64 | as the "upstream repo", or simply "upstream". As the chart shows, we will make | |
65 | a local copy of this repository, modify it, test our modifications, then submit | |
66 | the modifications for review and merging. | |
9f95a23c TL |
67 | |
68 | A local copy of the upstream code is made by | |
69 | ||
f67539c2 TL |
70 | 1. Forking the upstream repo on GitHub, and |
71 | 2. Cloning your fork to make a local working copy | |
9f95a23c | 72 | |
f67539c2 TL |
73 | |
74 | Forking The Ceph Repository | |
75 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
76 | ||
77 | See the `GitHub documentation | |
9f95a23c TL |
78 | <https://help.github.com/articles/fork-a-repo/#platform-linux>`_ for |
79 | detailed instructions on forking. In short, if your GitHub username is | |
f67539c2 TL |
80 | "mygithubaccount", your fork of the upstream repo will appear at |
81 | ``https://github.com/mygithubaccount/ceph``. | |
82 | ||
83 | Cloning Your Fork | |
84 | ^^^^^^^^^^^^^^^^^ | |
85 | ||
86 | Once you have created your fork, clone it by running: | |
9f95a23c | 87 | |
f67539c2 | 88 | .. prompt:: bash $ |
9f95a23c | 89 | |
f67539c2 | 90 | git clone https://github.com/mygithubaccount/ceph |
9f95a23c | 91 | |
f67539c2 TL |
92 | You must fork the Ceph repository before you clone it. Without forking, you cannot |
93 | open a `GitHub pull request | |
94 | <https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request>`_. | |
9f95a23c TL |
95 | |
96 | For more information on using GitHub, refer to `GitHub Help | |
97 | <https://help.github.com/>`_. | |
98 | ||
f67539c2 TL |
99 | Configuring Your Local Environment |
100 | ---------------------------------- | |
9f95a23c | 101 | |
f67539c2 TL |
102 | In the local environment created in the previous step, you now have a copy of |
103 | the ``master`` branch in ``remotes/origin/master``. This fork | |
9f95a23c TL |
104 | (https://github.com/mygithubaccount/ceph.git) is frozen in time and the |
105 | upstream repo (https://github.com/ceph/ceph.git, typically abbreviated to | |
f67539c2 TL |
106 | ``ceph/ceph.git``) is updated frequently by other contributors. This means that |
107 | you must sync your fork periodically. Failure to synchronize your fork may | |
108 | result in your commits and pull requests failing to merge because they refer to | |
109 | file contents that have changed since you last synchronized your fork. | |
110 | ||
111 | Configure your local git environment with your name and email address. | |
112 | ||
113 | .. prompt:: bash $ | |
114 | ||
115 | git config user.name "FIRST_NAME LAST_NAME" | |
116 | git config user.email "MY_NAME@example.com" | |
117 | ||
118 | Add the upstream repo as a "remote" and fetch it: | |
9f95a23c | 119 | |
f67539c2 | 120 | .. prompt:: bash $ |
9f95a23c | 121 | |
f67539c2 TL |
122 | git remote add ceph https://github.com/ceph/ceph.git |
123 | git fetch ceph | |
9f95a23c | 124 | |
20effc67 TL |
125 | These commands fetch all the branches and commits from ``ceph/ceph.git`` to the |
126 | local git repo as ``remotes/ceph/$BRANCH_NAME`` and can be referenced as | |
127 | ``ceph/$BRANCH_NAME`` in local git commands. | |
9f95a23c | 128 | |
9f95a23c | 129 | |
f67539c2 TL |
130 | Resetting Local Master to Upstream Master |
131 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
9f95a23c | 132 | |
f67539c2 TL |
133 | Your local ``master`` branch can be reset to the upstream Ceph ``master`` |
134 | branch by running the following commands: | |
9f95a23c | 135 | |
f67539c2 | 136 | .. prompt:: bash $ |
9f95a23c | 137 | |
f67539c2 TL |
138 | git fetch ceph |
139 | git checkout master | |
140 | git reset --hard ceph/master | |
141 | git push -u origin master | |
9f95a23c | 142 | |
f67539c2 TL |
143 | This procedure should be followed often, in order to keep your local ``master`` |
144 | in sync with upstream ``master``. | |
9f95a23c | 145 | |
f67539c2 TL |
146 | Creating a Bugfix branch |
147 | ------------------------ | |
9f95a23c | 148 | |
f67539c2 | 149 | Create a branch for your bugfix: |
9f95a23c | 150 | |
f67539c2 | 151 | .. prompt:: bash $ |
9f95a23c | 152 | |
f67539c2 TL |
153 | git checkout master |
154 | git checkout -b fix_1 | |
155 | git push -u origin fix_1 | |
9f95a23c | 156 | |
f67539c2 TL |
157 | This creates a local branch called ``fix_1`` in our GitHub fork. At this point, |
158 | the ``fix_1`` branch is identical to the ``master`` branch, but not for long! | |
159 | You are now ready to modify the code. Be careful to always run `git checkout | |
160 | master` first, otherwise you may find commits from an unrelated branch mixed | |
161 | with your new work. | |
9f95a23c | 162 | |
f67539c2 TL |
163 | Fixing the bug locally |
164 | ---------------------- | |
165 | ||
166 | In the `Ceph issue tracker <https://tracker.ceph.com>`_, change the status of | |
167 | the tracker issue to "In progress". This communicates to other Ceph | |
168 | contributors that you have begun working on a fix, which helps to avoid | |
169 | duplication of effort. If you don't have permission to change that field, your | |
170 | previous comment that you are working on the issue is sufficient. | |
171 | ||
172 | Your fix may be very simple and require only minimal testing. But that's not | |
173 | likely. It is more likely that the process of fixing your bug will be iterative | |
174 | and will involve trial and error, as well as skill. An explanation of how to | |
175 | fix bugs is beyond the scope of this document. Instead, we focus on the | |
176 | mechanics of the process in the context of the Ceph project. | |
177 | ||
178 | For a detailed discussion of the tools available for validating bugfixes, | |
9f95a23c TL |
179 | see the chapters on testing. |
180 | ||
f67539c2 TL |
181 | For now, let us assume that you have finished work on the bugfix, that you have |
182 | tested the bugfix , and that you believe that it works. Commit the changes to | |
183 | your local branch using the ``--signoff`` option (here represented as the `s` | |
184 | portion of the `-as` flag): | |
185 | ||
186 | .. prompt:: bash $ | |
9f95a23c | 187 | |
f67539c2 | 188 | git commit -as |
9f95a23c | 189 | |
f67539c2 | 190 | Push the changes to your fork: |
9f95a23c | 191 | |
f67539c2 | 192 | .. prompt:: bash $ |
9f95a23c | 193 | |
f67539c2 | 194 | git push origin fix_1 |
9f95a23c | 195 | |
f67539c2 TL |
196 | Opening a GitHub pull request |
197 | ----------------------------- | |
9f95a23c | 198 | |
f67539c2 TL |
199 | The next step is to open a GitHub pull request (PR). This makes your bugfix |
200 | visible to the community of Ceph contributors. They will review it and may | |
201 | perform additional testing and / or request changes. | |
9f95a23c | 202 | |
f67539c2 TL |
203 | This is the point where you "go public" with your modifications. Be prepared |
204 | to receive suggestions and constructive criticism in the form of comments | |
205 | within the PR. Don't worry! The Ceph project is a friendly place! | |
206 | ||
207 | If you are uncertain how to create and manage pull requests, you may read | |
9f95a23c TL |
208 | `this GitHub pull request tutorial`_. |
209 | ||
210 | .. _`this GitHub pull request tutorial`: | |
211 | https://help.github.com/articles/using-pull-requests/ | |
212 | ||
f67539c2 | 213 | For ideas on what constitutes a "good" pull request, see |
9f95a23c TL |
214 | the `Git Commit Good Practice`_ article at the `OpenStack Project Wiki`_. |
215 | ||
216 | .. _`Git Commit Good Practice`: https://wiki.openstack.org/wiki/GitCommitMessages | |
217 | .. _`OpenStack Project Wiki`: https://wiki.openstack.org/wiki/Main_Page | |
218 | ||
f67539c2 TL |
219 | and our own `Submitting Patches <https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst>`_ document. |
220 | ||
221 | Once your pull request (PR) is opened, update the :ref:`issue-tracker` by | |
222 | adding a comment directing other contributors to your PR. The comment can be | |
223 | as simple as:: | |
9f95a23c TL |
224 | |
225 | *PR*: https://github.com/ceph/ceph/pull/$NUMBER_OF_YOUR_PULL_REQUEST | |
226 | ||
f67539c2 TL |
227 | Understanding Automated PR validation |
228 | ------------------------------------- | |
9f95a23c | 229 | |
f67539c2 TL |
230 | When you create or update your PR, the Ceph project's `Continuous Integration |
231 | (CI) <https://en.wikipedia.org/wiki/Continuous_integration>`_ infrastructure | |
232 | automatically tests it. At the time of this writing (September 2020), the | |
20effc67 | 233 | automated CI testing included five tests: |
9f95a23c | 234 | |
20effc67 | 235 | #. a test to check that the commits are properly signed (see :ref:`submitting-patches`): |
f67539c2 TL |
236 | #. a test to check that the documentation builds |
237 | #. a test to check that the submodules are unmodified | |
238 | #. a test to check that the API is in order | |
20effc67 TL |
239 | #. a :ref:`make check<make-check>` test |
240 | ||
f67539c2 TL |
241 | Additional tests may be performed depending on which files your PR modifies. |
242 | ||
243 | The :ref:`make check<make-check>` test builds the PR and runs it through a battery of | |
244 | tests. These tests run on servers operated by the Ceph Continuous | |
9f95a23c TL |
245 | Integration (CI) team. When the tests complete, the result will be shown |
246 | on GitHub in the pull request itself. | |
247 | ||
f67539c2 | 248 | You should test your modifications before you open a PR. |
9f95a23c TL |
249 | Refer to the chapters on testing for details. |
250 | ||
251 | Notes on PR make check test | |
252 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
253 | ||
f67539c2 | 254 | The GitHub :ref:`make check<make-check>` test is driven by a Jenkins instance. |
9f95a23c | 255 | |
f67539c2 TL |
256 | Jenkins merges your PR branch into the latest version of the base branch before |
257 | starting tests. This means that you don't have to rebase the PR to pick up any fixes. | |
9f95a23c | 258 | |
f67539c2 | 259 | You can trigger PR tests at any time by adding a comment to the PR - the |
9f95a23c | 260 | comment should contain the string "test this please". Since a human subscribed |
f67539c2 TL |
261 | to the PR might interpret that as a request for him or her to test the PR, we |
262 | recommend that you address Jenkins directly. For example, write "jenkins retest | |
263 | this please". For efficiency a single re-test can also be requested with | |
264 | e.g. "jenkins test signed". For reference, a list of these requests is | |
265 | automatically added to the end of each new PR's description. | |
266 | ||
267 | If there is a build failure and you aren't sure what caused it, check the | |
268 | :ref:`make check<make-check>` log. To access it, click on the "details" (next | |
269 | to the :ref:`make check<make-check>` test in the PR) link to enter the Jenkins web | |
270 | GUI. Then click on "Console Output" (on the left). | |
271 | ||
272 | Jenkins is configured to search logs for strings known to have been associated | |
273 | with :ref:`make check<make-check>` failures in the past. However, there is no | |
274 | guarantee that these known strings are associated with any given | |
275 | :ref:`make check<make-check>` failure. You'll have to read through the log to determine the | |
276 | cause of your specific failure. | |
9f95a23c TL |
277 | |
278 | Integration tests AKA ceph-qa-suite | |
279 | ----------------------------------- | |
280 | ||
f67539c2 TL |
281 | Since Ceph is complex, it may be necessary to test your fix to |
282 | see how it behaves on real clusters running on physical or virtual | |
9f95a23c TL |
283 | hardware. Tests designed for this purpose live in the `ceph/qa |
284 | sub-directory`_ and are run via the `teuthology framework`_. | |
285 | ||
286 | .. _`ceph/qa sub-directory`: https://github.com/ceph/ceph/tree/master/qa/ | |
287 | .. _`teuthology repository`: https://github.com/ceph/teuthology | |
288 | .. _`teuthology framework`: https://github.com/ceph/teuthology | |
289 | ||
290 | The Ceph community has access to the `Sepia lab | |
20effc67 TL |
291 | <https://wiki.sepia.ceph.com/doku.php>`_ where `integration tests`_ can be run |
292 | on physical hardware. | |
293 | Other developers may add tags like "needs-qa" to your PR. This allows PRs that | |
294 | need testing to be merged into a single branch and tested all at the same time. | |
295 | Since teuthology suites can take hours (even days in some cases) to run, this | |
296 | can save a lot of time. | |
297 | ||
298 | To request access to the Sepia lab, start `here | |
299 | <https://wiki.sepia.ceph.com/doku.php?id=vpnaccess>`_. | |
9f95a23c | 300 | |
20effc67 TL |
301 | Integration testing is discussed in more detail in the `integration |
302 | tests`_ chapter. | |
9f95a23c | 303 | |
20effc67 | 304 | .. _integration tests: ../testing_integration_tests/tests-integration-testing-teuthology-intro |
9f95a23c TL |
305 | |
306 | Code review | |
307 | ----------- | |
308 | ||
309 | Once your bugfix has been thoroughly tested, or even during this process, | |
310 | it will be subjected to code review by other developers. This typically | |
f67539c2 TL |
311 | takes the form of comments in the PR itself, but can be supplemented |
312 | by discussions on :ref:`irc` and the :ref:`mailing-list`. | |
9f95a23c TL |
313 | |
314 | Amending your PR | |
315 | ---------------- | |
316 | ||
317 | While your PR is going through testing and `Code Review`_, you can | |
318 | modify it at any time by editing files in your local branch. | |
319 | ||
f67539c2 | 320 | After updates are committed locally (to the ``fix_1`` branch in our |
9f95a23c TL |
321 | example), they need to be pushed to GitHub so they appear in the PR. |
322 | ||
323 | Modifying the PR is done by adding commits to the ``fix_1`` branch upon | |
324 | which it is based, often followed by rebasing to modify the branch's git | |
325 | history. See `this tutorial | |
326 | <https://www.atlassian.com/git/tutorials/rewriting-history>`_ for a good | |
327 | introduction to rebasing. When you are done with your modifications, you | |
328 | will need to force push your branch with: | |
329 | ||
f67539c2 TL |
330 | .. prompt:: bash $ |
331 | ||
332 | git push --force origin fix_1 | |
9f95a23c | 333 | |
f67539c2 TL |
334 | Why do we take these extra steps instead of simply adding additional commits |
335 | the the PR? It is best practice for a PR to consist of a single commit; this | |
336 | makes for clean history, eases peer review of your changes, and facilitates | |
337 | merges. In rare circumstances it also makes it easier to cleanly revert | |
338 | changes. | |
9f95a23c | 339 | |
f67539c2 TL |
340 | Merging |
341 | ------- | |
9f95a23c | 342 | |
f67539c2 | 343 | The bugfix process completes when a project lead merges your PR. |
9f95a23c TL |
344 | |
345 | When this happens, it is a signal for you (or the lead who merged the PR) | |
f67539c2 | 346 | to change the :ref:`issue-tracker` status to "Resolved". Some issues may be |
9f95a23c | 347 | flagged for backporting, in which case the status should be changed to |
f67539c2 TL |
348 | "Pending Backport" (see the :ref:`backporting` chapter for details). |
349 | ||
350 | See also :ref:`merging` for more information on merging. | |
351 | ||
352 | Proper Merge Commit Format | |
353 | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
354 | ||
355 | This is the most basic form of a merge commit:: | |
356 | ||
357 | doc/component: title of the commit | |
358 | ||
359 | Reviewed-by: Reviewer Name <rname@example.com> | |
360 | ||
361 | This consists of two parts: | |
362 | ||
363 | #. The title of the commit / PR to be merged. | |
364 | #. The name and email address of the reviewer. Enclose the reviewer's email | |
365 | address in angle brackets. | |
366 | ||
367 | Using .githubmap to Find a Reviewer's Email Address | |
368 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
369 | If you cannot find the email address of the reviewer on his or her GitHub | |
370 | page, you can look it up in the **.githubmap** file, which can be found in | |
371 | the repository at **/ceph/.githubmap**. | |
372 | ||
373 | Using "git log" to find a Reviewer's Email Address | |
374 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
375 | If you cannot find a reviewer's email address by using the above methods, you | |
376 | can search the git log for their email address. Reviewers are likely to have | |
377 | committed something before. If they have made previous contributions, the git | |
378 | log will probably contain their email address. | |
379 | ||
380 | Use the following command | |
381 | ||
382 | .. prompt:: bash [branch-under-review]$ | |
383 | ||
384 | git log | |
385 | ||
386 | Using ptl-tool to Generate Merge Commits | |
387 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
388 | ||
389 | Another method of generating merge commits involves using Patrick Donnelly's | |
390 | **ptl-tool** pull commits. This tool can be found at | |
391 | **/ceph/src/script/ptl-tool.py**. Merge commits that have been generated by | |
392 | the **ptl-tool** have the following form:: | |
9f95a23c | 393 | |
f67539c2 TL |
394 | Merge PR #36257 into master |
395 | * refs/pull/36257/head: | |
396 | client: move client_lock to _unmount() | |
397 | client: add timer_lock support | |
398 | Reviewed-by: Patrick Donnelly <pdonnell@redhat.com> |