Extend script execution timeout (#15779)
#15196 This is the work of @ghernandez345 except for adding the `ResponseController` thing in Go to override the server timeout for that specific sync endpoint so that the calls don't timeout waiting for a script response (the default HTTP server timeout was 90s for our server). # Checklist for submitter If some of the following don't apply, delete the relevant line. <!-- Note that API documentation changes are now addressed by the product design team. --> - [x] Changes file added for user-visible changes in `changes/` or `orbit/changes/`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated tests - [x] Manual QA for all new/changed functionality - For Orbit and Fleet Desktop changes: - [x] Manual QA must be performed in the three main OSs, macOS, Windows and Linux. - [x] Auto-update manual QA, from released version of component to new version (see [tools/tuf/test](../tools/tuf/test/README.md)). --------- Co-authored-by: Gabriel Hernandez <ghernandez345@gmail.com> Co-authored-by: Roberto Dip <me@roperzh.com> Co-authored-by: Roberto Dip <dip.jesusr@gmail.com>
This commit is contained in:
co-authored by
Gabriel Hernandez
Roberto Dip
Roberto Dip
parent
a8a8080b3b
commit
d943fbbf8e
@@ -0,0 +1 @@
|
||||
* Extended the script execution timeout to 5 minutes
|
||||
+18
-1
@@ -871,7 +871,24 @@ the way that the Fleet server works.
|
||||
}
|
||||
}
|
||||
|
||||
rootMux.Handle("/api/", apiHandler)
|
||||
// We must wrap the Handler here to set special per-endpoint Write
|
||||
// timeouts, so that we have access to the raw http.ResponseWriter.
|
||||
// Otherwise, the handler is wrapped by the promhttp response delegator,
|
||||
// which does not support the Unwrap call needed to work with
|
||||
// ResponseController.
|
||||
//
|
||||
// See https://pkg.go.dev/net/http#NewResponseController which explains
|
||||
// the Unwrap method that the prometheus wrapper of http.ResponseWriter
|
||||
// does not implement.
|
||||
rootMux.HandleFunc("/api/", func(rw http.ResponseWriter, req *http.Request) {
|
||||
if req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/fleet/scripts/run/sync") {
|
||||
rc := http.NewResponseController(rw)
|
||||
if err := rc.SetWriteDeadline(time.Now().Add((5 * time.Minute) + (1 * time.Second))); err != nil {
|
||||
level.Error(logger).Log("msg", "http middleware failed to override endpoint write timeout", "err", err)
|
||||
}
|
||||
}
|
||||
apiHandler.ServeHTTP(rw, req)
|
||||
})
|
||||
rootMux.Handle("/", frontendHandler)
|
||||
|
||||
debugHandler := &debugMux{
|
||||
|
||||
@@ -2,6 +2,7 @@ package main
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"html/template"
|
||||
"net/http"
|
||||
"os"
|
||||
@@ -74,6 +75,8 @@ func runScriptCommand() *cli.Command {
|
||||
return err
|
||||
}
|
||||
|
||||
fmt.Println("\nScript is running. Please wait for it to finish...")
|
||||
|
||||
res, err := client.RunHostScriptSync(h.ID, b)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -150,10 +150,10 @@ Output:
|
||||
scriptResult: &fleet.HostScriptResult{
|
||||
ExitCode: ptr.Int64(-1),
|
||||
Output: "Oh no!",
|
||||
Message: "Timeout. Fleet stopped the script after 30 seconds to protect host performance.",
|
||||
Message: fleet.RunScriptScriptTimeoutErrMsg,
|
||||
},
|
||||
expectOutput: `
|
||||
Error: Timeout. Fleet stopped the script after 30 seconds to protect host performance.
|
||||
Error: Timeout. Fleet stopped the script after 5 minutes to protect host performance.
|
||||
|
||||
Output before timeout:
|
||||
|
||||
@@ -198,11 +198,13 @@ Fleet records the last 10,000 characters to prevent downtime.
|
||||
-------------------------------------------------------------------------------------
|
||||
`, maxChars),
|
||||
},
|
||||
{
|
||||
name: "host timeout",
|
||||
scriptPath: generateValidPath,
|
||||
expectErrMsg: fleet.RunScriptHostTimeoutErrMsg,
|
||||
},
|
||||
// TODO: this would take 5 minutes to run, we don't want that kind of slowdown in our test suite
|
||||
// but can be useful to have around for manual testing.
|
||||
//{
|
||||
// name: "host timeout",
|
||||
// scriptPath: generateValidPath,
|
||||
// expectErrMsg: fleet.RunScriptHostTimeoutErrMsg,
|
||||
//},
|
||||
}
|
||||
|
||||
setupDS := func(t *testing.T, c testCase) {
|
||||
|
||||
@@ -15,7 +15,7 @@ import (
|
||||
)
|
||||
|
||||
func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScriptRequestPayload, waitForResult time.Duration) (*fleet.HostScriptResult, error) {
|
||||
const maxPendingScriptAge = time.Minute // any script older than this is not considered pending anymore on that host
|
||||
const maxPendingScriptAge = 5 * time.Minute // any script older than this is not considered pending anymore on that host
|
||||
|
||||
// must load the host to get the team (cannot use lite, the last seen time is
|
||||
// required to check if it is online) to authorize with the proper team id.
|
||||
|
||||
+1
-1
@@ -89,7 +89,7 @@ const StatusMessage = ({
|
||||
// Expected API message: "Scripts are disabled for this host. To run scripts, deploy a Fleet installer with scripts enabled."
|
||||
return <StatusMessageError message={message} />;
|
||||
case -1:
|
||||
// Expected API message: "Timeout. Fleet stopped the script after 30 seconds to protect host performance."
|
||||
// Expected API message: "Timeout. Fleet stopped the script after 5 minutes to protect host performance."
|
||||
return <StatusMessageError message={message} />;
|
||||
case 0:
|
||||
// Expected API message: ""
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
* Extended the script execution timeout to 5 minutes
|
||||
@@ -17,7 +17,7 @@ import (
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
)
|
||||
|
||||
const scriptExecTimeout = 30 * time.Second
|
||||
const scriptExecTimeout = 5 * time.Minute
|
||||
|
||||
// Client defines the methods required for the API requests to the server. The
|
||||
// fleet.OrbitClient type satisfies this interface.
|
||||
|
||||
@@ -541,7 +541,8 @@ const (
|
||||
RunScriptHostNotFoundErrMsg = "Host doesn’t exist. Make sure you provide a valid hostname, UUID, osquery host ID, or node key."
|
||||
RunScriptForbiddenErrMsg = "You don’t have the right permissions in Fleet to run the script."
|
||||
RunScriptAlreadyRunningErrMsg = "A script is already running on this host. Please wait about 1 minute to let it finish."
|
||||
RunScriptHostTimeoutErrMsg = "Fleet hasn’t heard from the host in over 1 minute. Fleet doesn’t know if the script ran because the host went offline."
|
||||
RunScriptHostTimeoutErrMsg = "Fleet hasn’t heard from the host in over 5 minutes. Fleet doesn’t know if the script ran because the host went offline."
|
||||
RunScriptScriptTimeoutErrMsg = "Timeout. Fleet stopped the script after 5 minutes to protect host performance."
|
||||
)
|
||||
|
||||
// ConflictError is used to indicate a conflict, such as a UUID conflict in the DB.
|
||||
|
||||
@@ -203,7 +203,7 @@ func (hsr HostScriptResult) UserMessage(hostTimeout bool) string {
|
||||
}
|
||||
|
||||
if hsr.ExitCode == nil {
|
||||
if hsr.HostTimeout(1 * time.Minute) {
|
||||
if hsr.HostTimeout(5 * time.Minute) {
|
||||
return RunScriptHostTimeoutErrMsg
|
||||
}
|
||||
return RunScriptAlreadyRunningErrMsg
|
||||
@@ -211,7 +211,7 @@ func (hsr HostScriptResult) UserMessage(hostTimeout bool) string {
|
||||
|
||||
switch *hsr.ExitCode {
|
||||
case -1:
|
||||
return "Timeout. Fleet stopped the script after 30 seconds to protect host performance."
|
||||
return RunScriptScriptTimeoutErrMsg
|
||||
case -2:
|
||||
return "Scripts are disabled for this host. To run scripts, deploy a Fleet installer with scripts enabled."
|
||||
default:
|
||||
|
||||
@@ -594,6 +594,7 @@ func (e *authEndpointer) makeEndpoint(f handlerFunc, v interface{}) http.Handler
|
||||
mw := e.customMiddleware[i]
|
||||
endp = mw(endp)
|
||||
}
|
||||
|
||||
return newServer(endp, makeDecoder(v), e.opts)
|
||||
}
|
||||
|
||||
|
||||
@@ -77,7 +77,9 @@ func (r runScriptSyncResponse) Status() int {
|
||||
var testRunScriptWaitForResult time.Duration
|
||||
|
||||
// waitForResultTime is the default timeout for the synchronous script execution.
|
||||
const waitForResultTime = time.Minute
|
||||
// The extra seconds is to give a little extra time on top of the script
|
||||
// execution time.
|
||||
const waitForResultTime = (5 * time.Minute) + (30 * time.Second)
|
||||
|
||||
func runScriptSyncEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (errorer, error) {
|
||||
waitForResult := waitForResultTime
|
||||
|
||||
Reference in New Issue
Block a user