fix(targets): make target removal and reload transactional; prevent reappearing entries #662
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…#592) * Initial plan * Fix ETag format to comply with HTTP standards by wrapping with quotes Co-authored-by: overtrue <[email protected]> * bufigx --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: overtrue <[email protected]> Co-authored-by: overtrue <[email protected]>
* improve lock Signed-off-by: Mu junxiang <[email protected]> * feat(tests): add wait_for_object_absence helper and improve lifecycle test reliability Signed-off-by: Mu junxiang <[email protected]> * chore: remove dirty docs Signed-off-by: Mu junxiang <[email protected]> --------- Signed-off-by: Mu junxiang <[email protected]>
…599) * feat(append): implement object append operations with state tracking Signed-off-by: junxiang Mu <[email protected]> * chore: rebase Signed-off-by: junxiang Mu <[email protected]> --------- Signed-off-by: junxiang Mu <[email protected]>
Co-authored-by: loverustfs <[email protected]>
…d-logger # Conflicts: # Cargo.lock # Cargo.toml # crates/notify/src/rules/pattern_rules.rs # crates/utils/Cargo.toml # crates/utils/src/notify/mod.rs # rustfs/src/admin/handlers/event.rs # rustfs/src/admin/mod.rs # rustfs/src/storage/ecfs.rs
Dependency ReviewThe following issues were found:
License Issuesrustfs/Cargo.toml
OpenSSF ScorecardScorecard details
Scanned Files
|
…nto feature/audit-add-logger * 'feature/audit-add-logger' of github.com:rustfs/rustfs: fix: update ahm integration test fixture (#659) fix: correct HTTP range suffix handling
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR makes target removal and reload transactional to prevent removed targets from reappearing after reload. It also simplifies the observability crate by removing the custom logger/sinks and standardizing on OpenTelemetry telemetry.
- Make target config updates transactional and deterministic (lowercase normalization, rebuild-and-write-back of sections, deduplication).
- Remove custom obs logger/sinks; change init_obs() to return only an OtelGuard.
- Instrumentation cleanup and minor logging/typo fixes.
Reviewed Changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/run.sh | Removes env vars for legacy sinks/logger to align with new obs design. |
| rustfs/src/storage/ecfs.rs | Switch to tracing::instrument import, logging macro cleanup, ObjectInfo path simplifications, minor typo fix; one module path regression in get_replication_config. |
| rustfs/src/main.rs | Adjust to new init_obs() signature (no logger returned). |
| rustfs/Cargo.toml | Dependency cleanups/reordering and version bumps. |
| crates/utils/src/net.rs | Import cleanups. |
| crates/obs/* | Remove logger/sinks and related exports; keep telemetry; update types/paths. |
| crates/notify/src/registry.rs | Rebuild-and-write-back config per section; normalize section names; env parsing tweaks. |
| crates/notify/src/integration.rs | Case-insensitive removal of targets; add debug snapshot after removal. |
| crates/notify/examples/* | Align examples with new configuration paths and API changes. |
| crates/kms/src/backends/local.rs | Path simplifications for types/errors; potential missing imports for Nonce/KmsError. |
| crates/config/src/observability/* | Consolidate observability config; remove file/kafka/webhook sink configs. |
| crates/audit/* | Use std::sync::OnceLock; transactional audit target config writes similar to notify. |
| Cargo.toml | Version bumps and dependency adjustments across workspace. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 40 out of 42 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of Change
Related Issues
Summary of Changes
fix(targets): make target removal and reload transactional; prevent reappearing entries
remove_target_configupdates both in-memory and persisted config and emits a debug snapshot of the config after removal.remove_target_dataand defer destructive cleanup of.rustfs.sysuntil config persistence succeeds; on failure avoid or rollback destructive changes..rustfs.systo prevent removed targets from reappearing due to ordering differences.Checklist
make pre-commitImpact
Additional Notes
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.