diff --git a/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md b/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md index 13bcf99cf..2b6f1df25 100644 --- a/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md +++ b/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md @@ -7,7 +7,7 @@ github-issue: 2035 spec-path: docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md branch: 2035-fix-duplicate-port-zero-tracker-instance-bootstrap related-pr: null -last-updated-utc: 2026-07-28 13:06 +last-updated-utc: 2026-07-29 16:51 semantic-links: skill-links: - write-unit-test @@ -18,12 +18,14 @@ semantic-links: - docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level/ISSUE.md - docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md - docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md + - docs/issues/open/2041-migrate-runtime-service-registry-metadata/ISSUE.md - docs/events-architecture.md - evidence.md related-issues: - 1419 - 2036 - 2039 + - 2041 --- # Issue #2035 - Fix Duplicate Port-Zero Tracker Instance Bootstrap @@ -77,29 +79,30 @@ metrics policy correct: the UDP server has one application-wide event bus and repository, while producer-side metrics suppression can hide facts required by the independent banning listener. -Issue [#2035](https://github.com/torrust/torrust-tracker/issues/2035) must be -reimplemented after: +Issue [#2035](https://github.com/torrust/torrust-tracker/issues/2035) is +delivered in two phases. After #2036 defines canonical runtime +service/configuration-instance identity, this issue can reimplement bootstrap +identity preservation and prove that each duplicate port-zero configuration +starts with its matching container. This phase must not introduce registry +metadata or metrics-policy behavior. -1. #2036 defines canonical runtime service/configuration-instance identity. -2. [#2041](../2041-migrate-runtime-service-registry-metadata/ISSUE.md) - carries that identity through started-service registration metadata. -3. #2039 makes event publication independent of metrics policy and filters - metrics in listeners by that canonical identity. - -The new implementation will preserve bootstrap identity without creating a -second runtime identity or metrics-policy mechanism. +After bootstrap identity propagation is merged, [#2041](../2041-migrate-runtime-service-registry-metadata/ISSUE.md) +will carry the same identity through started-service registration metadata, and +Issue #2039 will make event publication independent of metrics policy and filter +metrics in listeners by canonical identity. Those follow-ups are prerequisites +only for this issue's metrics-related final verification and closure. ## Implementation Plan | ID | Status | Task | Notes / Expected Output | | --- | ------- | ------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | -| T1 | BLOCKED | Land [#2036](../2036-add-runtime-service-registry-metadata/ISSUE.md) canonical identity | Bootstrap identity must align with the canonical runtime identity contract. | -| T2 | BLOCKED | Land registry metadata migration | Register started services with canonical identity before #2039 consumes runtime identity. | -| T3 | BLOCKED | Land [#2039](../2039-normalize-per-instance-event-metrics-policy/ISSUE.md) event-metrics normalization | Objective-event publication and listener-side metrics filtering must be available. | -| T4 | TODO | Replace address-keyed container lookup | Use an order-preserving representation or canonical identity, not configured `SocketAddr`. | -| T5 | TODO | Start matching containers | Pass each configuration entry's matching container into HTTP and UDP startup. | -| T6 | TODO | Correlate lifecycle logs | Include canonical identity with configured and final binding logs. | -| T7 | TODO | Add regressions and validate | Cover duplicate port-zero HTTP/UDP listeners and record focused verification. | +| T1 | DONE | Land [#2036](../2036-add-runtime-service-registry-metadata/ISSUE.md) canonical identity | Bootstrap identity aligns with the canonical runtime identity contract. | +| T2 | TODO | Replace address-keyed container lookup | Use an order-preserving representation or canonical identity, not configured `SocketAddr`. | +| T3 | TODO | Start matching containers | Pass each configuration entry's matching container into HTTP and UDP startup. | +| T4 | TODO | Correlate lifecycle logs | Include canonical identity with configured and final binding logs. | +| T5 | TODO | Add bootstrap regressions and validate | Cover duplicate port-zero HTTP/UDP listeners and record focused verification. | +| T6 | BLOCKED | Land registry metadata migration | #2041 must expose started-service canonical identity before final verification. | +| T7 | BLOCKED | Land [#2039](../2039-normalize-per-instance-event-metrics-policy/ISSUE.md) event-metrics normalization | Required only for metrics-related final verification and issue closure. | ## Progress Tracking @@ -107,10 +110,11 @@ second runtime identity or metrics-policy mechanism. - [x] Specification drafted and approved by user/maintainer - [x] GitHub issue created: #2035 -- [ ] Prerequisite #2036 completed +- [x] Prerequisite #2036 completed +- [ ] Bootstrap identity preservation completed +- [ ] Registry metadata migration completed - [ ] Event-metrics normalization completed -- [ ] New implementation completed -- [ ] Automatic and manual verification completed +- [ ] Final automatic and manual verification completed - [ ] Acceptance criteria reviewed after implementation ### Progress Log @@ -120,6 +124,9 @@ second runtime identity or metrics-policy mechanism. [evidence.md](evidence.md). - 2026-07-29 00:00 UTC - agent - Archived the prior implementation attempt and deferred implementation until #2036 and event-metrics normalization are complete. +- 2026-07-29 16:51 UTC - agent - Clarified the two-phase delivery order from the #2036 handoff: + bootstrap identity propagation begins after #2036; #2041 and #2039 are required only for + metrics-related final verification and closure. ## Acceptance Criteria diff --git a/docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md b/docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md index aff3b2ba8..8269533b2 100644 --- a/docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md +++ b/docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md @@ -7,7 +7,7 @@ github-issue: 2036 spec-path: docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md branch: 2036-add-runtime-service-registry-metadata related-pr: null -last-updated-utc: 2026-07-29 14:45 +last-updated-utc: 2026-07-29 16:15 semantic-links: skill-links: - write-unit-test @@ -16,6 +16,11 @@ semantic-links: - docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level/ISSUE.md - docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md - docs/issues/open/2041-migrate-runtime-service-registry-metadata/ISSUE.md + - packages/axum-http-server/src/server.rs + - packages/axum-rest-api-server/src/server.rs + - packages/primitives/src/configuration_instance_id.rs + - packages/primitives/src/service_role.rs + - packages/udp-server/src/server/launcher.rs related-issues: - 1419 --- @@ -60,14 +65,32 @@ in [#2041](../2041-migrate-runtime-service-registry-metadata/ISSUE.md), which de - Public URLs, proxies, domain names, and deployment topology. - Dynamic service restart, deregistration, or configuration reload. +## Approved Design Decisions + +- The tracker-owned `primitives` package is the canonical home for both + identity types. They must not be added to the generic + `torrust-net-primitives` or `torrust-server-lib` packages. +- `ServiceRole` has `HttpTracker`, `UdpTracker`, `RestApi`, and + `HealthCheckApi` variants. HTTPS remains the `HttpTracker` role; its final + `ServiceBinding` differentiates HTTP from HTTPS. +- `ConfigurationInstanceId` combines a `ServiceRole` with a zero-based index + in that role's configuration-entry list. Its equality is structural over + those two values and never considers a configured or final `SocketAddr`. +- The index is derived during configuration/bootstrap enumeration and remains + immutable for the lifetime of the process. It correlates one configured + instance; it is not a user-supplied persistent service identifier. +- The public types provide the traits needed by their intended internal + consumers, including comparison, hashing, and serialization, without + introducing a parallel identity representation. + ## Implementation Plan | ID | Status | Task | Notes / Expected Output | | --- | ------ | ----------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | -| T1 | TODO | Define tracker-owned service role type | Keep tracker semantics out of generic network and server crates. | -| T2 | TODO | Define canonical configuration-instance identity type | Specify scope, equality, construction, documentation, and unit tests. | -| T3 | TODO | Verify consumer boundaries | Confirm #2035 bootstrap, registry migration, and #2039 can consume the same types without creating competing identifiers. | -| T4 | TODO | Run focused validation | Run `linter all` and relevant type/module tests. | +| T1 | DONE | Define tracker-owned service role type | Keep tracker semantics out of generic network and server crates. | +| T2 | DONE | Define canonical configuration-instance identity type | Specify scope, equality, construction, documentation, and unit tests. | +| T3 | DONE | Verify consumer boundaries | Confirm #2035 bootstrap, registry migration, and #2039 can consume the same types without creating competing identifiers. | +| T4 | DONE | Run focused validation | `cargo test -p torrust-tracker-primitives` and `linter all` passed. | ## Progress Tracking @@ -75,8 +98,8 @@ in [#2041](../2041-migrate-runtime-service-registry-metadata/ISSUE.md), which de - [x] Specification drafted and approved by user/maintainer - [x] GitHub issue created: #2036 -- [ ] Implementation completed -- [ ] Automatic verification completed (`linter all`, relevant tests) +- [x] Implementation completed +- [x] Automatic verification completed (`linter all`, relevant tests) - [ ] Acceptance criteria reviewed after implementation - [ ] Issue closed and specification moved to `docs/issues/closed/` @@ -84,13 +107,22 @@ in [#2041](../2041-migrate-runtime-service-registry-metadata/ISSUE.md), which de - 2026-07-28 14:51 UTC - agent - User-approved specification promoted to GitHub feature #2036. - 2026-07-29 14:45 UTC - agent - Split registry migration into a dedicated draft issue. #2036 now owns only canonical role and configuration-instance identity types, which can be implemented before #2035 bootstrap propagation. +- 2026-07-29 16:15 UTC - user and agent - Confirmed the canonical identity model: tracker-owned + primitives define the four service roles and a role-qualified, zero-based configuration instance + index. The identity is independent of socket addresses and is not a user-supplied persistent ID. +- 2026-07-29 16:28 UTC - agent - Added `ServiceRole` and `ConfigurationInstanceId` to the + tracker-owned primitives package. `cargo test -p torrust-tracker-primitives`, `linter all`, and + the full pre-commit check passed. +- 2026-07-29 16:28 UTC - agent - Replaced the HTTP, REST API, and UDP health-check + `TYPE_STRING` values with their corresponding `ServiceRole` identifiers. The REST API canonical + string is `tracker_rest_api` to preserve its existing health-check response value. ## Acceptance Criteria -- [ ] AC1: Tracker-owned canonical service-role values are defined without coupling generic server/network libraries to tracker variants. -- [ ] AC2: Canonical configuration-instance identity is typed, documented, and independent of configured socket addresses. -- [ ] AC3: The types can be used by #2035, the registry migration follow-up, and #2039 without conversion to competing identity types. -- [ ] AC4: Focused tests and `linter all` exit with code `0`. +- [x] AC1: Tracker-owned canonical service-role values are defined without coupling generic server/network libraries to tracker variants. +- [x] AC2: Canonical configuration-instance identity is typed, documented, and independent of configured socket addresses. +- [x] AC3: The types can be used by #2035, the registry migration follow-up, and #2039 without conversion to competing identity types. +- [x] AC4: Focused tests and `linter all` exit with code `0`. ## Verification Plan diff --git a/packages/axum-http-server/src/server.rs b/packages/axum-http-server/src/server.rs index b05e2a1b8..aadc458a3 100644 --- a/packages/axum-http-server/src/server.rs +++ b/packages/axum-http-server/src/server.rs @@ -15,12 +15,12 @@ use torrust_server_lib::signals::{Halted, Started}; use torrust_tracker_axum_server::custom_axum_server::{self, TimeoutAcceptor}; use torrust_tracker_axum_server::signals::graceful_shutdown; use torrust_tracker_http_core::container::HttpTrackerCoreContainer; +use torrust_tracker_primitives::ServiceRole; use tracing::instrument; use super::v1::routes::router; use crate::HTTP_TRACKER_LOG_TARGET; -const TYPE_STRING: &str = "http_tracker"; /// Error that can occur when starting or stopping the HTTP server. /// /// Some errors triggered while starting the server are: @@ -281,7 +281,12 @@ pub fn check_fn(service_binding: &ServiceBinding) -> ServiceHealthCheckJob { } }); - ServiceHealthCheckJob::new(service_binding.clone(), info, TYPE_STRING.to_string(), job) + ServiceHealthCheckJob::new( + service_binding.clone(), + info, + ServiceRole::HttpTracker.as_str().to_string(), + job, + ) } #[cfg(test)] diff --git a/packages/axum-rest-api-server/src/server.rs b/packages/axum-rest-api-server/src/server.rs index 74421a726..6a6d498b4 100644 --- a/packages/axum-rest-api-server/src/server.rs +++ b/packages/axum-rest-api-server/src/server.rs @@ -40,14 +40,13 @@ use torrust_server_lib::signals::{Halted, Started}; use torrust_tracker_axum_server::custom_axum_server::{self, TimeoutAcceptor}; use torrust_tracker_axum_server::signals::graceful_shutdown; use torrust_tracker_configuration::AccessTokens; +use torrust_tracker_primitives::ServiceRole; use torrust_tracker_rest_api_runtime_adapter::v1::container::TrackerHttpApiCoreContainer; use tracing::{Level, instrument}; use super::routes::router; use crate::API_LOG_TARGET; -const TYPE_STRING: &str = "tracker_rest_api"; - /// Errors that can occur when starting or stopping the API server. #[derive(Debug, Error)] pub enum Error { @@ -207,7 +206,7 @@ pub fn check_fn(service_binding: &ServiceBinding) -> ServiceHealthCheckJob { Err(err) => Err(err.to_string()), } }); - ServiceHealthCheckJob::new(service_binding.clone(), info, TYPE_STRING.to_string(), job) + ServiceHealthCheckJob::new(service_binding.clone(), info, ServiceRole::RestApi.as_str().to_string(), job) } /// A struct responsible for starting the API server. diff --git a/packages/primitives/src/configuration_instance_id.rs b/packages/primitives/src/configuration_instance_id.rs new file mode 100644 index 000000000..abf1fd722 --- /dev/null +++ b/packages/primitives/src/configuration_instance_id.rs @@ -0,0 +1,109 @@ +use serde::Serialize; + +use crate::ServiceRole; + +/// Identifies one configured tracker service instance for a process lifetime. +/// +/// Equality includes the tracker [`ServiceRole`] and the zero-based index in +/// that role's configuration-entry list. The identifier deliberately excludes +/// configured and final socket addresses, because repeated port-zero bindings +/// are valid. It is neither user supplied nor persistent across configuration +/// reordering. +#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Hash, Clone, Copy, Serialize)] +pub struct ConfigurationInstanceId { + service_role: ServiceRole, + instance_index: usize, +} + +impl ConfigurationInstanceId { + /// Creates an identifier for a role-qualified configuration entry. + #[must_use] + pub const fn new(service_role: ServiceRole, instance_index: usize) -> Self { + Self { + service_role, + instance_index, + } + } + + /// Returns the tracker role configured for this instance. + #[must_use] + pub const fn service_role(self) -> ServiceRole { + self.service_role + } + + /// Returns the zero-based index in the role's configuration-entry list. + #[must_use] + pub const fn instance_index(self) -> usize { + self.instance_index + } +} + +#[cfg(test)] +mod tests { + use crate::{ConfigurationInstanceId, ServiceRole}; + + #[test] + fn it_should_identify_equal_role_and_index_as_the_same_instance() { + // Arrange + let first_instance = ConfigurationInstanceId::new(ServiceRole::HttpTracker, 0); + let same_instance = ConfigurationInstanceId::new(ServiceRole::HttpTracker, 0); + + // Act + let are_equal = first_instance == same_instance; + + // Assert + assert!(are_equal); + } + + #[test] + fn it_should_distinguish_instances_with_the_same_role_and_different_indices() { + // Arrange + let first_instance = ConfigurationInstanceId::new(ServiceRole::HttpTracker, 0); + let second_instance = ConfigurationInstanceId::new(ServiceRole::HttpTracker, 1); + + // Act + let are_equal = first_instance == second_instance; + + // Assert + assert!(!are_equal); + } + + #[test] + fn it_should_distinguish_instances_with_the_same_index_and_different_roles() { + // Arrange + let http_instance = ConfigurationInstanceId::new(ServiceRole::HttpTracker, 0); + let udp_instance = ConfigurationInstanceId::new(ServiceRole::UdpTracker, 0); + + // Act + let are_equal = http_instance == udp_instance; + + // Assert + assert!(!are_equal); + } + + #[test] + fn it_should_expose_its_role_and_zero_based_index() { + // Arrange + let instance_id = ConfigurationInstanceId::new(ServiceRole::UdpTracker, 1); + + // Act + let service_role = instance_id.service_role(); + let instance_index = instance_id.instance_index(); + + // Assert + assert_eq!(service_role, ServiceRole::UdpTracker); + assert_eq!(instance_index, 1); + } + + #[test] + fn it_should_serialize_the_role_and_instance_index() { + // Arrange + let instance_id = ConfigurationInstanceId::new(ServiceRole::HttpTracker, 0); + + // Act + let serialized = serde_json::to_string(&instance_id).unwrap(); + + // Assert + assert_eq!(serialized, r#"{"service_role":"http_tracker","instance_index":0}"#); + } +} diff --git a/packages/primitives/src/lib.rs b/packages/primitives/src/lib.rs index 78a084dde..f24f7462c 100644 --- a/packages/primitives/src/lib.rs +++ b/packages/primitives/src/lib.rs @@ -5,6 +5,7 @@ //! by the tracker server crate, but also by other crates in the Torrust //! ecosystem. pub mod announce; +pub mod configuration_instance_id; pub mod driver; pub mod mode; pub mod number_of_bytes; @@ -18,16 +19,19 @@ pub mod peer; pub mod peer_id; pub mod policy; pub mod scrape; +pub mod service_role; pub mod swarm_metadata; use std::collections::BTreeMap; pub use announce::{AnnounceData, AnnounceEvent, AnnouncePolicy}; +pub use configuration_instance_id::ConfigurationInstanceId; pub use driver::Driver; pub use mode::PrivateMode; pub use number_of_bytes::NumberOfBytes; pub use policy::TrackerPolicy; pub use scrape::ScrapeData; +pub use service_role::ServiceRole; /// Duration since the Unix Epoch. /// /// **Deprecated**: import from [`torrust_clock::DurationSinceUnixEpoch`] instead. diff --git a/packages/primitives/src/service_role.rs b/packages/primitives/src/service_role.rs new file mode 100644 index 000000000..82bba6a75 --- /dev/null +++ b/packages/primitives/src/service_role.rs @@ -0,0 +1,95 @@ +use std::fmt; + +use serde::{Deserialize, Serialize}; + +// issue: #2036 +/// A tracker application service role. +/// +/// This role identifies the application behavior implemented by a listener. +/// It does not identify its transport or socket binding: HTTP and HTTPS both +/// use [`Self::HttpTracker`] and differ through their service binding. +#[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Debug, Hash, Clone, Copy)] +#[serde(rename_all = "snake_case")] +pub enum ServiceRole { + /// A `BitTorrent` HTTP or HTTPS tracker service. + HttpTracker, + /// A `BitTorrent` UDP tracker service. + UdpTracker, + /// The tracker management REST API service. + #[serde(rename = "tracker_rest_api")] + RestApi, + /// The tracker health-check API service. + HealthCheckApi, +} + +impl ServiceRole { + /// Returns the stable tracker-owned identifier for this role. + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::HttpTracker => "http_tracker", + Self::UdpTracker => "udp_tracker", + Self::RestApi => "tracker_rest_api", + Self::HealthCheckApi => "health_check_api", + } + } +} + +impl fmt::Display for ServiceRole { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(self.as_str()) + } +} + +#[cfg(test)] +mod tests { + use crate::ServiceRole; + + #[test] + fn it_should_return_the_canonical_identifier_for_each_role() { + // Arrange + let roles = [ + (ServiceRole::HttpTracker, "http_tracker"), + (ServiceRole::UdpTracker, "udp_tracker"), + (ServiceRole::RestApi, "tracker_rest_api"), + (ServiceRole::HealthCheckApi, "health_check_api"), + ]; + + // Act and Assert + for (service_role, identifier) in roles { + assert_eq!(service_role.as_str(), identifier); + } + } + + #[test] + fn it_should_display_the_canonical_identifier_for_each_role() { + // Arrange + let roles = [ + (ServiceRole::HttpTracker, "http_tracker"), + (ServiceRole::UdpTracker, "udp_tracker"), + (ServiceRole::RestApi, "tracker_rest_api"), + (ServiceRole::HealthCheckApi, "health_check_api"), + ]; + + // Act and Assert + for (service_role, identifier) in roles { + assert_eq!(service_role.to_string(), identifier); + } + } + + #[test] + fn it_should_serialize_each_role_to_its_canonical_identifier() { + // Arrange + let roles = [ + (ServiceRole::HttpTracker, r#""http_tracker""#), + (ServiceRole::UdpTracker, r#""udp_tracker""#), + (ServiceRole::RestApi, r#""tracker_rest_api""#), + (ServiceRole::HealthCheckApi, r#""health_check_api""#), + ]; + + // Act and Assert + for (service_role, identifier) in roles { + assert_eq!(serde_json::to_string(&service_role).unwrap(), identifier); + } + } +} diff --git a/packages/udp-server/src/server/launcher.rs b/packages/udp-server/src/server/launcher.rs index 801f5f642..8c3573a93 100644 --- a/packages/udp-server/src/server/launcher.rs +++ b/packages/udp-server/src/server/launcher.rs @@ -11,6 +11,7 @@ use torrust_server_lib::logging::STARTED_ON; use torrust_server_lib::registar::ServiceHealthCheckJob; use torrust_server_lib::signals::{Halted, Started, shutdown_signal_with_message}; use torrust_tracker_client::udp::client::check; +use torrust_tracker_primitives::ServiceRole; use torrust_tracker_udp_core::container::UdpTrackerCoreContainer; use torrust_tracker_udp_core::event::ConnectionContext; use torrust_tracker_udp_core::{self, ConnectionIdValidationPolicy, UDP_TRACKER_LOG_TARGET}; @@ -23,7 +24,6 @@ use crate::server::bound_socket::BoundSocket; use crate::server::processor::Processor; use crate::server::receiver::Receiver; -const TYPE_STRING: &str = "udp_tracker"; /// A UDP server instance launcher. #[derive(Constructor)] pub struct Launcher; @@ -132,7 +132,12 @@ impl Launcher { let job = tokio::spawn(async move { check(&service_binding_clone).await }); - ServiceHealthCheckJob::new(service_binding.clone(), info, TYPE_STRING.to_string(), job) + ServiceHealthCheckJob::new( + service_binding.clone(), + info, + ServiceRole::UdpTracker.as_str().to_string(), + job, + ) } // issue-spec: docs/issues/drafts/simplify-udp-server-main-loop.md