Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kuberpult-cli): add timeout to the args #2060

Merged
merged 2 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading