Browse Source

feat(options) Make the options automator render the value of drifted options (#50692)

The expected workflow when we encounter drifted options (the option on
the DB has been changed outside of the option automator configmap) is
that:
- the automator will skip the update and yell.
- we would update the configmap to the value contained in the DB
- we would try again to apply the change. Now the script would notice
that we are not changing the value but the the previous DB update was
not done by the automator.
- At this point the automator leaves the value in place (there is no
more drift in the value between configmap and DB) but switches the last
update field to automator in order to be able to make further changes.

The previous version of the script made this process hard because it,
well, did not print out the value present in the DB when drift was
detected. That would have made it really hard to fix the configmap file.
This PR adds the option (enabled by default) to display the DB value
when we encounter drift.
Filippo Pacifici 1 year ago
parent
commit
9b36bbce4d

+ 26 - 7
src/sentry/runner/commands/configoptions.py

@@ -2,30 +2,37 @@ import sys
 from typing import Any, Optional, Set
 
 import click
-import yaml
+from yaml import safe_dump, safe_load
 
 from sentry.runner.decorators import configuration
 
 # These messages are produced more than once and referenced in tests.
 # This is the reason they are constants.
 DRIFT_MSG = "[DRIFT] Option %s drifted and cannot be updated."
+DB_VALUE = "Value of option %s on DB:"
 CHANNEL_UPDATE_MSG = "[CHANNEL UPDATE] Option %s value unchanged. Last update channel updated."
 UPDATE_MSG = "[UPDATE] Option %s updated."
 UNSET_MSG = "[UNSET] Option %s unset."
 
 
-def _attempt_update(key: str, value: Any, drifted_options: Set[str], dry_run: bool) -> None:
+def _attempt_update(
+    key: str, value: Any, drifted_options: Set[str], dry_run: bool, hide_drift: bool
+) -> None:
     """
     Updates the option if it is not drifted and if we are not in dry
     run mode.
     """
     from sentry import options
 
+    db_value = options.get(key)
     if key in drifted_options:
         click.echo(DRIFT_MSG % key)
+        if not hide_drift:
+            click.echo(DB_VALUE % key)
+            click.echo(safe_dump(db_value))
         return
 
-    if options.get(key) == value:
+    if db_value == value:
         # This script is making changes with UpdateChannel.AUTOMATOR
         # channel. Thus, if the laast update channel was already
         # UpdateChannel.AUTOMATOR, and the value we are trying to set
@@ -60,9 +67,14 @@ def _attempt_update(key: str, value: Any, drifted_options: Set[str], dry_run: bo
     help="Prints the updates without applying them.",
 )
 @click.option("-f", "--file", help="File name to load. If not provided assume stdin.")
+@click.option(
+    "--hide-drift",
+    is_flag=True,
+    help="Hide the actual value of the option on DB when detecting drift.",
+)
 @click.pass_context
 @configuration
-def configoptions(ctx, dry_run: bool, file: Optional[str]) -> None:
+def configoptions(ctx, dry_run: bool, file: Optional[str], hide_drift: bool) -> None:
     """
     Makes changes to options in bulk starting from a yaml file.
     Contrarily to the `config` command, this is meant to perform
@@ -101,7 +113,7 @@ def configoptions(ctx, dry_run: bool, file: Optional[str]) -> None:
     ctx.obj["dry_run"] = dry_run
 
     with open(file) if file is not None else sys.stdin as stream:
-        options_to_update = yaml.safe_load(stream)
+        options_to_update = safe_load(stream)
 
     options_to_update = options_to_update["options"]
     ctx.obj["options_to_update"] = options_to_update
@@ -119,6 +131,7 @@ def configoptions(ctx, dry_run: bool, file: Optional[str]) -> None:
             drifted_options.add(key)
 
     ctx.obj["drifted_options"] = drifted_options
+    ctx.obj["hide_drift"] = hide_drift
 
 
 @configoptions.command()
@@ -135,7 +148,9 @@ def patch(ctx) -> None:
         click.echo("!!! Dry-run flag on. No update will be performed.")
 
     for key, value in ctx.obj["options_to_update"].items():
-        _attempt_update(key, value, ctx.obj["drifted_options"], dry_run)
+        _attempt_update(
+            key, value, ctx.obj["drifted_options"], dry_run, bool(ctx.obj["hide_drift"])
+        )
 
 
 @configoptions.command()
@@ -160,7 +175,11 @@ def sync(ctx):
     for opt in all_options:
         if opt.name in options_to_update:
             _attempt_update(
-                opt.name, options_to_update[opt.name], ctx.obj["drifted_options"], dry_run
+                opt.name,
+                options_to_update[opt.name],
+                ctx.obj["drifted_options"],
+                dry_run,
+                bool(ctx.obj["hide_drift"]),
             )
         else:
             if options.isset(opt.name):

+ 11 - 0
tests/sentry/runner/commands/test_configoptions.py

@@ -6,6 +6,7 @@ from sentry import options
 from sentry.options.manager import FLAG_AUTOMATOR_MODIFIABLE, FLAG_IMMUTABLE, UpdateChannel
 from sentry.runner.commands.configoptions import (
     CHANNEL_UPDATE_MSG,
+    DB_VALUE,
     DRIFT_MSG,
     UNSET_MSG,
     UPDATE_MSG,
@@ -76,6 +77,11 @@ class ConfigOptionsTest(CliTestCase):
                     UPDATE_MSG % "map_option",
                     UPDATE_MSG % "list_option",
                     DRIFT_MSG % "drifted_option",
+                    DB_VALUE % "drifted_option",
+                    "- 1",
+                    "- 2",
+                    "- 3",
+                    "",
                     CHANNEL_UPDATE_MSG % "change_channel_option",
                 ]
             )
@@ -138,6 +144,11 @@ class ConfigOptionsTest(CliTestCase):
                 UPDATE_MSG % "map_option",
                 UPDATE_MSG % "list_option",
                 DRIFT_MSG % "drifted_option",
+                DB_VALUE % "drifted_option",
+                "- 1",
+                "- 2",
+                "- 3",
+                "",
                 CHANNEL_UPDATE_MSG % "change_channel_option",
                 UNSET_MSG % "to_unset_option",
             ]