Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strengthen configuration change approval #8167

Closed
Tracked by #824
wuhuizuo opened this issue May 13, 2024 · 1 comment · Fixed by #8218
Closed
Tracked by #824

Strengthen configuration change approval #8167

wuhuizuo opened this issue May 13, 2024 · 1 comment · Fixed by #8218
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@wuhuizuo
Copy link
Contributor

Strengthen configuration change approval

An approval process for the change to the file path scope of global variables and configurations will be deployed.

Why

The current reviewers and approvers are at the code repository level and are not subdivided into specific areas, can not fine-grained control over changes in important areas. Configuration/variable changes can have a wide-ranging impact, affecting system stability and compatibility. Recently, there have been several online issues caused by configuration/variable changes.
To minimize the negative impact of configuration/variable changes, ensure that each change undergoes careful consideration and evaluation and comes with clear documentation, and reduce the risk of online issues, we plan to pilot pre-approval of configuration changes in the tikv/pd repo and delegate the approval responsibility to the person in charge of maintaining the TiDB product.

What's new for developer

The reviewing control framework will be migrated to the native "lgtm" and "approve" plug-ins. In terms of final approval, it will change from a single merge approval (a "/merge" command) to an approval mode by responsible area (one or more "/ approve" command).
All the changes need to be approved by committers or maintainers as before except if there are configurations changes, the PRs need also to be approved by approvers inside /OWNERS_ALIASES file. The approvers will be automatically recommended by bot on GitHub.

Ref:

Set up

  1. Now we use prow's OWNERS mechanism to control the approving for pull requests, here is the PR review flow.
  2. We setup an OWNERS_ALIASES file in root folder in repository to maintain the approver teams for variable or configuration changes.
  3. We setup serval OWNERS files in sub folders to support approving by folder or file paths. And make it not inherit from parent OWNERS files to ensure the changes must be approved by the dedicated approvers.
  4. The changes should be approved by the matched approvers that are configured in OWNERS files(layer by layer in folder level), and leaf approvers first.
  5. All the changes must be approved before merging.
  6. OWNERS and OWNERS_ALIASES files should be updated on time.
  7. If file paths need to be updated, we should update the OWNERS files in the leaf folders.
  8. If the approver member should be updated, we should update the /OWNERS_ALIASES file.

Role and Responsibility

  • For contributor
    • Assess the system's needs and requirements before making any configuration changes, considering factors such as performance, scalability, security, and compatibility.
    • Implement the configuration changes and verify that any changes made to the codebase or configuration maintain backward compatibility.
    • Conduct testing to verify the effects of configuration changes, checking for any adverse impact on system behavior, performance, or compatibility.
    • In case of any issues or unexpected outcomes resulting from the configuration changes, contributors actively troubleshoot and work towards resolving them.
    • Document the configuration parameters that affect user behavior and any system variable changes (adding, deleting, deprecating, modifying) by creating pull requests to documentation repositories (pingcap/docs, pingcap/docs-cn), adding the changes to release notes, and updating relevant documentation, to ensure that users have access to accurate and up-to-date information.
  • For approver
    1. Thoroughly review all proposed configuration change pull requests. This involves examining the details of the proposed modifications, such as the purpose, impact, and potential risks associated with the configuration change.
    2. Evaluate the impact these changes may have on the system or any related components. This includes considering potential risks, such as performance impact, security vulnerabilities, or conflicts with existing configurations.
    3. Validate that the proposed configuration changes in the pull requests have undergone appropriate testing.
    4. Confirm the necessary documentation PR has been created.
    5. Grant approval for the PR.
@wuhuizuo wuhuizuo added the type/enhancement The issue or PR belongs to an enhancement. label May 13, 2024
@wuhuizuo
Copy link
Contributor Author

/assign wuhuizuo

wuhuizuo added a commit to wuhuizuo/pd that referenced this issue May 27, 2024
wuhuizuo added a commit to wuhuizuo/pd that referenced this issue May 27, 2024
wuhuizuo added a commit to wuhuizuo/pd that referenced this issue Jun 4, 2024
@ti-chi-bot ti-chi-bot bot closed this as completed in #8218 Jun 4, 2024
ti-chi-bot bot pushed a commit that referenced this issue Jun 4, 2024
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 5, 2024
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 5, 2024
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 5, 2024
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 5, 2024
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 5, 2024
ti-chi-bot bot added a commit that referenced this issue Jun 7, 2024
…nfiguration files (#8218) (#8252)

close #8167

Signed-off-by: wuhuizuo <[email protected]>

Co-authored-by: wuhuizuo <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot pushed a commit that referenced this issue Jun 7, 2024
…nfiguration files (#8218) (#8250)

close #8167

Signed-off-by: wuhuizuo <[email protected]>

Co-authored-by: wuhuizuo <[email protected]>
ti-chi-bot bot pushed a commit that referenced this issue Jun 7, 2024
…nfiguration files (#8218) (#8249)

close #8167

Signed-off-by: wuhuizuo <[email protected]>

Co-authored-by: wuhuizuo <[email protected]>
ti-chi-bot bot pushed a commit that referenced this issue Jun 11, 2024
…nfiguration files (#8218) (#8248)

close #8167

Signed-off-by: wuhuizuo <[email protected]>

Co-authored-by: wuhuizuo <[email protected]>
Co-authored-by: husharp <[email protected]>
ti-chi-bot bot added a commit that referenced this issue Jun 11, 2024
…nfiguration files (#8218) (#8251)

close #8167

Signed-off-by: wuhuizuo <[email protected]>

Co-authored-by: wuhuizuo <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot pushed a commit that referenced this issue Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant