You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "jmsperu (via GitHub)" <gi...@apache.org> on 2026/03/26 22:29:16 UTC
[PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
jmsperu opened a new pull request, #12900:
URL: https://github.com/apache/cloudstack/pull/12900
## Summary
Adds a new `InfrastructureBackupTask` to the NAS backup plugin that performs daily backups of CloudStack infrastructure (database, management/agent configs, SSL certs) to NAS storage.
## Problem
CloudStack's NAS backup provider only backs up VM disks. The management server database, agent configurations, SSL certificates, and global settings are not backed up. If the management server fails, all metadata is lost unless someone manually configured mysqldump cron.
## Solution
A new background poll task that automatically backs up:
1. **MySQL databases** (`cloud` + optionally `cloud_usage`) using `mysqldump --single-transaction` for InnoDB-consistent hot backups
2. **Management server configs** (`/etc/cloudstack/management/`)
3. **Agent configs** (`/etc/cloudstack/agent/`) if present
4. **SSL certificates** (`/etc/cloudstack/management/cert/`) if present
5. **Automatic retention** management (removes old backup sets)
Database credentials are read from `/etc/cloudstack/management/db.properties` at runtime (no secrets stored in global config).
### Configuration
| Setting | Scope | Default | Description |
|---------|-------|---------|-------------|
| `nas.infra.backup.enabled` | Global | `false` | Master switch for infrastructure backup |
| `nas.infra.backup.location` | Global | (empty) | NAS mount path (e.g. `/mnt/nas-backup`) |
| `nas.infra.backup.retention` | Global | `7` | Number of backup sets to keep |
| `nas.infra.backup.include.usage.db` | Global | `true` | Include `cloud_usage` database |
### Backup Structure
```
/mnt/nas-backup/infra-backup/
├── 20260327-020000/
│ ├── cloud-20260327-020000.sql.gz
│ ├── cloud_usage-20260327-020000.sql.gz
│ ├── management-config.tar.gz
│ ├── agent-config.tar.gz
│ └── ssl-certs.tar.gz
├── 20260326-020000/
│ └── ...
```
### Changes
- **New:** `InfrastructureBackupTask.java` - background task implementing `BackgroundPollTask`
- **Modified:** `NASBackupProvider.java` - added 4 ConfigKeys, scheduled the backup task
## Test plan
- [ ] Enable backup framework and set provider to NAS
- [ ] Configure `nas.infra.backup.enabled=true` and `nas.infra.backup.location=/mnt/test-backup`
- [ ] Verify backup directory structure is created
- [ ] Verify `cloud-*.sql.gz` database dump is created and restorable
- [ ] Verify `management-config.tar.gz` contains `/etc/cloudstack/management/` files
- [ ] Verify `agent-config.tar.gz` is created only when agent config dir exists
- [ ] Set retention=2, trigger multiple cycles, verify old backups are cleaned up
- [ ] Verify task is a no-op when disabled
- [ ] Verify task handles missing `db.properties` gracefully
- [ ] Verify `cloud_usage` backup is included/excluded based on config
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4762925426
Thanks for the review @DaanHoogland (and the Copilot pass). Pushed `5b82495` addressing the open points:
- **Command injection / password in process list** (`backupDatabase`): `mysqldump` now runs without a shell (`ProcessBuilder` arg list) and reads credentials from a `0600` `--defaults-extra-file`, so values from `db.properties` can't be shell-interpreted and the password no longer appears in the process list. The dump is gzipped in-process.
- **`ArrayIndexOutOfBoundsException` on negative retention** (`cleanupOldBackups`): clamped to `0`.
- **Symlink-following delete** (`deleteDirectory`): rewritten with `Files.walkFileTree` (no symlink following) so a symlink inside the NAS tree can't cause deletion outside the backup path.
- **Concurrent backups across management servers** (`NASBackupProvider.submitTask` / `runInContext`): now guarded by a cluster-wide `GlobalLock` (one MS at a time); acquisition is an overridable seam so it stays unit-testable without the DB lock.
- `dir.mkdirs()` now only runs when the directory doesn't already exist.
Also extended `InfrastructureBackupTaskTest` with retention/delete coverage (negative-retention clamp, keeps-newest/deletes-oldest, and symlink-escape safety — the last two fail against the previous code). The refactor into a shared `backupDirectory` helper is already in the current revision.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4547333972
Hi @DaanHoogland @sureshanaparti @kiranchavala — this one's been green for a while now. Adds an automated NAS infrastructure backup (mgmt server db, configs, certs) on top of the existing NAS backup framework. CI is clean, no merge conflicts.
Could one of you take a look when you have a moment? Happy to clarify anything about the design choices in the PR description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5424434997
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4398237609
Code LGTM. However, I wonder whether backing up the CloudStack database from within CloudStack itself is the right approach.
In most deployments, database backups are usually handled externally by administrators (e.g., via cron jobs or existing backup tooling). That approach may be simpler and more flexible operationally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5414199471
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r4336718649
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,428 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials come from the management server's db.properties (via {@link DbProperties},
+ * which decrypts ENC(...) values when db.properties encryption is in use).
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error(describeDirectoryCreationFailure(backupDir, nasBackupPath));
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+ backupDirectoryIfPresent(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ backupDirectoryIfPresent(SSL_CERT_PATH, backupDir, "ssl-certs");
+ cleanupOldBackups(infraBackupRoot, retentionCount);
Review Comment:
that covers it, thanks
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,428 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials come from the management server's db.properties (via {@link DbProperties},
+ * which decrypts ENC(...) values when db.properties encryption is in use).
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error(describeDirectoryCreationFailure(backupDir, nasBackupPath));
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
Review Comment:
that covers it, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "abh1sar (via GitHub)" <gi...@apache.org>.
abh1sar commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4550380537
Hi @jmsperu you have lots of open PRs around improving the NAS provider.
While your contribution is very much appreciated, due diligence is needed in reviewing and testing the PRs, so that they adhere to the coding standard, the code remains easy to read and maintainable and doesn't break the existing functionality.
Since we have limited resources, we need to look at the PRs in a priority order.
Currently I am actively reviewing the incremental backup PR. IMO this PR is low priority. So will get to it later.
We will get to all your PRs, but it might take some time. So please be patient.
In the meantime, may I ask you, on all your open PRs, to
1. Answer any pending copilot review comments
2. Test the steps mentioned in the TestPlan (not looking for logs, but just a confirmation that they have been tested. For example, the test cases in the test plan mentioned in this PR are unchecked)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4552441905
Thanks @abh1sar — completely understood on the prioritisation, and no rush from me. Happy for this one to sit until #13074 is through your queue.
In the meantime, on all my open NAS-provider PRs I'll:
1. Sweep open Copilot threads and respond inline (resolving where addressed, explaining where the suggestion isn't quite right).
2. Walk each TestPlan and tick the boxes I can legitimately confirm — distinguishing CI-verified items (build, unit tests) from manual cases I've reproduced in my lab; the rest stay unchecked until I genuinely run them.
I'll scope the activity to test-plan confirmations + Copilot replies (no rebases / no force-pushes that would re-trigger reviewers). If you'd prefer I close any of these as low-priority and re-open after #13074 lands, say the word and I'll do that instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r4196851740
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,414 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+ backupDirectoryIfPresent(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ backupDirectoryIfPresent(SSL_CERT_PATH, backupDir, "ssl-certs");
+ cleanupOldBackups(infraBackupRoot, retentionCount);
+
+ LOG.info("Infrastructure backup completed successfully: {}", backupDir);
+
+ } catch (Exception e) {
+ LOG.error("Infrastructure backup failed: {}", e.getMessage(), e);
+ } finally {
+ releaseRunLock(lock);
+ }
+ }
+
+ /**
+ * Name of the sub-directory that keeps this management server's backups apart from those of the
+ * other servers in the cluster. Management configs and certificates are per server, so they must
+ * not overwrite each other, and the retention count applies per server. Uses the host name and
+ * falls back to the management server id.
+ */
+ protected String getManagementServerLabel() {
+ String label = null;
+ try {
+ label = InetAddress.getLocalHost().getHostName();
+ } catch (UnknownHostException e) {
+ LOG.debug("Could not determine the local host name for the infrastructure backup directory: {}", e.getMessage());
+ }
+ if (label == null || label.isBlank()) {
+ label = "ms-" + ManagementServerNode.getManagementServerId();
+ }
+ return label.replaceAll("[^A-Za-z0-9._-]", "_");
+ }
+
+ /**
+ * Dumps the cloud database, and the usage database when requested, into {@code backupDir}.
+ * The database component is opt-in ({@code nas.infra.backup.include.database}): production
+ * deployments typically run their own mysqldump jobs and leave it off; it exists for small and
+ * edge deployments that want unified disaster recovery on the same NAS as their VM backups.
+ */
+ protected void backupDatabases(String backupDir, String timestamp, boolean includeDatabase, boolean includeUsageDb) {
+ if (!includeDatabase) {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ return;
+ }
+ Properties dbProps = loadDbProperties();
Review Comment:
Thanks for testing with an encrypted db.properties. You were right: the task read the file directly, so mysqldump got the `ENC(...)` value. Fixed in 1b35a7be70: `loadDbProperties()` now uses `DbProperties.getDbProperties()`, which decrypts the values the same way the management server does for its own connection pool (and finds db.properties the same way). The new test `encryptedDbPasswordIsDecryptedBeforeMysqldump` encrypts a password with `EncryptionSecretKeyChecker` and checks that the decrypted value reaches the dump for both `cloud` and `cloud_usage`. `missingDbPropertiesSkipsOnlyTheDatabase` covers the failure case: configs, certificates and retention still run.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -85,6 +86,59 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
true,
BackupFrameworkEnabled.key());
+ static final ConfigKey<Boolean> NASInfraBackupEnabled = new ConfigKey<>("Advanced", Boolean.class,
+ "nas.infra.backup.enabled",
+ "false",
+ "Enable automated infrastructure backup to NAS storage. When enabled, the management " +
+ "server will perform a daily backup of CloudStack configuration files and SSL " +
+ "certificates to the configured NAS location. The CloudStack database is NOT included " +
+ "by default — for production deployments, manage database backups externally (e.g. via " +
+ "a cron job running mysqldump). To opt in to bundling the database with this backup " +
+ "(useful only for small / edge / single-MS deployments without separate ops tooling), " +
+ "also set nas.infra.backup.include.database=true.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<Boolean> NASInfraBackupIncludeDatabase = new ConfigKey<>("Advanced", Boolean.class,
+ "nas.infra.backup.include.database",
+ "false",
+ "Include the CloudStack database in the daily infrastructure backup. Defaults to false " +
+ "because production deployments typically manage DB backups via external tooling (e.g. " +
+ "cron + mysqldump, replication, dedicated backup appliance) and are better served " +
+ "doing so. Only set true when you want one-knob disaster recovery for a small/edge " +
+ "deployment and the same NAS that already holds your VM backups is an acceptable " +
+ "target. Has no effect unless nas.infra.backup.enabled is also true.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<String> NASInfraBackupLocation = new ConfigKey<>("Advanced", String.class,
+ "nas.infra.backup.location",
+ "",
+ "NAS mount path where infrastructure backups are stored (e.g. /mnt/nas-backup). " +
+ "Backups will be written to {location}/infra-backup/{timestamp}/.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<Integer> NASInfraBackupRetention = new ConfigKey<>("Advanced", Integer.class,
+ "nas.infra.backup.retention",
+ "7",
+ "Number of infrastructure backup sets to retain. Older backups are automatically removed.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<Boolean> NASInfraBackupUsageDb = new ConfigKey<>("Advanced", Boolean.class,
+ "nas.infra.backup.include.usage.db",
+ "true",
Review Comment:
Yes, that one is intentional. `nas.infra.backup.include.usage.db` only has an effect once `nas.infra.backup.include.database` is turned on, and that stays false by default (it was made opt-in earlier in this PR after @weizhouapache's review, since production setups usually back up the database with their own tooling). An operator who opts in to the database dump usually wants `cloud_usage` with it, so the sub-option defaults to true; both descriptions state the dependency. Happy to flip it to false if you prefer every part to be explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4624604273
Looking for an initial reviewer on this one when someone has bandwidth 🙏
Quick context: it adds automated infrastructure-level backup (CloudStack management DB + `cloud-secret` + ACS TLS certificates) on top of the existing per-VM backup story. Bootstrap-time backup that gives you a single tarball you can restore from when a mgmt server is rebuilt from scratch.
CI is green and the diff is contained to a new optional `bin/cloudstack-infra-backup.sh` + a packaging hook — no changes to the existing volume/backup hot paths. Happy to address any feedback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5452728893
Thanks for running it, @DaanHoogland.
**On the Trillian result (tid-16825): 148 OK, 1 error.** The failure is `test_01_redundant_vpc_site2site_vpn` in `test_vpc_vpn.py` — redundant-VPC site-to-site VPN. It does not touch the backup framework or this plugin, and this PR changes only `InfrastructureBackupTask`, `NASBackupProvider` and their tests. Unrelated failure.
**The open review threads are all addressed at `3fa88ea`** — they just were not resolved:
- *Negative `retentionCount` causing `ArrayIndexOutOfBoundsException`* — clamped at the top of `cleanupOldBackups`, so `toDelete` can never exceed `backups.length`.
- *Task scheduled on every management server* — serialised by `GlobalLock("infra-backup")`, and each server now writes to its own `<location>/infra-backup/<ms-hostname>/<timestamp>` directory with retention applied per server, so concurrent runs neither collide nor delete each other's backups.
- *Extract the repeated backup blocks into helpers, drop the prelude comments* — done as `backupDatabases(...)`, `backupDirectoryIfPresent(...)` and `backupDirectory(...)`; the comments came out with them.
Could you resolve them if you are happy, or say what still reads wrong. `@abh1sar`, this is the one Daan asked you to look at.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-6019007483
@Pearl1594 thank you for testing this on a real setup; both problems were real. Fixed in 1b35a7be70:
- Encrypted DB password: credentials now come from `DbProperties.getDbProperties()` (details in the thread).
- Backup location ownership: when the directory cannot be created, the log now says "Failed to create infrastructure backup directory <dir>. The management server runs as user 'cloud', which needs write access to <location> (nas.infra.backup.location); for example, run 'chown cloud <location>' on the management server after mounting the NAS there. Also check that the NAS is mounted at that path." The user name is taken from the running process, not hard-coded. The `nas.infra.backup.location` description now states that the path must be writable by the management server user, and gives the per-server layout.
Local results: InfrastructureBackupTaskTest 17/17, NASBackupProviderTest 21/21, checkstyle clean.
Agreed on the documentation. I have prepared a section for the NAS plugin page in cloudstack-documentation (settings, on-NAS layout, the `cloud` user's write permission with a mount and chown example, and the opt-in database dump) and will open that PR shortly and link it here.
On the failing `triage` check: that is the "PR Merge Conflict Check" workflow. Its log ends with `error removing "status:has-conflicts": HttpError: Resource not accessible by integration`, i.e. its token cannot edit labels when the run comes from a fork. It is not related to the change, and the branch has no conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-6132403017
Rebased on the current 4.22 (head now b1baa57bcb, the same five commits). The one conflict was in NASBackupProvider against "Nasbackup quiesce fixes" (#14305) and earlier 4.22 changes: kept both sides, so the incremental and quiesce timeout settings sit next to the four nas.infra.backup.* settings, BackgroundPollManager is injected after the new BackupDetailsDao and AlertManager, and getConfigKeys() lists all of them. InfrastructureBackupTask and its test are unchanged. NAS plugin builds with checkstyle; InfrastructureBackupTaskTest 17/17 and NASBackupProviderTest 23/23 pass. Ready for review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "daviftorres (via GitHub)" <gi...@apache.org>.
daviftorres commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4556146596
Dear @jmsperu, this is an excellent idea. Many ACS deployments lack thorough backups for real disaster recovery situations.
Here is our current approach for anyone looking for alternative methods:
- We run backups from a read-only database replica to avoid impacting performance.
- A script runs continuously (as a service) in a loop with a 15-minute sleep (instead of a cronjob) to prevent overlapping execution.
- Script:
- Dumps each database sequentially (never written to disk).
- Pipes the dump directly into gpg for PGP encryption.
- Uploads the file using a write-only API key for security.
- Credentials and keys are loaded as environment variables on service startup.
- Uploads alternate between three different S3 buckets across North America (West, North, and East) in a round-robin fashion. Each bucket has versioning and lifecycle policies enabled.
Example of the line that dumps, encrypts, and uploads backups:
```bash
mysqldump --no-tablespaces --lock-tables=false -R cloud | gpg -r "<PUB_KEY_ID>" --yes --encrypt - | mc pipe --quiet <BUCKET>/acs/cloud.sql.pgp || success=false
```
I hope it helps someone!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r3411664076
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
+
+ // 6. Cleanup old backups based on retention policy
Review Comment:
I think this comment does not add value to the call on the line below.
```suggestion
```
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
Review Comment:
```suggestion
backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
```
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
Review Comment:
this if-else in an extra methos and the prelude comment as javadoc?
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
Review Comment:
same as below, maybe even jus one utility with an extra parameter containing “agent-config” or "ssl-certs"
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
Review Comment:
small extra method , e.g. `backupSslCertificates(String SSL_CERTPATH, String backupDir)`. comment does not seem needed.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -129,6 +184,16 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
@Inject
private DiskOfferingDao diskOfferingDao;
+ @Inject
+ private BackgroundPollManager backgroundPollManager;
+
+ @Override
+ public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
+ super.configure(name, params);
+ backgroundPollManager.submitTask(new InfrastructureBackupTask(this));
+ return true;
Review Comment:
@jmsperu , this one can be quite serious as well. How to prevent for instance two MSses backing up the DB at the same time, or if we allow them, making sure they don’t use the same timestamp/back-up-ordinal?
Also when retention is low, what happens if retention is 2 but we have three MSses?
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
+
+ // 6. Cleanup old backups based on retention policy
+ cleanupOldBackups(nasBackupPath, retentionCount);
+
+ LOG.info("Infrastructure backup completed successfully: {}", backupDir);
+
+ } catch (Exception e) {
+ LOG.error("Infrastructure backup failed: {}", e.getMessage(), e);
+ }
+ }
+
+ protected Properties loadDbProperties() {
+ File propsFile = new File(DB_PROPERTIES_PATH);
+ if (!propsFile.exists()) {
+ LOG.warn("Database properties file not found: {}", DB_PROPERTIES_PATH);
+ return null;
+ }
+
+ Properties props = new Properties();
+ try (BufferedReader reader = new BufferedReader(new FileReader(propsFile))) {
+ props.load(reader);
+ return props;
+ } catch (IOException e) {
+ LOG.error("Failed to read database properties: {}", e.getMessage());
+ return null;
+ }
+ }
+
+ protected void backupDatabase(String dbName, String backupDir, String timestamp,
+ String dbHost, String dbUser, String dbPassword) {
+ String dumpFile = backupDir + "/" + dbName + "-" + timestamp + ".sql.gz";
+
+ // Use --single-transaction for InnoDB hot backup (no table locks, consistent snapshot)
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("mysqldump --single-transaction --routines --triggers --events " +
+ "-h '%s' -u '%s' -p'%s' '%s' | gzip > '%s'",
+ dbHost, dbUser, dbPassword, dbName, dumpFile)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(300, TimeUnit.SECONDS);
+
+ if (!completed) {
+ process.destroyForcibly();
+ LOG.error("Database backup timed out for {}", dbName);
+ return;
+ }
+
+ if (process.exitValue() != 0) {
+ LOG.error("Database backup failed for {} with exit code {}", dbName, process.exitValue());
+ return;
+ }
+
+ File dump = new File(dumpFile);
+ LOG.info("Database {} backed up: {} ({} bytes)", dbName, dumpFile, dump.length());
+
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup database {}: {}", dbName, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void backupDirectory(String sourcePath, String backupDir, String archiveName) {
+ File source = new File(sourcePath);
+ if (!source.exists() || !source.isDirectory()) {
+ LOG.debug("Directory {} does not exist, skipping", sourcePath);
+ return;
+ }
+
+ String tarFile = backupDir + "/" + archiveName + ".tar.gz";
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("tar czf '%s' -C '%s' .", tarFile, sourcePath)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(60, TimeUnit.SECONDS);
+
+ if (completed && process.exitValue() == 0) {
+ LOG.info("Directory {} backed up to {}", sourcePath, tarFile);
+ } else {
+ if (!completed) {
+ process.destroyForcibly();
+ }
+ LOG.warn("Directory backup failed for {} (exit code: {})",
+ sourcePath, completed ? process.exitValue() : "timeout");
+ }
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup directory {}: {}", sourcePath, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void cleanupOldBackups(String nasBackupPath, int retentionCount) {
+ File infraDir = new File(nasBackupPath + "/infra-backup");
+ if (!infraDir.exists()) {
+ return;
+ }
+
+ File[] backups = infraDir.listFiles(File::isDirectory);
+ if (backups == null || backups.length <= retentionCount) {
+ return;
+ }
+
+ // Sort by name (timestamp-based), oldest first
+ Arrays.sort(backups, Comparator.comparing(File::getName));
+
+ int toDelete = backups.length - retentionCount;
Review Comment:
co-pilot has a point here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5414309414
@abh1sar can you have a look as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-6122905134
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4411314364
@weizhouapache fair point — the intent here was never to replace external/operational DB backup tooling, but to give small and edge deployments a one-knob "backup the management plane to the same NAS that already holds my VM backups" option for disaster recovery (rebuild from a fresh management server using only what's on the NAS).
Concrete adjustments I can make:
1. Make the DB component opt-in via an explicit `infrastructure.backup.include.database` global setting, defaulting to **false** — so by default we only ship configs + certs (where there's no real ops alternative anyway).
2. Document explicitly that for production deployments with existing backup tooling, the DB component should stay disabled and be handled externally.
3. Keep the unified target (same NAS as VM backups) for the configs+certs path so recovery is in one place.
Does that split land closer to where you'd want it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4523012657
Pushed the scope reduction discussed above.
- New `nas.infra.backup.include.database` global, **default false** — DB component is fully gated on it. Configs + certs path runs regardless.
- Updated `nas.infra.backup.enabled` description to make the new split explicit and steer production users to the cron-job approach for the DB.
- Added `InfrastructureBackupTaskTest` (9 tests, all passing locally) covering the gating decisions — should materially close the codecov gap @weizhouapache flagged.
- Refactored a few methods to `protected` so the side-effecting paths (mysqldump, tar) can be overridden in tests without standing up real ProcessBuilders.
Ready for another look. If you'd still rather drop the DB component entirely and ship configs+certs only, happy to do that in a follow-up — let me know.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5414900269
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18981
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r3844112703
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -129,6 +184,16 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
@Inject
private DiskOfferingDao diskOfferingDao;
+ @Inject
+ private BackgroundPollManager backgroundPollManager;
+
+ @Override
+ public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
+ super.configure(name, params);
+ backgroundPollManager.submitTask(new InfrastructureBackupTask(this));
+ return true;
Review Comment:
Good challenge, and the two halves have different answers.
Concurrency: a cluster-wide `GlobalLock("infra-backup")` was added in 5b82495, so two management servers never run the task at the same time; the loser logs that another server is performing the backup and skips that run.
Collisions and retention across servers: that was still wrong, and 3fa88ea fixes it. The management-config and certificate archives are per server (each MS has its own /etc/cloudstack/management and cert directory), so writing them into one shared tree meant the last server to run would win and the others' configs would rotate out. Backups now go to `<location>/infra-backup/<ms-hostname>/<timestamp>` (falling back to the management server id when the hostname isn't available) and `cleanupOldBackups()` is applied to that per-server directory. So with retention 2 and three MSes you keep two backups per server, six in total, and no server ever deletes another's. The timestamp only needs to be unique within a server's own directory now.
The database dump is opt-in and, when enabled, each server's run includes it, so an N-server cluster produces N dumps a day. I think that is acceptable for the small deployments this switch is aimed at and it is documented as such; if you'd rather have a single cluster-wide dump per day I can add a "newest dump younger than 24h exists" check, but I've kept it simple for now.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
+
+ // 6. Cleanup old backups based on retention policy
+ cleanupOldBackups(nasBackupPath, retentionCount);
+
+ LOG.info("Infrastructure backup completed successfully: {}", backupDir);
+
+ } catch (Exception e) {
+ LOG.error("Infrastructure backup failed: {}", e.getMessage(), e);
+ }
+ }
+
+ protected Properties loadDbProperties() {
+ File propsFile = new File(DB_PROPERTIES_PATH);
+ if (!propsFile.exists()) {
+ LOG.warn("Database properties file not found: {}", DB_PROPERTIES_PATH);
+ return null;
+ }
+
+ Properties props = new Properties();
+ try (BufferedReader reader = new BufferedReader(new FileReader(propsFile))) {
+ props.load(reader);
+ return props;
+ } catch (IOException e) {
+ LOG.error("Failed to read database properties: {}", e.getMessage());
+ return null;
+ }
+ }
+
+ protected void backupDatabase(String dbName, String backupDir, String timestamp,
+ String dbHost, String dbUser, String dbPassword) {
+ String dumpFile = backupDir + "/" + dbName + "-" + timestamp + ".sql.gz";
+
+ // Use --single-transaction for InnoDB hot backup (no table locks, consistent snapshot)
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("mysqldump --single-transaction --routines --triggers --events " +
+ "-h '%s' -u '%s' -p'%s' '%s' | gzip > '%s'",
+ dbHost, dbUser, dbPassword, dbName, dumpFile)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(300, TimeUnit.SECONDS);
+
+ if (!completed) {
+ process.destroyForcibly();
+ LOG.error("Database backup timed out for {}", dbName);
+ return;
+ }
+
+ if (process.exitValue() != 0) {
+ LOG.error("Database backup failed for {} with exit code {}", dbName, process.exitValue());
+ return;
+ }
+
+ File dump = new File(dumpFile);
+ LOG.info("Database {} backed up: {} ({} bytes)", dbName, dumpFile, dump.length());
+
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup database {}: {}", dbName, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void backupDirectory(String sourcePath, String backupDir, String archiveName) {
+ File source = new File(sourcePath);
+ if (!source.exists() || !source.isDirectory()) {
+ LOG.debug("Directory {} does not exist, skipping", sourcePath);
+ return;
+ }
+
+ String tarFile = backupDir + "/" + archiveName + ".tar.gz";
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("tar czf '%s' -C '%s' .", tarFile, sourcePath)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(60, TimeUnit.SECONDS);
+
+ if (completed && process.exitValue() == 0) {
+ LOG.info("Directory {} backed up to {}", sourcePath, tarFile);
+ } else {
+ if (!completed) {
+ process.destroyForcibly();
+ }
+ LOG.warn("Directory backup failed for {} (exit code: {})",
+ sourcePath, completed ? process.exitValue() : "timeout");
+ }
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup directory {}: {}", sourcePath, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void cleanupOldBackups(String nasBackupPath, int retentionCount) {
+ File infraDir = new File(nasBackupPath + "/infra-backup");
+ if (!infraDir.exists()) {
+ return;
+ }
+
+ File[] backups = infraDir.listFiles(File::isDirectory);
+ if (backups == null || backups.length <= retentionCount) {
+ return;
+ }
+
+ // Sort by name (timestamp-based), oldest first
+ Arrays.sort(backups, Comparator.comparing(File::getName));
+
+ int toDelete = backups.length - retentionCount;
Review Comment:
Done in 5b82495: retention is clamped to 0 when negative, with a unit test (cleanupClampsNegativeRetentionInsteadOfThrowing).
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
Review Comment:
Done in 3fa88ea: the field and the constructor parameter are gone; the task is constructed with no arguments.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
Review Comment:
Done: it is now `backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb)` with the rationale as javadoc.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
Review Comment:
Done, the numbered comments are gone.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
Review Comment:
Done: `backupDirectoryIfPresent(path, backupDir, archiveName)` is used for both agent-config and ssl-certs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4140304039
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/12900?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 3.70%. Comparing base ([`c1af36f`](https://app.codecov.io/gh/apache/cloudstack/commit/c1af36f8fc9bcba6480f7f0daf2542c991f75e18?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`6a8e1d1`](https://app.codecov.io/gh/apache/cloudstack/commit/6a8e1d1ac4f74b975a6429186e18943f2728179e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (c1af36f) and HEAD (6a8e1d1). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (c1af36f) | HEAD (6a8e1d1) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 4.22 #12900 +/- ##
=============================================
- Coverage 17.61% 3.70% -13.91%
=============================================
Files 5917 448 -5469
Lines 531537 38042 -493495
Branches 64985 7038 -57947
=============================================
- Hits 93610 1409 -92201
+ Misses 427369 36446 -390923
+ Partials 10558 187 -10371
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/12900/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/12900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.70% <ø> (ø)` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/12900/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/12900?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4552487065
Quick clarification on my previous comment — rather than proactively sweeping the open Copilot threads / TestPlans, **I'll stand by for your direction**. When you (or whoever picks each PR up) want me to act on a specific thread, tick a specific TestPlan item, push a fix, or close a PR as superseded, just ping and I'll turn it around quickly.
Goal is to not add work to your queue while you're heads-down on #13074. Standing by.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5396229571
Thanks @DaanHoogland. Pushed 3fa88ea addressing all of your comments plus the two remaining Copilot ones. The multi-management-server question is answered in-thread: runs were already serialised by a cluster-wide GlobalLock, and each server now writes to its own `infra-backup/<ms-hostname>/` directory with retention counted per server, so configs and certificates of different servers never overwrite or rotate each other out. InfrastructureBackupTaskTest is at 14 tests, including two new ones for the per-server layout and per-server retention. Now that #13074 has landed, could this one go back into the queue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5424456258
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4487229987
> @weizhouapache fair point — the intent here was never to replace external/operational DB backup tooling, but to give small and edge deployments a one-knob "backup the management plane to the same NAS that already holds my VM backups" option for disaster recovery (rebuild from a fresh management server using only what's on the NAS).
>
> Concrete adjustments I can make:
>
> 1. Make the DB component opt-in via an explicit `infrastructure.backup.include.database` global setting, defaulting to **false** — so by default we only ship configs + certs (where there's no real ops alternative anyway).
> 2. Document explicitly that for production deployments with existing backup tooling, the DB component should stay disabled and be handled externally.
> 3. Keep the unified target (same NAS as VM backups) for the configs+certs path so recovery is in one place.
>
> Does that split land closer to where you'd want it?
@jmsperu
I personally suggest you create a cron job on the management server, it still works even when the cloudstack-management service is not running.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5712146282
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5859521701
Synced with 4.22 (merge b028852290): the only conflicts were additive in `NASBackupProvider` next to the incremental backup settings from #13877; no behavioural change. Local results (JDK 17): InfrastructureBackupTaskTest 14/14, NASBackupProviderTest 21/21.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r4333499253
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,428 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials come from the management server's db.properties (via {@link DbProperties},
+ * which decrypts ENC(...) values when db.properties encryption is in use).
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error(describeDirectoryCreationFailure(backupDir, nasBackupPath));
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+ backupDirectoryIfPresent(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ backupDirectoryIfPresent(SSL_CERT_PATH, backupDir, "ssl-certs");
+ cleanupOldBackups(infraBackupRoot, retentionCount);
Review Comment:
Good catch, thanks. Fixed in b99a0c25e9: each step now reports success, and a run where the DB dump or any archive failed is removed and skips retention entirely. A complete run writes a `BACKUP_COMPLETE` marker last, and retention only counts marked sets, so failed or partial runs can never push good backups out. Unmarked leftovers (a crash mid-run, or backups from before this change) are only removed once a full window of complete sets exists and they are older than the oldest one kept. Added tests for a failed dump, a failed archive and a week of failed runs keeping every good backup.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,428 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials come from the management server's db.properties (via {@link DbProperties},
+ * which decrypts ENC(...) values when db.properties encryption is in use).
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error(describeDirectoryCreationFailure(backupDir, nasBackupPath));
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
Review Comment:
Yes, agreed. In b99a0c25e9 the backup directories are created 0700 and the dumps, archives and marker 0600, with the mode set at creation (the files are created owner-only before mysqldump, the gzip or tar write into them), so there is no window where they are readable by others. They are owned by the user the management server runs as. Existing `infra-backup` directories from earlier runs are tightened to 0700 too, and a file system without POSIX permissions logs a warning asking to restrict the location on the NAS. Tests check the modes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r3302411568
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
+
+ // 6. Cleanup old backups based on retention policy
+ cleanupOldBackups(nasBackupPath, retentionCount);
+
+ LOG.info("Infrastructure backup completed successfully: {}", backupDir);
+
+ } catch (Exception e) {
+ LOG.error("Infrastructure backup failed: {}", e.getMessage(), e);
+ }
+ }
+
+ protected Properties loadDbProperties() {
+ File propsFile = new File(DB_PROPERTIES_PATH);
+ if (!propsFile.exists()) {
+ LOG.warn("Database properties file not found: {}", DB_PROPERTIES_PATH);
+ return null;
+ }
+
+ Properties props = new Properties();
+ try (BufferedReader reader = new BufferedReader(new FileReader(propsFile))) {
+ props.load(reader);
+ return props;
+ } catch (IOException e) {
+ LOG.error("Failed to read database properties: {}", e.getMessage());
+ return null;
+ }
+ }
+
+ protected void backupDatabase(String dbName, String backupDir, String timestamp,
+ String dbHost, String dbUser, String dbPassword) {
+ String dumpFile = backupDir + "/" + dbName + "-" + timestamp + ".sql.gz";
+
+ // Use --single-transaction for InnoDB hot backup (no table locks, consistent snapshot)
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("mysqldump --single-transaction --routines --triggers --events " +
+ "-h '%s' -u '%s' -p'%s' '%s' | gzip > '%s'",
+ dbHost, dbUser, dbPassword, dbName, dumpFile)};
Review Comment:
`backupDatabase` builds a shell command (`/bin/bash -c`) with values from `db.properties` interpolated into the command line. This allows command injection if any property contains quotes/shell metacharacters, and it also exposes the DB password via the process list (`-p'password'`). Use `ProcessBuilder` with argument lists (no shell), and pass credentials via a secure mechanism (e.g., `--defaults-extra-file` with 0600 permissions or `MYSQL_PWD` env) rather than embedding them in the command string.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
+
+ // 6. Cleanup old backups based on retention policy
+ cleanupOldBackups(nasBackupPath, retentionCount);
+
+ LOG.info("Infrastructure backup completed successfully: {}", backupDir);
+
+ } catch (Exception e) {
+ LOG.error("Infrastructure backup failed: {}", e.getMessage(), e);
+ }
+ }
+
+ protected Properties loadDbProperties() {
+ File propsFile = new File(DB_PROPERTIES_PATH);
+ if (!propsFile.exists()) {
+ LOG.warn("Database properties file not found: {}", DB_PROPERTIES_PATH);
+ return null;
+ }
+
+ Properties props = new Properties();
+ try (BufferedReader reader = new BufferedReader(new FileReader(propsFile))) {
+ props.load(reader);
+ return props;
+ } catch (IOException e) {
+ LOG.error("Failed to read database properties: {}", e.getMessage());
+ return null;
+ }
+ }
+
+ protected void backupDatabase(String dbName, String backupDir, String timestamp,
+ String dbHost, String dbUser, String dbPassword) {
+ String dumpFile = backupDir + "/" + dbName + "-" + timestamp + ".sql.gz";
+
+ // Use --single-transaction for InnoDB hot backup (no table locks, consistent snapshot)
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("mysqldump --single-transaction --routines --triggers --events " +
+ "-h '%s' -u '%s' -p'%s' '%s' | gzip > '%s'",
+ dbHost, dbUser, dbPassword, dbName, dumpFile)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(300, TimeUnit.SECONDS);
+
+ if (!completed) {
+ process.destroyForcibly();
+ LOG.error("Database backup timed out for {}", dbName);
+ return;
+ }
+
+ if (process.exitValue() != 0) {
+ LOG.error("Database backup failed for {} with exit code {}", dbName, process.exitValue());
+ return;
+ }
+
+ File dump = new File(dumpFile);
+ LOG.info("Database {} backed up: {} ({} bytes)", dbName, dumpFile, dump.length());
+
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup database {}: {}", dbName, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void backupDirectory(String sourcePath, String backupDir, String archiveName) {
+ File source = new File(sourcePath);
+ if (!source.exists() || !source.isDirectory()) {
+ LOG.debug("Directory {} does not exist, skipping", sourcePath);
+ return;
+ }
+
+ String tarFile = backupDir + "/" + archiveName + ".tar.gz";
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("tar czf '%s' -C '%s' .", tarFile, sourcePath)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(60, TimeUnit.SECONDS);
+
+ if (completed && process.exitValue() == 0) {
+ LOG.info("Directory {} backed up to {}", sourcePath, tarFile);
+ } else {
+ if (!completed) {
+ process.destroyForcibly();
+ }
+ LOG.warn("Directory backup failed for {} (exit code: {})",
+ sourcePath, completed ? process.exitValue() : "timeout");
+ }
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup directory {}: {}", sourcePath, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void cleanupOldBackups(String nasBackupPath, int retentionCount) {
+ File infraDir = new File(nasBackupPath + "/infra-backup");
+ if (!infraDir.exists()) {
+ return;
+ }
+
+ File[] backups = infraDir.listFiles(File::isDirectory);
+ if (backups == null || backups.length <= retentionCount) {
+ return;
+ }
+
+ // Sort by name (timestamp-based), oldest first
+ Arrays.sort(backups, Comparator.comparing(File::getName));
+
+ int toDelete = backups.length - retentionCount;
+ for (int i = 0; i < toDelete; i++) {
+ LOG.info("Removing old infrastructure backup: {}", backups[i].getName());
+ deleteDirectory(backups[i]);
+ }
+ }
+
+ private void deleteDirectory(File dir) {
+ File[] files = dir.listFiles();
+ if (files != null) {
+ for (File file : files) {
+ if (file.isDirectory()) {
+ deleteDirectory(file);
+ } else {
Review Comment:
`deleteDirectory` recursively deletes using `File.isDirectory()` and `listFiles()`, which can follow symlinks. If a symlink is introduced inside the NAS backup tree (NAS shares are often writable by multiple systems), this can delete files/directories outside the intended backup path. Consider using a safe delete that does not follow symlinks (e.g., `Files.walkFileTree` with `NOFOLLOW_LINKS`) and validate the canonical path stays under the expected `infra-backup` root before deleting.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
+
+ // 6. Cleanup old backups based on retention policy
+ cleanupOldBackups(nasBackupPath, retentionCount);
+
+ LOG.info("Infrastructure backup completed successfully: {}", backupDir);
+
+ } catch (Exception e) {
+ LOG.error("Infrastructure backup failed: {}", e.getMessage(), e);
+ }
+ }
+
+ protected Properties loadDbProperties() {
+ File propsFile = new File(DB_PROPERTIES_PATH);
+ if (!propsFile.exists()) {
+ LOG.warn("Database properties file not found: {}", DB_PROPERTIES_PATH);
+ return null;
+ }
+
+ Properties props = new Properties();
+ try (BufferedReader reader = new BufferedReader(new FileReader(propsFile))) {
+ props.load(reader);
+ return props;
+ } catch (IOException e) {
+ LOG.error("Failed to read database properties: {}", e.getMessage());
+ return null;
+ }
+ }
+
+ protected void backupDatabase(String dbName, String backupDir, String timestamp,
+ String dbHost, String dbUser, String dbPassword) {
+ String dumpFile = backupDir + "/" + dbName + "-" + timestamp + ".sql.gz";
+
+ // Use --single-transaction for InnoDB hot backup (no table locks, consistent snapshot)
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("mysqldump --single-transaction --routines --triggers --events " +
+ "-h '%s' -u '%s' -p'%s' '%s' | gzip > '%s'",
+ dbHost, dbUser, dbPassword, dbName, dumpFile)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(300, TimeUnit.SECONDS);
+
+ if (!completed) {
+ process.destroyForcibly();
+ LOG.error("Database backup timed out for {}", dbName);
+ return;
+ }
+
+ if (process.exitValue() != 0) {
+ LOG.error("Database backup failed for {} with exit code {}", dbName, process.exitValue());
+ return;
+ }
+
+ File dump = new File(dumpFile);
+ LOG.info("Database {} backed up: {} ({} bytes)", dbName, dumpFile, dump.length());
+
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup database {}: {}", dbName, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void backupDirectory(String sourcePath, String backupDir, String archiveName) {
+ File source = new File(sourcePath);
+ if (!source.exists() || !source.isDirectory()) {
+ LOG.debug("Directory {} does not exist, skipping", sourcePath);
+ return;
+ }
+
+ String tarFile = backupDir + "/" + archiveName + ".tar.gz";
+ String[] cmd = {"/bin/bash", "-c",
+ String.format("tar czf '%s' -C '%s' .", tarFile, sourcePath)};
+
+ try {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectErrorStream(true);
+ Process process = pb.start();
+ boolean completed = process.waitFor(60, TimeUnit.SECONDS);
+
+ if (completed && process.exitValue() == 0) {
+ LOG.info("Directory {} backed up to {}", sourcePath, tarFile);
+ } else {
+ if (!completed) {
+ process.destroyForcibly();
+ }
+ LOG.warn("Directory backup failed for {} (exit code: {})",
+ sourcePath, completed ? process.exitValue() : "timeout");
+ }
+ } catch (IOException | InterruptedException e) {
+ LOG.error("Failed to backup directory {}: {}", sourcePath, e.getMessage());
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ protected void cleanupOldBackups(String nasBackupPath, int retentionCount) {
+ File infraDir = new File(nasBackupPath + "/infra-backup");
+ if (!infraDir.exists()) {
+ return;
+ }
+
+ File[] backups = infraDir.listFiles(File::isDirectory);
+ if (backups == null || backups.length <= retentionCount) {
+ return;
+ }
+
+ // Sort by name (timestamp-based), oldest first
+ Arrays.sort(backups, Comparator.comparing(File::getName));
+
+ int toDelete = backups.length - retentionCount;
Review Comment:
`cleanupOldBackups` assumes `retentionCount` is non-negative. If the config is set to a negative value, `toDelete` becomes larger than `backups.length`, leading to `ArrayIndexOutOfBoundsException` in the delete loop (caught by the outer try/catch, but it will cause the whole backup run to be reported as failed). Validate/clamp `retentionCount` (e.g., treat values < 0 as 0, or require >= 1) before using it to compute `toDelete`.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
Review Comment:
`dir.mkdirs()` returns false both on failure and when the directory already exists. If the timestamp collides (or the directory already exists for any reason), this will log an error and abort even though the directory may be usable. Prefer checking `dir.exists()` separately and only treating it as an error when the directory does not exist and cannot be created.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
Review Comment:
`InfrastructureBackupTask` stores a `NASBackupProvider provider` field, but it is never used. This adds unnecessary coupling and forces tests to pass `null`. Either remove the field/constructor parameter or use it (e.g., for logging/name/config scoping) so the dependency is justified.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -129,6 +184,16 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
@Inject
private DiskOfferingDao diskOfferingDao;
+ @Inject
+ private BackgroundPollManager backgroundPollManager;
+
+ @Override
+ public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
+ super.configure(name, params);
+ backgroundPollManager.submitTask(new InfrastructureBackupTask(this));
+ return true;
Review Comment:
`backgroundPollManager.submitTask(new InfrastructureBackupTask(this))` will schedule the infrastructure backup task on every management server where this plugin is configured. In multi-management-server deployments this can lead to multiple servers concurrently writing/deleting in the same NAS `infra-backup` path (duplicate backups, races during retention cleanup, and potential accidental deletion). Add cluster-wide coordination (e.g., a DB-based `GlobalLock` around the run, or ensure only one management server instance performs infra backups) or write to per-MS subdirectories to avoid collisions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4556471622
Thanks @daviftorres — this is genuinely useful, and I appreciate you putting the operational detail in writing. A few things stand out as patterns this PR's docs should explicitly point operators toward rather than try to replicate in-tree:
1. **Read-only replica as the dump source** — avoids any contention with the live mgmt DB, especially under load. Anything we ship from inside `cloudstack-management` is inherently dumping from the primary, which is the wrong shape for production-scale deployments.
2. **Long-running service with sleep instead of cron** — kills the overlap class of bugs (slow dump + 15-min cron = compounding load) without needing a lock file. Clean.
3. **`mysqldump | gpg --encrypt | mc pipe`** — no plaintext ever lands on disk, write-only credentials, geographic spread with versioning + lifecycle. That's the right shape for compliance-sensitive backups, and it's also exactly the kind of thing we shouldn't try to replicate in a plugin.
This is also why the latest revision of this PR makes the DB component **opt-in via `nas.infra.backup.include.database` (default false)** — operators with real backup tooling like yours leave it off, and the configs+certs path (where there's no comparable alternative) ships as the default. I'll add a docs note pointing to patterns like yours as the recommended production approach, with the in-plugin DB option clearly framed as a greenfield/small-deployment convenience rather than a substitute.
Really helpful comment — thanks for taking the time to share.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r3969324077
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,414 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+ backupDirectoryIfPresent(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ backupDirectoryIfPresent(SSL_CERT_PATH, backupDir, "ssl-certs");
+ cleanupOldBackups(infraBackupRoot, retentionCount);
+
+ LOG.info("Infrastructure backup completed successfully: {}", backupDir);
+
+ } catch (Exception e) {
+ LOG.error("Infrastructure backup failed: {}", e.getMessage(), e);
+ } finally {
+ releaseRunLock(lock);
+ }
+ }
+
+ /**
+ * Name of the sub-directory that keeps this management server's backups apart from those of the
+ * other servers in the cluster. Management configs and certificates are per server, so they must
+ * not overwrite each other, and the retention count applies per server. Uses the host name and
+ * falls back to the management server id.
+ */
+ protected String getManagementServerLabel() {
+ String label = null;
+ try {
+ label = InetAddress.getLocalHost().getHostName();
+ } catch (UnknownHostException e) {
+ LOG.debug("Could not determine the local host name for the infrastructure backup directory: {}", e.getMessage());
+ }
+ if (label == null || label.isBlank()) {
+ label = "ms-" + ManagementServerNode.getManagementServerId();
+ }
+ return label.replaceAll("[^A-Za-z0-9._-]", "_");
+ }
+
+ /**
+ * Dumps the cloud database, and the usage database when requested, into {@code backupDir}.
+ * The database component is opt-in ({@code nas.infra.backup.include.database}): production
+ * deployments typically run their own mysqldump jobs and leave it off; it exists for small and
+ * edge deployments that want unified disaster recovery on the same NAS as their VM backups.
+ */
+ protected void backupDatabases(String backupDir, String timestamp, boolean includeDatabase, boolean includeUsageDb) {
+ if (!includeDatabase) {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ return;
+ }
+ Properties dbProps = loadDbProperties();
Review Comment:
When the DB password is encrypted in db.properties file, backup of DB fails as there's no decryption logic in place. Worth checking if we could use the `com.cloud.utils.db.DbProperties.getDbProperties();` which has the decryption logic.
I saw the following error in the MS log:
```
...
2026-09-09 13:34:00,736 ERROR [o.a.c.b.InfrastructureBackupTask] (BackgroundTaskPollManager-6:[ctx-cdc8ed08]) (logid:937a2066) Database backup failed for cloud with exit code 2
...
2026-09-09 13:34:00,807 ERROR [o.a.c.b.InfrastructureBackupTask] (BackgroundTaskPollManager-6:[ctx-cdc8ed08]) (logid:937a2066) Database backup failed for cloud_usage with exit code 2
```
Also noticed this in `/var/log/cloudstack/management/management-server.err`:
```
mysqldump: Got error: 1045: Access denied for user 'cloud'@'10.0.33.135' (using password: YES) when trying to connect
mysqldump: Got error: 1045: Access denied for user 'cloud'@'10.0.33.135' (using password: YES) when trying to connect
```
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -85,6 +86,59 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
true,
BackupFrameworkEnabled.key());
+ static final ConfigKey<Boolean> NASInfraBackupEnabled = new ConfigKey<>("Advanced", Boolean.class,
+ "nas.infra.backup.enabled",
+ "false",
+ "Enable automated infrastructure backup to NAS storage. When enabled, the management " +
+ "server will perform a daily backup of CloudStack configuration files and SSL " +
+ "certificates to the configured NAS location. The CloudStack database is NOT included " +
+ "by default — for production deployments, manage database backups externally (e.g. via " +
+ "a cron job running mysqldump). To opt in to bundling the database with this backup " +
+ "(useful only for small / edge / single-MS deployments without separate ops tooling), " +
+ "also set nas.infra.backup.include.database=true.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<Boolean> NASInfraBackupIncludeDatabase = new ConfigKey<>("Advanced", Boolean.class,
+ "nas.infra.backup.include.database",
+ "false",
+ "Include the CloudStack database in the daily infrastructure backup. Defaults to false " +
+ "because production deployments typically manage DB backups via external tooling (e.g. " +
+ "cron + mysqldump, replication, dedicated backup appliance) and are better served " +
+ "doing so. Only set true when you want one-knob disaster recovery for a small/edge " +
+ "deployment and the same NAS that already holds your VM backups is an acceptable " +
+ "target. Has no effect unless nas.infra.backup.enabled is also true.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<String> NASInfraBackupLocation = new ConfigKey<>("Advanced", String.class,
+ "nas.infra.backup.location",
+ "",
+ "NAS mount path where infrastructure backups are stored (e.g. /mnt/nas-backup). " +
+ "Backups will be written to {location}/infra-backup/{timestamp}/.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<Integer> NASInfraBackupRetention = new ConfigKey<>("Advanced", Integer.class,
+ "nas.infra.backup.retention",
+ "7",
+ "Number of infrastructure backup sets to retain. Older backups are automatically removed.",
+ true,
+ ConfigKey.Scope.Global,
+ BackupFrameworkEnabled.key());
+
+ static final ConfigKey<Boolean> NASInfraBackupUsageDb = new ConfigKey<>("Advanced", Boolean.class,
+ "nas.infra.backup.include.usage.db",
+ "true",
Review Comment:
Is this supposed to be true by default?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r3844114378
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
Review Comment:
Done, see `backupDirectoryIfPresent()`; the comment is gone.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials are read from /etc/cloudstack/management/db.properties.
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String DB_PROPERTIES_PATH = "/etc/cloudstack/management/db.properties";
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ private final NASBackupProvider provider;
+
+ public InfrastructureBackupTask(NASBackupProvider provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String backupDir = nasBackupPath + "/infra-backup/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.mkdirs()) {
+ LOG.error("Failed to create backup directory: {}", backupDir);
+ return;
+ }
+
+ // 1 & 2. Database backup — opt-in via nas.infra.backup.include.database.
+ // Production deployments typically run their own mysqldump cron jobs and disable this;
+ // it exists for small/edge deployments wanting unified DR on the same NAS as VM backups.
+ if (includeDatabase) {
+ Properties dbProps = loadDbProperties();
+ if (dbProps == null) {
+ LOG.error("Database backup requested but failed to load properties from {} — skipping DB component", DB_PROPERTIES_PATH);
+ } else {
+ String dbHost = dbProps.getProperty("db.cloud.host", "localhost");
+ String dbUser = dbProps.getProperty("db.cloud.username", "cloud");
+ String dbPassword = dbProps.getProperty("db.cloud.password", "");
+
+ backupDatabase("cloud", backupDir, timestamp, dbHost, dbUser, dbPassword);
+
+ if (includeUsageDb) {
+ String usageHost = dbProps.getProperty("db.usage.host", dbHost);
+ String usageUser = dbProps.getProperty("db.usage.username", dbUser);
+ String usagePassword = dbProps.getProperty("db.usage.password", dbPassword);
+ backupDatabase("cloud_usage", backupDir, timestamp, usageHost, usageUser, usagePassword);
+ }
+ }
+ } else {
+ LOG.debug("Database backup skipped (nas.infra.backup.include.database=false). " +
+ "Manage DB backups externally for production deployments.");
+ }
+
+ // 3. Backup management server configs
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+
+ // 4. Backup agent configs (if present on this host)
+ File agentDir = new File(AGENT_CONFIG_PATH);
+ if (agentDir.exists()) {
+ backupDirectory(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ }
+
+ // 5. Backup SSL certificates
+ File sslDir = new File(SSL_CERT_PATH);
+ if (sslDir.exists()) {
+ backupDirectory(SSL_CERT_PATH, backupDir, "ssl-certs");
+ }
+
+ // 6. Cleanup old backups based on retention policy
Review Comment:
Done, removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5414207319
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4486939493
> @weizhouapache fair point — the intent here was never to replace external/operational DB backup tooling, but to give small and edge deployments a one-knob "backup the management plane to the same NAS that already holds my VM backups" option for disaster recovery (rebuild from a fresh management server using only what's on the NAS).
>
> Concrete adjustments I can make:
>
> 1. Make the DB component opt-in via an explicit `infrastructure.backup.include.database` global setting, defaulting to **false** — so by default we only ship configs + certs (where there's no real ops alternative anyway).
> 2. Document explicitly that for production deployments with existing backup tooling, the DB component should stay disabled and be handled externally.
> 3. Keep the unified target (same NAS as VM backups) for the configs+certs path so recovery is in one place.
>
> Does that split land closer to where you'd want it?
sound like good improvements to the PR @jmsperu (not sure if @weizhouapache agrees ;) )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4522881449
Thanks both — and @weizhouapache, the cron-job suggestion is fair for any deployment with existing ops tooling. The hard mode here is the **first-day greenfield small deployment** where the operator has CloudStack + a NAS and not yet a cron infrastructure; they should be able to opt-in to a unified backup target without standing up a separate process. But that's an opt-in, not a default.
I'll push the three-point scope reduction in the next revision:
1. New `infrastructure.backup.include.database` global, **default false** — DB component skipped unless explicitly enabled
2. Docs explicitly recommending the cron-job approach for production
3. Configs + certs path (where there's no realistic alternative) stays as-is, on the same unified NAS target as VM backups
If even the opt-in DB path is undesirable (you'd rather we don't ship that code at all and just keep configs+certs), happy to drop it entirely — let me know.
Also need to fix the codecov failures — adding unit tests in the next push.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "jmsperu (via GitHub)" <gi...@apache.org>.
jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-4523099101
Friendly nudge — the opt-in DB ConfigKey push + new unit tests are queued but GH Actions has it as `action_required`. Could one of @weizhouapache / @DaanHoogland click **Approve and run** when convenient so the codecov / test runs actually fire? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5434051231
<b>[SF] Trillian test result (tid-16825)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 55774 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12900-t16825-kvm-ol8.zip
Smoke tests completed. 148 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_redundant_vpc_site2site_vpn | `Failure` | 419.47 | test_vpc_vpn.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#discussion_r4258558590
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,428 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials come from the management server's db.properties (via {@link DbProperties},
+ * which decrypts ENC(...) values when db.properties encryption is in use).
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error(describeDirectoryCreationFailure(backupDir, nasBackupPath));
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
+ backupDirectoryIfPresent(AGENT_CONFIG_PATH, backupDir, "agent-config");
+ backupDirectoryIfPresent(SSL_CERT_PATH, backupDir, "ssl-certs");
+ cleanupOldBackups(infraBackupRoot, retentionCount);
Review Comment:
should we skip the cleanup when the dump or the tar failed? right now a failed run still counts, so a week of failures deletes every good backup
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/InfrastructureBackupTask.java:
##########
@@ -0,0 +1,428 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.backup;
+
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFilePermission;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPOutputStream;
+
+/**
+ * Scheduled task that backs up CloudStack infrastructure to NAS storage:
+ * <ul>
+ * <li>MySQL databases (cloud, cloud_usage if enabled)</li>
+ * <li>Management server configuration files</li>
+ * <li>Agent configuration files</li>
+ * <li>SSL certificates and keystores</li>
+ * </ul>
+ *
+ * Database credentials come from the management server's db.properties (via {@link DbProperties},
+ * which decrypts ENC(...) values when db.properties encryption is in use).
+ * Backups are stored under {nasBackupPath}/infra-backup/{timestamp}/ with
+ * automatic retention management.
+ */
+public class InfrastructureBackupTask extends ManagedContextRunnable implements BackgroundPollTask {
+
+ private static final Logger LOG = LogManager.getLogger(InfrastructureBackupTask.class);
+
+ private static final String MANAGEMENT_CONFIG_PATH = "/etc/cloudstack/management";
+ private static final String AGENT_CONFIG_PATH = "/etc/cloudstack/agent";
+ private static final String SSL_CERT_PATH = "/etc/cloudstack/management/cert";
+
+ /** 24 hours in milliseconds */
+ private static final long DAILY_INTERVAL_MS = 86400L * 1000L;
+
+ @Override
+ public Long getDelay() {
+ return DAILY_INTERVAL_MS;
+ }
+
+ /** Indirection so tests can override without standing up the ConfigDepot. */
+ protected boolean isEnabled() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupEnabled.value());
+ }
+
+ protected String getBackupLocation() {
+ return NASBackupProvider.NASInfraBackupLocation.value();
+ }
+
+ protected int getRetentionCount() {
+ return NASBackupProvider.NASInfraBackupRetention.value();
+ }
+
+ protected boolean isDatabaseIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupIncludeDatabase.value());
+ }
+
+ protected boolean isUsageDbIncluded() {
+ return Boolean.TRUE.equals(NASBackupProvider.NASInfraBackupUsageDb.value());
+ }
+
+ @Override
+ protected void runInContext() {
+ if (!isEnabled()) {
+ LOG.debug("Infrastructure backup is disabled (nas.infra.backup.enabled=false)");
+ return;
+ }
+
+ String nasBackupPath = getBackupLocation();
+ if (nasBackupPath == null || nasBackupPath.isEmpty()) {
+ LOG.error("Infrastructure backup location not configured (nas.infra.backup.location is empty)");
+ return;
+ }
+
+ int retentionCount = getRetentionCount();
+ boolean includeDatabase = isDatabaseIncluded();
+ boolean includeUsageDb = isUsageDbIncluded();
+
+ String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+ String infraBackupRoot = nasBackupPath + "/infra-backup/" + getManagementServerLabel();
+ String backupDir = infraBackupRoot + "/" + timestamp;
+
+ LOG.info("Starting infrastructure backup to {} (database included: {})", backupDir, includeDatabase);
+
+ // Cluster-wide lock: in multi-management-server deployments only one MS should run the
+ // infrastructure backup at a time, otherwise they would write/delete concurrently in the
+ // same NAS infra-backup path (duplicate backups, retention races, accidental deletes).
+ GlobalLock lock = acquireRunLock();
+ if (lock == null) {
+ LOG.info("Another management server is performing the infrastructure backup; skipping this run");
+ return;
+ }
+
+ try {
+ File dir = new File(backupDir);
+ if (!dir.exists() && !dir.mkdirs()) {
+ LOG.error(describeDirectoryCreationFailure(backupDir, nasBackupPath));
+ return;
+ }
+
+ backupDatabases(backupDir, timestamp, includeDatabase, includeUsageDb);
+ backupDirectory(MANAGEMENT_CONFIG_PATH, backupDir, "management-config");
Review Comment:
should these files be readable only by the cloud user? the config archive has db.properties and the key file, and the hosts mount the same nas
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NAS backup: automated infrastructure backup (database, configs, certificates) [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-6315886611
thanks @jmsperu, retention and file modes look right. CLGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org