Skip to content

Commit ab22c47

Browse files
added command level timeouts
1 parent e3b2aad commit ab22c47

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+8256
-52
lines changed

go.mod

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@ require (
1414
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
1515
github.com/prometheus/client_golang v1.14.0
1616
github.com/robfig/cron/v3 v3.0.0
17+
github.com/samber/lo v1.39.0
1718
github.com/stretchr/testify v1.8.4
1819
github.com/tidwall/gjson v1.9.3
1920
go.uber.org/zap v1.21.0
21+
golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17
2022
golang.org/x/sys v0.15.0
2123
google.golang.org/grpc v1.56.3
2224
google.golang.org/protobuf v1.31.0
25+
gopkg.in/src-d/go-billy.v4 v4.3.2
2326
gopkg.in/src-d/go-git.v4 v4.13.1
2427
)
2528

@@ -60,7 +63,6 @@ require (
6063
golang.org/x/net v0.17.0 // indirect
6164
golang.org/x/text v0.14.0 // indirect
6265
google.golang.org/genproto/googleapis/rpc v0.0.0-20230525234030-28d5490b6b19 // indirect
63-
gopkg.in/src-d/go-billy.v4 v4.3.2 // indirect
6466
gopkg.in/warnings.v0 v0.1.2 // indirect
6567
gopkg.in/yaml.v3 v3.0.1 // indirect
6668
mellium.im/sasl v0.3.1 // indirect

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ github.com/prometheus/procfs v0.11.1 h1:xRC8Iq1yyca5ypa9n1EZnWZkt7dwcoRPQwX/5gwa
104104
github.com/prometheus/procfs v0.11.1/go.mod h1:eesXgaPo1q7lBpVMoMy0ZOFTth9hBn4W/y0/p/ScXhY=
105105
github.com/robfig/cron/v3 v3.0.0 h1:kQ6Cb7aHOHTSzNVNEhmp8EcWKLb4CbiMW9h9VyIhO4E=
106106
github.com/robfig/cron/v3 v3.0.0/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro=
107+
github.com/samber/lo v1.39.0 h1:4gTz1wUhNYLhFSKl6O+8peW0v2F4BCY034GRpU9WnuA=
108+
github.com/samber/lo v1.39.0/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA=
107109
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
108110
github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
109111
github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
@@ -146,6 +148,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
146148
golang.org/x/crypto v0.0.0-20210314154223-e6e6c4f2bb5b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
147149
golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k=
148150
golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
151+
golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17 h1:3MTrJm4PyNL9NBqvYDSj3DHl46qQakyfqfWo4jgfaEM=
152+
golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17/go.mod h1:lgLbSvA5ygNOMpwM/9anMpWVlVJ7Z+cHWq/eFuinpGE=
149153
golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
150154
golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
151155
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=

internal/Configuration.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import (
55
)
66

77
type Configuration struct {
8-
CommitStatsTimeoutInSec int `env:"COMMIT_STATS_TIMEOUT_IN_SEC" envDefault:"2"`
9-
EnableFileStats bool `env:"ENABLE_FILE_STATS" envDefault:"false"`
10-
GitHistoryCount int `env:"GIT_HISTORY_COUNT" envDefault:"15"`
11-
MinLimit int `env:"MIN_LIMIT_FOR_PVC" envDefault:"1"` // in MB
12-
UseGitCli bool `env:"USE_GIT_CLI" envDefault:"false"`
13-
ProcessTimeout int `env:"PROCESS_TIMEOUT" envDefault:"5"`
14-
CliCmdTimeout int `env:"CLI_CMD_TIMEOUT" envDefault:"0"`
8+
CommitStatsTimeoutInSec int `env:"COMMIT_STATS_TIMEOUT_IN_SEC" envDefault:"2"`
9+
EnableFileStats bool `env:"ENABLE_FILE_STATS" envDefault:"false"`
10+
GitHistoryCount int `env:"GIT_HISTORY_COUNT" envDefault:"15"`
11+
MinLimit int `env:"MIN_LIMIT_FOR_PVC" envDefault:"1"` // in MB
12+
UseGitCli bool `env:"USE_GIT_CLI" envDefault:"false"`
13+
ProcessTimeout int `env:"PROCESS_TIMEOUT" envDefault:"5"`
14+
CliCmdTimeoutGlobal int `env:"CLI_CMD_TIMEOUT_GLOBAL" envDefault:"0"`
15+
CliCmdTimeoutJson string `env:"CLI_CMD_TIMEOUT_JSON" envDefault:""`
1516
}
1617

1718
func ParseConfiguration() (*Configuration, error) {

pkg/git/GitBaseManager.go

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import (
88
"github.com/devtron-labs/git-sensor/internal"
99
"github.com/devtron-labs/git-sensor/internal/sql"
1010
"github.com/devtron-labs/git-sensor/util"
11+
"github.com/samber/lo"
1112
"go.uber.org/zap"
13+
"golang.org/x/exp/slices"
1214
"os"
1315
"os/exec"
1416
"regexp"
@@ -44,8 +46,28 @@ type GitManagerBase interface {
4446
ConfigureSshCommand(gitCtx GitContext, rootDir string, sshPrivateKeyPath string) (response, errMsg string, err error)
4547
}
4648
type GitManagerBaseImpl struct {
47-
logger *zap.SugaredLogger
48-
conf *internal.Configuration
49+
logger *zap.SugaredLogger
50+
conf *internal.Configuration
51+
commandTimeoutMap map[string]time.Duration
52+
}
53+
54+
func NewGitManagerBaseImpl(logger *zap.SugaredLogger, config *internal.Configuration) *GitManagerBaseImpl {
55+
56+
commandTimeoutMap, err := parseCmdTimeoutJson(config)
57+
if err != nil {
58+
logger.Errorw("error in parsing CLI_CMD_TIMEOUT_JSON", "value", config.CliCmdTimeoutJson, "err", err)
59+
}
60+
61+
return &GitManagerBaseImpl{logger: logger, conf: config, commandTimeoutMap: commandTimeoutMap}
62+
}
63+
64+
func parseCmdTimeoutJson(config *internal.Configuration) (map[string]time.Duration, error) {
65+
commandTimeoutMap := make(map[string]time.Duration)
66+
var err error
67+
if config.CliCmdTimeoutJson != "" {
68+
err = json.Unmarshal([]byte(config.CliCmdTimeoutJson), &commandTimeoutMap)
69+
}
70+
return commandTimeoutMap, err
4971
}
5072

5173
type GitManagerImpl struct {
@@ -240,9 +262,23 @@ func (impl *GitManagerBaseImpl) FetchDiffStatBetweenCommits(gitCtx GitContext, o
240262
func (impl *GitManagerBaseImpl) CreateCmdWithContext(ctx GitContext, name string, arg ...string) (*exec.Cmd, context.CancelFunc) {
241263
newCtx := ctx.Context
242264
cancel := func() {}
243-
if impl.conf.CliCmdTimeout > 0 {
244-
newCtx, cancel = context.WithTimeout(ctx.Context, time.Duration(impl.conf.CliCmdTimeout))
265+
266+
//TODO: how to make it generic, currently works because the
267+
// git command is placed at index 2 for current implementations
268+
timeout := impl.getCommandTimeout(arg[2])
269+
270+
if timeout > 0 {
271+
newCtx, cancel = context.WithTimeout(ctx.Context, timeout)
245272
}
246273
cmd := exec.CommandContext(newCtx, name, arg...)
247274
return cmd, cancel
248275
}
276+
277+
func (impl *GitManagerBaseImpl) getCommandTimeout(command string) time.Duration {
278+
timeout := time.Duration(impl.conf.CliCmdTimeoutGlobal)
279+
overridenCommands := lo.Keys[string, time.Duration](impl.commandTimeoutMap)
280+
if slices.Contains(overridenCommands, command) {
281+
timeout = impl.commandTimeoutMap[command]
282+
}
283+
return timeout
284+
}

pkg/git/GitCliManager.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package git
33
import (
44
"encoding/json"
55
"fmt"
6-
"github.com/devtron-labs/git-sensor/internal"
7-
"go.uber.org/zap"
86
"gopkg.in/src-d/go-billy.v4/osfs"
97
"os"
108
"path/filepath"
@@ -16,12 +14,13 @@ type GitCliManager interface {
1614
}
1715

1816
type GitCliManagerImpl struct {
19-
GitManagerBaseImpl
17+
*GitManagerBaseImpl
2018
}
2119

22-
func NewGitCliManagerImpl(logger *zap.SugaredLogger, config *internal.Configuration) *GitCliManagerImpl {
20+
func NewGitCliManagerImpl(baseManagerImpl *GitManagerBaseImpl) *GitCliManagerImpl {
21+
2322
return &GitCliManagerImpl{
24-
GitManagerBaseImpl: GitManagerBaseImpl{logger: logger, conf: config},
23+
GitManagerBaseImpl: baseManagerImpl,
2524
}
2625
}
2726

pkg/git/GoGitSDKManager.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package git
22

33
import (
44
"fmt"
5-
"github.com/devtron-labs/git-sensor/internal"
6-
"go.uber.org/zap"
75
"gopkg.in/src-d/go-git.v4"
86
"gopkg.in/src-d/go-git.v4/config"
97
"gopkg.in/src-d/go-git.v4/plumbing"
@@ -14,12 +12,12 @@ type GoGitSDKManager interface {
1412
GitManager
1513
}
1614
type GoGitSDKManagerImpl struct {
17-
GitManagerBaseImpl
15+
*GitManagerBaseImpl
1816
}
1917

20-
func NewGoGitSDKManagerImpl(logger *zap.SugaredLogger, config *internal.Configuration) *GoGitSDKManagerImpl {
18+
func NewGoGitSDKManagerImpl(baseManagerImpl *GitManagerBaseImpl) *GoGitSDKManagerImpl {
2119
return &GoGitSDKManagerImpl{
22-
GitManagerBaseImpl: GitManagerBaseImpl{logger: logger, conf: config},
20+
GitManagerBaseImpl: baseManagerImpl,
2321
}
2422
}
2523

pkg/git/RepositoryManagerIT_test.go

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,32 @@ import (
1111
"time"
1212
)
1313

14-
var gitRepoUrl = "https://github.com/devtron-labs/getting-started-nodejs.git"
15-
var location1 = "/tmp/git-base/1/github.com/devtron-labs/getting-started-nodejs.git"
16-
var location2 = "/tmp/git-base/2/github.com/devtron-labs/getting-started-nodejs.git"
14+
var gitRepoUrl = "https://github.com/devtron-labs/getting-started-nodejs"
15+
var location1 = baseDir + "/git-base/1/github.com/devtron-labs/getting-started-nodejs.git"
16+
var location2 = baseDir + "/git-base/2/github.com/devtron-labs/getting-started-nodejs.git"
1717
var commitHash = "dfde5ecae5cd1ae6a7e3471a63a8277177898a7d"
1818
var tag = "v0.0.2"
1919
var branchName = "do-not-touch-this-branch"
20-
var baseDir = "tmp/"
21-
var privateGitRepoUrl = "https://github.com/prakash100198/HelloWorldProject.git"
22-
var privateGitRepoLocation = "/tmp/git-base/42/github.com/prakash100198/HelloWorldProject.git"
23-
var username = "prakash100198"
24-
var password = ""
20+
var baseDir = "/Users/subhashish/tmp1"
21+
var privateGitRepoUrl = "https://github.com/devtron-labs/getting-started-nodejs.git"
22+
var privateGitRepoLocation = baseDir + "/git-base/42/github.com/devtron-labs/getting-started-nodejs.git"
23+
var username = "subhashish-devtron"
24+
var password = "Subs2303"
2525
var sshPrivateKey = ``
2626

2727
func getRepoManagerImpl(t *testing.T) *RepositoryManagerImpl {
2828
logger, err := utils.NewSugardLogger()
2929
assert.Nil(t, err)
30-
gitCliImpl := NewCliGitManagerImpl(logger)
31-
gogitImpl := NewGoGitManagerImpl(logger)
32-
repositoryManagerImpl := NewRepositoryManagerImpl(logger, &internal.Configuration{
30+
gitCliImpl := NewGitCliManagerImpl(logger)
31+
gogitImpl := NewGoGitSDKManagerImpl(logger)
32+
conf := &internal.Configuration{
3333
CommitStatsTimeoutInSec: 0,
3434
EnableFileStats: true,
3535
GitHistoryCount: 2,
36-
}, gitCliImpl, gogitImpl)
36+
UseGitCli: true,
37+
}
38+
gitUtil := NewGitManagerImpl(conf, gitCliImpl, gogitImpl)
39+
repositoryManagerImpl := NewRepositoryManagerImpl(logger, conf, gitUtil)
3740
return repositoryManagerImpl
3841
}
3942

@@ -52,7 +55,7 @@ func TestRepositoryManager_Add(t *testing.T) {
5255
gitProviderId int
5356
location string
5457
url string
55-
gitContext *GitContext
58+
gitCtx *GitContext
5659
authMode sql.AuthMode
5760
sshPrivateKeyContent string
5861
}
@@ -66,7 +69,7 @@ func TestRepositoryManager_Add(t *testing.T) {
6669
gitProviderId: 1,
6770
location: privateGitRepoLocation,
6871
url: privateGitRepoUrl,
69-
gitContext: &GitContext{
72+
gitCtx: &GitContext{
7073
Username: username,
7174
Password: password,
7275
},
@@ -79,7 +82,7 @@ func TestRepositoryManager_Add(t *testing.T) {
7982
gitProviderId: 1,
8083
location: privateGitRepoLocation,
8184
url: privateGitRepoUrl,
82-
gitContext: &GitContext{
85+
gitCtx: &GitContext{
8386
Username: "",
8487
Password: "",
8588
},
@@ -92,7 +95,7 @@ func TestRepositoryManager_Add(t *testing.T) {
9295
gitProviderId: 1,
9396
location: location1,
9497
url: gitRepoUrl + "dhs",
95-
gitContext: &GitContext{
98+
gitCtx: &GitContext{
9699
Username: "",
97100
Password: "",
98101
},
@@ -105,7 +108,7 @@ func TestRepositoryManager_Add(t *testing.T) {
105108
gitProviderId: 1,
106109
location: location2,
107110
url: gitRepoUrl,
108-
gitContext: &GitContext{
111+
gitCtx: &GitContext{
109112
Username: "",
110113
Password: "",
111114
},
@@ -117,11 +120,11 @@ func TestRepositoryManager_Add(t *testing.T) {
117120
repositoryManagerImpl := getRepoManagerImpl(t)
118121
for _, tt := range tests {
119122
if tt.payload.authMode == "SSH" {
120-
err := repositoryManagerImpl.CreateSshFileIfNotExistsAndConfigureSshCommand(tt.payload.location, tt.payload.gitProviderId, tt.payload.sshPrivateKeyContent)
123+
err := repositoryManagerImpl.CreateSshFileIfNotExistsAndConfigureSshCommand(
121124
assert.Nil(t, err)
122125
}
123126
t.Run(tt.name, func(t *testing.T) {
124-
err := repositoryManagerImpl.Add(tt.payload.gitProviderId, tt.payload.location, tt.payload.url, tt.payload.gitContext, tt.payload.authMode, tt.payload.sshPrivateKeyContent)
127+
err := repositoryManagerImpl.Add(tt.payload.gitProviderId, tt.payload.location, tt.payload.url, tt.payload.gitCtx, tt.payload.authMode, tt.payload.sshPrivateKeyContent)
125128
if (err != nil) != tt.wantErr {
126129
t.Errorf("Add() error in %s, error = %v, wantErr %v", tt.name, err, tt.wantErr)
127130
return
@@ -135,7 +138,7 @@ func TestRepositoryManager_Fetch(t *testing.T) {
135138
type args struct {
136139
location string
137140
url string
138-
gitContext *GitContext
141+
gitCtx *GitContext
139142
}
140143
tests := []struct {
141144
name string
@@ -146,7 +149,7 @@ func TestRepositoryManager_Fetch(t *testing.T) {
146149
name: "Test1_Fetch_InvokingWithValidGitUrlWithoutCreds", payload: args{
147150
location: location2,
148151
url: gitRepoUrl,
149-
gitContext: &GitContext{
152+
gitCtx: &GitContext{
150153
Username: "",
151154
Password: "",
152155
},
@@ -156,7 +159,7 @@ func TestRepositoryManager_Fetch(t *testing.T) {
156159
name: "Test2_Fetch_InvokingWithInvalidGitUrlWithoutCreds", payload: args{
157160
location: location1,
158161
url: gitRepoUrl + "dhs",
159-
gitContext: &GitContext{
162+
gitCtx: &GitContext{
160163
Username: "",
161164
Password: "",
162165
},
@@ -166,17 +169,17 @@ func TestRepositoryManager_Fetch(t *testing.T) {
166169
name: "Test3_Fetch_InvokingWithCorrectArgumentsWithCreds", payload: args{
167170
location: privateGitRepoLocation,
168171
url: privateGitRepoUrl,
169-
gitContext: &GitContext{
172+
gitCtx: &GitContext{
170173
Username: username,
171174
Password: password,
172175
},
173176
}, wantErr: false,
174177
},
175178
{
176179
name: "Test4_Fetch_InvokingWithWrongLocationOfLocalDir", payload: args{
177-
location: privateGitRepoLocation + "/hjwbwfdj",
180+
location: baseDir + "/git-base/42/github.com/devtron-labs/agetting-started-nodejsgits",
178181
url: privateGitRepoUrl,
179-
gitContext: &GitContext{
182+
gitCtx: &GitContext{
180183
Username: username,
181184
Password: password,
182185
},
@@ -186,7 +189,7 @@ func TestRepositoryManager_Fetch(t *testing.T) {
186189
repositoryManagerImpl := getRepoManagerImpl(t)
187190
for _, tt := range tests {
188191
t.Run(tt.name, func(t *testing.T) {
189-
_, _, err := repositoryManagerImpl.Fetch(tt.payload.gitContext, tt.payload.url, tt.payload.location)
192+
_, _, err := repositoryManagerImpl.Fetch(tt.payload.gitCtx, tt.payload.url, tt.payload.location)
190193

191194
if (err != nil) != tt.wantErr {
192195
t.Errorf("Fetch() error in %s, error = %v, wantErr %v", tt.name, err, tt.wantErr)
@@ -249,6 +252,7 @@ func TestRepositoryManager_GetCommitMetadata(t *testing.T) {
249252
got.Author = ""
250253
got.Message = ""
251254
got.Changes = nil
255+
got.CheckoutPath = ""
252256

253257
if !reflect.DeepEqual(*got, *tt.want) {
254258
t.Errorf("GetCommitMetadata() got = %v, want %v", got, tt.want)
@@ -380,6 +384,7 @@ func TestRepositoryManager_ChangesSince(t *testing.T) {
380384
}
381385

382386
got[index].FileStats = nil
387+
got[index].CheckoutPath = ""
383388
want.FileStats = nil
384389

385390
if !reflect.DeepEqual(*got[index], *want) {
@@ -561,10 +566,10 @@ func TestRepositoryManager_ChangesSinceByRepository(t *testing.T) {
561566
repositoryManagerImpl := getRepoManagerImpl(t)
562567
for _, tt := range tests {
563568
//r, err := git.PlainOpen(tt.payload.checkoutPath)
564-
r, err := repositoryManagerImpl.gitUtil.OpenRepoPlain(tt.payload.checkoutPath)
569+
r, err := repositoryManagerImpl.gitManager.OpenRepoPlain(tt.payload.checkoutPath)
565570
assert.Nil(t, err)
566571
t.Run(tt.name, func(t *testing.T) {
567-
got, err := repositoryManagerImpl.ChangesSinceByRepository(r, tt.payload.branch, tt.payload.from, tt.payload.to, tt.payload.count, "")
572+
got, err := repositoryManagerImpl.ChangesSinceByRepository(r, tt.payload.branch, tt.payload.from, tt.payload.to, tt.payload.count)
568573
if (err != nil) != tt.wantErr {
569574
t.Errorf("ChangesSinceByRepository() error in %s, error = %v, wantErr %v", tt.name, err, tt.wantErr)
570575
return
@@ -580,6 +585,7 @@ func TestRepositoryManager_ChangesSinceByRepository(t *testing.T) {
580585
got[index].Author = ""
581586
got[index].Message = ""
582587
got[index].Changes = nil
588+
got[index].CheckoutPath = ""
583589

584590
if !reflect.DeepEqual(*got[index], *want) {
585591
t.Errorf("ChangesSinceByRepository() got = %v, want %v", got, tt.want)
@@ -643,6 +649,7 @@ func TestRepositoryManager_GetCommitForTag(t *testing.T) {
643649
got.Author = ""
644650
got.Message = ""
645651
got.Changes = nil
652+
got.CheckoutPath = ""
646653

647654
if !reflect.DeepEqual(*got, *tt.want) {
648655
t.Errorf("GetCommitMetadata() got = %v, want %v", got, tt.want)

0 commit comments

Comments
 (0)