Skip to content

Commit 6c19add

Browse files
abtreeceabtreece
committed
fix: address PR review findings on EOL archive tooling
Addresses must-fix items from PR_REVIEW_FINDINGS.md: - Wire up GCloudStorageLock in all four archive scripts. Imports were present but unused, leaving migrations and prunes racy against the publish workflow's pointer updates. Both migrate scripts now also use begin/ensure for cleanup so a failure mid-run no longer leaves the imported GPG signing key on disk. - migrate-apt-to-archive: stop swallowing pool-merge errors via '|| true'. Real I/O/permission failures must surface; only no-clobber skips were ever benign. - migrate-apt-to-archive: comment claimed EOL detection compared against environments/, but the code compares against the DEB entries in config.yml. Fix the comment to match the code. - archiving-eol-packages.md: APT explicit-distros example listed centos-8, which is RPM-only and would fail in the APT script. Replace with debian-10,ubuntu-20.04.
1 parent e523f8a commit 6c19add

5 files changed

Lines changed: 309 additions & 200 deletions

File tree

‎dev-handbook/archiving-eol-packages.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ ARCHIVE_REPO_BUCKET_NAME=fsruby-server-edition-apt-repo-archive \
6969
./internal-scripts/ci-cd/archive/migrate-apt-to-archive.rb --dry-run
7070
~~~
7171

72-
The script auto-detects EOL distros by comparing `aptly repo list` output against the distributions defined in `config.yml`. You can also specify distros explicitly:
72+
The script auto-detects EOL distros by comparing `aptly repo list` output against the DEB distributions defined in `config.yml`. You can also specify distros explicitly (DEB-only — RPM distros belong to the YUM script):
7373

7474
~~~bash
75-
./internal-scripts/ci-cd/archive/migrate-apt-to-archive.rb --dry-run --distros centos-8,debian-9
75+
./internal-scripts/ci-cd/archive/migrate-apt-to-archive.rb --dry-run --distros debian-10,ubuntu-20.04
7676
~~~
7777

7878
**Execute the migration** (removes `--dry-run`):

‎internal-scripts/ci-cd/archive/migrate-apt-to-archive.rb‎

Lines changed: 76 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# ./internal-scripts/ci-cd/archive/migrate-apt-to-archive.rb [--dry-run] [--distros centos-8,debian-9]
1212
#
1313
# If --distros is not specified, automatically detects EOL distros by comparing
14-
# the Aptly state against the current environments/ directory.
14+
# the Aptly state against the DEB distributions defined in config.yml.
1515

1616
require_relative '../../../lib/gcloud_storage_lock'
1717
require_relative '../../../lib/ci_workflow_support'
@@ -42,55 +42,66 @@ def main
4242
create_temp_dirs
4343
ensure_gpg_state_isolated
4444
activate_wrappers_bin_dir
45+
initialize_locking
4546
initialize_aptly(@main_aptly_config_path, @main_state_path, @main_state_repo_path)
4647
fetch_and_import_signing_key
4748

48-
print_header 'Downloading main repository state'
49-
version = get_latest_production_repo_version
50-
if version == 0
51-
abort 'ERROR: No production repository exists yet'
52-
end
53-
fetch_main_state(version)
54-
55-
print_header 'Identifying EOL distributions'
56-
eol_distros = identify_eol_distros
57-
if eol_distros.empty?
58-
log_notice 'No EOL distributions found to archive'
59-
exit
60-
end
61-
log_notice "EOL distributions to archive: #{eol_distros.join(', ')}"
62-
63-
print_header 'Fetching existing archive state (if any)'
64-
@archive_version = get_latest_archive_version
65-
if @archive_version > 0
66-
log_notice "Existing archive at version #{@archive_version}, will merge"
67-
fetch_archive_state(@archive_version)
68-
else
69-
log_notice 'No existing archive — creating fresh'
70-
end
71-
72-
print_header 'Creating archive repository'
73-
create_archive_state(eol_distros)
74-
75-
print_header 'Trimming main repository state'
76-
trim_main_state(eol_distros)
49+
eol_distros = nil
50+
version = nil
51+
52+
begin
53+
synchronize do
54+
print_header 'Downloading main repository state'
55+
version = get_latest_production_repo_version
56+
if version == 0
57+
abort 'ERROR: No production repository exists yet'
58+
end
59+
fetch_main_state(version)
60+
61+
print_header 'Identifying EOL distributions'
62+
eol_distros = identify_eol_distros
63+
if eol_distros.empty?
64+
log_notice 'No EOL distributions found to archive'
65+
return
66+
end
67+
log_notice "EOL distributions to archive: #{eol_distros.join(', ')}"
68+
69+
print_header 'Fetching existing archive state (if any)'
70+
@archive_version = get_latest_archive_version
71+
if @archive_version > 0
72+
log_notice "Existing archive at version #{@archive_version}, will merge"
73+
fetch_archive_state(@archive_version)
74+
else
75+
log_notice 'No existing archive — creating fresh'
76+
end
77+
78+
print_header 'Creating archive repository'
79+
create_archive_state(eol_distros)
80+
check_lock_health
81+
82+
print_header 'Trimming main repository state'
83+
trim_main_state(eol_distros)
84+
check_lock_health
85+
86+
if @dry_run
87+
log_notice 'DRY RUN — not uploading changes'
88+
print_summary(eol_distros, version)
89+
return
90+
end
91+
92+
print_header 'Uploading archive repository'
93+
upload_archive
94+
check_lock_health
95+
96+
print_header 'Uploading trimmed main repository'
97+
upload_main(version)
98+
end
7799

78-
if @dry_run
79-
log_notice 'DRY RUN — not uploading changes'
80-
print_summary(eol_distros, version)
81-
exit
100+
print_header 'Success!'
101+
print_summary(eol_distros, version) if eol_distros && !eol_distros.empty?
102+
ensure
103+
cleanup
82104
end
83-
84-
print_header 'Uploading archive repository'
85-
upload_archive
86-
87-
print_header 'Uploading trimmed main repository'
88-
upload_main(version)
89-
90-
print_header 'Success!'
91-
print_summary(eol_distros, version)
92-
93-
cleanup
94105
end
95106

96107
private
@@ -161,6 +172,22 @@ def activate_wrappers_bin_dir
161172
ENV['PATH'] = "#{@wrappers_bin_dir}:#{ENV['PATH']}"
162173
end
163174

175+
def initialize_locking
176+
@lock = GCloudStorageLock.new(url: lock_url)
177+
end
178+
179+
def lock_url
180+
"gs://#{ENV['PRODUCTION_REPO_BUCKET_NAME']}/locks/apt"
181+
end
182+
183+
def synchronize(&block)
184+
@lock.synchronize(&block)
185+
end
186+
187+
def check_lock_health
188+
abort 'ERROR: lock is unhealthy. Aborting operation' if !@lock.healthy?
189+
end
190+
164191
def initialize_aptly(config_path, state_path, repo_path)
165192
log_notice "Creating Aptly config: #{config_path}"
166193
File.open(config_path, 'w:utf-8') do |f|
@@ -297,12 +324,13 @@ def create_archive_state(eol_distros)
297324
archive_pool = "#{@archive_state_path}/pool"
298325
if File.exist?(main_pool)
299326
if File.exist?(archive_pool)
300-
# Merge: copy new pool files that don't already exist
327+
# Merge: copy new pool files that don't already exist (-n: no-clobber).
328+
# Real I/O errors must surface — only no-clobber skips are benign.
301329
run_bash(
302-
sprintf('cp -rn %s/* %s/ 2>/dev/null || true',
330+
sprintf('cp -rn %s/* %s/',
303331
Shellwords.escape(main_pool),
304332
Shellwords.escape(archive_pool)),
305-
log_invocation: false, check_error: false, pipefail: false
333+
log_invocation: true, check_error: true, pipefail: false
306334
)
307335
else
308336
FileUtils.cp_r(main_pool, @archive_state_path)

‎internal-scripts/ci-cd/archive/migrate-yum-to-archive.rb‎

Lines changed: 67 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
# ./internal-scripts/ci-cd/archive/migrate-yum-to-archive.rb [--dry-run] [--distros centos-8]
1212
#
1313
# If --distros is not specified, automatically detects EOL distros by comparing
14-
# the repo contents against the current environments/ directory.
14+
# the repo contents against the RPM distributions defined in config.yml.
1515

16+
require_relative '../../../lib/gcloud_storage_lock'
1617
require_relative '../../../lib/ci_workflow_support'
1718
require_relative '../../../lib/shell_scripting_support'
1819
require_relative '../../../lib/publishing_support'
@@ -34,50 +35,59 @@ def main
3435
print_header 'Initializing'
3536
load_config
3637
create_temp_dirs
38+
initialize_locking
3739
fetch_and_import_signing_key
3840

39-
print_header 'Downloading main repository'
40-
version = get_latest_production_repo_version
41-
if version == 0
42-
abort 'ERROR: No production repository exists yet'
43-
end
44-
fetch_main_repo(version)
45-
46-
print_header 'Identifying EOL distributions'
47-
eol_distros = identify_eol_distros
48-
if eol_distros.empty?
49-
log_notice 'No EOL distributions found to archive'
50-
exit
51-
end
52-
log_notice "EOL distributions to archive: #{eol_distros.join(', ')}"
53-
54-
print_header 'Fetching existing archive (if any)'
55-
@archive_version = get_latest_archive_version
56-
if @archive_version > 0
57-
log_notice "Existing archive at version #{@archive_version}, will merge"
58-
fetch_archive_repo(@archive_version)
59-
else
60-
log_notice 'No existing archive — creating fresh'
61-
@archive_repo_path = "#{@temp_dir}/archive-repo"
62-
Dir.mkdir(@archive_repo_path)
63-
end
41+
eol_distros = nil
42+
version = nil
43+
44+
begin
45+
synchronize do
46+
print_header 'Downloading main repository'
47+
version = get_latest_production_repo_version
48+
if version == 0
49+
abort 'ERROR: No production repository exists yet'
50+
end
51+
fetch_main_repo(version)
52+
53+
print_header 'Identifying EOL distributions'
54+
eol_distros = identify_eol_distros
55+
if eol_distros.empty?
56+
log_notice 'No EOL distributions found to archive'
57+
return
58+
end
59+
log_notice "EOL distributions to archive: #{eol_distros.join(', ')}"
60+
61+
print_header 'Fetching existing archive (if any)'
62+
@archive_version = get_latest_archive_version
63+
if @archive_version > 0
64+
log_notice "Existing archive at version #{@archive_version}, will merge"
65+
fetch_archive_repo(@archive_version)
66+
else
67+
log_notice 'No existing archive — creating fresh'
68+
@archive_repo_path = "#{@temp_dir}/archive-repo"
69+
Dir.mkdir(@archive_repo_path)
70+
end
71+
72+
if @dry_run
73+
log_notice 'DRY RUN — not uploading changes'
74+
print_summary(eol_distros, version)
75+
return
76+
end
77+
78+
print_header 'Uploading EOL distros to archive'
79+
upload_archive(eol_distros)
80+
check_lock_health
81+
82+
print_header 'Removing EOL distros from main repository'
83+
remove_from_main(eol_distros, version)
84+
end
6485

65-
if @dry_run
66-
log_notice 'DRY RUN — not uploading changes'
67-
print_summary(eol_distros, version)
68-
exit
86+
print_header 'Success!'
87+
print_summary(eol_distros, version) if eol_distros && !eol_distros.empty?
88+
ensure
89+
cleanup
6990
end
70-
71-
print_header 'Uploading EOL distros to archive'
72-
upload_archive(eol_distros)
73-
74-
print_header 'Removing EOL distros from main repository'
75-
remove_from_main(eol_distros, version)
76-
77-
print_header 'Success!'
78-
print_summary(eol_distros, version)
79-
80-
cleanup
8191
end
8292

8393
private
@@ -101,6 +111,22 @@ def create_temp_dirs
101111
Dir.mkdir(@local_repo_path)
102112
end
103113

114+
def initialize_locking
115+
@lock = GCloudStorageLock.new(url: lock_url)
116+
end
117+
118+
def lock_url
119+
"gs://#{ENV['PRODUCTION_REPO_BUCKET_NAME']}/locks/yum"
120+
end
121+
122+
def synchronize(&block)
123+
@lock.synchronize(&block)
124+
end
125+
126+
def check_lock_health
127+
abort 'ERROR: lock is unhealthy. Aborting operation' if !@lock.healthy?
128+
end
129+
104130
def fetch_and_import_signing_key
105131
log_notice 'Fetching and importing signing key'
106132
File.open(@signing_key_path, 'wb') do |f|

0 commit comments

Comments
 (0)