Skip to content

Commit b088d23

Browse files
Alphadelta14Copilotdef-
authored
CLO-108 Pass app.kubernetes.io/* labels to clusterd/balancerd/console sts/pods/deploys/svcs (#37086)
This fixes missing app.kubernetes.io/* labels on clusterd pods/sts/svcs, environmentd svcs, balancerd svcs, console svcs. ### Motivation In doing some o11y work, I realized I could not easily select the clusterd pods in the cluster by their canonical app name. I also could not reach the Services for many other resources. I also added a few more canonical labels (part-of=materialize and managed-by=materialize-operator) ### Description Changes how default labels are propagated. Most resources have a canonical static app name (e.g., "clusterd" or "balancerd"). This is made to be a bit explicitly passed. Unfortunately, we set resources in quite a few places, so this change was made roughly 3 times. Definitely not super happy about that. ### Verification How do you know this change is correct? Describe new or existing automated tests, or manual steps you took. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Dennis Felsing <dennis@felsing.org>
1 parent 9fa3617 commit b088d23

9 files changed

Lines changed: 128 additions & 14 deletions

File tree

src/cloud-resources/src/crd.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,7 @@ pub trait ManagedResource: Resource<DynamicType = ()> + Sized {
7979

8080
fn managed_resource_meta(&self, name: String) -> ObjectMeta {
8181
let mut labels = self.default_labels();
82-
if let Some(app) = self.app_name() {
83-
labels.insert("app.kubernetes.io/name".to_owned(), app.to_owned());
84-
}
82+
labels.extend(recommended_k8s_labels(self.app_name()));
8583
ObjectMeta {
8684
namespace: Some(self.meta().namespace.clone().unwrap()),
8785
name: Some(name),
@@ -92,6 +90,26 @@ pub trait ManagedResource: Resource<DynamicType = ()> + Sized {
9290
}
9391
}
9492

93+
/// Get the recommended Kubernetes labels (app.kubernetes.io/*)
94+
/// WARNING: this is duplicated in src/orchestrator/src/lib.rs and src/orchestratord/src/k8s.rs
95+
pub fn recommended_k8s_labels(app_name: Option<&str>) -> BTreeMap<String, String> {
96+
let mut labels = BTreeMap::new();
97+
labels.insert(
98+
"app.kubernetes.io/managed-by".to_owned(),
99+
"materialize-operator".to_owned(),
100+
);
101+
labels.insert(
102+
"app.kubernetes.io/part-of".to_owned(),
103+
"materialize".to_owned(),
104+
);
105+
if let Some(app) = app_name {
106+
labels.insert("app.kubernetes.io/name".to_owned(), app.to_owned());
107+
// legacy label
108+
labels.insert("app".to_owned(), app.to_owned());
109+
}
110+
labels
111+
}
112+
95113
fn owner_reference<T: Resource<DynamicType = ()>>(t: &T) -> OwnerReference {
96114
OwnerReference {
97115
api_version: T::api_version(&()).to_string(),

src/controller/src/clusters.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ impl Controller {
729729
let service = self.orchestrator.ensure_service(
730730
&service_name,
731731
ServiceConfig {
732+
app_name: "clusterd".into(),
732733
image: self.clusterd_image.clone(),
733734
init_container_image: self.init_container_image.clone(),
734735
args: Box::new(move |assigned| {

src/orchestrator-kubernetes/src/lib.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ use mz_cloud_resources::crd::vpc_endpoint::v1::VpcEndpoint;
4949
use mz_orchestrator::{
5050
DiskLimit, LabelSelectionLogic, LabelSelector as MzLabelSelector, NamespacedOrchestrator,
5151
OfflineReason, Orchestrator, Service, ServiceAssignments, ServiceConfig, ServiceEvent,
52-
ServiceProcessMetrics, ServiceStatus, scheduling_config::*,
52+
ServiceProcessMetrics, ServiceStatus, recommended_k8s_labels, scheduling_config::*,
5353
};
5454
use mz_ore::cast::CastInto;
5555
use mz_ore::retry::Retry;
@@ -556,6 +556,7 @@ impl NamespacedOrchestrator for NamespacedKubernetesOrchestrator {
556556
&self,
557557
id: &str,
558558
ServiceConfig {
559+
app_name,
559560
image,
560561
init_container_image,
561562
args,
@@ -600,6 +601,11 @@ impl NamespacedOrchestrator for NamespacedKubernetesOrchestrator {
600601
labels.insert(self.make_label_key(&key), value);
601602
}
602603

604+
let standard_labels = recommended_k8s_labels(app_name);
605+
606+
// Standard Kubernetes labels
607+
labels.extend(standard_labels.clone());
608+
603609
labels.insert(self.make_label_key("scale"), scale.to_string());
604610

605611
for port in &ports_in {
@@ -645,6 +651,7 @@ impl NamespacedOrchestrator for NamespacedKubernetesOrchestrator {
645651
let service = K8sService {
646652
metadata: ObjectMeta {
647653
name: Some(name.clone()),
654+
labels: Some(standard_labels.clone()),
648655
..Default::default()
649656
},
650657
spec: Some(ServiceSpec {
@@ -1236,6 +1243,7 @@ impl NamespacedOrchestrator for NamespacedKubernetesOrchestrator {
12361243
let stateful_set = StatefulSet {
12371244
metadata: ObjectMeta {
12381245
name: Some(name.clone()),
1246+
labels: Some(standard_labels.clone()),
12391247
..Default::default()
12401248
},
12411249
spec: Some(StatefulSetSpec {

src/orchestrator/src/lib.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ pub struct LabelSelector {
191191
#[derive(Derivative)]
192192
#[derivative(Debug)]
193193
pub struct ServiceConfig {
194+
/// Static application name (usually present in labels)
195+
pub app_name: String,
194196
/// An opaque identifier for the executable or container image to run.
195197
///
196198
/// Often names a container on Docker Hub or a path on the local machine.
@@ -250,6 +252,24 @@ pub struct ServiceConfig {
250252
pub node_selector: BTreeMap<String, String>,
251253
}
252254

255+
/// Get the recommended Kubernetes labels (app.kubernetes.io/*)
256+
/// WARNING: this is duplicated in src/orchestratord/src/k8s.rs and src/cloud-resources/src/crd.rs
257+
pub fn recommended_k8s_labels(app_name: String) -> BTreeMap<String, String> {
258+
BTreeMap::from_iter([
259+
(
260+
"app.kubernetes.io/managed-by".to_owned(),
261+
"materialize-operator".to_owned(),
262+
),
263+
(
264+
"app.kubernetes.io/part-of".to_owned(),
265+
"materialize".to_owned(),
266+
),
267+
("app.kubernetes.io/name".to_owned(), app_name.to_owned()),
268+
// legacy label
269+
("app".to_owned(), app_name.to_owned()),
270+
])
271+
}
272+
253273
/// A named port associated with a service.
254274
#[derive(Debug, Clone, PartialEq, Eq)]
255275
pub struct ServicePort {

src/orchestratord/src/controller/balancer.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use tracing::{trace, warn};
3535

3636
use crate::{
3737
Error,
38-
k8s::{apply_resource, get_resource, replace_resource},
38+
k8s::{apply_resource, get_resource, recommended_k8s_labels, replace_resource},
3939
tls::{DefaultCertificateSpecs, create_certificate, issuer_ref_defined},
4040
};
4141
use mz_cloud_resources::crd::{
@@ -354,10 +354,13 @@ impl Context {
354354
..Default::default()
355355
};
356356

357+
let match_labels = pod_template_labels.clone();
358+
pod_template_labels.extend(recommended_k8s_labels(balancer.app_name()));
359+
357360
let deployment_spec = DeploymentSpec {
358361
replicas: Some(balancer.replicas()),
359362
selector: LabelSelector {
360-
match_labels: Some(pod_template_labels.clone()),
363+
match_labels: Some(match_labels),
361364
..Default::default()
362365
},
363366
strategy: Some(DeploymentStrategy {

src/orchestratord/src/controller/console.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ use tracing::{trace, warn};
3939

4040
use crate::{
4141
Error,
42-
k8s::{apply_resource, get_resource, replace_resource},
42+
k8s::{apply_resource, get_resource, recommended_k8s_labels, replace_resource},
4343
tls::{DefaultCertificateSpecs, create_certificate, issuer_ref_defined},
4444
};
4545
use mz_cloud_resources::crd::{
@@ -402,10 +402,13 @@ ssl_certificate_key /nginx/tls/tls.key;",
402402
..Default::default()
403403
};
404404

405+
let match_labels = pod_template_labels.clone();
406+
pod_template_labels.extend(recommended_k8s_labels("console".into()));
407+
405408
let deployment_spec = DeploymentSpec {
406409
replicas: Some(console.replicas()),
407410
selector: LabelSelector {
408-
match_labels: Some(pod_template_labels.clone()),
411+
match_labels: Some(match_labels),
409412
..Default::default()
410413
},
411414
template: PodTemplateSpec {

src/orchestratord/src/controller/materialize/generation.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ use super::matching_image_from_environmentd_image_ref;
4343
use crate::k8s::{apply_resource, delete_resource, get_resource};
4444
use crate::tls::issuer_ref_defined;
4545
use mz_cloud_provider::CloudProvider;
46-
use mz_cloud_resources::crd::ManagedResource;
4746
use mz_cloud_resources::crd::materialize::v1alpha1::Materialize;
47+
use mz_cloud_resources::crd::{ManagedResource, recommended_k8s_labels};
4848
use mz_ore::instrument;
4949

5050
static V140_DEV0: LazyLock<Version> = LazyLock::new(|| Version {
@@ -1131,11 +1131,7 @@ fn create_environmentd_statefulset_object(
11311131
"materialize.cloud/app".to_owned(),
11321132
mz.environmentd_app_name(),
11331133
);
1134-
pod_template_labels.insert("app".to_owned(), "environmentd".to_string());
1135-
pod_template_labels.insert(
1136-
"app.kubernetes.io/name".to_owned(),
1137-
"environmentd".to_string(),
1138-
);
1134+
pod_template_labels.extend(recommended_k8s_labels("environmentd".into()));
11391135
pod_template_labels.extend(
11401136
mz.spec
11411137
.pod_labels

src/orchestratord/src/k8s.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// the Business Source License, use of this software will be governed
88
// by the Apache License, Version 2.0.
99

10+
use std::collections::BTreeMap;
1011
use std::time::Duration;
1112

1213
use k8s_openapi::{
@@ -190,3 +191,18 @@ pub async fn register_crds(
190191

191192
Ok(())
192193
}
194+
195+
/// Get the recommended Kubernetes labels (app.kubernetes.io/*)
196+
/// WARNING: this is duplicated in src/orchestrator/src/lib.rs and src/cloud-resources/src/crd.rs
197+
pub fn recommended_k8s_labels(app_name: String) -> BTreeMap<String, String> {
198+
let mut labels = BTreeMap::new();
199+
labels.insert(
200+
"app.kubernetes.io/managed-by".into(),
201+
"materialize-operator".into(),
202+
);
203+
labels.insert("app.kubernetes.io/part-of".into(), "materialize".into());
204+
labels.insert("app.kubernetes.io/name".into(), app_name.to_owned());
205+
// legacy label
206+
labels.insert("app".into(), app_name.to_owned());
207+
labels
208+
}

test/orchestratord/mzcompose.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,6 +1627,55 @@ def check() -> None:
16271627
retry(check, 360)
16281628

16291629

1630+
class RecommendedK8sLabels(Modification):
1631+
@classmethod
1632+
def values(cls, version: MzVersion) -> list[Any]:
1633+
return [None]
1634+
1635+
@classmethod
1636+
def default(cls) -> Any:
1637+
return None
1638+
1639+
def modify(self, definition: dict[str, Any]) -> None:
1640+
pass
1641+
1642+
def validate(self, mods: dict[type[Modification], Any]) -> None:
1643+
if MzVersion.parse_mz(mods[EnvironmentdImageRef]) < MzVersion.parse_mz(
1644+
"v26.24.0"
1645+
):
1646+
return
1647+
1648+
def get(kind: str, name: str) -> dict[str, Any]:
1649+
return json.loads(
1650+
spawn.capture(
1651+
[
1652+
"kubectl",
1653+
"get",
1654+
kind,
1655+
name,
1656+
"-n",
1657+
"materialize-environment",
1658+
"-o",
1659+
"json",
1660+
]
1661+
)
1662+
)
1663+
1664+
def check() -> None:
1665+
pod = get_environmentd_data()["items"][0]
1666+
statefulset = get(
1667+
"statefulset", pod["metadata"]["labels"]["materialize.cloud/name"]
1668+
)
1669+
service = get("service", statefulset["spec"]["serviceName"])
1670+
for kind, obj in (("statefulset", statefulset), ("service", service)):
1671+
actual = obj["metadata"].get("labels", {}).get("app.kubernetes.io/name")
1672+
assert (
1673+
actual == "environmentd"
1674+
), f"Expected app.kubernetes.io/name=environmentd on {kind}/{obj['metadata']['name']}, got {actual!r}"
1675+
1676+
retry(check, 120)
1677+
1678+
16301679
class AuthenticatorKind(Modification):
16311680
@classmethod
16321681
def values(cls, version: MzVersion) -> list[Any]:

0 commit comments

Comments
 (0)