[recipe_bootstrap] Delete the recipe_version property.
We don't need to set the recipe_version property and pass it to child builders, instead we can look up the recipe version in the infra/recipes manifest at the specified integration_base_revision and use that.
Rename the "dir" arguments to "integrationDir" and "recipesDir" for clarity.
Bug: 64892
Change-Id: I3643bb1e520731d40a67a0012829854bcf5e744b
Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/infra/+/507359
Fuchsia-Auto-Submit: Mahesh Saripalli <maheshsr@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Reviewed-by: Oliver Newman <olivernewman@google.com>
diff --git a/cmd/recipe_bootstrap/main.go b/cmd/recipe_bootstrap/main.go
index 9820c76..e3d78b3 100644
--- a/cmd/recipe_bootstrap/main.go
+++ b/cmd/recipe_bootstrap/main.go
@@ -8,7 +8,6 @@
"bytes"
"context"
"encoding/json"
- "errors"
"flag"
"fmt"
"io/ioutil"
@@ -161,50 +160,39 @@
return nil
}
-// resolveRecipeVersionAndProperties resolves the recipe version and build properties to use.
-func resolveRecipeVersionAndProperties(ctx context.Context, build *buildbucketpb.Build) error {
+// getBuilderFilePath returns the builder file path to read the build input properties.
+func getBuilderFilePath(integrationDir string, build *buildbucketpb.Build) string {
+ // TODO(fxbug.dev//71232): switch to "builders" directory once all maintained release branches are newer.
+ return filepath.Join(
+ integrationDir, "infra", "config", "generated", build.Builder.Project,
+ "for_review_only", "buildbucket", build.Builder.Bucket,
+ fmt.Sprintf("%s.textproto", build.Builder.Builder),
+ )
+}
+
+// checkoutIntegration checks out the integration.git repo and returns the path
+// to the directory containing the checkout.
+func checkoutIntegration(ctx context.Context, build *buildbucketpb.Build) (string, error) {
// Do a checkout in a temporary directory to avoid checkout conflicts with
// the recipe's working directory.
cwd, err := os.Getwd()
if err != nil {
- return err
+ return "", err
}
- dir, err := ioutil.TempDir(cwd, "checkout")
+ integrationDir, err := ioutil.TempDir(cwd, "integration-checkout")
if err != nil {
- return err
+ return integrationDir, err
}
- defer os.RemoveAll(dir)
- if err := checkoutIntegration(ctx, dir, build); err != nil {
- return fmt.Errorf("failed to checkout integration bundle: %w", err)
- }
- if err := resolveRecipeVersion(dir, build); err != nil {
- return fmt.Errorf("failed to resolve recipe version: %w", err)
- }
- // Open builder-specific file.
- // TODO(fxbug.dev//71232): switch to "builders" directory once all maintained release branches are newer.
- builderFilePath := filepath.Join(
- dir, "infra", "config", "generated", build.Builder.Project,
- "for_review_only", "buildbucket", build.Builder.Bucket,
- fmt.Sprintf("%s.textproto", build.Builder.Builder),
- )
- if err := resolveBuildProperties(builderFilePath, build); err != nil {
- return fmt.Errorf("failed to resolve build properties: %w", err)
- }
- return nil
-}
-
-// checkoutIntegration checks out the integration.git repo in dir.
-func checkoutIntegration(ctx context.Context, dir string, build *buildbucketpb.Build) error {
integrationURL, integrationBaseRevision, err := resolveIntegration(ctx, build)
if err != nil {
- return err
+ return integrationDir, err
}
if err := props.SetInputProperty(build, "integration_base_revision", integrationBaseRevision); err != nil {
- return err
+ return integrationDir, err
}
tctx, cancel := context.WithTimeout(ctx, defaultCheckoutTimeout)
defer cancel()
- return checkout.Checkout(tctx, build.Input, *integrationURL, integrationBaseRevision, dir)
+ return integrationDir, checkout.Checkout(tctx, build.Input, *integrationURL, integrationBaseRevision, integrationDir)
}
// resolveIntegration resolves the integration URL and revision to checkout.
@@ -250,48 +238,10 @@
return integrationURL, revision, nil
}
-// resolveRecipeVersion resolves the recipe version to use.
-//
-// If Build.Input.GerritChanges indicates that the recipes repo is under test,
-// we short-circuit immediately.
-//
-// In all other cases, we ensure that the recipe version property is set in
-// Build.Input.Properties.
-func resolveRecipeVersion(dir string, build *buildbucketpb.Build) error {
- recipeVersion, err := props.GetStringInputProperty(build, "recipe_version")
- if err != nil {
- return err
- }
- // If the recipe version property is already set, we're done.
- if recipeVersion != "" {
- return nil
- }
- // If the recipes repo is under test, we're done. checkoutRecipes will use
- // the build input directly.
- hasRecipeChange, err := rbc.HasRepoChange(build.Input, recipesRemote)
- if err != nil {
- return fmt.Errorf("could not determine whether build input has recipe change: %w", err)
- }
- if hasRecipeChange {
- return nil
- }
- // Otherwise, resolve the recipe version from the checkout dir.
- manifestXML, err := os.Open(filepath.Join(dir, recipesManifest))
- if err != nil {
- return fmt.Errorf("failed to read recipes manifest file: %s: %v", filepath.Join(dir, recipesManifest), err)
- }
- defer manifestXML.Close()
- proj, err := manifest.ResolveRecipesProject(manifestXML, recipesRemote)
- if err != nil {
- return err
- }
- return props.SetInputProperty(build, "recipe_version", proj.Revision)
-}
-
// resolveBuildProperties resolves the build input properties to use.
//
-// If enable_property_versioning is set, we merge the build input properties
-// with intergration properties.
+// If enable_property_versioning is set, merge the build input properties with
+// integration properties.
func resolveBuildProperties(builderFilePath string, build *buildbucketpb.Build) error {
enablePropertyVersion, err := props.GetBoolInputProperty(build, "enable_property_versioning")
if err != nil {
@@ -339,46 +289,62 @@
return data, nil
}
-// checkoutRecipes checks out the recipes.git repo in dir.
+// checkoutRecipes resolves the recipe version to check out, checks out the recipes
+// repo, and returns the path to the checked-out repository.
//
// If the build input has a recipe change, then checkout directly using the
// build input.
//
// Otherwise, checkout at the revision specified by Build.Input.Properties.
-func checkoutRecipes(ctx context.Context, dir string, build *buildbucketpb.Build) error {
+func checkoutRecipes(ctx context.Context, integrationDir string, build *buildbucketpb.Build) (string, error) {
+ // Do recipes checkout in a temporary directory to avoid checkout conflicts
+ // with the recipe's working directory.
+ cwd, err := os.Getwd()
+ if err != nil {
+ return "", fmt.Errorf("failed to get working directory: %w", err)
+
+ }
+ recipesDir, err := ioutil.TempDir(cwd, "recipes-checkout")
+ if err != nil {
+ return recipesDir, fmt.Errorf("failed to get create tempdir: %w", err)
+
+ }
recipesURL, err := url.Parse(recipesRemote)
if err != nil {
- return fmt.Errorf("could not parse URL %s", recipesRemote)
+ return recipesDir, fmt.Errorf("could not parse URL %s", recipesRemote)
}
var input *buildbucketpb.Build_Input
hasRecipeChange, err := rbc.HasRepoChange(build.Input, recipesRemote)
if err != nil {
- return fmt.Errorf("could not determine whether build input has recipe change: %w", err)
+ return recipesDir, fmt.Errorf("could not determine whether build input has recipe change: %w", err)
}
if hasRecipeChange {
input = build.Input
} else {
- recipeVersion, err := props.GetStringInputProperty(build, "recipe_version")
+ // Use the recipe version from the checkout integrationDir.
+ manifestXML, err := os.Open(filepath.Join(integrationDir, recipesManifest))
if err != nil {
- return err
+ return recipesDir, fmt.Errorf("failed to read recipes manifest file: %s: %v", filepath.Join(integrationDir, recipesManifest), err)
}
- if recipeVersion == "" {
- return errors.New("recipe_version property is unexpectedly empty")
+ defer manifestXML.Close()
+ proj, err := manifest.ResolveRecipesProject(manifestXML, recipesRemote)
+ if err != nil {
+ return recipesDir, err
}
input = &buildbucketpb.Build_Input{
GitilesCommit: &buildbucketpb.GitilesCommit{
Host: recipesURL.Host,
Project: strings.TrimLeft(recipesURL.Path, "/"),
- Id: recipeVersion,
+ Id: proj.Revision,
},
}
}
tctx, cancel := context.WithTimeout(ctx, defaultCheckoutTimeout)
defer cancel()
- if err := checkout.Checkout(tctx, input, *recipesURL, "", dir); err != nil {
- return fmt.Errorf("failed to checkout: %w", err)
+ if err := checkout.Checkout(tctx, input, *recipesURL, "", recipesDir); err != nil {
+ return recipesDir, fmt.Errorf("failed to checkout: %w", err)
}
- return nil
+ return recipesDir, nil
}
// fetchRecipeDeps runs `recipes.py fetch`, which checks out all recipe git
@@ -387,8 +353,8 @@
// beforehand, but running `recipes.py fetch` separately makes it easier to
// distinguish between fetch errors (which are generally caused by transient Git
// server issues) and true build errors within the executed recipe.
-func fetchRecipeDeps(ctx context.Context, dir string) error {
- cmd, err := recipeCommand(ctx, dir, "fetch")
+func fetchRecipeDeps(ctx context.Context, recipesDir string) error {
+ cmd, err := recipeCommand(ctx, recipesDir, "fetch")
if err != nil {
return err
}
@@ -399,12 +365,12 @@
}
// executeRecipe exec's `recipes.py luciexe`, passing build.proto through stdin.
-func executeRecipe(ctx context.Context, dir string, build *buildbucketpb.Build, outputPath string) error {
+func executeRecipe(ctx context.Context, recipesDir string, build *buildbucketpb.Build, outputPath string) error {
args := []string{"luciexe"}
if outputPath != "" {
args = append(args, "--output", outputPath)
}
- cmd, err := recipeCommand(ctx, dir, args...)
+ cmd, err := recipeCommand(ctx, recipesDir, args...)
if err != nil {
return err
}
@@ -417,9 +383,9 @@
}
// recipeCommand constructs an exec.Cmd that will run the `recipes.py` script
-// found in `dir` with the specified arguments.
-func recipeCommand(ctx context.Context, dir string, recipesPyArgs ...string) (*exec.Cmd, error) {
- args := []string{filepath.Join(dir, "recipes.py")}
+// found in `recipesDir` with the specified arguments.
+func recipeCommand(ctx context.Context, recipesDir string, recipesPyArgs ...string) (*exec.Cmd, error) {
+ args := []string{filepath.Join(recipesDir, "recipes.py")}
args = append(args, recipesPyArgs...)
python, err := exec.LookPath("python")
if err != nil {
@@ -460,7 +426,7 @@
// to checkout, then checks out the recipes repo at that revision. It returns
// the deserialized input build.proto along with the path to the recipes
// checkout.
-func setUpRecipes(ctx context.Context) (build *buildbucketpb.Build, dir string, err error) {
+func setUpRecipes(ctx context.Context) (build *buildbucketpb.Build, recipesDir string, err error) {
data, err := ioutil.ReadAll(os.Stdin)
if err != nil {
err = fmt.Errorf("failed to read build.proto from stdin: %w", err)
@@ -475,26 +441,22 @@
err = fmt.Errorf("failed to ensure that build has a gitiles commit: %w", err)
return
}
- if err = resolveRecipeVersionAndProperties(ctx, build); err != nil {
- return
- }
- // Do recipes checkout in a temporary directory to avoid checkout conflicts
- // with the recipe's working directory.
- cwd, err := os.Getwd()
+ integrationDir, err := checkoutIntegration(ctx, build)
+ defer os.RemoveAll(integrationDir)
if err != nil {
- err = fmt.Errorf("failed to get working directory: %w", err)
+ err = fmt.Errorf("failed to checkout integration bundle: %w", err)
return
}
- dir, err = ioutil.TempDir(cwd, "recipes-checkout")
+ if err = resolveBuildProperties(getBuilderFilePath(integrationDir, build), build); err != nil {
+ err = fmt.Errorf("failed to resolve the build properties: %w", err)
+ return
+ }
+ recipesDir, err = checkoutRecipes(ctx, integrationDir, build)
if err != nil {
- err = fmt.Errorf("failed to get create tempdir: %w", err)
- return
- }
- if err = checkoutRecipes(ctx, dir, build); err != nil {
err = fmt.Errorf("failed to download recipe bundle: %w", err)
return
}
- if err = fetchRecipeDeps(ctx, dir); err != nil {
+ if err = fetchRecipeDeps(ctx, recipesDir); err != nil {
return
}
b, err := json.MarshalIndent(build.Input.Properties, "", " ")
@@ -506,24 +468,24 @@
// it's possible for recipe_bootstrap to emit build.proto to the same Logdog stream
// as the recipe engine.
logging.Infof(ctx, "resolved input properties: %s", string(b))
- return build, dir, nil
+ return build, recipesDir, nil
}
func execute(ctx context.Context) error {
outputPath := flag.String("output", "", "Path to write the final build.proto state to.")
flag.Parse()
- build, dir, err := setUpRecipes(ctx)
- // `dir` might not exist depending on the setup failure mode, so ignore any
+ build, recipesDir, err := setUpRecipes(ctx)
+ // `recipesDir` might not exist depending on the setup failure mode, so ignore any
// error when removing it.
- defer os.RemoveAll(dir)
+ defer os.RemoveAll(recipesDir)
if err != nil {
if outputErr := outputBuildSummary(ctx, err.Error()); outputErr != nil {
logging.Errorf(ctx, "Failed to output build summary after recipe setup failed: %v", outputErr)
}
return err
}
- if err := executeRecipe(ctx, dir, build, *outputPath); err != nil {
+ if err := executeRecipe(ctx, recipesDir, build, *outputPath); err != nil {
return fmt.Errorf("failed to execute recipe: %w", err)
}
return nil
diff --git a/cmd/recipe_bootstrap/main_test.go b/cmd/recipe_bootstrap/main_test.go
index 69d5f76..90e7d32 100644
--- a/cmd/recipe_bootstrap/main_test.go
+++ b/cmd/recipe_bootstrap/main_test.go
@@ -110,7 +110,7 @@
t.Fatalf("expected error, got nil")
}
if diff := cmp.Diff(test.expectProperties, build.Input.Properties.AsMap()); diff != "" {
- t.Errorf("resolveBuildProperties(): output differs (-want, +got):\n%s", diff)
+ t.Errorf("mergeProperties(): output differs (-want, +got):\n%s", diff)
}
})
}
