From 8ff637a1e6b713fc3ba1c59ee9d0f8010b38842a Mon Sep 17 00:00:00 2001 From: Aayush Gupta <43479002+Aayyush@users.noreply.github.com> Date: Thu, 3 Mar 2022 10:28:28 -0800 Subject: [PATCH] Remove unused feature flags (#198) --- .../test-repos/automerge/exp-output-apply-dir1.txt | 1 + .../test-repos/automerge/exp-output-apply-dir2.txt | 1 + .../test-repos/automerge/exp-output-autoplan.txt | 2 ++ server/core/terraform/terraform_client.go | 8 +------- server/core/terraform/terraform_client_internal_test.go | 3 --- server/core/terraform/terraform_client_test.go | 6 ------ server/lyft/feature/names.go | 4 +--- 7 files changed, 6 insertions(+), 19 deletions(-) diff --git a/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir1.txt b/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir1.txt index 19bf762f1b..8c358550ec 100644 --- a/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir1.txt +++ b/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir1.txt @@ -6,5 +6,6 @@ null_resource.automerge[0]: Creation complete after *s [id=*******************] Apply complete! Resources: 1 added, 0 changed, 0 destroyed. + ``` diff --git a/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir2.txt b/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir2.txt index f486966159..c4f1a9ec09 100644 --- a/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir2.txt +++ b/server/controllers/events/testfixtures/test-repos/automerge/exp-output-apply-dir2.txt @@ -6,5 +6,6 @@ null_resource.automerge[0]: Creation complete after *s [id=*******************] Apply complete! Resources: 1 added, 0 changed, 0 destroyed. + ``` diff --git a/server/controllers/events/testfixtures/test-repos/automerge/exp-output-autoplan.txt b/server/controllers/events/testfixtures/test-repos/automerge/exp-output-autoplan.txt index 8aa62370f3..5a232aa80f 100644 --- a/server/controllers/events/testfixtures/test-repos/automerge/exp-output-autoplan.txt +++ b/server/controllers/events/testfixtures/test-repos/automerge/exp-output-autoplan.txt @@ -21,6 +21,7 @@ Terraform will perform the following actions: Plan: 1 to add, 0 to change, 0 to destroy. + ``` * :arrow_forward: To **apply** this plan, comment: @@ -50,6 +51,7 @@ Terraform will perform the following actions: Plan: 1 to add, 0 to change, 0 to destroy. + ``` * :arrow_forward: To **apply** this plan, comment: diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 5783bac655..efc3e088b4 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -249,15 +249,9 @@ func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Vers // See Client.RunCommandWithVersion. func (c *DefaultClient) RunCommandWithVersion(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (string, error) { - shouldAllocate, err := c.featureAllocator.ShouldAllocate(feature.LogStreaming, ctx.BaseRepo.FullName) - - if err != nil { - ctx.Log.Err("unable to allocate for feature: %s, error: %s", feature.LogStreaming, err) - } - // if the feature is enabled, we use the async workflow else we default to the original sync workflow // Don't stream terraform show output to outCh - if shouldAllocate && isAsyncEligibleCommand(args[0]) { + if len(args) > 0 && isAsyncEligibleCommand(args[0]) { outCh := c.RunCommandAsync(ctx, path, args, customEnvVars, v, workspace) var lines []string diff --git a/server/core/terraform/terraform_client_internal_test.go b/server/core/terraform/terraform_client_internal_test.go index de0a75134f..7c4931040c 100644 --- a/server/core/terraform/terraform_client_internal_test.go +++ b/server/core/terraform/terraform_client_internal_test.go @@ -14,7 +14,6 @@ import ( "github.com/runatlantis/atlantis/server/events/models" jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" "github.com/runatlantis/atlantis/server/logging" - "github.com/runatlantis/atlantis/server/lyft/feature" fmocks "github.com/runatlantis/atlantis/server/lyft/feature/mocks" . "github.com/runatlantis/atlantis/testing" ) @@ -50,7 +49,6 @@ func TestDefaultClient_Synchronous_RunCommandWithVersion(t *testing.T) { AsyncClient: asyncClient, featureAllocator: allocator, } - When(allocator.ShouldAllocate(feature.LogStreaming, "owner/repo")).ThenReturn(false, nil) When(mockBuilder.Build(nil, workspace, path, args)).ThenReturn(echoCommand, nil) customEnvVars := map[string]string{} @@ -126,7 +124,6 @@ func TestDefaultClient_Synchronous_RunCommandWithVersion_Error(t *testing.T) { featureAllocator: allocator, } - When(allocator.ShouldAllocate(feature.LogStreaming, "owner/repo")).ThenReturn(false, nil) When(mockBuilder.Build(nil, workspace, path, args)).ThenReturn(echoCommand, nil) out, err := client.RunCommandWithVersion(ctx, path, args, map[string]string{}, nil, workspace) ErrEquals(t, fmt.Sprintf(`running "/bin/sh -c echo dying && exit 1" in %q: exit status 1`, path), err) diff --git a/server/core/terraform/terraform_client_test.go b/server/core/terraform/terraform_client_test.go index a8c591d26f..29d1dcad90 100644 --- a/server/core/terraform/terraform_client_test.go +++ b/server/core/terraform/terraform_client_test.go @@ -20,14 +20,12 @@ import ( "path/filepath" "testing" - . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/cmd" "github.com/runatlantis/atlantis/server/core/terraform" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" "github.com/runatlantis/atlantis/server/logging" - "github.com/runatlantis/atlantis/server/lyft/feature" fmocks "github.com/runatlantis/atlantis/server/lyft/feature/mocks" . "github.com/runatlantis/atlantis/testing" ) @@ -44,8 +42,6 @@ func TestNewClient_NoTF(t *testing.T) { defer tempSetEnv(t, "PATH", tmp)() allocator := fmocks.NewMockAllocator() - When(allocator.ShouldAllocate(feature.LogStreaming, "owner/repo")).ThenReturn(false, nil) - _, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler, allocator) ErrEquals(t, "getting default version: terraform not found in $PATH. Set --default-tf-version or download terraform from https://www.terraform.io/downloads.html", err) } @@ -72,7 +68,6 @@ func TestNewClient_DefaultTFFlagInPath(t *testing.T) { defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() allocator := fmocks.NewMockAllocator() - When(allocator.ShouldAllocate(feature.LogStreaming, "owner/repo")).ThenReturn(false, nil) c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler, allocator) Ok(t, err) @@ -105,7 +100,6 @@ func TestNewClient_DefaultTFFlagInBinDir(t *testing.T) { defer tempSetEnv(t, "PATH", fmt.Sprintf("%s:%s", tmp, os.Getenv("PATH")))() allocator := fmocks.NewMockAllocator() - When(allocator.ShouldAllocate(feature.LogStreaming, "owner/repo")).ThenReturn(false, nil) c, err := terraform.NewClient(logging.NewNoopLogger(t), binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, nil, true, projectCmdOutputHandler, allocator) Ok(t, err) diff --git a/server/lyft/feature/names.go b/server/lyft/feature/names.go index bb4738f1aa..d63af12b62 100644 --- a/server/lyft/feature/names.go +++ b/server/lyft/feature/names.go @@ -3,6 +3,4 @@ package feature type Name string // list of feature names used in the code base. These must be kept in sync with any external config. -const LogStreaming Name = "log-streaming" -const ForceApply Name = "force-apply" -const LogPersistence Name = "log-persistence" +const LogPersistence Name = "log-persistence" \ No newline at end of file