[VAULT-28313] enos: decode scenarios iteratively

Significantly improve the speeed and reliability of `list` and `validate` in large repositories. While the prior implementation did list and validate in parallel, it also retained whole sets in memory even if they were not being used. Holding onto those references was catastrophic on machines with scarce resources. We replace the old implementation with a scenario decoder that's an iterator which allows the caller to handle the scenario if necessary or allow it to be garbage collected when we're through with it. This improves our memory usage with `list` and `validate` significantly. When testing `validate` against the Vault `replication` scenario with our prior implementation, our memory usage grew linerally until reaching ~11GB. Now our memory stays flat in the 300-400mb range. Overall it improves listing wall clock speed by 10% and our validate speed by 15% when validating a subset of scenarios. On my machine you couldn't successfully validate all of them with the prior implementation and now it works just fine. There's still room for improvement on listing speed. If we cared less about format padding we could change the UI to be incremental for the basic UI and then render the scenarios on screen while they are being decoded. I chose not to do that as it's outside the scope of what we're trying to resolve. There's also significant memory usage when creating a sample frame and validating and/or making an observation it. There are likely subsequent improvements we could make there but those don't appear to be a blocker to fixing our validator. Backwards incompatible changes: * We now return an error when attempting to list scenarios in a directory without any defined scenario blocks. * We now sort our matrix block by variant and elements and the ordering can be slightly different than before. I doubt anybody would notice but it's possible. * We don't guarantee perfectly sorted output when listing as it is done in parallel and we aren't caching and sorting. Changes: * Create a scenario decoder that implements an iterator pattern * Utilize the iterator pattern in various endpoints and tests * Fix a race in the warnings as errors check acceptance test * Bump version * Pin to the latest actions to resolve deprecation warnings Signed-off-by: Ryan Cragun <[email protected]>
hashicorp · Jun 20, 2024 · a0d7988 · a0d7988
1 parent 96c036c
commit a0d7988
Show file tree

Hide file tree

Showing 44 changed files with 1,324 additions and 879 deletions.
diff --git a/.github/actions/profile-build/action.yml b/.github/actions/profile-build/action.yml
@@ -47,7 +47,7 @@ runs:
         fi
         cp ${{ inputs.profile-out }} default.pgo
     - if: ${{ inputs.upload-profile == 'true' }}
-      uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
+      uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
       with:
         name: default.pgo
         path: default.pgo

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -18,7 +18,7 @@ jobs:
       filepath: ${{ steps.generate-metadata-file.outputs.filepath }}
       product-version: ${{ steps.product-metadata.outputs.product-version }}
     steps:
-      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
       - id: product-metadata
         run: |
           make version
@@ -28,7 +28,7 @@ jobs:
         with:
           version: ${{ steps.product-metadata.outputs.product-version }}
           product: ${{ env.PKG_NAME }}
-      - uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
+      - uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
         with:
           name: metadata.json
           path: ${{ steps.generate-metadata-file.outputs.filepath }}
@@ -41,8 +41,8 @@ jobs:
     outputs:
       profile-path: ${{ steps.final-profile.outputs.profile-path }}
     steps:
-      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
-      - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version-file: go.mod
       - id: product-metadata
@@ -89,11 +89,11 @@ jobs:
     outputs:
       artifact-name: ${{ steps.build.outputs.artifact-name }}
     steps:
-      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
-      - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version-file: go.mod
-      - uses: actions/download-artifact@c850b930e6ba138125429b7e5c93fc707a7f8427 # v4.1.4
+      - uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
         id: download
         with:
           name: ${{ needs.profile-binary.outputs.profile-path }}
@@ -106,7 +106,7 @@ jobs:
           pgo: true
           version: ${{ needs.product-metadata.outputs.product-version }}
       - name: Upload Artifacts
-        uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
+        uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
         with:
           name: ${{ steps.build.outputs.artifact-name }}
           path: ${{ steps.build.outputs.artifact-path }}
@@ -130,13 +130,13 @@ jobs:
           echo "RPM_PACKAGE=$(basename out/*.rpm)" >> "$GITHUB_ENV"
           echo "DEB_PACKAGE=$(basename out/*.deb)" >> "$GITHUB_ENV"
       - name: Upload RHEL Packages
-        uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
+        uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
         if: ${{ matrix.goos == 'linux' }}
         with:
           name: ${{ env.RPM_PACKAGE }}
           path: out/${{ env.RPM_PACKAGE }}
       - name: Upload Debian Packages
-        uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
+        uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
         if: ${{ matrix.goos == 'linux' }}
         with:
           name: ${{ env.DEB_PACKAGE }}
@@ -157,7 +157,7 @@ jobs:
       GOPRIVATE: 'github.com/hashicorp/*'
       TOKEN: ${{ secrets.ELEVATED_GITHUB_TOKEN }}
     steps:
-      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
       - uses: hashicorp/actions-docker-build@v1
         with:
           version: ${{env.version}}

diff --git a/.github/workflows/create_release.yml b/.github/workflows/create_release.yml
@@ -43,7 +43,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
 
       # Set up bob CLI
       - name: Setup bob CLI

diff --git a/.github/workflows/pr_build.yml b/.github/workflows/pr_build.yml
@@ -27,8 +27,8 @@ jobs:
       GOARCH: ${{ matrix.goarch }}
       CI: true
     steps:
-      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
-      - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version-file: go.mod
       - name: Get Product Version
@@ -43,7 +43,7 @@ jobs:
           goos: ${{ matrix.goos }}
           version: ${{ steps.get-product-version.outputs.product-version }}
       - name: Upload
-        uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
+        uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
         with:
           name: ${{ env.PKG_NAME }}_${{ steps.get-product-version.outputs.product-version }}_${{ matrix.goos }}_${{ matrix.goarch }}.zip
           path: out/${{ env.PKG_NAME }}_${{ steps.get-product-version.outputs.product-version }}_${{ matrix.goos }}_${{ matrix.goarch }}.zip

diff --git a/.github/workflows/update_homebrew_formula.yml b/.github/workflows/update_homebrew_formula.yml
@@ -68,10 +68,10 @@ jobs:
       # Checkout Enos repo and place it in the specified relative path within the runner's main directory,
       # in order to accommodate checking out multiple repos.
       - name: Checkout enos repo
-        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
         with:
           path: enos-checkout
-      - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
+      - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version-file: enos-checkout/go.mod
       # Set up bob CLI
@@ -100,7 +100,7 @@ jobs:
       # in order to accommodate checking out multiple repos.
       # A token with sufficient permissions for the target repo is required.
       - name: Checkout homebrew-tap
-        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
         with:
           repository: ${{ env.TARGET_REPO }}
           path: ${{ env.TARGET_REPO_FILEPATH }}

diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml
@@ -22,8 +22,8 @@ jobs:
     name: "Format"
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
-      - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version-file: go.mod
       - uses: ./.github/actions/set-up-buf
@@ -34,7 +34,7 @@ jobs:
           # the terraform wrapper will break terraform execution in enos because
           # it changes the output to text when we expect it to be JSON.
           terraform_wrapper: false
-      - uses: actions/download-artifact@9bc31d5ccc31df68ecc42ccf4149144866c47d8a # v3.0.2
+      - uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
         with:
           name: ${{inputs.artifact-name }}
       - name: unzip
@@ -52,8 +52,8 @@ jobs:
     env:
       CI: true
     steps:
-      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
-      - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version-file: go.mod
       - uses: hashicorp/setup-terraform@v3
@@ -76,7 +76,7 @@ jobs:
           echo "${{ secrets.ENOS_CI_SSH_PRIVATE_KEY }}" > ./acceptance/support/private_key.pem
           chmod 600 ./acceptance/support/private_key.pem
       - name: Download Artifact
-        uses: actions/download-artifact@9bc31d5ccc31df68ecc42ccf4149144866c47d8a # v3.0.2
+        uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # v4.1.7
         with:
           name: ${{inputs.artifact-name }}
       - name: Run all Go tests (unit, acceptance, external deps)

diff --git a/...e/scenarios/scenario_list_pass_0/enos.txt → ...arios/scenario_list_no_scenarios/enos.txt b/...e/scenarios/scenario_list_pass_0/enos.txt → ...arios/scenario_list_no_scenarios/enos.txt
diff --git a/acceptance/scenario_check_test.go b/acceptance/scenario_check_test.go
@@ -41,11 +41,9 @@ func TestAcc_Cmd_Scenario_Check(t *testing.T) {
 			t.Parallel()
 
 			enos := newAcceptanceRunner(t, skipUnlessTerraformCLI())
-			tmpDir, err := os.MkdirTemp("/tmp", "enos.check")
-			require.NoError(t, err)
-			t.Cleanup(func() { os.RemoveAll(tmpDir) })
+			tmpDir := t.TempDir()
 			outDir := filepath.Join(tmpDir, test.dir)
-			err = os.MkdirAll(outDir, 0o755)
+			err := os.MkdirAll(outDir, 0o755)
 			require.NoError(t, err)
 			outDir, err = filepath.EvalSymlinks(outDir)
 			require.NoError(t, err)
@@ -123,14 +121,11 @@ func TestAcc_Cmd_Scenario_Check_WithWarnings(t *testing.T) {
 		skipUnlessExtEnabled(), // since we need the random provider
 	)
 
-	tmpDir, err := os.MkdirTemp("/tmp", "enos.validate")
-	require.NoError(t, err)
-	t.Cleanup(func() { os.RemoveAll(tmpDir) })
-
+	tmpDir := t.TempDir()
 	for _, failOnWarnings := range []bool{true, false} {
 		t.Run(fmt.Sprintf("fail_on_warnings_%t", failOnWarnings), func(t *testing.T) {
 			t.Parallel()
-			outDir := filepath.Join(tmpDir, "scenario_generate_has_warnings")
+			outDir := filepath.Join(tmpDir, fmt.Sprintf("scenario_generate_has_warnings_%t", failOnWarnings))
 			err := os.MkdirAll(outDir, 0o755)
 			require.NoError(t, err)
 			outDir, err = filepath.EvalSymlinks(outDir)

diff --git a/acceptance/scenario_list_test.go b/acceptance/scenario_list_test.go
@@ -23,8 +23,8 @@ func TestAcc_Cmd_Scenario_List(t *testing.T) {
 		fail bool
 	}{
 		{
-			dir: "scenarios/scenario_list_pass_0",
-			out: &pb.ListScenariosResponse{},
+			dir:  "invalid_scenarios/scenario_list_no_scenarios",
+			fail: true,
 		},
 		{
 			dir: "scenarios/scenario_list_pass_1",

diff --git a/acceptance/scenario_sample_list_test.go b/acceptance/scenario_sample_list_test.go
@@ -18,16 +18,15 @@ import (
 func TestAcc_Cmd_Scenario_Sample_List(t *testing.T) {
 	t.Parallel()
 	for _, test := range []struct {
-		dir  string
-		out  *pb.ListSamplesResponse
-		fail bool
+		dir string
+		out *pb.ListSamplesResponse
 	}{
 		{
-			dir: "scenario_list_pass_0",
+			dir: "./invalid_scenarios/scenario_list_no_scenarios",
 			out: &pb.ListSamplesResponse{},
 		},
 		{
-			dir: "sample_list",
+			dir: "./scenarios/sample_list",
 			out: &pb.ListSamplesResponse{
 				Samples: []*pb.Ref_Sample{
 					{
@@ -48,17 +47,11 @@ func TestAcc_Cmd_Scenario_Sample_List(t *testing.T) {
 			t.Parallel()
 			enos := newAcceptanceRunner(t)
 
-			path, err := filepath.Abs(filepath.Join("./scenarios", test.dir))
+			path, err := filepath.Abs(test.dir)
 			require.NoError(t, err)
 			cmd := fmt.Sprintf("scenario sample list --chdir %s --format json", path)
 			fmt.Println(path)
 			out, _, err := enos.run(context.Background(), cmd)
-			if test.fail {
-				require.Error(t, err)
-
-				return
-			}
-
 			require.NoError(t, err)
 			got := &pb.ListSamplesResponse{}
 			require.NoError(t, protojson.Unmarshal(out, got))

diff --git a/acceptance/scenario_sample_observe_test.go b/acceptance/scenario_sample_observe_test.go
@@ -112,12 +112,12 @@ func TestAcc_Cmd_Scenario_Sample_Observe(t *testing.T) {
 							Scenario: &pb.Ref_Scenario{
 								Id: &pb.Scenario_ID{
 									Name:   "smoke",
-									Uid:    "21610357b0083126896a0072a429e677fd6381fa98e94b15ac2101f697100b42",
-									Filter: "smoke arch:s390x distro:ubuntu",
+									Uid:    "bdc1d6d8a1d8332d0ee5ea09e1cb6ad06d47953f8e225de8dc1f0ec3be5eb6a0",
+									Filter: "smoke arch:s390x distro:amz",
 									Variants: &pb.Matrix_Vector{
 										Elements: []*pb.Matrix_Element{
 											{Key: "arch", Value: "s390x"},
-											{Key: "distro", Value: "ubuntu"},
+											{Key: "distro", Value: "amz"},
 										},
 									},
 								},
@@ -144,12 +144,12 @@ func TestAcc_Cmd_Scenario_Sample_Observe(t *testing.T) {
 							Scenario: &pb.Ref_Scenario{
 								Id: &pb.Scenario_ID{
 									Name:   "upgrade",
-									Uid:    "f662b9deac220ad5bc22f2618098925b47d06fc342d8c0d0e6240bb67547b9e8",
-									Filter: "upgrade arch:arm64 distro:ubuntu",
+									Uid:    "3f481b933e5406b3ccafc1a6bbe9fbf18f7f407bbb8546b89dab69345a39dc1b",
+									Filter: "upgrade arch:amd64 distro:amz",
 									Variants: &pb.Matrix_Vector{
 										Elements: []*pb.Matrix_Element{
-											{Key: "arch", Value: "arm64"},
-											{Key: "distro", Value: "ubuntu"},
+											{Key: "arch", Value: "amd64"},
+											{Key: "distro", Value: "amz"},
 										},
 									},
 								},

diff --git a/acceptance/scenario_validate_test.go b/acceptance/scenario_validate_test.go
@@ -88,7 +88,7 @@ func TestAcc_Cmd_Scenario_Validate_filtered(t *testing.T) {
 				return
 			}
 
-			require.NoError(t, err)
+			require.NoErrorf(t, err, string(out))
 			got := &pb.ValidateScenariosConfigurationResponse{}
 			require.NoError(t, protojson.Unmarshal(out, got))
 		})

diff --git a/go.mod b/go.mod
@@ -11,23 +11,23 @@ require (
 	github.com/google/uuid v1.6.0
 	github.com/hashicorp/go-hclog v1.6.3
 	github.com/hashicorp/go-multierror v1.1.1
-	github.com/hashicorp/hcl/v2 v2.20.1
+	github.com/hashicorp/hcl/v2 v2.21.0
 	github.com/hashicorp/terraform-exec v0.21.0
 	github.com/hashicorp/terraform-json v0.22.1
 	github.com/hexops/gotextdiff v1.0.3
 	github.com/mitchellh/cli v1.1.5
 	github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db
 	github.com/mitchellh/go-wordwrap v1.0.1
 	github.com/olekukonko/tablewriter v0.0.5
-	github.com/spf13/cobra v1.8.0
+	github.com/spf13/cobra v1.8.1
 	github.com/stretchr/testify v1.8.4
 	github.com/zclconf/go-cty v1.14.4
 	github.com/zclconf/go-cty-yaml v1.0.3
-	golang.org/x/net v0.25.0
-	golang.org/x/term v0.20.0
-	golang.org/x/text v0.15.0
+	golang.org/x/net v0.26.0
+	golang.org/x/term v0.21.0
+	golang.org/x/text v0.16.0
 	google.golang.org/grpc v1.64.0
-	google.golang.org/protobuf v1.34.1
+	google.golang.org/protobuf v1.34.2
 )
 
 require (
@@ -53,7 +53,7 @@ require (
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-version v1.7.0 // indirect
-	github.com/huandu/xstrings v1.4.0 // indirect
+	github.com/huandu/xstrings v1.5.0 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/mattn/go-colorable v0.1.13 // indirect
@@ -67,11 +67,11 @@ require (
 	github.com/shopspring/decimal v1.4.0 // indirect
 	github.com/spf13/cast v1.6.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
-	golang.org/x/crypto v0.23.0 // indirect
-	golang.org/x/mod v0.17.0 // indirect
+	golang.org/x/crypto v0.24.0 // indirect
+	golang.org/x/mod v0.18.0 // indirect
 	golang.org/x/sync v0.7.0 // indirect
-	golang.org/x/sys v0.20.0 // indirect
-	golang.org/x/tools v0.21.0 // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20240528184218-531527333157 // indirect
+	golang.org/x/sys v0.21.0 // indirect
+	golang.org/x/tools v0.22.0 // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20240617180043-68d350f18fd4 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )