feat: Add config-based SRE Recipe Implementation #826

Gan-Tu · 2021-08-21T06:13:14Z

In this PR, we support both config-based and implementation-based SRE Recipe implementation.

Recipe Types

The config-based SRE Recipe makes it fast to iterate and add new SRE Recipes, as discussed in the internal design review. It also helps reduce redundant, and hacky code.
The implementation-based SRE Recipe is kept for backward compatibility and allows for complex implementation needs that config-based SRE Recipe cannot do.
Documentations are added for both, with support for hiding/disabling recipes.

Refactoring

Migrated all existing SRE Recipe implementation to config-based YAML recipes
Added one dummy implementation-based recipe for future reference.
Updated sandboxctl to support running both types of recipes, using the samesandboxctl syntax right now.
Refactored generic, reusable shell commands to a standalone utils file, instead of putting them as static methods in (conceptually irrelevant) recipe classes.

Future Work

I have native validation tests for config-based recipes themselves during runtime as of now. Since this PR is getting too large, I plan to have a dedicated PR for adding integration tests for config validations during e2e CI and release.

Many SRE Recipes (except the one added in #817) don't have integration tests either, so tests for SRE Recipes prob deserve a new PR and teamwork for the future anyways.

Manually verified all sandboxctl sre-recipes command and sandboxctl describe still work as intended.

…dent_cause

daniel-sanche

This is great Gan, I'm super excited about this change! I added some comments, but I think most of it is non-blocking. Maybe we can turn some comments into issues to address later.

Also, I haven't tested the recipes much yet. I'll try to do a final manual run through after the code is fully reviewed, and possibly open a new PR with recipe tests

sre-recipes/recipes/impl_based/dummy_recipe.py

sre-recipes/recipe_runner.py

sre-recipes/recipes/configs_based/README.md

sre-recipes/recipe_runner.py

daniel-sanche · 2021-09-03T21:49:25Z

sre-recipes/recipes/configs_based/disabled/recipe1.yaml

@@ -0,0 +1,61 @@
+# Copyright 2021 Google LLC


FYI: I want to start writing simple tests for the recipes like this one. It's probably out of scope for this PR, but we should probably add more before the next release to be safe. Let me know if you have thoughts

(bug here)

simonz130

Excellent work! Thank you @Gan-Tu!

sre-recipes/recipe_runner.py

simonz130 · 2021-09-08T18:55:44Z

sre-recipes/recipe_runner.py

+
+    def __init__(self, recipe_name):
+        filepath = path.join(path.dirname(
+            path.abspath(__file__)), f"recipes/configs_based/{recipe_name}.yaml")


nit: we might want to be tolerant and allow yml extension too

Given .yaml is recommended by YAML and we own this repo. Let's go with the consistent .yaml extension

https://yaml.org/faq.html

simonz130 · 2021-09-08T18:57:24Z

sre-recipes/recipe_runner.py

+        filepath = path.join(path.dirname(
+            path.abspath(__file__)), f"recipes/configs_based/{recipe_name}.yaml")
+        with open(filepath, "r") as file:
+            self.recipe = yaml.safe_load(file.read())


Is there a way to define a grammar(structure) for the yaml config?

We cannot. But since the parsed YAML is in JSON format, we can use JSON-Schema library to do validation checks. I am planning to do that as a part of the follow-up PR for integration testing and validations.

https://stackoverflow.com/questions/3262569/validating-a-yaml-document-in-python

sre-recipes/recipe_runner.py

simonz130 · 2021-09-08T19:09:51Z

sre-recipes/recipe_runner.py

+
+    def run_restore(self):
+        print('Restoring broken service...')
+        for action in self.config.get("restore", []):


can we extract all the operation names into consts so it's consolidated in one place?

sre-recipes/recipe_runner.py

sre-recipes/recipes/configs_based/README.md

sre-recipes/utils.py

daniel-sanche

Looks great, thanks!

Gan-Tu added 30 commits June 4, 2021 16:25

docs: fix sre recipes documentation

ef597ae

configs: add YAML representation of SRE Recipes

70d2131

feat: add a new recipe runner, with refactored base class

45029f4

configs: add selectors

ab110d9

feat: implement recipe runner, and leave shell commands in utils

07401f6

use multiline format for hints and descriptions

e4124c9

add informative status msg when running recipe commands

2df1748

Use selector correctly, and move sre recipe configs to new folder

e757f28

Merge branch 'master' into gan-refactor-sre-recipe

ca255b6

Merge branch 'develop' into gan-refactor-sre-recipe

0b16538

[docs]: add sandboxctl describe to hints

635ad40

[docs] add deprecation warning to original Recipe class

828453e

refactor: do not use command lib

a48b7c9

fix: update selector param to pod_selector in config

f16af3f

fix: update new recipes

4ff9864

feat: add recipe runner without verify implementation

0ad0639

fix: rename run_steps to handle_actions

7311472

feat: implement verify

bf710cf

feat: rename broken_service -> affected_service, broken_cause -> inci…

ed9cb55

…dent_cause

docs: add README for SRE Recipe configs

f92d826

feat: rename broken_service -> affected_service, broken_cause -> inci…

601d4b0

…dent_cause

docs: fix run_hint message, when there is no hints available

3a8887a

refactor: improve multiple choice logic

fa4985e

minor fixes

0ec4e76

docs: improve README to suggest more than action is supported

079081c

fix: fix bugs

685625b

docs: add COPYRIGHT notices

6622595

chore: ignore srerecipe logs in .gitignore

829d01b

refactor: move common commands to utils

d7ef1e3

feat: use config based recipe runner; WIP

8dd8aea

Gan-Tu added 3 commits August 27, 2021 18:56

docs: update readme for sre recipe configs

bd6807b

fix: print error when sandboxctl describe fails

9318457

Stop printing SRE Recipe API response in break/restore

36902b6

Gan-Tu marked this pull request as ready for review August 31, 2021 00:04

Gan-Tu requested review from daniel-sanche and simonz130 as code owners August 31, 2021 00:04

blunderbuss-gcf bot assigned daniel-sanche Aug 31, 2021

daniel-sanche reviewed Sep 3, 2021

View reviewed changes

Gan-Tu added 8 commits September 3, 2021 17:19

docs: improve documentation

5932ca8

refactor: use new config format for actions

943deb8

feat: use action handler for multiple choice

82a82ba

refactor: extract action handlers out to its own class

f81718a

docs: update README for sre recipe config

c2bb35c

refactor: rename multiple choice quiz, and add handler docstring

1a38a7e

fix: update missed legacy action type

ef47ab8

fix: use subprocess.run instead to avoid merging of stdout,stderr

9a4ac8a

Gan-Tu requested a review from daniel-sanche September 4, 2021 02:57

simonz130 approved these changes Sep 8, 2021

View reviewed changes

daniel-sanche approved these changes Sep 9, 2021

View reviewed changes

Gan-Tu added 3 commits September 10, 2021 13:33

fix typos

1783171

refactor: disable dummy recipe

63911ed

Merge branch 'develop' into gan-refactor-sre-recipe

bb28dee

Gan-Tu added this to the v0.8.0 milestone Sep 10, 2021

Gan-Tu linked an issue Sep 10, 2021 that may be closed by this pull request

feat: Support both config based and implementation based SRE Recipes #832

Closed

Gan-Tu merged commit 13e46f7 into develop Sep 10, 2021

Gan-Tu deleted the gan-refactor-sre-recipe branch September 10, 2021 22:28

github-actions bot mentioned this pull request Oct 19, 2021

chore: release/v0.7.1 #847

Closed

github-actions bot mentioned this pull request Oct 29, 2021

chore: release/v0.7.1 #862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add config-based SRE Recipe Implementation #826

feat: Add config-based SRE Recipe Implementation #826

Gan-Tu commented Aug 21, 2021 •

edited

Loading

daniel-sanche left a comment

daniel-sanche Sep 3, 2021

simonz130 left a comment

simonz130 Sep 8, 2021

Gan-Tu Sep 9, 2021

simonz130 Sep 8, 2021

Gan-Tu Sep 8, 2021

simonz130 Sep 8, 2021

daniel-sanche left a comment •

edited

Loading

feat: Add config-based SRE Recipe Implementation #826

feat: Add config-based SRE Recipe Implementation #826

Conversation

Gan-Tu commented Aug 21, 2021 • edited Loading

daniel-sanche left a comment

Choose a reason for hiding this comment

daniel-sanche Sep 3, 2021

Choose a reason for hiding this comment

simonz130 left a comment

Choose a reason for hiding this comment

simonz130 Sep 8, 2021

Choose a reason for hiding this comment

Gan-Tu Sep 9, 2021

Choose a reason for hiding this comment

simonz130 Sep 8, 2021

Choose a reason for hiding this comment

Gan-Tu Sep 8, 2021

Choose a reason for hiding this comment

simonz130 Sep 8, 2021

Choose a reason for hiding this comment

daniel-sanche left a comment • edited Loading

Choose a reason for hiding this comment

Gan-Tu commented Aug 21, 2021 •

edited

Loading

daniel-sanche left a comment •

edited

Loading