From a40ee0b2581228daeb0a97c9fa04a13c3f102d2d Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky Date: Fri, 3 Nov 2023 12:15:37 -0500 Subject: [PATCH] Web UI no longer gives an error when deleting a large number of hosts. (#14896) After 30 seconds, the 'Delete host' modal closes and the delete operation continues in the background. The following text has been added to the modal when deleting 500 or more hosts: "When deleting a large volume of hosts, it may take some time for this change to be reflected in the UI." #14097 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [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] Manual QA for all new/changed functionality - [x] Backend test added --- changes/14097-deleting-large-number-of-hosts | 4 ++ .../hosts/ManageHostsPage/ManageHostsPage.tsx | 1 + .../DeleteHostModal/DeleteHostModal.tsx | 11 ++++- server/service/hosts.go | 46 +++++++++++++++++-- server/service/integration_core_test.go | 38 +++++++++++++++ 5 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 changes/14097-deleting-large-number-of-hosts diff --git a/changes/14097-deleting-large-number-of-hosts b/changes/14097-deleting-large-number-of-hosts new file mode 100644 index 0000000000..d1a74b45a6 --- /dev/null +++ b/changes/14097-deleting-large-number-of-hosts @@ -0,0 +1,4 @@ +Web UI no longer gives an error when deleting a large number of hosts. + +After 30 seconds, the 'Delete host' modal closes and the delete operation continues in the background. +The following text has been added to the modal when deleting 500 or more hosts: "When deleting a large volume of hosts, it may take some time for this change to be reflected in the UI." \ No newline at end of file diff --git a/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx b/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx index 93f0006325..b832c48c64 100644 --- a/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx +++ b/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx @@ -1179,6 +1179,7 @@ const ManageHostsPage = ({ onSubmit={onDeleteHostSubmit} onCancel={toggleDeleteHostModal} isAllMatchingHostsSelected={isAllMatchingHostsSelected} + hostsCount={hostsCount} isUpdating={isUpdatingHosts} /> ); diff --git a/frontend/pages/hosts/components/DeleteHostModal/DeleteHostModal.tsx b/frontend/pages/hosts/components/DeleteHostModal/DeleteHostModal.tsx index 307092ad05..4f00b2c11a 100644 --- a/frontend/pages/hosts/components/DeleteHostModal/DeleteHostModal.tsx +++ b/frontend/pages/hosts/components/DeleteHostModal/DeleteHostModal.tsx @@ -13,6 +13,8 @@ interface IDeleteHostModalProps { isAllMatchingHostsSelected?: boolean; /** Manage host page only */ selectedHostIds?: number[]; + /** Manage host page only */ + hostsCount?: number; /** Host details page only */ hostName?: string; isUpdating: boolean; @@ -23,6 +25,7 @@ const DeleteHostModal = ({ onCancel, isAllMatchingHostsSelected, selectedHostIds, + hostsCount, hostName, isUpdating, }: IDeleteHostModalProps): JSX.Element => { @@ -34,6 +37,12 @@ const DeleteHostModal = ({ } return hostName; }; + const largeVolumeText = (): string => { + if (selectedHostIds && isAllMatchingHostsSelected && hostsCount && hostsCount >= 500) { + return " When deleting a large volume of hosts, it may take some time for this change to be reflected in the UI." + } + return "" + } return (

- This action will delete {hostText()} from your Fleet instance. + This action will delete {hostText()} from your Fleet instance.{largeVolumeText()}

If the hosts come back online, they will automatically re-enroll.

diff --git a/server/service/hosts.go b/server/service/hosts.go index 9be38b2bc9..8b85b772b0 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -161,6 +161,12 @@ func (svc *Service) ListHosts(ctx context.Context, opt fleet.HostListOptions) ([ // Delete Hosts ///////////////////////////////////////////////////////////////////////////////// +// These values are modified during testing. +var ( + deleteHostsTimeout = 30 * time.Second + deleteHostsSkipAuthorization = false +) + type deleteHostsRequest struct { IDs []uint `json:"ids"` Filters struct { @@ -172,11 +178,15 @@ type deleteHostsRequest struct { } type deleteHostsResponse struct { - Err error `json:"error,omitempty"` + Err error `json:"error,omitempty"` + StatusCode int `json:"-"` } func (r deleteHostsResponse) error() error { return r.Err } +// Status implements statuser interface to send out custom HTTP success codes. +func (r deleteHostsResponse) Status() int { return r.StatusCode } + func deleteHostsEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (errorer, error) { req := request.(*deleteHostsRequest) listOpt := fleet.HostListOptions{ @@ -186,11 +196,33 @@ func deleteHostsEndpoint(ctx context.Context, request interface{}, svc fleet.Ser StatusFilter: req.Filters.Status, TeamFilter: req.Filters.TeamID, } - err := svc.DeleteHosts(ctx, req.IDs, listOpt, req.Filters.LabelID) - if err != nil { - return deleteHostsResponse{Err: err}, nil + + // Since bulk deletes can take a long time, after DeleteHostsTimeout, we will return a 202 (Accepted) status code + // and allow the delete operation to proceed. + var err error + deleteDone := make(chan bool, 1) + ctx = context.WithoutCancel(ctx) // to make sure DB operations don't get killed after we return a 202 + go func() { + err = svc.DeleteHosts(ctx, req.IDs, listOpt, req.Filters.LabelID) + if err != nil { + // logging the error for future debug in case we already sent http.StatusAccepted + logging.WithErr(ctx, err) + } + deleteDone <- true + }() + select { + case <-deleteDone: + if err != nil { + return deleteHostsResponse{Err: err}, nil + } + return deleteHostsResponse{StatusCode: http.StatusOK}, nil + case <-time.After(deleteHostsTimeout): + if deleteHostsSkipAuthorization { + // Only called during testing. + svc.(validationMiddleware).Service.(*Service).authz.SkipAuthorization(ctx) + } + return deleteHostsResponse{StatusCode: http.StatusAccepted}, nil } - return deleteHostsResponse{}, nil } func (svc *Service) DeleteHosts(ctx context.Context, ids []uint, opts fleet.HostListOptions, lid *uint) error { @@ -198,6 +230,10 @@ func (svc *Service) DeleteHosts(ctx context.Context, ids []uint, opts fleet.Host return err } + if len(ids) == 0 && lid == nil && opts.Empty() { + return &fleet.BadRequestError{Message: "list of ids or filters must be specified"} + } + if len(ids) > 0 && (lid != nil || !opts.Empty()) { return &fleet.BadRequestError{Message: "Cannot specify a list of ids and filters at the same time"} } diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 9b9bf8c2ed..7d3d9ee343 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -963,6 +963,44 @@ func (s *integrationTestSuite) TestBulkDeleteHostByIDs() { require.NoError(t, err) } +func (s *integrationTestSuite) TestBulkDeleteHostByIDsWithTimeout() { + t := s.T() + + hosts := s.createHosts(t, "debian") + + req := deleteHostsRequest{ + IDs: []uint{hosts[0].ID}, + } + resp := deleteHostsResponse{} + originalTimeout := deleteHostsTimeout + deleteHostsTimeout = 0 + deleteHostsSkipAuthorization = true + defer func() { + deleteHostsTimeout = originalTimeout + deleteHostsSkipAuthorization = false + }() + s.DoJSON("POST", "/api/latest/fleet/hosts/delete", req, http.StatusAccepted, &resp) + + // Make sure the host was actually deleted. + deleteDone := make(chan bool) + go func() { + for { + _, err := s.ds.Host(context.Background(), hosts[0].ID) + if err != nil { + deleteDone <- true + break + } + } + }() + select { + case <-deleteDone: + return + case <-time.After(2 * time.Second): + t.Log("http.StatusAccepted (202) means that delete should continue in the background, but we did not see the host deleted after 2 seconds.") + t.Error("Timeout: delete did not occur.") + } +} + func (s *integrationTestSuite) createHosts(t *testing.T, platforms ...string) []*fleet.Host { var hosts []*fleet.Host if len(platforms) == 0 {