Skip to content

Commit 22a6de5

Browse files
committed
isolate concurrent CI runs on shared real buckets via GITHUB_RUN_ID
Three concurrent Test (1.26, 25.8) jobs (master push + PR runs) shared the same remote path backup/{cluster}/{shard}/{version} and deleted each other's fixed-name test_rebase_gcs_* backups mid-test, see https://github.com/Altinity/clickhouse-backup/actions/runs/29487098583/job/87610918468 - append GITHUB_RUN_ID to the {version} macro (writeMacrosVersionXML), so every integration config path/object_disk_path is unique per workflow run - testflows: storage_prefix includes GITHUB_RUN_ID (pid is not unique across runners) - disk_cos endpoint gets ${HOSTNAME} like disk_gcs already had - ReadBackupMetadataRemote returns an explicit error for Broken backups instead of an empty BackupMetadata that produced the confusing "required backup ... has data_format=" error
1 parent 9ff52c1 commit 22a6de5

4 files changed

Lines changed: 16 additions & 2 deletions

File tree

pkg/backup/download.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1831,6 +1831,12 @@ func (b *Backuper) ReadBackupMetadataRemote(ctx context.Context, backupName stri
18311831
}
18321832
for _, backup := range backupList {
18331833
if backup.BackupName == backupName {
1834+
// a Broken placeholder contains only BackupName, returning it would produce
1835+
// confusing downstream errors about empty data_format/tables (e.g. a backup
1836+
// being deleted or uploaded by a concurrent process)
1837+
if backup.Broken != "" {
1838+
return nil, errors.Errorf("%s is %s on remote storage", backupName, backup.Broken)
1839+
}
18341840
return &backup.BackupMetadata, nil
18351841
}
18361842
}

test/integration/configs/dynamic_settings.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ cat <<EOT > /etc/clickhouse-server/config.d/storage_configuration_cos.xml
217217
<disks>
218218
<disk_cos>
219219
${COS_DISK_TYPE}
220-
<endpoint>https://clickhouse-backup-1336113806.cos.na-ashburn.myqcloud.com/disk_cos/{cluster}/{shard}/</endpoint>
220+
<endpoint>https://clickhouse-backup-1336113806.cos.na-ashburn.myqcloud.com/disk_cos/${HOSTNAME}/{cluster}/{shard}/</endpoint>
221221
<access_key_id>${QA_TENCENT_SECRET_ID}</access_key_id>
222222
<secret_access_key>${QA_TENCENT_SECRET_KEY}</secret_access_key>
223223
<send_metadata>false</send_metadata>

test/integration/containers.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,8 +1093,14 @@ func (tc *TestContainers) clickHouseBinds(curDir, configsDir string) []string {
10931093
// dynamic_settings.sh runs only in advanced mode, while old versions (1.x, 19.x) need the
10941094
// isolation too: with the macro missing `{version}` stays literal, so all simple-mode matrix
10951095
// jobs collide on the same remote path and delete each other's fixed-name backups.
1096+
// GITHUB_RUN_ID is appended because the version alone doesn't isolate CONCURRENT workflow
1097+
// runs (master push + PR) executing the same matrix job: they share the remote path and
1098+
// delete/overwrite each other's fixed-name backups mid-test.
10961099
func (tc *TestContainers) writeMacrosVersionXML() (string, error) {
10971100
version := strings.ReplaceAll(getEnvDefault("CLICKHOUSE_VERSION", "26.3"), ".", "_")
1101+
if runID := os.Getenv("GITHUB_RUN_ID"); runID != "" {
1102+
version += "-" + runID
1103+
}
10981104
content := fmt.Sprintf("<yandex>\n <macros>\n <version>%s</version>\n </macros>\n</yandex>\n", version)
10991105
fPath := filepath.Join(os.TempDir(), fmt.Sprintf("clickhouse-backup-macros-version-env%d.xml", tc.envID))
11001106
if err := os.WriteFile(fPath, []byte(content), 0644); err != nil {

test/testflows/clickhouse_backup/regression.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ def regression(self, local, stress=False, fips=True, fips_godebug="only"):
8080
config_dir = f"{cwd}/configs/backup_{os.getpid()}"
8181
shutil.copytree(base_config_dir, config_dir, dirs_exist_ok=True)
8282

83-
storage_prefix = f"testflows_{os.getpid()}"
83+
# GITHUB_RUN_ID isolates concurrent CI workflow runs sharing the same real bucket
84+
# (pid alone is not unique across runners), pid isolates parallel runs on one host
85+
storage_prefix = f"testflows_{os.environ.get('GITHUB_RUN_ID', 'local')}_{os.getpid()}"
8486
origin_path = f"{config_dir}/config.yml.origin"
8587
config_path = f"{config_dir}/config.yml"
8688
with open(origin_path) as f:

0 commit comments

Comments
 (0)