NationTech/harmony
6
0
Fork 1
Code Issues 32 Pull Requests 6 Actions Packages Projects Releases 1 Wiki Activity

refactor/ns #74

Merged
wjro merged 8 commits from refactor/ns into master 2025-07-02 19:54:33 +00:00
Conversation 0 Commits 8 Files Changed 14 +502 -67
wjro commented 2025-07-02 15:16:41 +00:00
Owner
Copy Link
No description provided.
wjro added 3 commits 2025-07-02 15:16:46 +00:00
refactor: monitoring takes namespace from tenant
All checks were successful
Run Check Script / check (pull_request) Successful in -6s
Details
7de9860dcf
wjro added 1 commit 2025-07-02 15:27:40 +00:00
fix merge conflict
All checks were successful
Run Check Script / check (pull_request) Successful in 1m50s
Details
e857efa92f
johnride requested changes 2025-07-02 15:58:13 +00:00
harmony/src/domain/topology/k8s_anywhere.rs Outdated
@ -287,1 +296,4 @@
}
fn get_tenant_config(&self) -> Option<TenantConfig> {
self.tenant_manager_config.get().cloned()
johnride commented 2025-07-02 15:51:32 +00:00
Owner
Copy Link

K8sTenantManager is responsible for concrete implementation. K8sAnywhere should delegate

K8sTenantManager is responsible for concrete implementation. K8sAnywhere should delegate
harmony/src/domain/topology/tenant/k8s.rs Outdated
@ -346,4 +380,7 @@ impl TenantManager for K8sTenantManager {
);
Ok(())
}
fn get_tenant_config(&self) -> Option<TenantConfig> {
johnride commented 2025-07-02 15:49:16 +00:00
Owner
Copy Link

Smells like interface segregation issue

Smells like interface segregation issue
harmony/src/modules/monitoring/kube_prometheus/helm/config.rs Outdated
@ -43,3 +40,1 @@
kube_controller_manager: false,
kube_etcd: false,
kube_proxy: false,
kubernetes_api_server: true,
johnride commented 2025-07-02 15:52:27 +00:00
Owner
Copy Link

FIXME only required components.

FIXME only required components.
harmony/src/modules/monitoring/kube_prometheus/helm/types.rs Outdated
@ -0,0 +1,142 @@
// // in your build_score function...
johnride commented 2025-07-02 15:53:07 +00:00
Owner
Copy Link

Delete this file

Delete this file
harmony/src/modules/monitoring/kube_prometheus/prometheus.rs
@ -53,0 +63,4 @@
}
pub fn configure_with_topology<T: TenantManager>(&self, topology: &T) {
let ns = topology
johnride commented 2025-07-02 15:57:21 +00:00
Owner
Copy Link

Higher level components such as Monitoring, Alerting, etc. Should not be aware of the Tenant concept.

The topology itself should manage internally the logic related to the tenant.

error!("This must be refactored, see comments in pr #74");

Higher level components such as Monitoring, Alerting, etc. Should not be aware of the Tenant concept. The topology itself should manage internally the logic related to the tenant. error!("This must be refactored, see comments in pr #74");
wjro added 1 commit 2025-07-02 17:35:48 +00:00
fix: comments from pr
All checks were successful
Run Check Script / check (pull_request) Successful in 5s
Details
f2a350fae6
wjro added 1 commit 2025-07-02 17:46:47 +00:00
fix: merge conflict
Some checks failed
Run Check Script / check (pull_request) Failing after 41s
Details
82119076cf
taha approved these changes 2025-07-02 18:32:58 +00:00
harmony/src/modules/monitoring/kube_prometheus/helm_prometheus_alert_score.rs
@ -20,3 +21,3 @@
}
impl<T: Topology + HelmCommand> Score<T> for HelmPrometheusAlertingScore {
impl<T: Topology + HelmCommand + TenantManager> Score<T> for HelmPrometheusAlertingScore {
taha commented 2025-07-02 18:32:28 +00:00
Collaborator
Copy Link

as noted in a comment below, should remove TenantManager later

as noted in a comment below, should remove TenantManager later
harmony/src/modules/monitoring/kube_prometheus/types.rs
@ -46,0 +58,4 @@
}
#[derive(Debug, Clone, Serialize)]
pub struct Resources {
taha commented 2025-07-02 18:31:56 +00:00
Collaborator
Copy Link

I wonder if we can replace this with structs from k8s-openapi or kube-rs

I wonder if we can replace this with structs from k8s-openapi or kube-rs
wjro added 2 commits 2025-07-02 19:52:32 +00:00
fix: K8sTenantManager is responsible for concrete implementation. K8sAnywhere should delegate
All checks were successful
Run Check Script / check (pull_request) Successful in 4s
Details
3eecc2f590
wjro merged commit 6bf10b093c into master 2025-07-02 19:54:33 +00:00
wjro deleted branch refactor/ns 2025-07-02 19:54:34 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
johnride
taha
Labels
Clear labels   
Architecture Decision Record

   
Good first issue

   
RFC
No Label
Architecture Decision Record
Good first issue
RFC
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: NationTech/harmony#74
No description provided.
Delete Branch "refactor/ns"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?