Commit ab6bd10e authored by Garrett Beaty's avatar Garrett Beaty Committed by Commit Bot

Add a presubmit check to guard changes with an active outages config.

Change-Id: Ic04dbb979d49437347ccb46e31f134b826165dcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432860Reviewed-by: default avatarStephen Martinis <martiniss@chromium.org>
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812900}
parent 5e851297
...@@ -49,3 +49,93 @@ def CheckLucicfgGenOutputDev(input_api, output_api): ...@@ -49,3 +49,93 @@ def CheckLucicfgGenOutputDev(input_api, output_api):
def CheckChangedLUCIConfigs(input_api, output_api): def CheckChangedLUCIConfigs(input_api, output_api):
return input_api.canned_checks.CheckChangedLUCIConfigs( return input_api.canned_checks.CheckChangedLUCIConfigs(
input_api, output_api) input_api, output_api)
# Footer indicating a CL that is trying to address an outage by some mechanism
# other than those in infra/config/outages
_OUTAGE_ACTION_FOOTER = 'Infra-Config-Outage-Action'
# Footer acknowledging that an outages configuration is in effect when making an
# unrelated change
_IGNORE_OUTAGE_FOOTER = 'Infra-Config-Ignore-Outage'
def CheckOutagesConfigOnCommit(input_api, output_api):
outages_pyl = input_api.os_path.join(
input_api.PresubmitLocalPath(), 'generated/outages.pyl')
with open(outages_pyl) as f:
outages_config = input_api.ast.literal_eval(f.read())
if not outages_config:
footers = input_api.change.GitFootersFromDescription()
return [
output_api.PresubmitError(
'There is no outages configuration in effect, '
'please remove the {} footer from your CL description.'
.format(footer))
for footer in (_OUTAGE_ACTION_FOOTER, _IGNORE_OUTAGE_FOOTER)
if footer in footers
]
# Any of the config files under infra/config/outages
outages_config_files = set()
# Any of the config files under infra/config/generated
generated_config_files = set()
# Any config files that are not under infra/config/outages or
# infra/config/generated
config_files = set()
for p in input_api.LocalPaths():
if p in ('README.md', 'OWNERS'):
continue
if p.startswith('infra/config/outages/'):
outages_config_files.add(p)
continue
if p.startswith('infra/config/generated/'):
generated_config_files.add(p)
continue
config_files.add(p)
# If the only changes to non-generated config fies were the outages files,
# assume the change was addressing an outage and that no additional mechanism
# needs to be added
if outages_config_files and not config_files:
# REVIEWER: Should we prevent the footers from being here in this case?
return []
# If any non-generated, non-outages files were modified or if the generated
# config files were modified without any config files being modified (lucicfg
# change, etc.) then make sure the user knows that when the outages
# configuration is disabled, the generated configuration may change
if config_files or generated_config_files:
footers = input_api.change.GitFootersFromDescription()
has_action_footer = _OUTAGE_ACTION_FOOTER in footers
has_ignore_footer = _IGNORE_OUTAGE_FOOTER in footers
if has_action_footer and has_ignore_footer:
return [
output_api.PresubmitError(
'Only one of {} or {} should be present in your CL description'
.format(_OUTAGE_ACTION_FOOTER, _IGNORE_OUTAGE_FOOTER)),
]
if not has_action_footer and not has_ignore_footer:
outages_config_lines = ['{}: {}'.format(k, v)
for k, v in sorted(outages_config.items())]
return [
output_api.PresubmitError('\n'.join([
'The following outages configuration is in effect:\n {}'
.format('\n '.join(outages_config_lines)),
('The effect of your change may not be visible '
'in the generated configuration.'),
('If your change is addressing the outage, '
'please add the footer {} with a link for the outage.')
.format(_OUTAGE_ACTION_FOOTER),
('If your change is not addressing the outage '
'but you still wish to land it, please add the footer '
'{} with a reason.')
.format(_IGNORE_OUTAGE_FOOTER),
('For more information on outages configuration, '
'see https://chromium.googlesource.com/chromium/src/+/refs/heads/master/infra/config/outages'),
])),
]
return []
# This is a non-LUCI generated file
# This details the current configuration modifications for outages settings
{}
...@@ -32,6 +32,7 @@ lucicfg.config( ...@@ -32,6 +32,7 @@ lucicfg.config(
"luci-notify.cfg", "luci-notify.cfg",
"luci-notify/email-templates/*.template", "luci-notify/email-templates/*.template",
"luci-scheduler.cfg", "luci-scheduler.cfg",
"outages.pyl",
"project.cfg", "project.cfg",
"project.pyl", "project.pyl",
"realms.cfg", "realms.cfg",
......
...@@ -29,3 +29,27 @@ available parameters are: ...@@ -29,3 +29,27 @@ available parameters are:
other workarounds to improve the CQ experience for developers rather than other workarounds to improve the CQ experience for developers rather than
tracking down which experimental tryjobs would be using the capacity that is tracking down which experimental tryjobs would be using the capacity that is
needed for the particular outage. needed for the particular outage.
# Making changes during an outage
If you attempt to make a configuration change while an outages configuration is
enabled then the actual effect of your change may not be apparent if the outages
configuration impacts the portion of the configuration that you are modifying.
To prevent someone from unknowingly making such a change, a presubmit check will
prevent changes from being landed if there is an outages configuration in effect
unless all of the following are true:
* 1 or more LUCI configuration files within //infra/config/outages are modified.
* 0 or more LUCI configuration files within //infra/config/generated are
modified.
* 0 other LUCI configuration files within //infra/config are modified.
If an outages configuration is in effect and you need to make a change that
doesn't meet these conditions to try and address the outage, add the footer
`Infra-Config-Outage-Action`. The value for the footer should be some link that
provides context on the outage being addressed.
If an outages configuration is in effect and you need to make an unrelated
change that cannot wait until the outage has been addressed, add the footer
`Infra-Config-Ignore-Outage`. The value for the footer should explain why it is
necessary to make such a change.
...@@ -7,6 +7,10 @@ def _outages_config(*, disable_cq_experiments = False): ...@@ -7,6 +7,10 @@ def _outages_config(*, disable_cq_experiments = False):
disable_cq_experiments = disable_cq_experiments, disable_cq_experiments = disable_cq_experiments,
) )
# The default value that is used by a generator to compare against the current
# config to output the effective outages configuration
DEFAULT_CONFIG = _outages_config()
# See README.md for documentation on allowable configuration values # See README.md for documentation on allowable configuration values
config = _outages_config( config = _outages_config(
) )
...@@ -2,9 +2,24 @@ ...@@ -2,9 +2,24 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
load("./config.star", "config") load("./config.star", "DEFAULT_CONFIG", "config")
load("@stdlib//internal/luci/proto.star", "cq_pb") load("@stdlib//internal/luci/proto.star", "cq_pb")
def _generate_outages_file(ctx):
config_to_write = {}
for a in dir(config):
value = getattr(config, a)
if value != getattr(DEFAULT_CONFIG, a):
config_to_write[a] = value
ctx.output["outages.pyl"] = "\n".join([
"# This is a non-LUCI generated file",
"# This details the current configuration modifications for outages settings",
repr(config_to_write),
"",
])
lucicfg.generator(_generate_outages_file)
def _disable_cq_experiments(ctx): def _disable_cq_experiments(ctx):
if not config.disable_cq_experiments: if not config.disable_cq_experiments:
return return
...@@ -12,8 +27,7 @@ def _disable_cq_experiments(ctx): ...@@ -12,8 +27,7 @@ def _disable_cq_experiments(ctx):
for c in ctx.output["commit-queue.cfg"].config_groups: for c in ctx.output["commit-queue.cfg"].config_groups:
if c.verifiers.tryjob == cq_pb.Verifiers.Tryjob(): if c.verifiers.tryjob == cq_pb.Verifiers.Tryjob():
# Accessing the tryjob field where it wasn't set causes it to be set # Accessing the tryjob field where it wasn't set causes it to be set
# to an empty message and added to the output, setting to None # to an empty message and added to the output
# prevents the change to the output
c.verifiers.tryjob = None c.verifiers.tryjob = None
continue continue
for b in c.verifiers.tryjob.builders: for b in c.verifiers.tryjob.builders:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment