Skip to content

Commit

Permalink
fix(kuberpult-cli): add timeout to the args (#2060)
Browse files Browse the repository at this point in the history
Ref: SRX-880CZF
  • Loading branch information
AminSlk authored Oct 24, 2024
1 parent 41b04df commit 7874cca
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 11 deletions.
12 changes: 6 additions & 6 deletions cli/pkg/cli_utils/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ const (
HttpDefaultTimeout = 180
)

func doRequest(request *http.Request) (*http.Response, []byte, error) {
func doRequest(request *http.Request, timeoutSeconds int) (*http.Response, []byte, error) {
//exhaustruct:ignore
client := &http.Client{
Timeout: HttpDefaultTimeout * time.Second,
Timeout: time.Duration(timeoutSeconds) * time.Second,
// We don't want to follow redirects. If we get a redirect, we want to return the original status code.
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
Expand All @@ -52,10 +52,10 @@ func doRequest(request *http.Request) (*http.Response, []byte, error) {
return resp, body, nil
}

func IssueHttpRequest(req http.Request, retries uint64) error {
func IssueHttpRequest(req http.Request, retries uint64, timeoutSeconds int) error {
var i uint64
for i = 0; i < retries+1; i++ {
response, body, err := doRequest(&req)
response, body, err := doRequest(&req, timeoutSeconds)
if err != nil {
log.Printf("error issuing http request: %v\n", err)
} else if response.StatusCode != http.StatusCreated && response.StatusCode != http.StatusOK {
Expand All @@ -73,8 +73,8 @@ func IssueHttpRequest(req http.Request, retries uint64) error {
return fmt.Errorf("could not perform a successful call to kuberpult")
}

func IssueHttpRequestWithBodyReturn(req http.Request) ([]byte, error) {
response, body, err := doRequest(&req)
func IssueHttpRequestWithBodyReturn(req http.Request, timeoutSeconds int) ([]byte, error) {
response, body, err := doRequest(&req, timeoutSeconds)
if err != nil {
return nil, err
} else if response.StatusCode != http.StatusCreated && response.StatusCode != http.StatusOK {
Expand Down
1 change: 1 addition & 0 deletions cli/pkg/cmd/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type kuberpultClientParameters struct {
authorEmail *string
iapToken *string
dexToken *string
timeout uint64
}

func RunCLI() ReturnCode {
Expand Down
2 changes: 1 addition & 1 deletion cli/pkg/cmd/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func handleReleaseTrain(kpClientParams kuberpultClientParameters, args []string)
requestParameters := kutil.RequestParameters{
Url: &kpClientParams.url,
Retries: kpClientParams.retries,
HttpTimeout: cli_utils.HttpDefaultTimeout,
HttpTimeout: int(kpClientParams.timeout),
}

if err = releasetrain.HandleReleaseTrain(requestParameters, authParams, *parsedArgs); err != nil {
Expand Down
13 changes: 13 additions & 0 deletions cli/pkg/cmd/parsing.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type commandLineArguments struct {
authorEmail cli_utils.RepeatedString
authorName cli_utils.RepeatedString
retries cli_utils.RepeatedInt
timeout cli_utils.RepeatedInt
}

func readArgs(args []string) (*commandLineArguments, []string, error) {
Expand All @@ -40,6 +41,7 @@ func readArgs(args []string) (*commandLineArguments, []string, error) {
fs.Var(&cmdArgs.authorName, "author_name", "the name of the git author who eventually will write to the manifest repo (must be set at most once)")
fs.Var(&cmdArgs.authorEmail, "author_email", "the email of the git author who eventially will write to the manifest repo (must be set at most once)")
fs.Var(&cmdArgs.retries, "retries", "number of times the cli will retry http requests to kuberpult upon failure (must be set at most once) - default=3")
fs.Var(&cmdArgs.timeout, "timeout", "requests timeout seconds (must be set at most once) - default=180")

if err := fs.Parse(args); err != nil {
return nil, nil, fmt.Errorf("error while reading command line arguments, error: %w", err)
Expand All @@ -63,6 +65,9 @@ func argsValid(cmdArgs *commandLineArguments) (bool, string) {
if len(cmdArgs.retries.Values) > 1 {
return false, "the --retries arg must be set at most once"
}
if len(cmdArgs.timeout.Values) > 1 {
return false, "the --timeout arg must be set at most once"
}

return true, ""
}
Expand Down Expand Up @@ -92,6 +97,14 @@ func convertToParams(cmdArgs *commandLineArguments) (*kuberpultClientParameters,
} else {
params.retries = DefaultRetries
}
if len(cmdArgs.timeout.Values) == 1 {
if cmdArgs.timeout.Values[0] <= 0 {
return nil, fmt.Errorf("--timeout arg value must be positive")
}
params.timeout = uint64(cmdArgs.timeout.Values[0])
} else {
params.timeout = cli_utils.HttpDefaultTimeout
}

return &params, nil
}
Expand Down
23 changes: 23 additions & 0 deletions cli/pkg/cmd/parsing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func TestParseArgs(t *testing.T) {
expectedParams: &kuberpultClientParameters{
url: "something.somewhere",
retries: DefaultRetries,
timeout: 180,
},
},
{
Expand All @@ -93,6 +94,7 @@ func TestParseArgs(t *testing.T) {
expectedParams: &kuberpultClientParameters{
url: "something.somewhere",
retries: DefaultRetries,
timeout: 180,
},
expectedOther: []string{"potato", "--tomato"},
},
Expand All @@ -103,6 +105,7 @@ func TestParseArgs(t *testing.T) {
url: "something.somewhere",
authorName: ptrStr("john"),
retries: DefaultRetries,
timeout: 180,
},
expectedOther: []string{"subcommand", "--arg1", "val1", "etc", "etc"},
},
Expand All @@ -120,6 +123,7 @@ func TestParseArgs(t *testing.T) {
url: "something.somewhere",
authorEmail: ptrStr("john"),
retries: DefaultRetries,
timeout: 180,
},
expectedOther: []string{"subcommand", "--arg1", "val1", "etc", "etc"},
},
Expand All @@ -136,6 +140,7 @@ func TestParseArgs(t *testing.T) {
expectedParams: &kuberpultClientParameters{
url: "something.somewhere",
retries: DefaultRetries,
timeout: 180,
},
expectedOther: []string{"potato", "--tomato"},
},
Expand All @@ -145,6 +150,7 @@ func TestParseArgs(t *testing.T) {
expectedParams: &kuberpultClientParameters{
url: "something.somewhere",
retries: 10,
timeout: 180,
},
expectedOther: []string{"potato", "--tomato"},
},
Expand All @@ -155,6 +161,23 @@ func TestParseArgs(t *testing.T) {
msg: "error while creating kuberpult client parameters, error: --retries arg value must be positive",
},
},
{
name: "overriding default timeout",
cmdArgs: "--url something.somewhere --retries 10 --timeout 7200 potato --tomato",
expectedParams: &kuberpultClientParameters{
url: "something.somewhere",
retries: 10,
timeout: 7200,
},
expectedOther: []string{"potato", "--tomato"},
},
{
name: "--timeout is negative",
cmdArgs: "--url something.somewhere --timeout -1 --author_email john subcommand --arg1 val1 etc etc",
expectedError: errMatcher{
msg: "error while creating kuberpult client parameters, error: --timeout arg value must be positive",
},
},
}

for _, tc := range tcs {
Expand Down
2 changes: 1 addition & 1 deletion cli/pkg/deployments/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func HandleGetCommitDeployments(requestParams kutil.RequestParameters, authParam
if err != nil {
return fmt.Errorf("error while preparing HTTP request, error: %w", err)
}
body, err := cli_utils.IssueHttpRequestWithBodyReturn(*req)
body, err := cli_utils.IssueHttpRequestWithBodyReturn(*req, requestParams.HttpTimeout)
if err != nil {
return fmt.Errorf("error while issuing HTTP request, error: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cli/pkg/locks/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func HandleLockRequest(requestParams kutil.RequestParameters, authParams kutil.A
if err != nil {
return fmt.Errorf("error while preparing HTTP request, error: %w", err)
}
if err := cli_utils.IssueHttpRequest(*req, requestParams.Retries); err != nil {
if err := cli_utils.IssueHttpRequest(*req, requestParams.Retries, requestParams.HttpTimeout); err != nil {
return fmt.Errorf("error while issuing HTTP request, error: %v", err)
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion cli/pkg/release/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func Release(requestParams kutil.RequestParameters, authParams kutil.Authenticat
if err != nil {
return fmt.Errorf("error while preparing HTTP request, error: %w", err)
}
if err := cli_utils.IssueHttpRequest(*req, requestParams.Retries); err != nil {
if err := cli_utils.IssueHttpRequest(*req, requestParams.Retries, requestParams.HttpTimeout); err != nil {
return fmt.Errorf("error while issuing HTTP request, error: %v", err)
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion cli/pkg/releasetrain/releasetrain.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func HandleReleaseTrain(requestParams kutil.RequestParameters, authParams kutil.
if err != nil {
return fmt.Errorf("error while preparing HTTP request, error: %w", err)
}
if err := cli_utils.IssueHttpRequest(*req, requestParams.Retries); err != nil {
if err := cli_utils.IssueHttpRequest(*req, requestParams.Retries, requestParams.HttpTimeout); err != nil {
return fmt.Errorf("error while issuing HTTP request, error: %v", err)
}
return nil
Expand Down

0 comments on commit 7874cca

Please sign in to comment.