kompose [completion] Add header #compdef

Contribution Guidelines

[x] I've read the contribution guidelines and wholeheartedly agree them

What does this PR include?

Adding print_usage function to backup_and_restore.sh script
Add colored messages in backup_and_restore.sh script
Move the thread count notice into the print_usage
Fix: Double quoted variable BACKUP_LOCATION. - Critical: Since it might has spaces therefore will corrupt the backup/restore process.
Bug Fix: overwriting filesystem possible - Critical: Previous code was not checking if the path is a symbolic to a filesystem, if user is root and the given path is a symbolic of filesystem then it will overwrite it!
Simplify the check expression of THREADS
Implement check_required_tools function - Implement check_required_tools function in backup_and_restore.sh script, as well call it in the beginning of the script instead of at the end
Clarifying the duplicate assigning of MAILCOW_BACKUP_LOCATION
Fix Bug: backup_and_restore.sh script, increment i only if needed
Fix: Crash on entering non-numeric value
Fix: backup_location should be exists, for the restore
Fix: Crash on entering non-numeric value
Implement declare_restore_point function
Typo in calling declare_restore_point
Fix: grep: warning: stray \ before :
Implement declare_file_selection function
Implement restore_docker_component to restore components in one place
Fix last grep: warning: stray \ before :
Write a clarification comment for function restore_docker_component
Introduce new help screen, with new flags depends script

Short Description

This PR is not finished yet, I'll include many re-write code for the backup_and_restore.sh script. Enhancement, refactor and improved code, before starting to implement a new features.

Affected Containers

None

Did you run tests?

Tested manually

What did you tested?

backup_and_restore.sh script

What were the final results? (Awaited, got)

All good

Jul 31 '24 12:07 sazriel26

Hey @DerLinkman, you can now resolve this PR, since I will need a few time to refactor and fully test the backup and restore functionality, since then you can resolve this PR now to avoid conflicts.

Aug 20 '24 00:08 h3ssan

Hey @DerLinkman, you can now resolve this PR, since I will need a few time to refactor and fully test the backup and restore functionality, since then you can resolve this PR now to avoid conflicts.

Hello @h3ssan! Is this pr now ready to merge or not? If not, we can convert it to a draft.

Aug 20 '24 06:08 DerLinkman

Hey there @DerLinkman , yes it’s ready to be merged.

still there’s 2 functions I need to look into but might take some time so yes, resolve it

Aug 20 '24 07:08 h3ssan

Clarification

Commit Fix Bug: backup_and_restore.sh script, increment i only if needed, why?

Original Code

  i=1 # Starts at 1, even before finding any folder
  declare -A FOLDER_SELECTION
  if [[ $(find "${BACKUP_LOCATION}"/mailcow-* -maxdepth 1 -type d 2> /dev/null| wc -l) -lt 1 ]]; then
    echo -e "\e[31mSelected backup location has no subfolders\e[0m"
    exit 1
  fi
  for folder in $(ls -d "${BACKUP_LOCATION}"/mailcow-*/); do
    echo "[ ${i} ] - ${folder}"
    FOLDER_SELECTION[${i}]="${folder}"
    ((i++)) 
  done

If i is initial to 1, this will cause:

i = 1 - Initial
Checking if there's a folder, then increment i, therefore 1 = 2

Here we have i equals to 2 but we only found one folder ! Then, when the user choose which folder to restore, the user can simply choose 2 not 1 and the script will continue working, check the following image:

Aug 20 '24 15:08 h3ssan

Clarification

Commit: Fix: Crash on entering non-numeric value, fixes the following issue:

Enter , (non-numeric value, actually) will case a crash:

Aug 20 '24 16:08 h3ssan

Till this point, what I have tested is below:

Tested Command	Expected	Status
`./backup_and_restore.sh backup vmail`	Backed up¹	Good ✓
`./backup_and_restore.sh backup all`	Backed up¹	Good ✓
`./backup_and_restore.sh backup anythingelse`	Throw `Unknown argument: anythingelse`	Good ✓
`./backup_and_restore.sh anythingelse`	print the usage	Good ✓
`./backup_and_restore.sh restore` with correct folder	Restored¹	Good ✓
`./backup_and_restore.sh restore` with non-existence folder	Throw `location not found`	Good ✓
`./backup_and_restore.sh restore` with location with no data	Throw `no subfolders`	Good ✓
`./backup_and_restore.sh restore` with location to `/dev/sda`	Throw `invalid path`	Good ✓
`./backup_and_restore.sh backup vmail` with location to `/dev/sda`	Throw `invalid path`	Good ✓

¹ I DID NOT check the actual process, just trusted what the script said! Therefore it might need extensive/dedicated testing.

Aug 20 '24 21:08 h3ssan

Sorry about the last commit, there was an issue with the committer and author details. I soft reset, fixed it and push again.

Aug 21 '24 00:08 h3ssan

Please note that this commit Implement declare_restore_point function still need testing. I'll do the test in a few hours.

Aug 21 '24 00:08 h3ssan

What to be expected in the commit: Introduce new help screen, with new flags depends script

Features

Restore multiple components at once, such as ./backup_and_restore.sh --restore /path/to/restore -c vmail -c crypt -c mysql or even using ./backup_and_restore.sh --restore /path/to/restore -c all to restore everything at once.
Making the script as flexible as possible.

Enhancement

Introduce a new help menu, check it out with ./backup_and_restore.sh --help
New flag system, also check it with ./backup_and_restore.sh --help
Define MAILCOW_BACKUP_LOCATION in your ~/.bashrc to shorten time ! then just do ./backup_and_restore.sh -c all to backup everything without specifying --backup flag!
Use named colors, instead of typing a coded color value.
Add many comments in the script to avoid overwhelmed code.
Avoid nested statements.
Big improvement in --delete-days flag.

Aug 22 '24 19:08 h3ssan

Using 4 thread(s) for this run.
Found project name mailcowdockerized
Using /opt/backup as restore location...
[ 1 ] - /opt/backup/mailcow-2024-08-24-08-35-01/
[ 2 ] - /opt/backup/mailcow-2024-08-25-08-35-01/
[ 3 ] - /opt/backup/mailcow-2024-08-26-08-35-01/
[ 4 ] - /opt/backup/mailcow-2024-08-27-08-14-51/

Select a restore point: 4


Matching available components to restore:  
here is nothing displayed
Where on the otherhand if you select other types like vmail, rspamd etc. those are printed. It seems mysql is missing when you manually select this here.

If you select all as component type then mysql/mariadb is shown in the list

Okay, so the mysql is not shown, since there's mix in use between mysql and mariadb but the actual database is mariadb and the actual backup file is backup_mariadb.tar.gz therefore I will completely change mysql to mariadb. Are you ok with this change?

Edit: I do not know if it was previously storing backups as mysql instead of now mariadb, so, I will just fix the issue instead of changing everything to mariadb.

Aug 27 '24 07:08 h3ssan

Btw: What are the reason of these numbers for? As you now cannot select them anymore? Is that the order they get restored now?

The reason of the numbers is for:

Ordering - which going to be restored in that sequence.
Identifying - so you'll know which components are going to be restored, because you might choose to restore vmail on a restore path that did not backup vmail at all.

Example: If you backup only the crypt, and then you choose to restore the vmail, it will now show up, since there's no vmail in the restore directory you specified.

Aug 27 '24 07:08 h3ssan

Thank you @DerLinkman for the testing process. I have fixed the issues, please do the testing process again!

sorry I know it's noisy but necessary.

Aug 27 '24 08:08 h3ssan

How does this script behave when it is automated? E.g runned via cron?

Especially the part with -d (delete backups) you have to enter YES to confirm deletion?

Aug 27 '24 09:08 DerLinkman

How does this script behave when it is automated? E.g runned via cron?

Especially the part with -d (delete backups) you have to enter YES to confirm deletion?

For now, it won't. In the next feat PR, I'll make it automated.

Aug 27 '24 09:08 h3ssan

For now, it won't. In the next feat PR, I'll make it automated.

That makes it hard for us to track... as we always take all staging commits into the next update and we await the full functionality of that. If you say that the automation is not implemented yet inside this pr, then we won't merge it... i'm sorry. As this is heavily used by other users.

In general: Why did you want to make multiple prs instead of one which includes all of your changes, even the new additions? I did not get it.

Aug 27 '24 09:08 DerLinkman

For now, it won't. In the next feat PR, I'll make it automated.

That makes it hard for us to track... as we always take all staging commits into the next update and we await the full functionality of that. If you say that the automation is not implemented yet inside this pr, then we won't merge it... i'm sorry. As this is heavily used by other users.

In general: Why did you want to make multiple prs instead of one which includes all of your changes, even the new additions? I did not get it.

Then, If something happened unexpected, we will revert everything instead of the PR that has the problem. Please correct me if I am wrong.

As well, many features inside one PR will make the testing harder.

If you resist, then it's ok for me. Please confirm that you need the full implementation inside this PR and I'll start make the script fully automated.

Aug 27 '24 09:08 h3ssan

I definitely unterstand your doing. I prefer this. But currently your backup script is removing a automated part. So yes: Please readd the automation into your scripts remake PR.

Aug 27 '24 10:08 DerLinkman

The script now can be automated using --yes flag.

Example:

./helper-scripts/backup_and_restore.sh --delete /opt/backup 7 --yes

Please note that since --yes flag is critical and won't ask for any confirmation, therefore there will not be any short flag for it. Only --yes.

Aug 27 '24 15:08 h3ssan

See comment (only one :D) rest looks fine to me.

However, we need to adjust the mailcow docs too. Do you want to do it by yourself or should we adapt it?

Just asking, that we don't do the same work parallel :D

Sure, I’ll do adjust the docs!

Aug 29 '24 10:08 h3ssan

@MAGICCC could you try to test this too? It works and looks good for me but if a seperate user can take a look at this i would be happy

Sep 10 '24 15:09 DerLinkman

Yes I will check on the weekend!

Sep 10 '24 15:09 MAGICCC

Yes I will check on the weekend!

Thank you!

Sep 10 '24 15:09 h3ssan

Would be awesome to include this as well: https://github.com/mailcow/mailcow-dockerized/pull/4833

Sep 17 '24 13:09 mrclschstr

Would be awesome to include this as well: #4833

@h3ssan could you adapt this in your next step (adding new features)?

Oct 15 '24 09:10 DerLinkman

Would be awesome to include this as well: #4833

@h3ssan could you adapt this in your next step (adding new features)?

I went through the PR, interesting. I will implement it later (after completing this step).

Oct 15 '24 14:10 h3ssan

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Jan 15 '25 00:01 milkmaker

Can this be reopened?

Feb 19 '25 07:02 mrclschstr

Can this be reopened?

This pull request cannot be reopened, as we have decided to keep backup_and_restore.sh as a legacy component. However, the good news is that we will develop a new solution with better options and capability.

Feb 19 '25 07:02 h3ssan

kompose kompose copied to clipboard

[completion] Add header #compdef

Contribution Guidelines

What does this PR include?

Short Description

Affected Containers

Did you run tests?

What did you tested?

What were the final results? (Awaited, got)

Clarification

Original Code

Clarification

Features

Enhancement

kompose
kompose copied to clipboard