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.

Sign up for GitHub

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

Jump to bottom

WIP Azure Backups #855

Closed
wants to merge 9 commits into from
Closed

WIP Azure Backups #855

wants to merge 9 commits into from

Conversation

ianstanton
Copy link
Member

No description provided.

sjmiller609
sjmiller609 reviewed Jun 26, 2024
View reviewed changes
conductor/src/main.rs
Comment on lines +729 to +739
if cloud_provider == "azure" {
let write_path = read_msg
.message
.backups_write_path
.clone()
.unwrap_or(format!("{}", read_msg.message.namespace));
destination_path = format!(
"https://{}.blob.core.windows.net/{}/{}",
azure_storage_account, backup_archive_bucket, write_path
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may need a step to provision in Azure the permissions for the workload required for this instance to access the bucket where its backups go

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Yep we will need that

sjmiller609
sjmiller609 reviewed Jun 26, 2024
View reviewed changes
tembo-operator/src/apis/coredb_types.rs
Comment on lines +92 to +94
/// AzureCredentials is the type for the credentials to be used to upload files to Azure Blob Storage.
#[derive(Serialize, Deserialize, Clone, Debug, Default, JsonSchema)]
pub struct AzureCredentials {
Copy link
Contributor

Choose a reason for hiding this comment

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

are these types already present in the cnpg file, or could be kopium imported from the CNPG crd

Copy link
Member Author

@ianstanton ianstanton Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes they are already present. The S3Credentials values (right below this section) are also present there, but it looks like we copied them in and renamed slightly. I followed the same pattern here. Happy to go with either approach

@ianstanton
Copy link
Member Author

Dry run

    backup:
      azureCredentials:
        storageAccount:
          key: ""
          name: iantembobackups
      destinationPath: https://iantembobackups.blob.core.windows.net/tembo-backups/v2/merrily-copious-herring
      encryption: AES256
      endpointURL: null
      retentionPolicy: "30"
      s3Credentials: null
      schedule: 9 9 * * *
      volumeSnapshot:
        enabled: false

@ianstanton
Copy link
Member Author

Closing for now but coming back to this soon :)

@ianstanton ianstanton closed this Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjmiller609 sjmiller609 sjmiller609 left review comments

@nhudson nhudson Awaiting requested review from nhudson nhudson will be requested when the pull request is marked ready for review nhudson is a code owner

@ChuckHend ChuckHend Awaiting requested review from ChuckHend ChuckHend will be requested when the pull request is marked ready for review ChuckHend is a code owner

@vrmiguel vrmiguel Awaiting requested review from vrmiguel vrmiguel will be requested when the pull request is marked ready for review vrmiguel is a code owner

@shahadarsh shahadarsh Awaiting requested review from shahadarsh shahadarsh will be requested when the pull request is marked ready for review shahadarsh is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ianstanton @sjmiller609