Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storage: Add support for storage bucket backup (from Incus) #13924

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

boltmark
Copy link
Contributor

@boltmark boltmark commented Aug 12, 2024

This PR adds support for storage bucket backups. It includes cherry-picks from lxc/incus#365, and the below description comes in part from the associated incus PR.

Description

API

The following endpoints are added, along with the storage_bucket_backup extension:

GET /1.0/storage-pools/<pool>/buckets/<bucket>/backups

POST /1.0/storage-pools/<pool>/buckets/<bucket>/backups

GET /1.0/storage-pools/<pool>/buckets/<bucket>/backups/<name>

POST /1.0/storage-pools/<pool>/buckets/<bucket>/backups/<name>

DELETE /1.0/storage-pools/<pool>/buckets/<bucket>/backups/<name>

GET /1.0/storage-pools/<pool>/buckets/<bucket>/backups/<name>/export

Export

CLI

$ lxc storage bucket export
Description:
  Export storage buckets as tarball.

Usage:
  lxc storage bucket export [<remote>:]<pool> <bucket> [<path>] [flags]

Examples:
  lxc storage bucket default b1
      Download a backup tarball of the b1 storage bucket.

Flags:
      --compression   Define a compression algorithm: for backup or none
      --target        Cluster member name

Global Flags:
      --debug          Show all debug messages
      --force-local    Force using the local unix socket
  -h, --help           Print help
      --project        Override the source project
  -q, --quiet          Don't show progress information
      --sub-commands   Use with help or --help to view sub-commands
  -v, --verbose        Show all information messages
      --version        Print version number
$ lxc storage bucket export default bucket0
Backup exported successfully!                      

Archive

The export command creates a tarball from the defined bucket, which is structured as follows:

bucket0.tar.gz
├── backup
│   └── index.yaml
│   └── bucket
    │   └── file1
    │   └── file2
  • The bucket directory contains the actual data
  • backup.yml contains the bucket metadata and keys:
name: bucket0
backend: dir
pool: default
config:
  bucket:
    config: {}
    description: ""
    name: bucket0
    s3_url: ""
    location: none
  bucket_keys:
  - description: Admin user
    role: admin
    access-key: OO9O3OXLJWMED0W8P2I5
    secret-key: Ny4W9glDRFks7DzxlPQibREOAUSc1De9Yv0mOkuY
    name: admin
  - description: ""
    role: read-only
    access-key: JIY5PI6RPQWP7CAKJN57
    secret-key: APC95jzb9u7XUKw7J7AWAdVnaNVbjqj5dOLddkhT
    name: reader

Import

CLI

$ lxc storage bucket import
Description:
  Import backups of storage buckets.

Usage:
  lxc storage bucket import [<remote>:]<pool> <backup file> [<bucket>] [flags]

Examples:
  lxc storage bucket import default backup0.tar.gz
                Create a new storage bucket using backup0.tar.gz as the source.

Flags:
      --target   Cluster member name

Global Flags:
      --debug          Show all debug messages
      --force-local    Force using the local unix socket
  -h, --help           Print help
      --project        Override the source project
  -q, --quiet          Don't show progress information
      --sub-commands   Use with help or --help to view sub-commands
  -v, --verbose        Show all information messages
      --version        Print version number
$ lxc storage bucket import default backup.tar.gz bucket0
Backup exported successfully!                      

Design decisions of note

  • One of the existing bucket keys is used by default, as everyone can at least read the bucket.
  • A TransferManager struct is implemented, which utilizes MinIO to handle downloading files from a bucket to create the backup, and uploading files to a bucket when creating a bucket from a backup.

Overall, what's been changed

  • The command storage bucket export and storage bucket import has been added to the command line interface (CLI).
  • New APIs added
  • Added new functionality for performing storage bucket backups and cleaning up expired backups
  • Added new functionality for importing storage bucket backups
  • Updated API and CLI documentation

@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Aug 12, 2024
@boltmark boltmark changed the title Storage: Add support for storage buckets backup Storage: Add support for storage bucket backup Aug 12, 2024
@boltmark boltmark changed the title Storage: Add support for storage bucket backup Storage: Add support for storage bucket backup (from Incus) Aug 12, 2024
lxd/backup.go Fixed Show fixed Hide fixed
lxd/backup.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
IdleConnTimeout: 30 * time.Second,
DisableCompression: true,
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,

Check failure

Code scanning / CodeQL

Disabled TLS certificate check High

InsecureSkipVerify should not be used in production code.
@boltmark boltmark force-pushed the storage-buckets-backup branch from 08a58ba to 8fdf38f Compare August 15, 2024 16:15
Copy link

Heads up @mionaalex - the "Documentation" label was applied to this issue.

@boltmark boltmark force-pushed the storage-buckets-backup branch 4 times, most recently from 19cae2b to 8b2a40c Compare August 16, 2024 01:30
@boltmark

This comment was marked as outdated.

@boltmark boltmark force-pushed the storage-buckets-backup branch from 8b2a40c to 94eb9bf Compare August 22, 2024 00:29
@boltmark boltmark force-pushed the storage-buckets-backup branch 6 times, most recently from eb04463 to ccbbddd Compare September 4, 2024 05:44
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
lxd/backup/backup_bucket.go Fixed Show fixed Hide fixed
@boltmark boltmark force-pushed the storage-buckets-backup branch 9 times, most recently from d70a96f to b1894d6 Compare September 5, 2024 15:35
@boltmark boltmark force-pushed the storage-buckets-backup branch 3 times, most recently from a3ece3c to 8a38733 Compare December 10, 2024 00:34
@tomponline
Copy link
Member

please can you rebase

maveonair and others added 21 commits December 15, 2024 21:48
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 21ed02ae159abd398ea623406101f877d4092b59)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 95bfa8881566ab6a66135a4709065d97feeaaa6b)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit b2dbe44d3447c554d9d7e5d4ee855238a7b27c6e)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 8f0061f699db14ba101378ca4314c3ee33afd93c)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit f4b0e4dad87ebc1bd0cc94d9b6b0cdaa0a848020)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit db96183b5d2f728bcf92f65b54e9265a8ff7a5f5)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 5f6dbb7bea79b315797533d792ff81c3a0bc8fbb)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
ValidateBackupName ensures that a backup name does not
contain '..', forward slashes, or  back slashes.

Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit d5b4350adc2f318ed9fdfd1078eaf8e7f00e8f88)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 679ce9355b1f16cbf2a825eb3f0b901b80f35298)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 2e6756328f2e72b1d2b1704a2b2b4373230e2cee)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 98693c062efc05ec500022e7032e63ca96c291ba)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
As is suggested by GitHub Advanced Security, we are
making the validation of backup names a bit more
strict (now checking for '\\' and '..'). We want
to add these checks for all backups to ensure
consistency across LXD.

Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
Signed-off-by: Fabian Mettler <dev@maveonair.com>
(cherry picked from commit 50f73d42057032403b29e5986763d265cfcb5256)
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
License: Apache-2.0
Signed-off-by: Mark Bolton <mark.bolton@canonical.com>
@boltmark boltmark force-pushed the storage-buckets-backup branch from 8a38733 to 97ff9ae Compare December 16, 2024 05:48
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind landing this change and the associated cherry-pick for the function in a separate PR to isolate the new functionality only in this PR. Thanks

func (c *ClusterTx) CreateStoragePoolBucketBackup(ctx context.Context, args StoragePoolBucketBackup) error {
_, err := c.getStoragePoolBucketBackupID(ctx, args.Name)
if err == nil {
return ErrAlreadyDefined
Copy link
Member

Choose a reason for hiding this comment

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

we should use api.StatusErrorf() here like we do in CreateStoragePoolBucketKey then we wont need the new ErrAlreadyDefined var.

 api.StatusErrorf(http.StatusConflict, "A bucket key using that access key already exists on this server")

Then we can check it in the call site using:

api.StatusErrorCheck(err, http.StatusConflict)


// DownloadAllFiles downloads all files from a bucket and writes them to a tar writer.
func (t TransferManager) DownloadAllFiles(bucketName string, tarWriter *instancewriter.InstanceTarWriter) error {
logger.Debugf("Downloading all files from bucket %s", bucketName)
Copy link
Member

Choose a reason for hiding this comment

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

we dont use logger.*f functions, but we can probably just remove these right?

@tomponline
Copy link
Member

please can you rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants