Skip to content

Commit d3162e6

Browse files
hugodutkahugodutka
committed
change terraform download warning to debug log
1 parent 9e8c55f commit d3162e6

File tree

4 files changed

+86
-38
lines changed

4 files changed

+86
-38
lines changed

‎provisioner/terraform/install.go

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package terraform
22

33
import (
44
"context"
5+
"fmt"
56
"os"
67
"path/filepath"
8+
"sync/atomic"
79
"time"
810

911
"github.com/gofrs/flock"
@@ -30,7 +32,9 @@ var (
3032

3133
// Install implements a thread-safe, idempotent Terraform Install
3234
// operation.
33-
func Install(ctx context.Context, log slog.Logger, dir string, wantVersion *version.Version) (string, error) {
35+
//
36+
//nolint:revive // verbose is a control flag that controls the verbosity of the log output.
37+
func Install(ctx context.Context, log slog.Logger, verbose bool, dir string, wantVersion *version.Version) (string, error) {
3438
err := os.MkdirAll(dir, 0o750)
3539
if err != nil {
3640
return "", err
@@ -64,13 +68,37 @@ func Install(ctx context.Context, log slog.Logger, dir string, wantVersion *vers
6468
Version: TerraformVersion,
6569
}
6670
installer.SetLogger(slog.Stdlib(ctx, log, slog.LevelDebug))
67-
log.Debug(
68-
ctx,
69-
"installing terraform",
71+
72+
logInstall := log.Debug
73+
if verbose {
74+
logInstall = log.Info
75+
}
76+
77+
logInstall(ctx, "installing terraform",
7078
slog.F("prev_version", hasVersionStr),
7179
slog.F("dir", dir),
72-
slog.F("version", TerraformVersion),
73-
)
80+
slog.F("version", TerraformVersion))
81+
82+
prolongedInstall := atomic.Bool{}
83+
prolongedInstallCtx, prolongedInstallCancel := context.WithCancel(ctx)
84+
go func() {
85+
seconds := 15
86+
select {
87+
case <-time.After(time.Duration(seconds) * time.Second):
88+
prolongedInstall.Store(true)
89+
// We always want to log this at the info level.
90+
log.Info(
91+
prolongedInstallCtx,
92+
fmt.Sprintf("terraform installation is taking longer than %d seconds, still in progress", seconds),
93+
slog.F("prev_version", hasVersionStr),
94+
slog.F("dir", dir),
95+
slog.F("version", TerraformVersion),
96+
)
97+
case <-prolongedInstallCtx.Done():
98+
return
99+
}
100+
}()
101+
defer prolongedInstallCancel()
74102

75103
path, err := installer.Install(ctx)
76104
if err != nil {
@@ -83,5 +111,9 @@ func Install(ctx context.Context, log slog.Logger, dir string, wantVersion *vers
83111
return "", xerrors.Errorf("%s should be %s", path, binPath)
84112
}
85113

114+
if prolongedInstall.Load() {
115+
log.Info(ctx, "terraform installation complete")
116+
}
117+
86118
return path, nil
87119
}

‎provisioner/terraform/install_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestInstall(t *testing.T) {
4040
wg.Add(1)
4141
go func() {
4242
defer wg.Done()
43-
p, err := terraform.Install(ctx, log, dir, version)
43+
p, err := terraform.Install(ctx, log, false, dir, version)
4444
assert.NoError(t, err)
4545
paths <- p
4646
}()

‎provisioner/terraform/serve.go

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package terraform
22

33
import (
44
"context"
5+
"errors"
56
"path/filepath"
67
"sync"
78
"time"
89

910
"github.com/cli/safeexec"
11+
"github.com/hashicorp/go-version"
1012
semconv "go.opentelemetry.io/otel/semconv/v1.14.0"
1113
"go.opentelemetry.io/otel/trace"
1214
"golang.org/x/xerrors"
@@ -41,10 +43,15 @@ type ServeOptions struct {
4143
ExitTimeout time.Duration
4244
}
4345

44-
func absoluteBinaryPath(ctx context.Context, logger slog.Logger) (string, error) {
46+
type systemBinaryDetails struct {
47+
absolutePath string
48+
version *version.Version
49+
}
50+
51+
func systemBinary(ctx context.Context) (*systemBinaryDetails, error) {
4552
binaryPath, err := safeexec.LookPath("terraform")
4653
if err != nil {
47-
return "", xerrors.Errorf("Terraform binary not found: %w", err)
54+
return nil, xerrors.Errorf("Terraform binary not found: %w", err)
4855
}
4956

5057
// If the "coder" binary is in the same directory as
@@ -54,59 +61,68 @@ func absoluteBinaryPath(ctx context.Context, logger slog.Logger) (string, error)
5461
// to execute this properly!
5562
absoluteBinary, err := filepath.Abs(binaryPath)
5663
if err != nil {
57-
return "", xerrors.Errorf("Terraform binary absolute path not found: %w", err)
64+
return nil, xerrors.Errorf("Terraform binary absolute path not found: %w", err)
5865
}
5966

6067
// Checking the installed version of Terraform.
6168
installedVersion, err := versionFromBinaryPath(ctx, absoluteBinary)
6269
if err != nil {
63-
return "", xerrors.Errorf("Terraform binary get version failed: %w", err)
70+
return nil, xerrors.Errorf("Terraform binary get version failed: %w", err)
6471
}
6572

66-
logger.Info(ctx, "detected terraform version",
67-
slog.F("installed_version", installedVersion.String()),
68-
slog.F("min_version", minTerraformVersion.String()),
69-
slog.F("max_version", maxTerraformVersion.String()))
70-
71-
if installedVersion.LessThan(minTerraformVersion) {
72-
logger.Warn(ctx, "installed terraform version too old, will download known good version to cache")
73-
return "", terraformMinorVersionMismatch
73+
details := &systemBinaryDetails{
74+
absolutePath: absoluteBinary,
75+
version: installedVersion,
7476
}
7577

76-
// Warn if the installed version is newer than what we've decided is the max.
77-
// We used to ignore it and download our own version but this makes it easier
78-
// to test out newer versions of Terraform.
79-
if installedVersion.GreaterThanOrEqual(maxTerraformVersion) {
80-
logger.Warn(ctx, "installed terraform version newer than expected, you may experience bugs",
81-
slog.F("installed_version", installedVersion.String()),
82-
slog.F("max_version", maxTerraformVersion.String()))
78+
if installedVersion.LessThan(minTerraformVersion) {
79+
return details, terraformMinorVersionMismatch
8380
}
8481

85-
return absoluteBinary, nil
82+
return details, nil
8683
}
8784

8885
// Serve starts a dRPC server on the provided transport speaking Terraform provisioner.
8986
func Serve(ctx context.Context, options *ServeOptions) error {
9087
if options.BinaryPath == "" {
91-
absoluteBinary, err := absoluteBinaryPath(ctx, options.Logger)
88+
binaryDetails, err := systemBinary(ctx)
9289
if err != nil {
9390
// This is an early exit to prevent extra execution in case the context is canceled.
9491
// It generally happens in unit tests since this method is asynchronous and
9592
// the unit test kills the app before this is complete.
96-
if xerrors.Is(err, context.Canceled) {
97-
return xerrors.Errorf("absolute binary context canceled: %w", err)
93+
if errors.Is(err, context.Canceled) {
94+
return xerrors.Errorf("system binary context canceled: %w", err)
9895
}
9996

100-
options.Logger.Warn(ctx, "no usable terraform binary found, downloading to cache dir",
101-
slog.F("terraform_version", TerraformVersion.String()),
102-
slog.F("cache_dir", options.CachePath))
103-
binPath, err := Install(ctx, options.Logger, options.CachePath, TerraformVersion)
97+
if errors.Is(err, terraformMinorVersionMismatch) {
98+
options.Logger.Warn(ctx, "installed terraform version too old, will download known good version to cache, or use a previously cached version",
99+
slog.F("installed_version", binaryDetails.version.String()),
100+
slog.F("min_version", minTerraformVersion.String()))
101+
}
102+
103+
binPath, err := Install(ctx, options.Logger, options.ExternalProvisioner, options.CachePath, TerraformVersion)
104104
if err != nil {
105105
return xerrors.Errorf("install terraform: %w", err)
106106
}
107107
options.BinaryPath = binPath
108108
} else {
109-
options.BinaryPath = absoluteBinary
109+
logVersion := options.Logger.Debug
110+
if options.ExternalProvisioner {
111+
logVersion = options.Logger.Info
112+
}
113+
logVersion(ctx, "detected terraform version",
114+
slog.F("installed_version", binaryDetails.version.String()),
115+
slog.F("min_version", minTerraformVersion.String()),
116+
slog.F("max_version", maxTerraformVersion.String()))
117+
// Warn if the installed version is newer than what we've decided is the max.
118+
// We used to ignore it and download our own version but this makes it easier
119+
// to test out newer versions of Terraform.
120+
if binaryDetails.version.GreaterThanOrEqual(maxTerraformVersion) {
121+
options.Logger.Warn(ctx, "installed terraform version newer than expected, you may experience bugs",
122+
slog.F("installed_version", binaryDetails.version.String()),
123+
slog.F("max_version", maxTerraformVersion.String()))
124+
}
125+
options.BinaryPath = binaryDetails.absolutePath
110126
}
111127
}
112128
if options.Tracer == nil {

‎provisioner/terraform/serve_internal_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ func Test_absoluteBinaryPath(t *testing.T) {
5454
t.Skip("Dummy terraform executable on Windows requires sh which isn't very practical.")
5555
}
5656

57-
log := testutil.Logger(t)
5857
// Create a temp dir with the binary
5958
tempDir := t.TempDir()
6059
terraformBinaryOutput := fmt.Sprintf(`#!/bin/sh
@@ -85,11 +84,12 @@ func Test_absoluteBinaryPath(t *testing.T) {
8584
}
8685

8786
ctx := testutil.Context(t, testutil.WaitShort)
88-
actualAbsoluteBinary, actualErr := absoluteBinaryPath(ctx, log)
87+
actualBinaryDetails, actualErr := systemBinary(ctx)
8988

90-
require.Equal(t, expectedAbsoluteBinary, actualAbsoluteBinary)
9189
if tt.expectedErr == nil {
9290
require.NoError(t, actualErr)
91+
require.Equal(t, expectedAbsoluteBinary, actualBinaryDetails.absolutePath)
92+
require.Equal(t, tt.terraformVersion, actualBinaryDetails.version.String())
9393
} else {
9494
require.EqualError(t, actualErr, tt.expectedErr.Error())
9595
}

0 commit comments

Comments
 (0)