VPP/DistributedOwnership

From fd.io
< VPP
Revision as of 12:50, 4 October 2024 by Otroan (Talk | contribs)

(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)
Jump to: navigation, search

Proposal: Distributed ownership of VPP plugins

Background

In the fd.io VPP project a maintainer has mainly a role as a gatekeeper for a particular feature. Unfortunately there is no automation ensuring that a maintainer is involved in reviewing all changes to the component they are responsible for. This can lead to a situation where a maintainer is not aware of changes to their component until after they have been merged.

Many open source project have a distributed ownership model, where the maintainers of a component are responsible for review and upstream merging of code changes.

In VPP this role has been centralized to the project committers. This has worked well for the project, but as the project grows and more plugins are added, it may be beneficial to delegate some of the review and merge responsibilities to the maintainers of the plugins.

Purpose

This proposal is two fold. Firstly, add tooling so that maintainers are automatically added to the review process for changes to their component. Secondly, to delegate the ability to merge changes to the maintainers of the component.

The intent is that the +1/-1 (review) is added for all components. And that the +2/-2 (merge) is added for the component maintainers initially on a per-request basis. Treat it as an example to learn how well the distributed ownership model works for the VPP project.

To ensure that the quality of the codebase is maintained, the project committers will still have the ability to override the maintainers decisions.

Steps for Implementation:

1. Define Plugin Maintainership in the `MAINTAINERS` File

Ensure that the `MAINTAINERS` file in the VPP repository is properly structured to specify maintainers for each plugin. The `MAINTAINERS` file should follow a format like:

``` plaintext Plugin - linux-cp I: linux-cp M: Neale Ranns <neale@graphiant.com> M: Matthew Smith <mgsmith@netgate.com> F: src/plugins/linux-cp/

Plugin - SRTP I: srtp M: Florin Coras <fcoras@cisco.com> F: src/plugins/srtp/

Plugin - bufmon I: bufmon M: Benoît Ganne <bganne@cisco.com> F: src/plugins/bufmon/ ```

2. Create Gerrit Maintainer Groups

For each plugin, create a corresponding Gerrit group that contains the maintainers responsible for reviewing and merging code in that plugin's directory.

The synchronisation between Gerrit Groups and the MAINTAINERS file should be handled automatically by a CI job.

Example Groups:
  • `fdio-vpp-linux-cp-maintainers` for the `linux-cp` plugin
  • `fdio-vpp-srtp-maintainers` for the `srtp` plugin
  • `fdio-vpp-bufmon-maintainers` for the `bufmon` plugin

3. Modify the Repository’s `project.config` File

Update the `project.config` for the VPP repository to delegate Code Review and submit permissions to the plugin maintainers for their respective plugin directories.

Example `project.config`:

``` [access "refs/heads/*"]

   # Permissions for linux-cp plugin
   label-Code-Review = -2..+2 group linux-cp-maintainers
   submit = group linux-cp-maintainers
   exclusiveGroupPermissions = label-Code-Review submit
   path = "src/plugins/linux-cp/"

[access "refs/heads/*"]

   # Permissions for SRTP plugin
   label-Code-Review = -2..+2 group srtp-maintainers
   submit = group srtp-maintainers
   exclusiveGroupPermissions = label-Code-Review submit
   path = "src/plugins/srtp/"

[access "refs/heads/*"]

   # Permissions for bufmon plugin
   label-Code-Review = -2..+2 group bufmon-maintainers
   submit = group bufmon-maintainers
   exclusiveGroupPermissions = label-Code-Review submit
   path = "src/plugins/bufmon/"

```

General permissions (if needed for other parts of the repository)

``` [access "refs/heads/*"]

   label-Code-Review = -1..+1 group vpp-reviewers
   submit = group vpp-submitters

```

This configuration ensures that only the relevant maintainer group can provide +2 Code Review and submit changes for their respective plugin directory.

The `project.config` file is in the project `refs/meta/config` branch.

4. Test the Configuration

Before rolling out the changes to all users, create a test or sandbox project to verify that the `project.config` changes work as expected. Perform the following tests:

  • Ensure that maintainers of each plugin can review and merge code only in their assigned directories.
  • Ensure that other maintainers or users cannot review or merge code outside their areas of responsibility.
  • Verify that project committers can still review and merge code across all directories.

5. Gather Feedback

This document is a draft proposal. Please provide feedback or suggestions for improving the process. Key areas for consideration include:

  • Should additional permissions be granted to non-maintainers for reviewing plugin code?
  • How do we handle cases where a patch affects multiple plugins?

The understanding, which needs to be tested, is that for changes affecting multiple plugins, then each plugin maintainer would need to approve the change. This could be a limitation or a feature, depending on the use case.

  • Do we need to ensure that commiters can override? E.g., for emergency fixes or style fixes or fixes to the core VPP API?

Conclusion

By delegating review and merge responsibilities to plugin maintainers, we aim to increase the efficiency and accuracy of the code review process. Plugin maintainers have deep knowledge of the code in their area, and allowing them to merge code directly will speed up development without compromising quality.

Please share your feedback on this proposal by [insert deadline].