Skip to content

GitLab

Kea
Kea
Last edited by Razvan Becheriu
Page history

gitlab howto

This page describes Kea project workflow. Everyone who contribute to Kea project is expected to read it at least once.

Gitlab setup

  • create an account if you don't have it yet.
  • go to gitlab.isc.org, click on your user icon in right top corner and click settings from the drop menu. Then click ssh keys icon on the left panel.
  • There are links there that explain how to generate your keys if you somehow haven't done that already. Anyway, the idea is to paste the content of your ~/.ssh/key-name.pub file there.
  • As a completely optional step, I configured my ssh client to use separate key for gitlab, by adding this to my ~/.ssh/config file:
Host gitlab.isc.org
IdentityFile ~/.ssh/id_ed25519_gitlab

Set up your git repository

To set up a new repository do:

git clone git@gitlab.isc.org:isc-projects/kea.git
cd kea

Adding an issue

  • If you have something new to work on, create an issue in gitlab: go to https://gitlab.isc.org/isc-projects/kea. Make sure it's not assigned to any milestone, unless you discussed this with the project manager and he is ok with adding it directly to the current milestone. We have a process to triage unassigned tickets weekly and assign them to specific milestone.

Working on an issue

  • Once you start working on an issue, assign the issue to yourself. You should also assign Doing label. There are two ways of doing that. First, you can add the label manually when browsing an issue. Alternatively, you can go to https://gitlab.isc.org/isc-projects/kea/-/boards and move your issue to appropriate stage.

  • Open the issue page, e.g. #3 (closed) and click create a MR. A good trick is to click on the triangle button to get the extended create MR menu. Start the title with Draft: or WIP: to prevent merging for now. In particular, you can use a shorter branch name. The branch name MUST start with the issue number.

  • git pull. Note the branch that was created will be there. Check it out and start working on the code. If you dislike the branch name being too long, consider using shorter description for the issue. However, it must still be human readable and people need to have a chance to understand what the issue is about. Also, if you have bash-completion package installed (and using bash), you can type: git checkout 3, and hit tab. The full name of the branch will be completed for you.

  • Commit your changes, push it on that branch.

  • Once you are done, do the following: unassign yourself from the issue and MR, remove "doing" label, add "review" label. The doing and review labels should be put on both the issue and the MR. This indicates the MR is ready for a review. This looks like a duplication of work, but it serves different purposes. The issues are tracked using a board (see this development board for example). Once you assign a label to an issue it is shown in the appropriate column. Now, the review label on MR indicates that particular one is ready for review. The illusion of duplication disappears when there are multiple MRs assigned to a single issue.

Reviewing an issue

  • as a reviewer: go to Issues->Board page. Look for an issue in review state that has no assignee. Assign to yourself. Review. Put some snarky comments. Add some more comments. Once you're done, reassign back to the developer.

  • as a developer: look for issues that are assigned to you. Do your best with addressing the comments. Push your improvements to the branch. Once done reassign back to the reviewer. DO NOT MERGE until the reviewer says the code is ready.

  • as a reviewer: Once all your comments are addressed, put a note that the code is ready to merge, open the MR, edit it and click on "remove the Draft/WIP prefix". Removal of the WIP status is a clear indication the code is ready to go.

  • If you can't reach an agreement after several review rounds, you can do two things: ask for a third opinion (any developer will do) or ask the manager to solve the problem.

Merging code

  • Kea project is currently set up in a way that allows only fast-forward merges. This is not the only possible option, it looks like the right way to go. It keeps the repo history much cleaner. If you complain about tons of merge conflicts, you tried to push a ticket that was too big.

  • Currently we don't have any significant CI integrated with gitlab. We have shellcheck and are planning to integrate danger in gitlab CI. If any of them fail, you won't be able to merge anything. Please fix the issue before merging. Before merging, please run unit-tests on your own.

  • Using rebase button on MR page is a very nice way of rebasing your branch. If there are conflicts, here's how you can rebase the code:

# commit everything on your branch
git checkout master
git pull
git checkout <your-branch>
git rebase master
# solve conflicts
git rebase --continue
git rebase --skip # if there are specific commits that you decided to skip, e.g. because they're already on master
  • Once you push your changes to master, gitlab should close the merge request and the ticket.

PROCESS CHANGE: We previously (before Oct 2019) put three things in the Changelog: Gitlab issue number, MR number and sha of the commit-id. This uniquely identified the changes, but was annoying and it was difficult to automate (in principle it could be done, but you'd have to develop a script that would use gitlab API to retrieve the MR #). After discussion with several members of the team, I propose to put only issue #. All other details are available using GL web interface.

GIT commit logs

PROCESS CHANGE: Previously (before Oct 2019) we were supposed to start each commit with [#Issue number,!MR number]. This was problematic, because you'd have to look up the MR in gitlab. Also, in cases when you're working on an experiment and you're not sure if there even will be a merge request, you may not have that number at all. As a result, the change proposal is to use [#Issue number]. This is something that can be fully automated with a pre-commit hook in gitlab, so you won't need to type it manually.

Commit logs should be descriptive. They are addressed at developers, so code familiarity by the reader is assumed, but please be considerate. "Minor changes" is not a good description. "Fixed typo in addHost()" is better.

Since we sometimes get code from places outside of our control (MRs on gitlab, PRs on github), we cannot assume the [#Issue number] information will always be available.

We don't do commit squashes in general. Sometimes the reviewer pushes fixes or we get commits from contributors (it's essential to get those preserved. Users being able to point to their code and brag about it is one of the reasons they submit patches).

If you like, you can squash your own commits.

Changelog

Changelog entries are requires for every change that is visible by a user. This covers things like new features (they may want to start using it), bug fixes (they may be affected and a fix may be a reason for them to upgrade), changes to build process (they may need to install new software dependency or, even when the dependency installation is automated, they should be aware that new piece of software will appear in their system) or documentation. Changelog should be modified on a branch and then merged to master. This the same change as everything else, and should go through the usual review process.

An example changelog entry looks like this:

1662.	[bug]		marcin
	Prevent deadlock in the Kea DHCP servers caused by allocating
	memory in the system signal handler. The issue was found on
	CentOS 7.6, but could possibly affect Kea running on any other
	OS.
	(Gitlab #796)

Branch names

Branch names must start with an issue number. Yes, there has to be an issue, even if you're doing an experiment and are uncertain if the code will ever be merged. It takes 30 seconds to create one. Tomek wants to be able to dig up what you were working on when writing release notes.

Using labels

Violet labels are for priority. There are four priority labels: critical, high, medium and low.

Orange labels are for components.

Green labels designate type and state of the issue (feature request, QA needed, etc)

Yellow labels designate specific libraries.

Red labels are for things that should stand out. Currently there's one label: bug.

This is just a proposal. Tomek's idea was to use dark colors for important things and light colors for less important aspects. If you don't like it, we can come up with an alternative naming/coloring scheme.

Working with multiple repositories (optional)

Very infrequently we need to access repos other than the official public one. Here's the list of optional ones, with some explanation why we could possibly need to use them.

  • trac - We migrated to gitlab in 2018 and copied over all branches. If did some pushes to trac after you were supposed to do everything in trac, here's how you can still access it. git remote add trac ssh://tomasz@git.kea.isc.org/git/kea
  • github - We have a public clone of our repo on github. While we prefer users to send MRs on gitlab, some of thmem send PRs on github. git remote add github https://github.com/isc-projects/kea
  • private - When we're dealing with code changes that must not be public for a while, we use private repo. In general, it's easier to set up separate repo copy for this, but if you want to do everything on one repo, you can git remote add private git@gitlab.isc.org:isc-private/kea.git

Here are a couple commands that may be useful during transition (trac->gitlab) period. However, they are useful if you happen to work with multiple repositories (such as processing pull request from github or working with internal repo).

This will pull the changes from private repo

git fetch private

To list your local branches with the tracking branches on remote repo:

git branch -vv

To list remote repos you are set to work with:

git remote -v

To set that your local branch should trac a remote branch, use:

git branch -u remote_repo/remote_branch local_branch

Fun fact: git pull is really a shortcut to git pull origin. You can do git pull private or git pull github. The pull itself does two commands: git fetch origin and git merge master origin/master.

Finally, if you can't be bothered anymore and want to get rid of this multiple repos nonsense, do this: git remote remove trac. It will unassociate your local copy with remote repo.

Reopens

The usual issue life cycle is as follows: opened by a reporter, a dev looks at the problem and comes up with some code or doc changes, which are later reviewed. If reporter is part of ISC team, he/she is often also a reviewer, but these two roles in principles are different. Once reviewer and dev agree that the code is ready, the change is merged and ticket is closed. However, this process has one flaw: it doesn't take into consideration whether the proposed fix actually addressed the problem raised by the original reporter.

The alternative - to ask for confirmation that the original problem is addressed - has many flaws. The reporter may not be skilled enough to know how to pull a branch from git, may be unwilling or unable to compile it or more likely be simply unresponsive. This would leave us with many tickets stuck in almost-done state. So we chose to use the approach of merging and closing, with reopen as an escape clause. If the reporter didn't confirm that the fix working, it's good to add something like "We think this should fix the original problem. If it doesn't work or you disagree with the solution, please reopen". This covers 99% of the cases. The text below discusses the remaining 1%.

There's the scenario: Someone reports a problem, we look at it, come up with a solution that we think addressed the problem, merge it and close. The reporter looked at the solution and discovered it doesn't solve the problem and reopens. There may be several reasons for this:

  • the fix didn't work (or works in some cases, such as our tests, but doesn't in the real life deployment as experienced by reporter)
  • the dev didn't understand the core of the problem (this may be caused be inadequate or incomplete description of the problem)
  • the reporter doesn't like the solution and would like to get something different
  • completely fixing the problem is too difficult or not reasonable (e.g. we will not rewrite whole parser just to improve an error message)

There may be other reasons. In all cases the major point is that the original problem remains unsolved and there is more to be done here. What should be done in such cases? You can only close a ticket once. If there's a reopen, a third person needs to get involved and independently confirm that the solution is valid and complete. This is kind of natural extension of the process we have for review where if dev and reviewer cannot reach an agreement, they ask for a third opinion. Feel free to ask the third person depending on the situation - a dev that's expert in the area, maybe someone from QA or get a manager involved.

In any case, closing a ticket that was reopened can easily be considered as rude behavior and can damage the public image of the whole project. If there's really nothing reasonable to be done in the short term, perhaps moving to %Outstanding is a good alternative to closing?

Bump lib versions for development or release artifacts

The safest way to bump lib version for newer development or stable release artifacts is:

  • if the library has not changed at all, leave version info untouched
  • if any change has been made to the library source code, bump 'current' and set 'revision' and 'age' to 0

This will guarantee that the new library needs to be recompiled every time with entire source tree, avoiding any bad linkage or dynamic symbol loading.

This can also be automate the process by the release procedure.

Because of the current parallel model of using development and stable release versions, development libraries versions must never overlap with stable release libraries versions (for the same Major version). To implement this, after each release, all development versions will be incremented with 10 + 1, but only the first time the library is modified and diverges from the release code.

example:

kea-1.8.0

lib1 version 10.0.0
lib2 version 4.0.0
lib3 version 23.0.0
lib4 version 7.0.0

kea-1.9.0

lib1 version 10.0.0 (unchanged)
lib2 version 15.0.0 (add 10 + 1 because the library code has diverged from release)
lib3 version 23.0.0 (unchanged)
lib4 version 18.0.0 (add 10 + 1 because the library code has diverged from release)

kea-1.9.1

lib1 version 10.0.0 (unchanged)
lib2 version 16.0.0 (add 1 because the library code has changed yet again)
lib3 version 34.0.0 (add 10 + 1 because the library code has diverged from release)
lib4 version 18.0.0

kea-1.8.1
lib1 version 10.0.0 (unchanged)
lib2 version 5.0.0 (add 1 because the code has changed) - merged changes from development into stable release
lib3 version 24.0.0 (add 1 because the code has changed) - merged changes from development into stable release
lib4 version 7.0.0 (unchanged) - nothing merged from development into stable release

After a new major release, the process starts again with development versions matching release versions.

example:

kea-2.0.0

lib1 version 10.0.0
lib2 version 5.0.0
lib3 version 24.0.0
lib4 version 8.0.0

kea-2.1.0

lib1 version 10.0.0 (unchanged)
lib2 version 16.0.0 (add 10 + 1 because the library code has diverged from release)
lib3 version 35.0.0 (add 10 + 1 because the library code has diverged from release)
lib4 version 8.0.0 (unchanged)

This means that the current versioning scheme support unlimited number of development versions, but up to 10 stable release versions sharing the same Major version. This also means that this version scheme will avoid version conflicts only between versions (development or stable release) sharing the same Major version.

Also, the value for KEA_HOOKS_VERSION in src/lib/hooks/hooks.h must be incremented for every development or stable release version. Note that development versions have a different (higher) values:

kea-1.8.0

const int KEA_HOOKS_VERSION = 21;

kea-1.8.1

const int KEA_HOOKS_VERSION = 22;

kea-1.9.0

const int KEA_HOOKS_VERSION = 10900;

kea-1.9.1

const int KEA_HOOKS_VERSION = 10901;

This means that up to 99 development versions will be supported before releasing a stable version.