Page MenuHomePhabricator

Warn against potentially deploying a config patch NOT specified
Closed, ResolvedPublicFeature

Description

NOTE: I don't know if this feature already exist, let me know if it does. Thanks!

Per https://wikitech.wikimedia.org/wiki/Incidents/2023-05-30_Unintentional_%2B2_on_a_config_patch_without_deployment, there was a suggestion on the meeting whether to (if not already available), warn against the deployment of a config patch if not specified by the scap backport ... command.

So assuming that a deployer is in the process of deploying a patch or patches and there is a patch that was recently merged or merged and forgotten to be deployed (but shows up as part of the diff), scap should detect that and warn the deployer about it.

Let's assume a patch: abc that was recently merged not to be deployed immediately (in a deploy window) or merged and forgotten, and a deployer wants to deploy several config patches: xyz, ijk, then runs the command: scap backport xyz ijki. Scap should detect that abc is part of the diff but not specified explicitly to be deployed and warn the deployer about it.

This way, the deployer can make a revert of the patch (abc) or ping the author whether they're around to deploy the change or not (schedule for another window).

Resolution

scap backport does have a check for this situation. If an unexpected commit is fetched during a backport, scap backport will generate output like the following:

19:14:06 The following are unexpected commits pulled from origin for /srv/mediawiki-staging:
commit c5e2ce837b52418164db3d24346127f5ca127c05 (origin/train-dev)
Author: Train Conductor <tconductor@wikimedia.org>
Date:   Thu Jun 29 19:13:07 2023 +0000

    setup_config_change(backport_with_extra_commits)

    Change-Id: I8ec0ac3409e5ecc12f4a66253627a32ae41703ea
Would you like to see the diff? (y/n): y

commit c5e2ce837b52418164db3d24346127f5ca127c05 (origin/train-dev)
Author: Train Conductor <tconductor@wikimedia.org>
Date:   Thu Jun 29 19:13:07 2023 +0000

    setup_config_change(backport_with_extra_commits)

    Change-Id: I8ec0ac3409e5ecc12f4a66253627a32ae41703ea

diff --git a/README b/README
index 843875375..673796b95 100644
--- a/README
+++ b/README
@@ -237,3 +237,5 @@ Added by setup_config_change(already_merged, backport_with_extra_commits) on 202
 Added by setup_config_change(normal, test_config_changes(normal)) on 2023-06-29 19:07:21

 Added by setup_config_change(already_merged, test_config_changes(already_merged)) on 2023-06-29 19:08:13
+
+Added by setup_config_change(already_merged, backport_with_extra_commits) on 2023-06-29 19:13:07
19:14:06 There were unexpected commits pulled from origin for /srv/mediawiki-staging.
Continue with backport? (y/n):

Details

TitleReferenceAuthorSource BranchDest Branch
backport: add tests for extra commitsrepos/releng/scap!156dancyT338168master
Customize query in GitLab

Event Timeline

Hi @xSavitar, this feature exists already. I'll be adding an integration test for it soon.

Aklapper renamed this task from [FEAT][REQUEST] Warn against potentially deploying a config patch NOT specified to Warn against potentially deploying a config patch NOT specified.Jun 19 2023, 8:46 AM
Aklapper changed the subtype of this task from "Task" to "Feature Request".
dancy triaged this task as Medium priority.