From 5f04fad003faef35017b18a4a3d7d67585ba06fd Mon Sep 17 00:00:00 2001 From: Jarek Kowalski Date: Sat, 4 Dec 2021 22:13:10 -0800 Subject: [PATCH] ui: major improvements to new snapshot flow (#1565) * ui: changed how PolicyEditor is instantiated via a route * server: added paths/resolve API * server: refresh affected source manager after policy change Also switched 15-second refresh cycle which is way too aggressive to 30-minute cycle (manual refresh button can be used if needed). * policy: allow overriding top-level policy for estimation * server: changed source create API to always require policy * ui: streamlined new snapshot and estimate flow * linter fix --- htmlui/src/App.css | 12 +- htmlui/src/App.js | 8 +- htmlui/src/EstimateResults.js | 116 ++++++++++++++++++++ htmlui/src/NewSnapshot.js | 122 +++++++++------------ htmlui/src/PoliciesTable.js | 35 ++---- htmlui/src/PolicyEditor.js | 36 ++---- htmlui/src/PolicyEditor.test.js | 5 +- htmlui/src/PolicyEditorPage.js | 23 ++++ htmlui/src/SourcesTable.js | 5 +- htmlui/src/uiutil.js | 25 +++++ internal/server/api_estimate.go | 5 +- internal/server/api_paths.go | 28 +++++ internal/server/api_paths_test.go | 36 ++++++ internal/server/api_policies.go | 32 +++--- internal/server/api_sources.go | 40 +++---- internal/server/api_sources_test.go | 63 ++++++++++- internal/server/server.go | 15 +++ internal/server/source_manager.go | 13 ++- internal/server/util_test.go | 5 +- internal/serverapi/serverapi.go | 22 +++- snapshot/policy/policy_manager.go | 31 +++++- snapshot/policy/policy_manager_test.go | 2 +- tests/end_to_end_test/server_start_test.go | 10 +- 23 files changed, 487 insertions(+), 202 deletions(-) create mode 100644 htmlui/src/EstimateResults.js create mode 100644 htmlui/src/PolicyEditorPage.js create mode 100644 internal/server/api_paths.go create mode 100644 internal/server/api_paths_test.go diff --git a/htmlui/src/App.css b/htmlui/src/App.css index 27497a70f..8ab4ea15a 100644 --- a/htmlui/src/App.css +++ b/htmlui/src/App.css @@ -143,8 +143,7 @@ div.tab-body { } .logs-table { - border: 1px solid #ccd; - background-color: #f0f0ff; + background-color: #f8f8ff; font-family: monospace; font-size: 12px; font-weight: bold; @@ -175,12 +174,9 @@ div.tab-body { padding-left: 40px; } -.estimate-results { - margin: 10px; - margin-left: 15px; - padding: 15px; - border: 1px solid #ccc; - background-color: #f8f8ff; +.estimateResults { + color: #008; + font-size: 95%; } .list-actions { diff --git a/htmlui/src/App.js b/htmlui/src/App.js index 5dda500ab..ca997748f 100644 --- a/htmlui/src/App.js +++ b/htmlui/src/App.js @@ -14,9 +14,9 @@ import { RepoStatus } from "./RepoStatus"; import { SnapshotsTable } from "./SnapshotsTable"; import { SourcesTable } from "./SourcesTable"; import { TaskDetails } from './TaskDetails'; -import { TasksTable } from './TasksTable'; +import { TasksTable } from './TasksTable'; import { NewSnapshot } from './NewSnapshot'; -import { PolicyEditor } from './PolicyEditor'; +import { PolicyEditorPage } from './PolicyEditorPage'; function useInterval(callback, delay) { const savedCallback = useRef(); @@ -72,9 +72,9 @@ function App() { - + - + diff --git a/htmlui/src/EstimateResults.js b/htmlui/src/EstimateResults.js new file mode 100644 index 000000000..da1841c8b --- /dev/null +++ b/htmlui/src/EstimateResults.js @@ -0,0 +1,116 @@ + +import { faChevronCircleDown, faChevronCircleUp, faStopCircle } from '@fortawesome/free-solid-svg-icons'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import axios from 'axios'; +import React, { Component } from 'react'; +import Button from 'react-bootstrap/Button'; +import Spinner from 'react-bootstrap/esm/Spinner'; +import Form from 'react-bootstrap/Form'; +import { TaskLogs } from './TaskLogs'; +import { cancelTask, redirectIfNotConnected, sizeDisplayName } from './uiutil'; + +export class EstimateResults extends Component { + constructor() { + super(); + this.state = { + isLoading: true, + error: null, + showLog: false, + }; + + this.taskID = this.taskID.bind(this); + this.fetchTask = this.fetchTask.bind(this); + + // poll frequently, we will stop as soon as the task ends. + this.interval = window.setInterval(() => this.fetchTask(this.props), 500); + } + + componentDidMount() { + this.setState({ + isLoading: true, + }); + + this.fetchTask(this.props); + } + + componentWillUnmount() { + if (this.interval) { + window.clearInterval(this.interval); + } + } + + taskID(props) { + return props.taskID || props.match.params.tid; + } + + fetchTask(props) { + axios.get('/api/v1/tasks/' + this.taskID(props)).then(result => { + this.setState({ + task: result.data, + isLoading: false, + }); + + if (result.data.endTime) { + window.clearInterval(this.interval); + this.interval = null; + } + }).catch(error => { + redirectIfNotConnected(error); + this.setState({ + error, + isLoading: false + }); + }); + } + + componentDidUpdate(prevProps) { + if (prevProps !== this.props) { + this.fetchTask(this.props); + } + } + + taskStatusDescription(task) { + if (task.status === "RUNNING") { + return <> + } + + if (task.status === "SUCCESS") { + return "Total" + } + + if (task.status === "CANCELED") { + return "(Canceled)" + } + + return task.status; + } + + render() { + const { task, isLoading, error } = this.state; + if (error) { + return

{error.message}

; + } + + if (isLoading) { + return

Loading ...

; + } + + return <> + {task.counters && + {this.taskStatusDescription(task)} Bytes: {sizeDisplayName(task.counters["Bytes"]?.value)} ({sizeDisplayName(task.counters["Excluded Bytes"]?.value)} excluded) + Files: {task.counters["Files"]?.value} ({task.counters["Excluded Files"]?.value} excluded) + Directories: {task.counters["Directories"]?.value} ({task.counters["Excluded Directories"]?.value} excluded) + Errors: {task.counters["Errors"]?.value} ({task.counters["Ignored Errors"]?.value} ignored) + + } + {task.status === "RUNNING" && <> +   + } + {this.state.showLog ? <> + + + : } + + ; + } +} diff --git a/htmlui/src/NewSnapshot.js b/htmlui/src/NewSnapshot.js index d4b79cee9..10539c5a7 100644 --- a/htmlui/src/NewSnapshot.js +++ b/htmlui/src/NewSnapshot.js @@ -1,15 +1,13 @@ -import { faWindowClose } from '@fortawesome/free-solid-svg-icons'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import axios from 'axios'; import React, { Component } from 'react'; import Button from 'react-bootstrap/Button'; import Col from 'react-bootstrap/Col'; import Form from 'react-bootstrap/Form'; import Row from 'react-bootstrap/Row'; -import { handleChange, validateRequiredFields } from './forms'; +import { handleChange } from './forms'; import { PolicyEditor } from './PolicyEditor'; -import { TaskDetails } from './TaskDetails'; -import { cancelTask, DirectorySelector, errorAlert, GoBackButton, redirectIfNotConnected } from './uiutil'; +import { EstimateResults } from './EstimateResults'; +import { DirectorySelector, errorAlert, GoBackButton, redirectIfNotConnected } from './uiutil'; export class NewSnapshot extends Component { constructor() { @@ -18,6 +16,7 @@ export class NewSnapshot extends Component { path: "", estimateTaskID: null, estimateTaskVisible: false, + lastEstimatedPath: "", policyEditorVisibleFor: "n/a", localUsername: null, }; @@ -26,8 +25,6 @@ export class NewSnapshot extends Component { this.handleChange = handleChange.bind(this); this.estimate = this.estimate.bind(this); this.snapshotNow = this.snapshotNow.bind(this); - this.cancelEstimate = this.cancelEstimate.bind(this); - this.togglePolicyEditor = this.togglePolicyEditor.bind(this); } componentDidMount() { @@ -41,36 +38,46 @@ export class NewSnapshot extends Component { }); } - togglePolicyEditor() { - if (!this.state.path) { - return; + componentDidUpdate() { + if (this.state.lastResolvedPath !== this.state.path) { + axios.post('/api/v1/paths/resolve', { path: this.state.path }).then(result => { + this.setState({ + lastResolvedPath: this.state.path, + resolvedSource: result.data.source, + }); + }).catch(error => { + redirectIfNotConnected(error); + }); } - if (this.state.policyEditorVisibleFor === this.state.path) { + if (this.state.estimateTaskVisible && this.state.lastEstimatedPath !== this.state.resolvedSource.path) { this.setState({ - policyEditorVisibleFor: "n/a", - }); - } else { - this.setState({ - policyEditorVisibleFor: this.state.path, - }); + estimateTaskVisible: false, + }) } } estimate(e) { e.preventDefault(); - if (!validateRequiredFields(this, ["path"])) { + if (!this.state.resolvedSource.path) { + return; + } + + const pe = this.policyEditorRef.current; + if (!pe) { return; } let req = { - root: this.state.path, + root: this.state.resolvedSource.path, maxExamplesPerBucket: 10, + policyOverride: pe.state.policy, } axios.post('/api/v1/estimate', req).then(result => { this.setState({ + lastEstimatedPath: this.state.resolvedSource.path, estimateTaskID: result.data.id, estimatingPath: result.data.description, estimateTaskVisible: true, @@ -81,22 +88,23 @@ export class NewSnapshot extends Component { }); } - cancelEstimate() { - this.setState({ estimateTaskVisible: false }); - cancelTask(this.state.estimateTaskID); - } - snapshotNow(e) { e.preventDefault(); - if (!this.state.path) { + if (!this.state.resolvedSource.path) { alert('Must specify directory to snapshot.'); return } + const pe = this.policyEditorRef.current; + if (!pe) { + return; + } + axios.post('/api/v1/sources', { - path: this.state.path, + path: this.state.resolvedSource.path, createSnapshot: true, + policy: pe.state.policy, }).then(result => { this.props.history.goBack(); }).catch(error => { @@ -121,62 +129,40 @@ export class NewSnapshot extends Component { - this.setState({ path: p })} autoFocus placeholder="enter path to snapshot" name="path" value={this.state.path} onChange={this.handleChange} - readOnly={this.state.policyEditorVisible || this.state.estimateTaskVisible} /> - - Click the Estimate button to estimate the size of the snapshot. - To specify frequency of snapshots or exclude some files, click Policy. - + this.setState({ path: p })} autoFocus placeholder="enter path to snapshot" name="path" value={this.state.path} onChange={this.handleChange} /> - - - + onClick={this.estimate}>Estimate +   + {this.state.estimateTaskID && this.state.estimateTaskVisible && -
-
-   - Estimate Snapshot Size for {this.state.estimatingPath}... -
- -
+ }
- {this.state.path && this.state.policyEditorVisibleFor === this.state.path && - - - - } + {this.state.resolvedSource && + + {this.state.resolvedSource ? this.state.resolvedSource.path : this.state.path} + - - - - - + + } ; } } diff --git a/htmlui/src/PoliciesTable.js b/htmlui/src/PoliciesTable.js index ababcfa2d..4ed35bd06 100644 --- a/htmlui/src/PoliciesTable.js +++ b/htmlui/src/PoliciesTable.js @@ -8,10 +8,10 @@ import Col from 'react-bootstrap/Col'; import Dropdown from 'react-bootstrap/Dropdown'; import Form from 'react-bootstrap/Form'; import Row from 'react-bootstrap/Row'; +import { Link } from 'react-router-dom'; import { handleChange } from './forms'; -import { PolicyEditor } from './PolicyEditor'; import MyTable from './Table'; -import { compare, DirectorySelector, isAbsolutePath, ownerName, redirectIfNotConnected } from './uiutil'; +import { compare, DirectorySelector, isAbsolutePath, ownerName, policyEditorURL, redirectIfNotConnected } from './uiutil'; const localPolicies = "Local Policies" const allPolicies = "All Policies" @@ -31,7 +31,6 @@ export class PoliciesTable extends Component { sources: [], }; - this.editorClosed = this.editorClosed.bind(this); this.editPolicyForPath = this.editPolicyForPath.bind(this); this.handleChange = handleChange.bind(this); this.fetchPolicies = this.fetchPolicies.bind(this); @@ -94,13 +93,6 @@ export class PoliciesTable extends Component { }); } - editorClosed() { - this.setState({ - editorTarget: null, - }); - this.fetchPolicies(); - } - editPolicyForPath(e) { e.preventDefault(); @@ -113,13 +105,11 @@ export class PoliciesTable extends Component { return; } - this.setState({ - editorTarget: { - userName: this.state.localUsername, - host: this.state.localHost, - path: this.state.policyPath, - }, - }) + this.props.history.push(policyEditorURL({ + userName: this.state.localUsername, + host: this.state.localHost, + path: this.state.policyPath, + })); } selectOwner(h) { @@ -233,12 +223,7 @@ export class PoliciesTable extends Component { id: 'edit', Header: '', width: 50, - Cell: x => , + Cell: x => , }] return
@@ -277,14 +262,12 @@ export class PoliciesTable extends Component {
} - {items.length > 0 ?
+ {items.length > 0 ?

Found {items.length} policies matching criteria.

: ((this.state.selectedOwner === localPolicies && this.state.policyPath) ?

No policy found for directory {this.state.policyPath}. Click Set Policy to define it.

:

No policies found.

)} - - {this.state.editorTarget && }
; } } diff --git a/htmlui/src/PolicyEditor.js b/htmlui/src/PolicyEditor.js index d0a3c5e96..95a9d2905 100644 --- a/htmlui/src/PolicyEditor.js +++ b/htmlui/src/PolicyEditor.js @@ -1,4 +1,4 @@ -import { faCalendarTimes, faChevronLeft, faClock, faExclamationTriangle, faFileAlt, faFileArchive, faFolderOpen, faMagic } from '@fortawesome/free-solid-svg-icons'; +import { faCalendarTimes, faClock, faExclamationTriangle, faFileAlt, faFileArchive, faFolderOpen, faMagic } from '@fortawesome/free-solid-svg-icons'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import axios from 'axios'; import moment from 'moment'; @@ -10,24 +10,9 @@ import Row from 'react-bootstrap/Row'; import Spinner from 'react-bootstrap/Spinner'; import Accordion from 'react-bootstrap/Accordion'; import { handleChange, LogDetailSelector, OptionalBoolean, OptionalNumberField, RequiredBoolean, stateProperty, StringList, valueToNumber } from './forms'; -import { errorAlert, sourceQueryStringParams } from './uiutil'; +import { errorAlert, PolicyEditorLink, sourceQueryStringParams } from './uiutil'; import { getDeepStateProperty } from './deepstate'; -function PolicyTypeName(s) { - if (!s.host && !s.userName) { - return "Global Policy" - } - - if (!s.userName) { - return "Host: " + s.host; - } - - if (!s.path) { - return "User: " + s.userName + "@" + s.host; - } - - return "Directory: " + s.userName + "@" + s.host + ":" + s.path; -} function LabelColumn(props) { return @@ -118,7 +103,7 @@ function UpcomingSnapshotTimes(times) { function SectionHeaderRow() { return -
Values
 +
Effective
 ; } @@ -152,7 +137,11 @@ export class PolicyEditor extends Component { }); } - componentDidUpdate() { + componentDidUpdate(prevProps) { + if (sourceQueryStringParams(this.props) !== sourceQueryStringParams(prevProps)) { + this.fetchPolicy(this.props); + } + const pjs = JSON.stringify(this.state.policy); if (pjs !== this.lastResolvedPolicy) { this.resolvePolicy(this.props); @@ -203,7 +192,7 @@ export class PolicyEditor extends Component { return "(Defined by this policy)"; } - return <>Defined by {PolicyTypeName(p)}; + return <>Defined by {PolicyEditorLink(p)}; } saveChanges(e) { @@ -288,10 +277,7 @@ export class PolicyEditor extends Component { } return
- {!this.props.embedded &&

-   {PolicyTypeName(this.props)}

}
-  Snapshot Retention @@ -538,8 +524,8 @@ export class PolicyEditor extends Component { - - {!this.state.isNew && <>  + {!this.props.embedded && } + {!this.state.isNew && !this.props.embedded && <>  } {this.state.saving && <> diff --git a/htmlui/src/PolicyEditor.test.js b/htmlui/src/PolicyEditor.test.js index 09bcbcd12..64cf1d375 100644 --- a/htmlui/src/PolicyEditor.test.js +++ b/htmlui/src/PolicyEditor.test.js @@ -121,8 +121,9 @@ it('e2e', async () => { if (false) { logDOM(getByTestId("definition-retention.keepHourly")); } - await waitFor(() => expect(getByTestId("definition-retention.keepLatest").innerHTML).toEqual("Defined by Directory: some-user@h1:some-path")); - await waitFor(() => expect(getByTestId("definition-retention.keepMonthly").innerHTML).toEqual("Defined by Directory: u1@h1:p1")); + + await waitFor(() => expect(getByTestId("definition-retention.keepLatest").innerHTML).toContain(`Directory: some-user@h1:some-path`)); + await waitFor(() => expect(getByTestId("definition-retention.keepMonthly").innerHTML).toContain("Directory: u1@h1:p1")); const expectedUpcomingSnapshotTimes = [ `
  • ${moment(ust1).format('L LT')} (${moment(ust1).fromNow()})
    • `, diff --git a/htmlui/src/PolicyEditorPage.js b/htmlui/src/PolicyEditorPage.js new file mode 100644 index 000000000..800521006 --- /dev/null +++ b/htmlui/src/PolicyEditorPage.js @@ -0,0 +1,23 @@ +import React, { Component, createRef } from 'react'; +import { PolicyEditor } from './PolicyEditor'; +import { GoBackButton, parseQuery, PolicyTypeName } from './uiutil'; + +export class PolicyEditorPage extends Component { + constructor() { + super(); + + this.editorRef = createRef(); + } + + render() { + const source = parseQuery(this.props.location.search); + const { userName, host, path } = source; + + return <> +

    + +   {PolicyTypeName(source)}

    + + ; + } +} diff --git a/htmlui/src/SourcesTable.js b/htmlui/src/SourcesTable.js index f369b977d..712debe06 100644 --- a/htmlui/src/SourcesTable.js +++ b/htmlui/src/SourcesTable.js @@ -12,7 +12,7 @@ import Spinner from 'react-bootstrap/Spinner'; import { Link } from 'react-router-dom'; import { handleChange } from './forms'; import MyTable from './Table'; -import { compare, errorAlert, ownerName, redirectIfNotConnected, sizeDisplayName, sizeWithFailures, sourceQueryStringParams } from './uiutil'; +import { compare, errorAlert, ownerName, policyEditorURL, redirectIfNotConnected, sizeDisplayName, sizeWithFailures, sourceQueryStringParams } from './uiutil'; const localSnapshots = "Local Snapshots" const allSnapshots = "All Snapshots" @@ -96,6 +96,7 @@ export class SourcesTable extends Component { +   ; case "PENDING": @@ -233,7 +234,7 @@ export class SourcesTable extends Component { : '', }, { id: 'status', - Header: 'Status', + Header: '', width: 300, accessor: x => x.status, Cell: x => this.statusCell(x, this), diff --git a/htmlui/src/uiutil.js b/htmlui/src/uiutil.js index fc9705c18..b96652223 100644 --- a/htmlui/src/uiutil.js +++ b/htmlui/src/uiutil.js @@ -7,6 +7,7 @@ import Form from 'react-bootstrap/Form'; import FormControl from 'react-bootstrap/FormControl'; import InputGroup from 'react-bootstrap/InputGroup'; import Spinner from 'react-bootstrap/Spinner'; +import { Link } from 'react-router-dom'; const base10UnitPrefixes = ["", "K", "M", "G", "T"]; @@ -165,6 +166,30 @@ export function GoBackButton(props) { return ; } +export function PolicyTypeName(s) { + if (!s.host && !s.userName) { + return "Global Policy" + } + + if (!s.userName) { + return "Host: " + s.host; + } + + if (!s.path) { + return "User: " + s.userName + "@" + s.host; + } + + return "Directory: " + s.userName + "@" + s.host + ":" + s.path; +} + +export function policyEditorURL(s) { + return '/policies/edit?' + sourceQueryStringParams(s); +} + +export function PolicyEditorLink(s) { + return {PolicyTypeName(s)}; +} + export function sourceQueryStringParams(src) { return 'userName=' + encodeURIComponent(src.userName) + '&host=' + encodeURIComponent(src.host) + '&path=' + encodeURIComponent(src.path); } diff --git a/internal/server/api_estimate.go b/internal/server/api_estimate.go index da75cd789..34a6f98ff 100644 --- a/internal/server/api_estimate.go +++ b/internal/server/api_estimate.go @@ -39,6 +39,7 @@ func (p estimateTaskProgress) Error(ctx context.Context, dirname string, err err func (p estimateTaskProgress) Stats(ctx context.Context, st *snapshot.Stats, included, excluded snapshotfs.SampleBuckets, excludedDirs []string, final bool) { p.ctrl.ReportCounters(map[string]uitask.CounterValue{ "Bytes": uitask.BytesCounter(st.TotalFileSize), + "Excluded Bytes": uitask.BytesCounter(st.ExcludedTotalFileSize), "Files": uitask.SimpleCounter(int64(st.TotalFileCount)), "Directories": uitask.SimpleCounter(int64(st.TotalDirectoryCount)), "Excluded Files": uitask.SimpleCounter(int64(st.ExcludedFileCount)), @@ -129,11 +130,11 @@ func (s *Server) handleEstimate(ctx context.Context, r *http.Request, body []byt ctrl.OnCancel(cancel) - policyTree, err := policy.TreeForSource(ctx, s.rep, snapshot.SourceInfo{ + policyTree, err := policy.TreeForSourceWithOverride(ctx, s.rep, snapshot.SourceInfo{ Host: s.rep.ClientOptions().Hostname, UserName: s.rep.ClientOptions().Username, Path: resolvedRoot, - }) + }, req.PolicyOverride) if err != nil { return errors.Wrap(err, "unable to get policy tree") } diff --git a/internal/server/api_paths.go b/internal/server/api_paths.go new file mode 100644 index 000000000..ffac9b988 --- /dev/null +++ b/internal/server/api_paths.go @@ -0,0 +1,28 @@ +package server + +import ( + "context" + "encoding/json" + "net/http" + "path/filepath" + + "github.com/kopia/kopia/internal/ospath" + "github.com/kopia/kopia/internal/serverapi" + "github.com/kopia/kopia/snapshot" +) + +func (s *Server) handlePathResolve(ctx context.Context, r *http.Request, body []byte) (interface{}, *apiError) { + var req serverapi.ResolvePathRequest + + if err := json.Unmarshal(body, &req); err != nil { + return nil, requestError(serverapi.ErrorMalformedRequest, "malformed request body") + } + + return &serverapi.ResolvePathResponse{ + SourceInfo: snapshot.SourceInfo{ + Path: filepath.Clean(ospath.ResolveUserFriendlyPath(req.Path, true)), + Host: s.rep.ClientOptions().Hostname, + UserName: s.rep.ClientOptions().Username, + }, + }, nil +} diff --git a/internal/server/api_paths_test.go b/internal/server/api_paths_test.go new file mode 100644 index 000000000..66df7789a --- /dev/null +++ b/internal/server/api_paths_test.go @@ -0,0 +1,36 @@ +package server_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kopia/kopia/internal/apiclient" + "github.com/kopia/kopia/internal/repotesting" + "github.com/kopia/kopia/internal/serverapi" + "github.com/kopia/kopia/internal/testutil" +) + +func TestPathsAPI(t *testing.T) { + ctx, env := repotesting.NewEnvironment(t, repotesting.FormatNotImportant) + srvInfo := startServer(t, env, false) + + cli, err := apiclient.NewKopiaAPIClient(apiclient.Options{ + BaseURL: srvInfo.BaseURL, + TrustedServerCertificateFingerprint: srvInfo.TrustedServerCertificateFingerprint, + Username: testUIUsername, + Password: testUIPassword, + }) + + require.NoError(t, err) + + dir0 := testutil.TempDirectory(t) + + req := &serverapi.ResolvePathRequest{ + Path: dir0, + } + resp := &serverapi.ResolvePathResponse{} + require.NoError(t, cli.Post(ctx, "paths/resolve", req, resp)) + + require.Equal(t, localSource(env, dir0), resp.SourceInfo) +} diff --git a/internal/server/api_policies.go b/internal/server/api_policies.go index 9cc124aa4..6be45c797 100644 --- a/internal/server/api_policies.go +++ b/internal/server/api_policies.go @@ -73,7 +73,7 @@ func (s *Server) handlePolicyResolve(ctx context.Context, r *http.Request, body target := getPolicyTargetFromURL(r.URL) // build a list of parents - policies, err := policy.GetPolicyHierarchy(ctx, s.rep, target) + policies, err := policy.GetPolicyHierarchy(ctx, s.rep, target, nil) if err != nil { return nil, internalServerError(err) } @@ -106,18 +106,21 @@ func (s *Server) handlePolicyResolve(ctx context.Context, r *http.Request, body } func (s *Server) handlePolicyDelete(ctx context.Context, r *http.Request, body []byte) (interface{}, *apiError) { - w, ok := s.rep.(repo.RepositoryWriter) - if !ok { + if _, ok := s.rep.(repo.RepositoryWriter); !ok { return nil, repositoryNotWritableError() } - if err := policy.RemovePolicy(ctx, w, getPolicyTargetFromURL(r.URL)); err != nil { + sourceInfo := getPolicyTargetFromURL(r.URL) + + if err := repo.WriteSession(ctx, s.rep, repo.WriteSessionOptions{ + Purpose: "PolicyDelete", + }, func(ctx context.Context, w repo.RepositoryWriter) error { + return errors.Wrap(policy.RemovePolicy(ctx, w, sourceInfo), "unable to delete policy") + }); err != nil { return nil, internalServerError(err) } - if err := w.Flush(ctx); err != nil { - return nil, internalServerError(err) - } + s.triggerRefreshSource(sourceInfo) return &serverapi.Empty{}, nil } @@ -128,18 +131,21 @@ func (s *Server) handlePolicyPut(ctx context.Context, r *http.Request, body []by return nil, requestError(serverapi.ErrorMalformedRequest, "malformed request body") } - w, ok := s.rep.(repo.RepositoryWriter) - if !ok { + if _, ok := s.rep.(repo.RepositoryWriter); !ok { return nil, repositoryNotWritableError() } - if err := policy.SetPolicy(ctx, w, getPolicyTargetFromURL(r.URL), newPolicy); err != nil { + sourceInfo := getPolicyTargetFromURL(r.URL) + + if err := repo.WriteSession(ctx, s.rep, repo.WriteSessionOptions{ + Purpose: "PolicyPut", + }, func(ctx context.Context, w repo.RepositoryWriter) error { + return errors.Wrap(policy.SetPolicy(ctx, w, sourceInfo, newPolicy), "unable to set policy") + }); err != nil { return nil, internalServerError(err) } - if err := w.Flush(ctx); err != nil { - return nil, internalServerError(err) - } + s.triggerRefreshSource(sourceInfo) return &serverapi.Empty{}, nil } diff --git a/internal/server/api_sources.go b/internal/server/api_sources.go index 0bba4bcfc..e81e12e12 100644 --- a/internal/server/api_sources.go +++ b/internal/server/api_sources.go @@ -52,6 +52,10 @@ func (s *Server) handleSourcesCreate(ctx context.Context, r *http.Request, body return nil, requestError(serverapi.ErrorMalformedRequest, "missing path") } + if req.Policy == nil { + return nil, requestError(serverapi.ErrorMalformedRequest, "missing policy") + } + req.Path = ospath.ResolveUserFriendlyPath(req.Path, true) _, err := os.Stat(req.Path) @@ -71,33 +75,13 @@ func (s *Server) handleSourcesCreate(ctx context.Context, r *http.Request, body resp := &serverapi.CreateSnapshotSourceResponse{} - // ensure we have the policy for this source, otherwise it will not show up in the - // list of sources at all. - _, err = policy.GetDefinedPolicy(ctx, s.rep, sourceInfo) - - switch { - case err == nil: - // already have policy, do nothing - log(ctx).Debugf("policy for %v already exists", sourceInfo) - - resp.Created = false - - case errors.Is(err, policy.ErrPolicyNotFound): - resp.Created = true - // don't have policy - create an empty one - log(ctx).Debugf("policy for %v not found, creating empty one", sourceInfo) - - if err = repo.WriteSession(ctx, s.rep, repo.WriteSessionOptions{ - Purpose: "handleSourcesCreate", - }, func(ctx context.Context, w repo.RepositoryWriter) error { - // nolint:wrapcheck - return policy.SetPolicy(ctx, w, sourceInfo, &req.InitialPolicy) - }); err != nil { - return nil, internalServerError(errors.Wrap(err, "unable to set initial policy")) - } - - default: - return nil, internalServerError(err) + if err = repo.WriteSession(ctx, s.rep, repo.WriteSessionOptions{ + Purpose: "handleSourcesCreate", + }, func(ctx context.Context, w repo.RepositoryWriter) error { + // nolint:wrapcheck + return policy.SetPolicy(ctx, w, sourceInfo, req.Policy) + }); err != nil { + return nil, internalServerError(errors.Wrap(err, "unable to set initial policy")) } // upgrade to exclusive lock to ensure we have source manager @@ -108,6 +92,8 @@ func (s *Server) handleSourcesCreate(ctx context.Context, r *http.Request, body sm := newSourceManager(sourceInfo, s) s.sourceManagers[sourceInfo] = sm + sm.refreshStatus(ctx) + go sm.run(ctx) } s.mu.Unlock() diff --git a/internal/server/api_sources_test.go b/internal/server/api_sources_test.go index 5cfc92425..82081fe22 100644 --- a/internal/server/api_sources_test.go +++ b/internal/server/api_sources_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/kopia/kopia/internal/apiclient" + "github.com/kopia/kopia/internal/clock" "github.com/kopia/kopia/internal/repotesting" "github.com/kopia/kopia/internal/serverapi" "github.com/kopia/kopia/internal/testutil" @@ -33,7 +34,7 @@ func TestSnapshotCounters(t *testing.T) { dir := testutil.TempDirectory(t) si := localSource(env, dir) - mustCreateSource(t, cli, dir) + mustCreateSource(t, cli, dir, &policy.Policy{}) require.Len(t, mustListSources(t, cli, &snapshot.SourceInfo{}), 1) mustSetPolicy(t, cli, si, &policy.Policy{ @@ -73,3 +74,63 @@ func TestSnapshotCounters(t *testing.T) { require.Equal(t, ut.Counters["Excluded Files"], uitask.SimpleCounter(1)) require.Equal(t, ut.Counters["Processed Files"], uitask.SimpleCounter(3)) } + +func TestSourceRefreshesAfterPolicy(t *testing.T) { + ctx, env := repotesting.NewEnvironment(t, repotesting.FormatNotImportant) + srvInfo := startServer(t, env, false) + + _ = ctx + + cli, err := apiclient.NewKopiaAPIClient(apiclient.Options{ + BaseURL: srvInfo.BaseURL, + TrustedServerCertificateFingerprint: srvInfo.TrustedServerCertificateFingerprint, + Username: testUIUsername, + Password: testUIPassword, + }) + + require.NoError(t, err) + + dir := testutil.TempDirectory(t) + si := localSource(env, dir) + + currentHour := clock.Now().Hour() + + mustCreateSource(t, cli, dir, &policy.Policy{ + SchedulingPolicy: policy.SchedulingPolicy{ + TimesOfDay: []policy.TimeOfDay{ + {Hour: (currentHour + 2) % 24, Minute: 33}, + }, + }, + }) + + sources := mustListSources(t, cli, &snapshot.SourceInfo{}) + require.Len(t, sources, 1) + require.NotNil(t, sources[0].NextSnapshotTime) + require.Equal(t, 33, sources[0].NextSnapshotTime.Minute()) + + mustSetPolicy(t, cli, si, &policy.Policy{ + SchedulingPolicy: policy.SchedulingPolicy{ + TimesOfDay: []policy.TimeOfDay{ + {Hour: (currentHour + 2) % 24, Minute: 55}, + }, + }, + }) + + // make sure that soon after setting policy, the next snapshot time is up-to-date. + match := false + + for attempt := 0; attempt < 3; attempt++ { + sources = mustListSources(t, cli, &snapshot.SourceInfo{}) + require.Len(t, sources, 1) + require.NotNil(t, sources[0].NextSnapshotTime) + + if sources[0].NextSnapshotTime.Minute() == 55 { + match = true + break + } + + time.Sleep(time.Second) + } + + require.True(t, match) +} diff --git a/internal/server/server.go b/internal/server/server.go index c6c06a2fe..963e46ba1 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -96,6 +96,9 @@ func (s *Server) APIHandlers(legacyAPI bool) http.Handler { m.HandleFunc("/api/v1/restore", s.handleAPI(requireUIUser, s.handleRestore)).Methods(http.MethodPost) m.HandleFunc("/api/v1/estimate", s.handleAPI(requireUIUser, s.handleEstimate)).Methods(http.MethodPost) + // path APIs + m.HandleFunc("/api/v1/paths/resolve", s.handleAPI(requireUIUser, s.handlePathResolve)).Methods(http.MethodPost) + // methods that can be called by any authenticated user (UI or remote user). m.HandleFunc("/api/v1/flush", s.handleAPI(anyAuthenticatedUser, s.handleFlush)).Methods(http.MethodPost) m.HandleFunc("/api/v1/repo/status", s.handleAPIPossiblyNotConnected(anyAuthenticatedUser, s.handleRepoStatus)).Methods(http.MethodGet) @@ -432,6 +435,18 @@ func (s *Server) endUpload(ctx context.Context, src snapshot.SourceInfo) { <-s.uploadSemaphore } +func (s *Server) triggerRefreshSource(sourceInfo snapshot.SourceInfo) { + sm := s.sourceManagers[sourceInfo] + if sm == nil { + return + } + + select { + case sm.refreshRequested <- struct{}{}: + default: + } +} + // SetRepository sets the repository (nil is allowed and indicates server that is not // connected to the repository). func (s *Server) SetRepository(ctx context.Context, rep repo.Repository) error { diff --git a/internal/server/source_manager.go b/internal/server/source_manager.go index 1ada6cb6d..74e084964 100644 --- a/internal/server/source_manager.go +++ b/internal/server/source_manager.go @@ -21,7 +21,7 @@ import ( ) const ( - statusRefreshInterval = 15 * time.Second // how frequently to refresh source status + statusRefreshInterval = 30 * time.Minute failedSnapshotRetryInterval = 5 * time.Minute refreshTimeout = 30 * time.Second // max amount of time to refresh a single source oneDay = 24 * time.Hour @@ -42,6 +42,7 @@ type sourceManager struct { src snapshot.SourceInfo closed chan struct{} snapshotRequests chan struct{} + refreshRequested chan struct{} // tickled externally to trigger refresh wg sync.WaitGroup mu sync.RWMutex @@ -142,6 +143,10 @@ func (s *sourceManager) runLocal(ctx context.Context) { continue + case <-s.refreshRequested: + s.refreshStatus(ctx) + continue + case <-time.After(statusRefreshInterval): s.refreshStatus(ctx) @@ -360,12 +365,11 @@ func (s *sourceManager) refreshStatus(ctx context.Context) { break } } - - s.nextSnapshotTime = s.findClosestNextSnapshotTime() } else { - s.nextSnapshotTime = nil s.lastSnapshot = nil } + + s.nextSnapshotTime = s.findClosestNextSnapshotTime() } type uitaskProgress struct { @@ -476,6 +480,7 @@ func newSourceManager(src snapshot.SourceInfo, server *Server) *sourceManager { state: "UNKNOWN", closed: make(chan struct{}), snapshotRequests: make(chan struct{}, 1), + refreshRequested: make(chan struct{}, 1), progress: &snapshotfs.CountingUploadProgress{}, } diff --git a/internal/server/util_test.go b/internal/server/util_test.go index c9906d437..02572141f 100644 --- a/internal/server/util_test.go +++ b/internal/server/util_test.go @@ -16,11 +16,12 @@ import ( "github.com/kopia/kopia/snapshot/policy" ) -func mustCreateSource(t *testing.T, cli *apiclient.KopiaAPIClient, path string) { +func mustCreateSource(t *testing.T, cli *apiclient.KopiaAPIClient, path string, pol *policy.Policy) { t.Helper() _, err := serverapi.CreateSnapshotSource(testlogging.Context(t), cli, &serverapi.CreateSnapshotSourceRequest{ - Path: path, + Path: path, + Policy: pol, }) require.NoError(t, err) } diff --git a/internal/serverapi/serverapi.go b/internal/serverapi/serverapi.go index d7e8822ac..3e65faff4 100644 --- a/internal/serverapi/serverapi.go +++ b/internal/serverapi/serverapi.go @@ -136,14 +136,13 @@ type SupportedAlgorithmsResponse struct { // CreateSnapshotSourceRequest contains request to create snapshot source and optionally create first snapshot. type CreateSnapshotSourceRequest struct { - Path string `json:"path"` - CreateSnapshot bool `json:"createSnapshot"` - InitialPolicy policy.Policy `json:"initialPolicy"` // policy to set on the source when first created, ignored if already exists + Path string `json:"path"` + CreateSnapshot bool `json:"createSnapshot"` + Policy *policy.Policy `json:"policy"` // policy to set on the path } // CreateSnapshotSourceResponse contains response of creating snapshot source. type CreateSnapshotSourceResponse struct { - Created bool `json:"created"` // whether the source was created (false==previously existed) SnapshotStarted bool `json:"snapshotted"` // whether snapshotting has been started } @@ -217,8 +216,9 @@ type RestoreRequest struct { // EstimateRequest contains request to estimate the size of the snapshot in a given root. type EstimateRequest struct { - Root string `json:"root"` - MaxExamplesPerBucket int `json:"maxExamplesPerBucket"` + Root string `json:"root"` + MaxExamplesPerBucket int `json:"maxExamplesPerBucket"` + PolicyOverride *policy.Policy `json:"policyOverride"` } // ResolvePolicyRequest contains request structure to ResolvePolicy. @@ -234,3 +234,13 @@ type ResolvePolicyResponse struct { Defined *policy.Policy `json:"defined"` UpcomingSnapshotTimes []time.Time `json:"upcomingSnapshotTimes"` } + +// ResolvePathRequest contains request to resolve a particular path to ResolvePathResponse. +type ResolvePathRequest struct { + Path string `json:"path"` +} + +// ResolvePathResponse contains response to resolve a particular path. +type ResolvePathResponse struct { + SourceInfo snapshot.SourceInfo `json:"source"` +} diff --git a/snapshot/policy/policy_manager.go b/snapshot/policy/policy_manager.go index c6353807a..ca2c32ebd 100644 --- a/snapshot/policy/policy_manager.go +++ b/snapshot/policy/policy_manager.go @@ -41,7 +41,14 @@ var log = logging.Module("kopia/snapshot/policy") // with parent policies. The source must contain a path. // Returns the effective policies and all source policies that contributed to that (most specific first). func GetEffectivePolicy(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) (effective *Policy, definition *Definition, sources []*Policy, e error) { - policies, err := GetPolicyHierarchy(ctx, rep, si) + return GetEffectivePolicyWithOverride(ctx, rep, si, nil) +} + +// GetEffectivePolicyWithOverride calculates effective snapshot policy for a given source by combining the source-specifc policy (if any) +// with parent policies. The source must contain a path. +// Returns the effective policies and all source policies that contributed to that (most specific first). +func GetEffectivePolicyWithOverride(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo, optionalPolicyOverride *Policy) (effective *Policy, definition *Definition, sources []*Policy, e error) { + policies, err := GetPolicyHierarchy(ctx, rep, si, optionalPolicyOverride) if err != nil { return nil, nil, nil, errors.Wrap(err, "unable to get parent policies") } @@ -52,7 +59,7 @@ func GetEffectivePolicy(ctx context.Context, rep repo.Repository, si snapshot.So } // GetPolicyHierarchy returns the set of parent policies that apply to the path in most-specific-to-most-general order. -func GetPolicyHierarchy(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) ([]*Policy, error) { +func GetPolicyHierarchy(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo, optionalPolicyOverride *Policy) ([]*Policy, error) { var md []*manifest.EntryMetadata // Find policies applying to paths all the way up to the root. @@ -62,7 +69,9 @@ func GetPolicyHierarchy(ctx context.Context, rep repo.Repository, si snapshot.So return nil, errors.Wrapf(err, "unable to find manifest for source %v", tmp) } - md = append(md, manifests...) + if si != tmp || optionalPolicyOverride == nil { + md = append(md, manifests...) + } parentPath := getParentPathOSIndependent(tmp.Path) if parentPath == tmp.Path { @@ -98,6 +107,11 @@ func GetPolicyHierarchy(ctx context.Context, rep repo.Repository, si snapshot.So var policies []*Policy + if optionalPolicyOverride != nil { + optionalPolicyOverride.Labels = LabelsForSource(si) + policies = append(policies, optionalPolicyOverride) + } + for _, em := range md { p := &Policy{} if err := loadPolicyFromManifest(ctx, rep, em.ID, p); err != nil { @@ -232,7 +246,12 @@ func (m SubdirectoryPolicyMap) GetPolicyForPath(relativePath string) (*Policy, e // TreeForSource returns policy Tree for a given source. func TreeForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) (*Tree, error) { - pols, err := applicablePoliciesForSource(ctx, rep, si) + return TreeForSourceWithOverride(ctx, rep, si, nil) +} + +// TreeForSourceWithOverride returns policy Tree for a given source with the root policy overridden. +func TreeForSourceWithOverride(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo, optionalPolicyOverride *Policy) (*Tree, error) { + pols, err := applicablePoliciesForSource(ctx, rep, si, optionalPolicyOverride) if err != nil { return nil, errors.Wrap(err, "unable to get policies") } @@ -240,10 +259,10 @@ func TreeForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceI return BuildTree(pols, DefaultPolicy), nil } -func applicablePoliciesForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) (map[string]*Policy, error) { +func applicablePoliciesForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo, optionalPolicyOverride *Policy) (map[string]*Policy, error) { result := map[string]*Policy{} - pol, _, _, err := GetEffectivePolicy(ctx, rep, si) + pol, _, _, err := GetEffectivePolicyWithOverride(ctx, rep, si, optionalPolicyOverride) if err != nil { return nil, err } diff --git a/snapshot/policy/policy_manager_test.go b/snapshot/policy/policy_manager_test.go index b5a1779e5..e661c84ae 100644 --- a/snapshot/policy/policy_manager_test.go +++ b/snapshot/policy/policy_manager_test.go @@ -432,7 +432,7 @@ func TestApplicablePoliciesForSource(t *testing.T) { for _, tc := range cases { tc := tc t.Run(tc.si.String(), func(t *testing.T) { - res, err := applicablePoliciesForSource(ctx, env.RepositoryWriter, tc.si) + res, err := applicablePoliciesForSource(ctx, env.RepositoryWriter, tc.si, nil) if err != nil { t.Fatalf("error in applicablePoliciesForSource(%v): %v", tc.si, err) } diff --git a/tests/end_to_end_test/server_start_test.go b/tests/end_to_end_test/server_start_test.go index cd43c72a1..779c5a65e 100644 --- a/tests/end_to_end_test/server_start_test.go +++ b/tests/end_to_end_test/server_start_test.go @@ -127,11 +127,11 @@ func TestServerStart(t *testing.T) { require.Equal(t, int64(0), et.Counters["Ignored Errors"].Value) createResp, err := serverapi.CreateSnapshotSource(ctx, cli, &serverapi.CreateSnapshotSourceRequest{ - Path: sharedTestDataDir2, + Path: sharedTestDataDir2, + Policy: &policy.Policy{}, }) require.NoError(t, err) - require.True(t, createResp.Created) require.False(t, createResp.SnapshotStarted) verifySourceCount(t, cli, nil, 2) @@ -162,13 +162,13 @@ func TestServerStart(t *testing.T) { keepDaily := 77 createResp, err = serverapi.CreateSnapshotSource(ctx, cli, &serverapi.CreateSnapshotSourceRequest{ - Path: sharedTestDataDir3, - CreateSnapshot: true, - InitialPolicy: policy.Policy{ + Path: sharedTestDataDir3, + Policy: &policy.Policy{ RetentionPolicy: policy.RetentionPolicy{ KeepDaily: &keepDaily, }, }, + CreateSnapshot: true, }) require.NoError(t, err)