Skip to content

ua_restore: enable restore from archive#1372

Merged
arogge merged 15 commits intobareos:masterfrom
sebsura:dev/ssura/master/enable-restore-from-archive
Mar 2, 2023
Merged

ua_restore: enable restore from archive#1372
arogge merged 15 commits intobareos:masterfrom
sebsura:dev/ssura/master/enable-restore-from-archive

Conversation

@sebsura
Copy link
Contributor

@sebsura sebsura commented Feb 7, 2023 •
edited by bruno-at-bareos
Loading

Thank you for contributing to the Bareos Project!

Adds an 'archive' keyword to the restore command. When that keyword is present the restore command looks for jobs of type 'A', i.e. archive jobs, when selecting a suitable job for restoration instead of jobs of type 'B'.

This PR also adds 'Archive' to the possible tab completions of update volstatus=.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@sebsura sebsura changed the title Dev/ssura/master/enable restore from archive ua_restore: enable restore from archive Feb 7, 2023
@sebsura sebsura force-pushed the dev/ssura/master/enable-restore-from-archive branch 6 times, most recently from 12ac7d7 to 66ba50e Compare February 13, 2023 14:12
@bruno-at-bareos bruno-at-bareos self-requested a review February 16, 2023 10:55
@bruno-at-bareos bruno-at-bareos self-assigned this Feb 16, 2023
@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Feb 20, 2023 •
edited
Loading

On first try doing a restore archive while no archive exist give a mixed result of sentences

*restore archive

First you select one or more JobIds that contain files
to be restored. You will be presented several methods
of specifying the JobIds. Then you will be allowed to
select which files from those JobIds are to be restored.

To select the JobIds, you have the following choices:
 1: List last 20 Jobs run
 2: List Jobs where a given File is saved
 3: Enter list of comma separated JobIds to select
 4: Enter SQL list command
 5: Select the most recent backup for a client
 6: Select backup for a client before a specified time
 7: Enter a list of files to restore
 8: Enter a list of files to restore before a specified time
 9: Find the JobIds of the most recent backup for a client
10: Find the JobIds for a backup for a client before a specified time
11: Enter a list of directories to restore for found JobIds
12: Select full restore to a specified Job date
13: Cancel
Select item:  (1-13): 5
Automatically selected Client: bareos-fd
Automatically selected FileSet: SelfTest
No Full backup archive before 2023-02-20 13:07:37 found.
A suitable full backup was found. Try restore <...> instead.

The last line shouldn't be there.

@sebsura
Copy link
Contributor Author

sebsura commented Feb 20, 2023 •
edited
Loading

My idea was that we should also tell the user that a normal backup exists if trying to restore an archive similar to when an archive backup exists while doing a normal restore. If this behaviour is not wanted, i can easily remove it.

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Feb 20, 2023 •
edited
Loading

My idea was that we should also tell the user that a normal backup exists if trying to restore an archive similar to when an archive backup exists while doing a normal restore. If this behaviour is not wanted, i can easily remove it.

Ok then we can improve the message. maybe by using ". Try a normal restore <...> instead."
which can help people like the inverse is quiet clear.

Select the Client (1-7): 4
Automatically selected FileSet: openSUSE-Full-btrfs
No Full backup before 2022-11-30 22:00:00 found.
A suitable full backup archive was found. Try restore archive <...> instead.
```

bruno-at-bareos
bruno-at-bareos requested changes Feb 21, 2023
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Sebastian, I've requested comments, and changes. Let discuss them.

bruno-at-bareos
bruno-at-bareos requested changes Feb 22, 2023
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there's still an error 2025 in systemtests/tests/restore-test/testrunner-archive-restore-file
We can remove the second volume creation on systemtests/tests/restore-test/testrunner-create-backup
Then I will wait @pstorz feedback about the accurate, you made a point about restore 12. and also on the singular/plural form in core/src/dird/ua.h

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Feb 22, 2023

There's still an opened question @sebsura the total time consumed to run the different tests is quite large

Test project /bareos/git/build
      Start 413: system:restore-test:setup
 1/11 Test #413: system:restore-test:setup ..................   Passed    3.13 sec
      Start 418: system:restore-test:create-backup
 2/11 Test #418: system:restore-test:create-backup ..........   Passed   34.45 sec
      Start 414: system:restore-test:archive-full-restore
 3/11 Test #414: system:restore-test:archive-full-restore ...   Passed   36.67 sec
      Start 415: system:restore-test:archive-restore-dir
 4/11 Test #415: system:restore-test:archive-restore-dir ....   Passed   36.65 sec
      Start 416: system:restore-test:archive-restore-file
 5/11 Test #416: system:restore-test:archive-restore-file ...   Passed   47.00 sec
      Start 417: system:restore-test:check-hints
 6/11 Test #417: system:restore-test:check-hints ............   Passed   31.83 sec
      Start 419: system:restore-test:full-restore
 7/11 Test #419: system:restore-test:full-restore ...........   Passed   36.60 sec
      Start 420: system:restore-test:restore-dir
 8/11 Test #420: system:restore-test:restore-dir ............   Passed   36.50 sec
      Start 421: system:restore-test:restore-file
 9/11 Test #421: system:restore-test:restore-file ...........   Passed   45.47 sec
      Start 422: system:restore-test:restore-old-archive
10/11 Test #422: system:restore-test:restore-old-archive ....   Passed   40.60 sec
      Start 423: system:restore-test:cleanup
11/11 Test #423: system:restore-test:cleanup ................   Passed    2.11 sec

100% tests passed, 0 tests failed out of 11
Total Test time (real) = 351.04 sec

And that's on my computer, I didn't yet check at Jenkins.
If we multiply this by the number of pr open it may become a kind of bottlenecks.
Don't know if we can speed that up a bit.

@sebsura
Copy link
Contributor Author

sebsura commented Feb 22, 2023 •
edited
Loading

There's still an opened question @sebsura the total time consumed to run the different tests is quite large

Test project /bareos/git/build
      Start 413: system:restore-test:setup
 1/11 Test #413: system:restore-test:setup ..................   Passed    3.13 sec
      Start 418: system:restore-test:create-backup
 2/11 Test #418: system:restore-test:create-backup ..........   Passed   34.45 sec
      Start 414: system:restore-test:archive-full-restore
 3/11 Test #414: system:restore-test:archive-full-restore ...   Passed   36.67 sec
      Start 415: system:restore-test:archive-restore-dir
 4/11 Test #415: system:restore-test:archive-restore-dir ....   Passed   36.65 sec
      Start 416: system:restore-test:archive-restore-file
 5/11 Test #416: system:restore-test:archive-restore-file ...   Passed   47.00 sec
      Start 417: system:restore-test:check-hints
 6/11 Test #417: system:restore-test:check-hints ............   Passed   31.83 sec
      Start 419: system:restore-test:full-restore
 7/11 Test #419: system:restore-test:full-restore ...........   Passed   36.60 sec
      Start 420: system:restore-test:restore-dir
 8/11 Test #420: system:restore-test:restore-dir ............   Passed   36.50 sec
      Start 421: system:restore-test:restore-file
 9/11 Test #421: system:restore-test:restore-file ...........   Passed   45.47 sec
      Start 422: system:restore-test:restore-old-archive
10/11 Test #422: system:restore-test:restore-old-archive ....   Passed   40.60 sec
      Start 423: system:restore-test:cleanup
11/11 Test #423: system:restore-test:cleanup ................   Passed    2.11 sec

100% tests passed, 0 tests failed out of 11
Total Test time (real) = 351.04 sec

And that's on my computer, I didn't yet check at Jenkins. If we multiply this by the number of pr open it may become a kind of bottlenecks. Don't know if we can speed that up a bit.

Can you tell me how long it the bareos-basic:simple-backup-and-restore test takes for you ?

Edit: the master build took 13m57s for tests while this build took 14m12s on jenkins. I cannot tell how high the increase in cpu time was though. Do you know of a way to check ?

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Feb 22, 2023 

Test project /bareos/git/build
    Start 285: system:bareos-basic:setup
1/3 Test #285: system:bareos-basic:setup .......................   Passed    1.77 sec
    Start 293: system:bareos-basic:simple-backup-and-restore
2/3 Test #293: system:bareos-basic:simple-backup-and-restore ...   Passed   39.84 sec
    Start 298: system:bareos-basic:cleanup
3/3 Test #298: system:bareos-basic:cleanup .....................   Passed    2.14 sec

100% tests passed, 0 tests failed out of 3
Total Test time (real) =  43.78 sec

We can open a discussion tomorrow with other to see if it is a none problem.

@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Feb 23, 2023 •
edited
Loading

Running on rebooted computer and recreated containers seems to fix the long runtime

Test #293: system:bareos-basic:simple-backup-and-restore ...............................   Passed   10.10 sec
 59/432 Test #419: system:restore-test:full-restore ............................................   Passed    6.48 sec
 72/432 Test #422: system:restore-test:restore-old-archive .....................................   Passed    9.46 sec

100% tests passed, 0 tests failed out of 394
Total Test time (real) = 272.91 sec

Let's drop the concern about this subject.

@sebsura sebsura force-pushed the dev/ssura/master/enable-restore-from-archive branch from f41ccb7 to 9f1ac27 Compare February 23, 2023 13:51
bruno-at-bareos
bruno-at-bareos previously approved these changes Feb 23, 2023
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question raised about bvfs_get_jobids can be eluded. By default the functionality will stay with jobtype=B and there's no need to implement research for other related job with jobtype=A

There's a tests systemtests/tests/python-bareos/test_bvfs.py:97 which check that functionality, and we don't break it here.
Users were already able to restore jobtype=A with the /bareos-webui/restore/versions and they will still able (because only one jobid is used in that case).

I approve then the code here.

@bruno-at-bareos bruno-at-bareos dismissed their stale review February 23, 2023 14:40

We need to squash the commit before the approval.

bruno-at-bareos
bruno-at-bareos requested changes Feb 23, 2023
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are ok globally. so now we can move to the squash commit together stage.

@sebsura sebsura force-pushed the dev/ssura/master/enable-restore-from-archive branch 2 times, most recently from 9e5faed to 6807469 Compare February 24, 2023 09:20
@sebsura
Copy link
Contributor Author

sebsura commented Feb 24, 2023

Squashed commits; also added the test we spoke about.

bruno-at-bareos
bruno-at-bareos reviewed Feb 27, 2023
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be fixed freebsd tests failing in error_full_restore and closing "hidden" converstations.

@sebsura sebsura force-pushed the dev/ssura/master/enable-restore-from-archive branch 2 times, most recently from bd14835 to c3e5c56 Compare February 28, 2023 11:41
@sebsura sebsura force-pushed the dev/ssura/master/enable-restore-from-archive branch 3 times, most recently from f91b44e to 4a41eda Compare February 28, 2023 14:48
bruno-at-bareos
bruno-at-bareos approved these changes Mar 1, 2023
View reviewed changes
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final approvement, ready to merge

@sebsura
Copy link
Contributor Author

sebsura commented Mar 1, 2023

I resolved the last open conversations since they were already addressed.

sebsura and others added 15 commits March 2, 2023 18:14
@sebsura @arogge
ua_restore: enable support for restoring from archives
d0f299e 
Add the archive argument to the restore command.  If that argument is
suppied we instead only look at archives instead of normal backups.

Show an informational message if trying to restore from an
archive/normal backup if no suitable backup exists, but one of the
other kind does exist.
@sebsura @arogge
restore-test: add tests for restore behaviour
94f38b5 
We create some new tests that specifically test the restore behaviour
of normal backups as well as archives.
@sebsura @arogge
dird: add Archive to update volstatus= tab completion
4524428
@sebsura @arogge
weird-files: update directory names
fafa53d 
A (non-existent) directory that contained a file that was softlinked
by a file in weird-files, was still named bacula.  This commit
recreated those softlinks to instead link into a directory named bareos.
@sebsura @arogge
test-config: Set Maximum Concurrent Jobs to remove warning
28e1b5b
@sebsura @arogge
test-conf: Remove unused options
bdb4530
@sebsura @arogge
test-config: delete unused config files
e4abaea
@sebsura @arogge
restore-test: fix bad grep string
56031b8
@sebsura @arogge
restore-test: check for warnings
e8cd706
@sebsura @arogge
restore-test: enhance preconditions
a24a05b
@sebsura @arogge
restore-test: add postcondition test to create-backups
955fd58
@sebsura @arogge
restore-test: fix preconditions for bsd
12ae769
@sebsura @arogge
restore: change test names
71fa6b8
@sebsura @arogge
restore: fix formatting of CMakeLists.txt
5b28df5
@arogge
Update CHANGELOG.md
0cc358f
@arogge arogge force-pushed the dev/ssura/master/enable-restore-from-archive branch from b68355d to 0cc358f Compare March 2, 2023 17:14
@arogge arogge merged commit eac5a87 into bareos:master Mar 2, 2023
@sebsura sebsura mentioned this pull request Mar 22, 2023
Merged
14 tasks
@SamuelBoerlin SamuelBoerlin mentioned this pull request Jun 30, 2023
Merged
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bruno-at-bareos bruno-at-bareos bruno-at-bareos approved these changes

Assignees

@bruno-at-bareos bruno-at-bareos

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sebsura @bruno-at-bareos @arogge