fix: resolve CI failures (fmt, clippy, tests)
- Fix rustfmt formatting in cache.rs, patches.rs, system.rs, routes.rs, main.rs - Add Default impl for PackageCacheState (clippy new_without_default) - Change apply_with_cache_retry generic bound from Fn to FnMut - Add mut to refresh_fn parameter for FnMut compatibility - Replace bool comparison with ! operator (clippy bool_comparison) - Update todo.md with completed status
This commit is contained in:
@ -126,7 +126,12 @@ pub async fn apply_patches(
|
||||
|
||||
// MANDATORY: Refresh package cache before applying patches
|
||||
let _ = job_manager_clone
|
||||
.update_job(&job_id_clone, JobStatus::Running, Some(0), Some("Refreshing package index...".to_string()))
|
||||
.update_job(
|
||||
&job_id_clone,
|
||||
JobStatus::Running,
|
||||
Some(0),
|
||||
Some("Refreshing package index...".to_string()),
|
||||
)
|
||||
.await;
|
||||
let _ = job_manager_clone
|
||||
.add_job_log(&job_id_clone, "Refreshing package cache...".to_string())
|
||||
@ -135,10 +140,18 @@ pub async fn apply_patches(
|
||||
match backend_clone.refresh_package_cache(&cache_state_clone) {
|
||||
Ok(_) => {
|
||||
let _ = job_manager_clone
|
||||
.add_job_log(&job_id_clone, "Package cache refreshed successfully".to_string())
|
||||
.add_job_log(
|
||||
&job_id_clone,
|
||||
"Package cache refreshed successfully".to_string(),
|
||||
)
|
||||
.await;
|
||||
let _ = job_manager_clone
|
||||
.update_job(&job_id_clone, JobStatus::Running, Some(10), Some("Cache refreshed, applying patches...".to_string()))
|
||||
.update_job(
|
||||
&job_id_clone,
|
||||
JobStatus::Running,
|
||||
Some(10),
|
||||
Some("Cache refreshed, applying patches...".to_string()),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Err(e) => {
|
||||
@ -194,17 +207,25 @@ pub async fn apply_patches(
|
||||
// 404/fetch error: refresh cache and retry once
|
||||
info!(job_id = %job_id_clone, "Patch apply failed with fetch error, refreshing cache and retrying");
|
||||
let _ = job_manager_clone
|
||||
.add_job_log(&job_id_clone, "Fetch error detected, refreshing cache and retrying...".to_string())
|
||||
.add_job_log(
|
||||
&job_id_clone,
|
||||
"Fetch error detected, refreshing cache and retrying..."
|
||||
.to_string(),
|
||||
)
|
||||
.await;
|
||||
|
||||
match backend_clone.refresh_package_cache(&cache_state_clone) {
|
||||
Ok(_) => {
|
||||
let _ = job_manager_clone
|
||||
.add_job_log(&job_id_clone, "Cache refreshed, retrying patch apply...".to_string())
|
||||
.add_job_log(
|
||||
&job_id_clone,
|
||||
"Cache refreshed, retrying patch apply...".to_string(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Err(refresh_err) => {
|
||||
let err_msg = format!("Cache refresh on retry failed: {}", refresh_err);
|
||||
let err_msg =
|
||||
format!("Cache refresh on retry failed: {}", refresh_err);
|
||||
let _ = job_manager_clone.fail_job(&job_id_clone, err_msg).await;
|
||||
error!(job_id = %job_id_clone, error = %refresh_err, "Cache refresh on retry failed");
|
||||
return;
|
||||
@ -228,29 +249,40 @@ pub async fn apply_patches(
|
||||
),
|
||||
)
|
||||
.await;
|
||||
match backend_clone.reboot_system(request.reboot_delay_seconds) {
|
||||
match backend_clone.reboot_system(request.reboot_delay_seconds)
|
||||
{
|
||||
Ok(_) => {
|
||||
let _ = job_manager_clone
|
||||
.add_job_log(&job_id_clone, "Reboot command executed".to_string())
|
||||
.add_job_log(
|
||||
&job_id_clone,
|
||||
"Reboot command executed".to_string(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Err(e) => {
|
||||
let _ = job_manager_clone
|
||||
.add_job_log(&job_id_clone, format!("Reboot failed: {}", e))
|
||||
.add_job_log(
|
||||
&job_id_clone,
|
||||
format!("Reboot failed: {}", e),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(retry_err) => {
|
||||
let _ = job_manager_clone.fail_job(&job_id_clone, retry_err.to_string()).await;
|
||||
let _ = job_manager_clone
|
||||
.fail_job(&job_id_clone, retry_err.to_string())
|
||||
.await;
|
||||
error!(job_id = %job_id_clone, error = %retry_err, "Patch application failed after retry");
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
// Non-fetch error: fail immediately
|
||||
let _ = job_manager_clone.fail_job(&job_id_clone, e.to_string()).await;
|
||||
let _ = job_manager_clone
|
||||
.fail_job(&job_id_clone, e.to_string())
|
||||
.await;
|
||||
error!(job_id = %job_id_clone, error = %e, "Patch application failed");
|
||||
}
|
||||
}
|
||||
|
||||
@ -42,11 +42,11 @@ pub struct SystemInfoData {
|
||||
/// Health check response data
|
||||
#[derive(Debug, Serialize)]
|
||||
pub struct HealthData {
|
||||
pub status: String, // "healthy" or "degraded"
|
||||
pub status: String, // "healthy" or "degraded"
|
||||
pub uptime_seconds: u64,
|
||||
pub version: String,
|
||||
pub last_cache_update: Option<String>, // RFC3339 timestamp
|
||||
pub cache_status: String, // "fresh", "stale", "unknown", "failed"
|
||||
pub last_cache_update: Option<String>, // RFC3339 timestamp
|
||||
pub cache_status: String, // "fresh", "stale", "unknown", "failed"
|
||||
}
|
||||
|
||||
/// Service status response data
|
||||
@ -138,15 +138,27 @@ pub async fn health_check(
|
||||
match backend.refresh_package_cache(&cache_state) {
|
||||
Ok(_) => {
|
||||
let updated = cache_state.status();
|
||||
("healthy".to_string(), "fresh".to_string(), updated.last_update.map(|dt| dt.to_rfc3339()))
|
||||
(
|
||||
"healthy".to_string(),
|
||||
"fresh".to_string(),
|
||||
updated.last_update.map(|dt| dt.to_rfc3339()),
|
||||
)
|
||||
}
|
||||
Err(e) => {
|
||||
error!("Health check cache refresh failed: {}", e);
|
||||
("degraded".to_string(), "failed".to_string(), cache_status_val.last_update.map(|dt| dt.to_rfc3339()))
|
||||
(
|
||||
"degraded".to_string(),
|
||||
"failed".to_string(),
|
||||
cache_status_val.last_update.map(|dt| dt.to_rfc3339()),
|
||||
)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
("healthy".to_string(), "fresh".to_string(), cache_status_val.last_update.map(|dt| dt.to_rfc3339()))
|
||||
(
|
||||
"healthy".to_string(),
|
||||
"fresh".to_string(),
|
||||
cache_status_val.last_update.map(|dt| dt.to_rfc3339()),
|
||||
)
|
||||
};
|
||||
|
||||
let response = ApiResponse::success(HealthData {
|
||||
|
||||
@ -26,21 +26,24 @@ pub fn configure_api_routes(
|
||||
) {
|
||||
info!("Configuring API v1 routes");
|
||||
|
||||
cfg.app_data(job_manager).app_data(backend).app_data(cache_state).service(
|
||||
web::scope("/api/v1")
|
||||
// VULN-005: Default handler for unsupported methods returns 405 instead of 404
|
||||
.default_service(web::route().to(method_not_allowed))
|
||||
// Package Management Endpoints
|
||||
.configure(packages::configure_routes)
|
||||
// Patch Management Endpoints
|
||||
.configure(patches::configure_routes)
|
||||
// System Management Endpoints
|
||||
.configure(system::configure_routes)
|
||||
// Job Management Endpoints
|
||||
.configure(jobs::configure_routes)
|
||||
// WebSocket Endpoint
|
||||
.configure(websocket::configure_routes),
|
||||
);
|
||||
cfg.app_data(job_manager)
|
||||
.app_data(backend)
|
||||
.app_data(cache_state)
|
||||
.service(
|
||||
web::scope("/api/v1")
|
||||
// VULN-005: Default handler for unsupported methods returns 405 instead of 404
|
||||
.default_service(web::route().to(method_not_allowed))
|
||||
// Package Management Endpoints
|
||||
.configure(packages::configure_routes)
|
||||
// Patch Management Endpoints
|
||||
.configure(patches::configure_routes)
|
||||
// System Management Endpoints
|
||||
.configure(system::configure_routes)
|
||||
// Job Management Endpoints
|
||||
.configure(jobs::configure_routes)
|
||||
// WebSocket Endpoint
|
||||
.configure(websocket::configure_routes),
|
||||
);
|
||||
}
|
||||
|
||||
/// Health check route (outside API scope for load balancer checks)
|
||||
|
||||
@ -166,7 +166,12 @@ async fn main() -> Result<()> {
|
||||
|
||||
// Configure API routes
|
||||
app = app.configure(|cfg| {
|
||||
configure_api_routes(cfg, job_manager_data.clone(), backend_data.clone(), cache_state.clone());
|
||||
configure_api_routes(
|
||||
cfg,
|
||||
job_manager_data.clone(),
|
||||
backend_data.clone(),
|
||||
cache_state.clone(),
|
||||
);
|
||||
});
|
||||
|
||||
// Configure health route (outside API scope)
|
||||
|
||||
@ -21,7 +21,7 @@ const CACHE_REFRESH_TIMEOUT_SECS: u64 = 120;
|
||||
/// Persistent cache state (written to cache.json)
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct CacheStateFile {
|
||||
pub last_cache_update: Option<String>, // RFC3339
|
||||
pub last_cache_update: Option<String>, // RFC3339
|
||||
pub last_update_success: bool,
|
||||
}
|
||||
|
||||
@ -44,6 +44,12 @@ struct CacheStateInner {
|
||||
last_update_error: Option<String>,
|
||||
}
|
||||
|
||||
impl Default for PackageCacheState {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
impl PackageCacheState {
|
||||
pub fn new() -> Self {
|
||||
// Try to load from state file on startup
|
||||
@ -83,8 +89,7 @@ impl PackageCacheState {
|
||||
Some(t) => {
|
||||
let threshold = Duration::from_secs(STALE_THRESHOLD_SECS);
|
||||
Utc::now() - t
|
||||
> chrono::Duration::from_std(threshold)
|
||||
.unwrap_or(chrono::TimeDelta::MAX)
|
||||
> chrono::Duration::from_std(threshold).unwrap_or(chrono::TimeDelta::MAX)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -151,12 +156,9 @@ pub fn is_fetch_error(error: &anyhow::Error) -> bool {
|
||||
|
||||
/// Execute a patch apply with automatic cache refresh retry on 404/fetch errors.
|
||||
/// Hardcoded 1 retry after cache refresh.
|
||||
pub fn apply_with_cache_retry<F>(
|
||||
refresh_fn: F,
|
||||
apply_fn: impl Fn() -> Result<()>,
|
||||
) -> Result<()>
|
||||
pub fn apply_with_cache_retry<F>(mut refresh_fn: F, apply_fn: impl Fn() -> Result<()>) -> Result<()>
|
||||
where
|
||||
F: Fn() -> Result<()>,
|
||||
F: FnMut() -> Result<()>,
|
||||
{
|
||||
match apply_fn() {
|
||||
Ok(()) => Ok(()),
|
||||
@ -226,7 +228,7 @@ mod tests {
|
||||
let status = state.status();
|
||||
// Fresh state should have no last_update (unless state file exists)
|
||||
// Just verify it doesn't panic
|
||||
assert!(status.last_update_success == false || status.last_update.is_some());
|
||||
assert!(!status.last_update_success || status.last_update.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -259,19 +261,14 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_apply_with_cache_retry_success() {
|
||||
let result = apply_with_cache_retry(
|
||||
|| Ok(()),
|
||||
|| Ok(()),
|
||||
);
|
||||
let result = apply_with_cache_retry(|| Ok(()), || Ok(()));
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_apply_with_cache_retry_non_fetch_error() {
|
||||
let result: Result<()> = apply_with_cache_retry(
|
||||
|| Ok(()),
|
||||
|| Err(anyhow::anyhow!("Permission denied")),
|
||||
);
|
||||
let result: Result<()> =
|
||||
apply_with_cache_retry(|| Ok(()), || Err(anyhow::anyhow!("Permission denied")));
|
||||
assert!(result.is_err());
|
||||
let err = result.unwrap_err();
|
||||
assert!(!is_fetch_error(&err));
|
||||
|
||||
@ -2,33 +2,49 @@
|
||||
|
||||
**Spec:** tasks/issue-2-package-cache-refresh.md
|
||||
**Version:** 1.1.17
|
||||
**Status:** In Progress
|
||||
**Status:** Complete - PR #3 Open
|
||||
|
||||
---
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] 1. Create `src/packages/cache.rs` - Core cache types, stale detection, state persistence, 404 retry logic
|
||||
- [ ] 2. Add `mod cache;` to `src/packages/mod.rs`
|
||||
- [ ] 3. Implement `refresh_package_cache()` on AptBackend
|
||||
- [ ] 4. Implement `refresh_package_cache()` on DnfBackend
|
||||
- [ ] 5. Implement `refresh_package_cache()` on YumBackend
|
||||
- [ ] 6. Implement `refresh_package_cache()` on ApkBackend
|
||||
- [ ] 7. Implement `refresh_package_cache()` on PacmanBackend
|
||||
- [ ] 8. Implement `last_cache_update()` on all backends (shared state)
|
||||
- [ ] 9. Add `refresh_package_cache` and `last_cache_update` to PackageManagerBackend trait
|
||||
- [ ] 10. Enhance health check in `src/api/handlers/system.rs` - add cache status, trigger refresh
|
||||
- [ ] 11. Update HealthData struct with `last_cache_update` and `cache_status` fields
|
||||
- [ ] 12. Add pre-apply cache refresh in `src/api/handlers/patches.rs`
|
||||
- [ ] 13. Bump version in `Cargo.toml` to 1.1.17
|
||||
- [ ] 14. Update `ARCHITECTURE.md` with cache refresh flow
|
||||
- [ ] 15. Update `REQUIREMENTS.md` with FR-007
|
||||
- [ ] 16. Implement state file persistence (cache.json read/write)
|
||||
- [ ] 17. Write unit tests for cache module
|
||||
- [ ] 18. Build and verify compilation
|
||||
- [ ] 19. Commit and push to fix/package-cache-refresh branch
|
||||
- [ ] 20. Create PR and reference Issue #2
|
||||
- [x] 1. Create `src/packages/cache.rs` - Core cache types, stale detection, state persistence, 404 retry logic
|
||||
- [x] 2. Add `mod cache;` to `src/packages/mod.rs`
|
||||
- [x] 3. Implement `refresh_package_cache()` on AptBackend
|
||||
- [x] 4. Implement `refresh_package_cache()` on DnfBackend
|
||||
- [x] 5. Implement `refresh_package_cache()` on YumBackend
|
||||
- [x] 6. Implement `refresh_package_cache()` on ApkBackend
|
||||
- [x] 7. Implement `refresh_package_cache()` on PacmanBackend
|
||||
- [x] 8. Implement `last_cache_update()` on all backends (shared state)
|
||||
- [x] 9. Add `refresh_package_cache` and `last_cache_update` to PackageManagerBackend trait
|
||||
- [x] 10. Enhance health check in `src/api/handlers/system.rs` - add cache status, trigger refresh
|
||||
- [x] 11. Update HealthData struct with `last_cache_update` and `cache_status` fields
|
||||
- [x] 12. Add pre-apply cache refresh in `src/api/handlers/patches.rs`
|
||||
- [x] 13. Bump version in `Cargo.toml` to 1.1.17
|
||||
- [x] 14. Update `ARCHITECTURE.md` with cache refresh flow
|
||||
- [x] 15. Update `REQUIREMENTS.md` with FR-007
|
||||
- [x] 16. Implement state file persistence (cache.json read/write)
|
||||
- [x] 17. Write unit tests for cache module
|
||||
- [x] 18. Build and verify compilation
|
||||
- [x] 19. Commit and push to fix/package-cache-refresh branch
|
||||
- [x] 20. Create PR and reference Issue #2
|
||||
|
||||
## Review
|
||||
|
||||
_To be filled after implementation_
|
||||
**PR:** https://gitea-lxc.moon-dragon.us/git-echo/linux_patch_api/pulls/3
|
||||
**Branch:** fix/package-cache-refresh
|
||||
**Commit:** cf3d597
|
||||
**Files Changed:** 12 files, 944 insertions, 15 deletions
|
||||
|
||||
### Issue Resolution
|
||||
|
||||
All 4 requirements from Issue #2 addressed:
|
||||
1. ✅ Pre-Upgrade Cache Refresh (MUST) - Mandatory cache refresh before every patch_apply
|
||||
2. ✅ Regular Interval Cache Refresh (MUST) - Cache refresh triggered on health check when stale (>4h)
|
||||
3. ✅ 404/Fetch Error Handling (SHOULD) - Auto-retry with cache refresh on fetch errors (1 retry)
|
||||
4. ✅ Stale Cache Detection (SHOULD) - Tracks last_cache_update, reports in health response
|
||||
|
||||
### Known Issue
|
||||
- SSH key `git_echo_id_ed25519` was rejected by Gitea on port 2222 - pushed via HTTPS + API token instead
|
||||
- Root cause: Key fingerprint SHA256:W1BK9fCA53/or7iJkONbFSf3KJ6+oiAggPgisZNPhsc not registered in git-echo Gitea account
|
||||
- Needs investigation: SSH key may need re-registration in Gitea
|
||||
|
||||
Reference in New Issue
Block a user