From 592fdc1f6eef12e29519a029d048b0b78adf8e76 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Tue, 1 Jul 2025 12:45:14 +0200 Subject: [PATCH 01/12] added kubelet.rs --- Cargo.lock | 1 + Cargo.toml | 1 + crates/stackable-operator/Cargo.toml | 1 + .../stackable-operator/src/utils/kubelet.rs | 75 +++++++++++++++++++ crates/stackable-operator/src/utils/mod.rs | 1 + 5 files changed, 79 insertions(+) create mode 100644 crates/stackable-operator/src/utils/kubelet.rs diff --git a/Cargo.lock b/Cargo.lock index c89c0e3e1..4b3e00997 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2988,6 +2988,7 @@ dependencies = [ "educe", "either", "futures", + "http", "indexmap 2.9.0", "json-patch", "k8s-openapi", diff --git a/Cargo.toml b/Cargo.toml index 4bef733c1..3d3dd0253 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ educe = { version = "0.6.0", default-features = false, features = ["Clone", "De either = "1.13.0" futures = "0.3.30" futures-util = "0.3.30" +http = "1.3.1" indexmap = "2.5.0" indoc = "2.0.6" insta = { version= "1.40", features = ["glob"] } diff --git a/crates/stackable-operator/Cargo.toml b/crates/stackable-operator/Cargo.toml index af065061f..ff34fd296 100644 --- a/crates/stackable-operator/Cargo.toml +++ b/crates/stackable-operator/Cargo.toml @@ -28,6 +28,7 @@ dockerfile-parser.workspace = true either.workspace = true educe.workspace = true futures.workspace = true +http.workspace = true indexmap.workspace = true json-patch.workspace = true k8s-openapi.workspace = true diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs new file mode 100644 index 000000000..a3e19ba1b --- /dev/null +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -0,0 +1,75 @@ +use k8s_openapi::api::core::v1::Node; +use kube::{ + Api, + api::{ListParams, ResourceExt}, + client::Client, +}; +use serde::Deserialize; +use http; +use snafu::{Snafu, ResultExt, OptionExt}; + +#[derive(Debug, Snafu)] +pub enum Error { + + #[snafu(display("failed to list nodes"))] + ListNodes { + source: kube::Error, + }, + #[snafu(display("failed to build proxy/configz request"))] + ConfigzRequest { + source: http::Error, + }, + + #[snafu(display("failed to fetch kubelet config from node {node}"))] + FetchNodeKubeletConfig { + source: kube::Error, + node: String, + }, + + #[snafu(display("failed to fetch `kubeletconfig` JSON key from configz response"))] + KubeletConfigJsonKey, + + #[snafu(display("failed to deserialize kubelet config JSON"))] + KubeletConfigJson { + source: serde_json::Error, + }, + + #[snafu(display("empty Kubernetes nodes list"))] + EmptyKubernetesNodesList, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct KubeletConfig { + cluster_domain: String, +} + +impl KubeletConfig { + pub async fn fetch(client: &Client) -> Result { + let api: Api = Api::all(client.clone()); + let nodes = api.list(&ListParams::default()).await.context(ListNodesSnafu)?; + let node = nodes.iter().next().context(EmptyKubernetesNodesListSnafu)?; + + let name = node.name_any(); + + // Query node stats by issuing a request to the admin endpoint. + // See https://kubernetes.io/docs/reference/instrumentation/node-metrics/ + let url = format!("/api/v1/nodes/{}/proxy/configz", name); + let req = http::Request::get(url).body(Default::default()).context(ConfigzRequestSnafu)?; + + // Deserialize JSON response as a JSON value. Alternatively, a type that + // implements `Deserialize` can be used. + let resp = client.request::(req).await.context(FetchNodeKubeletConfigSnafu { node: name })?; + + // Our JSON value is an object so we can treat it like a dictionary. + let summary = resp + .get("kubeletconfig") + .context(KubeletConfigJsonKeySnafu)?; + + // The base JSON representation includes a lot of metrics, including + // container metrics. Use a `NodeMetrics` type to deserialize only the + // values we care about. + serde_json::from_value::(summary.to_owned()).context(KubeletConfigJsonSnafu) + } + +} diff --git a/crates/stackable-operator/src/utils/mod.rs b/crates/stackable-operator/src/utils/mod.rs index b79b1d621..c905607e9 100644 --- a/crates/stackable-operator/src/utils/mod.rs +++ b/crates/stackable-operator/src/utils/mod.rs @@ -1,6 +1,7 @@ pub mod bash; pub mod cluster_info; pub mod crds; +mod kubelet; pub mod logging; mod option; mod url; From 77e8e1b7a0678dc6aa6338101a1feb509249e79e Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Tue, 1 Jul 2025 14:15:20 +0200 Subject: [PATCH 02/12] fetch kubelet config when initializing operators --- crates/stackable-operator/src/client.rs | 13 ++++- .../src/utils/cluster_info.rs | 18 +++---- .../stackable-operator/src/utils/kubelet.rs | 49 +++++++++---------- crates/stackable-operator/src/utils/mod.rs | 2 +- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/crates/stackable-operator/src/client.rs b/crates/stackable-operator/src/client.rs index 5a9ded61d..c51dae96f 100644 --- a/crates/stackable-operator/src/client.rs +++ b/crates/stackable-operator/src/client.rs @@ -21,7 +21,10 @@ use tracing::trace; use crate::{ kvp::LabelSelectorExt, - utils::cluster_info::{KubernetesClusterInfo, KubernetesClusterInfoOpts}, + utils::{ + cluster_info::{KubernetesClusterInfo, KubernetesClusterInfoOpts}, + kubelet, + }, }; pub type Result = std::result::Result; @@ -84,6 +87,9 @@ pub enum Error { #[snafu(display("unable to create kubernetes client"))] CreateKubeClient { source: kube::Error }, + + #[snafu(display("unable to fetch kubelet config"))] + KubeletConfig { source: kubelet::Error }, } /// This `Client` can be used to access Kubernetes. @@ -651,7 +657,10 @@ pub async fn initialize_operator( .context(InferKubeConfigSnafu)?; let default_namespace = kubeconfig.default_namespace.clone(); let client = kube::Client::try_from(kubeconfig).context(CreateKubeClientSnafu)?; - let cluster_info = KubernetesClusterInfo::new(cluster_info_opts); + let kubelet_config = kubelet::KubeletConfig::fetch(&client) + .await + .context(KubeletConfigSnafu)?; + let cluster_info = KubernetesClusterInfo::new(&kubelet_config, cluster_info_opts); Ok(Client::new( client, diff --git a/crates/stackable-operator/src/utils/cluster_info.rs b/crates/stackable-operator/src/utils/cluster_info.rs index d31275668..b9b6f8067 100644 --- a/crates/stackable-operator/src/utils/cluster_info.rs +++ b/crates/stackable-operator/src/utils/cluster_info.rs @@ -1,9 +1,6 @@ -use std::str::FromStr; - +use super::kubelet::KubeletConfig; use crate::commons::networking::DomainName; -const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local"; - /// Some information that we know about the Kubernetes cluster. #[derive(Debug, Clone)] pub struct KubernetesClusterInfo { @@ -21,7 +18,10 @@ pub struct KubernetesClusterInfoOpts { } impl KubernetesClusterInfo { - pub fn new(cluster_info_opts: &KubernetesClusterInfoOpts) -> Self { + pub fn new( + kubelet_config: &KubeletConfig, + cluster_info_opts: &KubernetesClusterInfoOpts, + ) -> Self { let cluster_domain = match &cluster_info_opts.kubernetes_cluster_domain { Some(cluster_domain) => { tracing::info!(%cluster_domain, "Using configured Kubernetes cluster domain"); @@ -29,12 +29,8 @@ impl KubernetesClusterInfo { cluster_domain.clone() } None => { - // TODO(sbernauer): Do some sort of advanced auto-detection, see https://github.com/stackabletech/issues/issues/436. - // There have been attempts of parsing the `/etc/resolv.conf`, but they have been - // reverted. Please read on the linked issue for details. - let cluster_domain = DomainName::from_str(KUBERNETES_CLUSTER_DOMAIN_DEFAULT) - .expect("KUBERNETES_CLUSTER_DOMAIN_DEFAULT constant must a valid domain"); - tracing::info!(%cluster_domain, "Defaulting Kubernetes cluster domain as it has not been configured"); + let cluster_domain = kubelet_config.cluster_domain.clone(); + tracing::info!(%cluster_domain, "Using kubelet config cluster domain"); cluster_domain } diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs index a3e19ba1b..83ff17640 100644 --- a/crates/stackable-operator/src/utils/kubelet.rs +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -1,3 +1,4 @@ +use http; use k8s_openapi::api::core::v1::Node; use kube::{ Api, @@ -5,34 +6,25 @@ use kube::{ client::Client, }; use serde::Deserialize; -use http; -use snafu::{Snafu, ResultExt, OptionExt}; +use snafu::{OptionExt, ResultExt, Snafu}; + +use crate::commons::networking::DomainName; #[derive(Debug, Snafu)] pub enum Error { - #[snafu(display("failed to list nodes"))] - ListNodes { - source: kube::Error, - }, + ListNodes { source: kube::Error }, #[snafu(display("failed to build proxy/configz request"))] - ConfigzRequest { - source: http::Error, - }, + ConfigzRequest { source: http::Error }, #[snafu(display("failed to fetch kubelet config from node {node}"))] - FetchNodeKubeletConfig { - source: kube::Error, - node: String, - }, + FetchNodeKubeletConfig { source: kube::Error, node: String }, #[snafu(display("failed to fetch `kubeletconfig` JSON key from configz response"))] KubeletConfigJsonKey, #[snafu(display("failed to deserialize kubelet config JSON"))] - KubeletConfigJson { - source: serde_json::Error, - }, + KubeletConfigJson { source: serde_json::Error }, #[snafu(display("empty Kubernetes nodes list"))] EmptyKubernetesNodesList, @@ -40,36 +32,39 @@ pub enum Error { #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] -struct KubeletConfig { - cluster_domain: String, +pub struct KubeletConfig { + pub cluster_domain: DomainName, } impl KubeletConfig { pub async fn fetch(client: &Client) -> Result { let api: Api = Api::all(client.clone()); - let nodes = api.list(&ListParams::default()).await.context(ListNodesSnafu)?; + let nodes = api + .list(&ListParams::default()) + .await + .context(ListNodesSnafu)?; let node = nodes.iter().next().context(EmptyKubernetesNodesListSnafu)?; let name = node.name_any(); - // Query node stats by issuing a request to the admin endpoint. - // See https://kubernetes.io/docs/reference/instrumentation/node-metrics/ + // Query kukbelet config let url = format!("/api/v1/nodes/{}/proxy/configz", name); - let req = http::Request::get(url).body(Default::default()).context(ConfigzRequestSnafu)?; + let req = http::Request::get(url) + .body(Default::default()) + .context(ConfigzRequestSnafu)?; // Deserialize JSON response as a JSON value. Alternatively, a type that // implements `Deserialize` can be used. - let resp = client.request::(req).await.context(FetchNodeKubeletConfigSnafu { node: name })?; + let resp = client + .request::(req) + .await + .context(FetchNodeKubeletConfigSnafu { node: name })?; // Our JSON value is an object so we can treat it like a dictionary. let summary = resp .get("kubeletconfig") .context(KubeletConfigJsonKeySnafu)?; - // The base JSON representation includes a lot of metrics, including - // container metrics. Use a `NodeMetrics` type to deserialize only the - // values we care about. serde_json::from_value::(summary.to_owned()).context(KubeletConfigJsonSnafu) } - } diff --git a/crates/stackable-operator/src/utils/mod.rs b/crates/stackable-operator/src/utils/mod.rs index c905607e9..a08b24a43 100644 --- a/crates/stackable-operator/src/utils/mod.rs +++ b/crates/stackable-operator/src/utils/mod.rs @@ -1,7 +1,7 @@ pub mod bash; pub mod cluster_info; pub mod crds; -mod kubelet; +pub mod kubelet; pub mod logging; mod option; mod url; From dd4b5d1aad41f44b715a0de2b9a0b3461cf6f1c0 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Tue, 1 Jul 2025 16:13:47 +0200 Subject: [PATCH 03/12] deserialize proxy reposponse once --- .../stackable-operator/src/utils/kubelet.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs index 83ff17640..e4491b1f0 100644 --- a/crates/stackable-operator/src/utils/kubelet.rs +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -30,6 +30,12 @@ pub enum Error { EmptyKubernetesNodesList, } +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct ProxyConfigResponse { + kubeletconfig: KubeletConfig, +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct KubeletConfig { @@ -37,6 +43,8 @@ pub struct KubeletConfig { } impl KubeletConfig { + + /// Fetches the kubelet configuration from the "first" node in the Kubernetes cluster. pub async fn fetch(client: &Client) -> Result { let api: Api = Api::all(client.clone()); let nodes = api @@ -53,18 +61,11 @@ impl KubeletConfig { .body(Default::default()) .context(ConfigzRequestSnafu)?; - // Deserialize JSON response as a JSON value. Alternatively, a type that - // implements `Deserialize` can be used. let resp = client - .request::(req) + .request::(req) .await .context(FetchNodeKubeletConfigSnafu { node: name })?; - // Our JSON value is an object so we can treat it like a dictionary. - let summary = resp - .get("kubeletconfig") - .context(KubeletConfigJsonKeySnafu)?; - - serde_json::from_value::(summary.to_owned()).context(KubeletConfigJsonSnafu) + Ok(resp.kubeletconfig) } } From f6b4aef0ca71e97146df8cbfe2ab788c8497da60 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Tue, 1 Jul 2025 16:56:10 +0200 Subject: [PATCH 04/12] don't fetch cluster domain from kubelet if the user has set it already --- crates/stackable-operator/CHANGELOG.md | 13 +++++++---- crates/stackable-operator/src/client.rs | 23 +++++++++++++++---- .../src/utils/cluster_info.rs | 18 +++++++++------ .../stackable-operator/src/utils/kubelet.rs | 1 - 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 4a740e29a..84a190f3f 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- The default Kubernetes cluster domain name is now fetched from the kubelet API unless explicitely configured ([#1068]). + ### Changed - Update `kube` to `1.1.0` ([#1049]). @@ -23,6 +27,7 @@ All notable changes to this project will be documented in this file. [#1058]: https://github.com/stackabletech/operator-rs/pull/1058 [#1060]: https://github.com/stackabletech/operator-rs/pull/1060 [#1064]: https://github.com/stackabletech/operator-rs/pull/1064 +[#1068]: https://github.com/stackabletech/operator-rs/pull/1068 ## [0.93.2] - 2025-05-26 @@ -148,7 +153,7 @@ All notable changes to this project will be documented in this file. ### Added - Add Deployments to `ClusterResource`s ([#992]). -- Add `DeploymentConditionBuilder` ([#993]). +- Add `DeploymentConditionBuilder` ([#993]). ### Changed @@ -369,7 +374,7 @@ All notable changes to this project will be documented in this file. ### Fixed - BREAKING: `KeyValuePairs::insert` (as well as `Labels::`/`Annotations::` via it) now overwrites - the old value if the key already exists. Previously, `iter()` would return *both* values in + the old value if the key already exists. Previously, `iter()` would return _both_ values in lexicographical order (causing further conversions like `Into` to prefer the maximum value) ([#888]). @@ -634,7 +639,7 @@ All notable changes to this project will be documented in this file. ### Changed -- Implement `PartialEq` for most *Snafu* Error enums ([#757]). +- Implement `PartialEq` for most _Snafu_ Error enums ([#757]). - Update Rust to 1.77 ([#759]) ### Fixed @@ -1385,7 +1390,7 @@ This is a rerelease of 0.25.1 which some last-minute incompatible API changes to ### Changed - Objects are now streamed rather than polled when waiting for them to be deleted ([#452]). -- serde\_yaml 0.8.26 -> 0.9.9 ([#450]) +- serde_yaml 0.8.26 -> 0.9.9 ([#450]) [#450]: https://github.com/stackabletech/operator-rs/pull/450 [#452]: https://github.com/stackabletech/operator-rs/pull/452 diff --git a/crates/stackable-operator/src/client.rs b/crates/stackable-operator/src/client.rs index c51dae96f..04428da00 100644 --- a/crates/stackable-operator/src/client.rs +++ b/crates/stackable-operator/src/client.rs @@ -657,10 +657,25 @@ pub async fn initialize_operator( .context(InferKubeConfigSnafu)?; let default_namespace = kubeconfig.default_namespace.clone(); let client = kube::Client::try_from(kubeconfig).context(CreateKubeClientSnafu)?; - let kubelet_config = kubelet::KubeletConfig::fetch(&client) - .await - .context(KubeletConfigSnafu)?; - let cluster_info = KubernetesClusterInfo::new(&kubelet_config, cluster_info_opts); + + let local_cluster_info_opts = match cluster_info_opts.kubernetes_cluster_domain { + None => { + trace!("Cluster domain not set, fetching kubelet config to determine cluster domain."); + + let kubelet_config = kubelet::KubeletConfig::fetch(&client) + .await + .context(KubeletConfigSnafu)?; + + KubernetesClusterInfoOpts { + kubernetes_cluster_domain: Some(kubelet_config.cluster_domain), + } + } + _ => KubernetesClusterInfoOpts { + kubernetes_cluster_domain: cluster_info_opts.kubernetes_cluster_domain.clone(), + }, + }; + + let cluster_info = KubernetesClusterInfo::new(&local_cluster_info_opts); Ok(Client::new( client, diff --git a/crates/stackable-operator/src/utils/cluster_info.rs b/crates/stackable-operator/src/utils/cluster_info.rs index b9b6f8067..d31275668 100644 --- a/crates/stackable-operator/src/utils/cluster_info.rs +++ b/crates/stackable-operator/src/utils/cluster_info.rs @@ -1,6 +1,9 @@ -use super::kubelet::KubeletConfig; +use std::str::FromStr; + use crate::commons::networking::DomainName; +const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local"; + /// Some information that we know about the Kubernetes cluster. #[derive(Debug, Clone)] pub struct KubernetesClusterInfo { @@ -18,10 +21,7 @@ pub struct KubernetesClusterInfoOpts { } impl KubernetesClusterInfo { - pub fn new( - kubelet_config: &KubeletConfig, - cluster_info_opts: &KubernetesClusterInfoOpts, - ) -> Self { + pub fn new(cluster_info_opts: &KubernetesClusterInfoOpts) -> Self { let cluster_domain = match &cluster_info_opts.kubernetes_cluster_domain { Some(cluster_domain) => { tracing::info!(%cluster_domain, "Using configured Kubernetes cluster domain"); @@ -29,8 +29,12 @@ impl KubernetesClusterInfo { cluster_domain.clone() } None => { - let cluster_domain = kubelet_config.cluster_domain.clone(); - tracing::info!(%cluster_domain, "Using kubelet config cluster domain"); + // TODO(sbernauer): Do some sort of advanced auto-detection, see https://github.com/stackabletech/issues/issues/436. + // There have been attempts of parsing the `/etc/resolv.conf`, but they have been + // reverted. Please read on the linked issue for details. + let cluster_domain = DomainName::from_str(KUBERNETES_CLUSTER_DOMAIN_DEFAULT) + .expect("KUBERNETES_CLUSTER_DOMAIN_DEFAULT constant must a valid domain"); + tracing::info!(%cluster_domain, "Defaulting Kubernetes cluster domain as it has not been configured"); cluster_domain } diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs index e4491b1f0..d7bdd5543 100644 --- a/crates/stackable-operator/src/utils/kubelet.rs +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -43,7 +43,6 @@ pub struct KubeletConfig { } impl KubeletConfig { - /// Fetches the kubelet configuration from the "first" node in the Kubernetes cluster. pub async fn fetch(client: &Client) -> Result { let api: Api = Api::all(client.clone()); From 5fe91bdc446d36525c5c5ee9f3e4b2a47b3bd087 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Tue, 1 Jul 2025 17:09:29 +0200 Subject: [PATCH 05/12] remove comment with typo --- crates/stackable-operator/src/utils/kubelet.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs index d7bdd5543..add9e2eeb 100644 --- a/crates/stackable-operator/src/utils/kubelet.rs +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -54,7 +54,6 @@ impl KubeletConfig { let name = node.name_any(); - // Query kukbelet config let url = format!("/api/v1/nodes/{}/proxy/configz", name); let req = http::Request::get(url) .body(Default::default()) From ecd63cdf4ca52ac092c12e5fc6fffbc84b0af4eb Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Tue, 1 Jul 2025 17:14:50 +0200 Subject: [PATCH 06/12] revert unintended auto-format --- crates/stackable-operator/CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 84a190f3f..a44dabece 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -153,7 +153,7 @@ All notable changes to this project will be documented in this file. ### Added - Add Deployments to `ClusterResource`s ([#992]). -- Add `DeploymentConditionBuilder` ([#993]). +- Add `DeploymentConditionBuilder` ([#993]). ### Changed @@ -374,7 +374,7 @@ All notable changes to this project will be documented in this file. ### Fixed - BREAKING: `KeyValuePairs::insert` (as well as `Labels::`/`Annotations::` via it) now overwrites - the old value if the key already exists. Previously, `iter()` would return _both_ values in + the old value if the key already exists. Previously, `iter()` would return *both* values in lexicographical order (causing further conversions like `Into` to prefer the maximum value) ([#888]). @@ -639,7 +639,7 @@ All notable changes to this project will be documented in this file. ### Changed -- Implement `PartialEq` for most _Snafu_ Error enums ([#757]). +- Implement `PartialEq` for most *Snafu* Error enums ([#757]). - Update Rust to 1.77 ([#759]) ### Fixed @@ -1390,7 +1390,7 @@ This is a rerelease of 0.25.1 which some last-minute incompatible API changes to ### Changed - Objects are now streamed rather than polled when waiting for them to be deleted ([#452]). -- serde_yaml 0.8.26 -> 0.9.9 ([#450]) +- serde\_yaml 0.8.26 -> 0.9.9 ([#450]) [#450]: https://github.com/stackabletech/operator-rs/pull/450 [#452]: https://github.com/stackabletech/operator-rs/pull/452 From ea12f243544a1130dd4f2fa87c43063d4bdb8687 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Wed, 2 Jul 2025 15:56:16 +0200 Subject: [PATCH 07/12] Apply suggestions from code review Co-authored-by: Sebastian Bernauer --- crates/stackable-operator/src/utils/kubelet.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs index add9e2eeb..3dca0d15a 100644 --- a/crates/stackable-operator/src/utils/kubelet.rs +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -14,10 +14,11 @@ use crate::commons::networking::DomainName; pub enum Error { #[snafu(display("failed to list nodes"))] ListNodes { source: kube::Error }, - #[snafu(display("failed to build proxy/configz request"))] + + #[snafu(display("failed to build \"/proxy/configz\" request"))] ConfigzRequest { source: http::Error }, - #[snafu(display("failed to fetch kubelet config from node {node}"))] + #[snafu(display("failed to fetch kubelet config from node {node:?}"))] FetchNodeKubeletConfig { source: kube::Error, node: String }, #[snafu(display("failed to fetch `kubeletconfig` JSON key from configz response"))] @@ -51,10 +52,9 @@ impl KubeletConfig { .await .context(ListNodesSnafu)?; let node = nodes.iter().next().context(EmptyKubernetesNodesListSnafu)?; + let node_name = node.name_any(); - let name = node.name_any(); - - let url = format!("/api/v1/nodes/{}/proxy/configz", name); + let url = format!("/api/v1/nodes/{name_name}/proxy/configz"); let req = http::Request::get(url) .body(Default::default()) .context(ConfigzRequestSnafu)?; From 357a62de0595f8cbd2c4dfaf3ff2c3b0f5b8e8e5 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Wed, 2 Jul 2025 16:03:28 +0200 Subject: [PATCH 08/12] better error messages --- .../stackable-operator/src/utils/kubelet.rs | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs index 3dca0d15a..97be91d81 100644 --- a/crates/stackable-operator/src/utils/kubelet.rs +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -15,8 +15,11 @@ pub enum Error { #[snafu(display("failed to list nodes"))] ListNodes { source: kube::Error }, - #[snafu(display("failed to build \"/proxy/configz\" request"))] - ConfigzRequest { source: http::Error }, + #[snafu(display("failed to build \"/api/v1/nodes/{node_name}/proxy/configz\" request"))] + ConfigzRequest { + source: http::Error, + node_name: String, + }, #[snafu(display("failed to fetch kubelet config from node {node:?}"))] FetchNodeKubeletConfig { source: kube::Error, node: String }, @@ -27,7 +30,9 @@ pub enum Error { #[snafu(display("failed to deserialize kubelet config JSON"))] KubeletConfigJson { source: serde_json::Error }, - #[snafu(display("empty Kubernetes nodes list"))] + #[snafu(display( + "empty Kubernetes nodes list. At least one node is required to fetch the cluster domain from the kubelet config" + ))] EmptyKubernetesNodesList, } @@ -54,15 +59,18 @@ impl KubeletConfig { let node = nodes.iter().next().context(EmptyKubernetesNodesListSnafu)?; let node_name = node.name_any(); - let url = format!("/api/v1/nodes/{name_name}/proxy/configz"); - let req = http::Request::get(url) - .body(Default::default()) - .context(ConfigzRequestSnafu)?; + let url = format!("/api/v1/nodes/{node_name}/proxy/configz"); + let req = + http::Request::get(url) + .body(Default::default()) + .context(ConfigzRequestSnafu { + node_name: node_name.clone(), + })?; let resp = client .request::(req) .await - .context(FetchNodeKubeletConfigSnafu { node: name })?; + .context(FetchNodeKubeletConfigSnafu { node: node_name })?; Ok(resp.kubeletconfig) } From 2e71f6751c344e4d0d46b05737c8dd75990247cf Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Wed, 2 Jul 2025 16:32:52 +0200 Subject: [PATCH 09/12] move kubelet query to cluster_info mod --- crates/stackable-operator/src/client.rs | 33 +++++-------------- .../src/utils/cluster_info.rs | 31 ++++++++++------- 2 files changed, 27 insertions(+), 37 deletions(-) diff --git a/crates/stackable-operator/src/client.rs b/crates/stackable-operator/src/client.rs index 04428da00..897a3d843 100644 --- a/crates/stackable-operator/src/client.rs +++ b/crates/stackable-operator/src/client.rs @@ -21,10 +21,7 @@ use tracing::trace; use crate::{ kvp::LabelSelectorExt, - utils::{ - cluster_info::{KubernetesClusterInfo, KubernetesClusterInfoOpts}, - kubelet, - }, + utils::cluster_info::{KubernetesClusterInfo, KubernetesClusterInfoOpts}, }; pub type Result = std::result::Result; @@ -88,8 +85,10 @@ pub enum Error { #[snafu(display("unable to create kubernetes client"))] CreateKubeClient { source: kube::Error }, - #[snafu(display("unable to fetch kubelet config"))] - KubeletConfig { source: kubelet::Error }, + #[snafu(display("unable to fetch cluster information from kubelet"))] + NewKubeletClusterInfo { + source: crate::utils::cluster_info::Error, + }, } /// This `Client` can be used to access Kubernetes. @@ -657,25 +656,9 @@ pub async fn initialize_operator( .context(InferKubeConfigSnafu)?; let default_namespace = kubeconfig.default_namespace.clone(); let client = kube::Client::try_from(kubeconfig).context(CreateKubeClientSnafu)?; - - let local_cluster_info_opts = match cluster_info_opts.kubernetes_cluster_domain { - None => { - trace!("Cluster domain not set, fetching kubelet config to determine cluster domain."); - - let kubelet_config = kubelet::KubeletConfig::fetch(&client) - .await - .context(KubeletConfigSnafu)?; - - KubernetesClusterInfoOpts { - kubernetes_cluster_domain: Some(kubelet_config.cluster_domain), - } - } - _ => KubernetesClusterInfoOpts { - kubernetes_cluster_domain: cluster_info_opts.kubernetes_cluster_domain.clone(), - }, - }; - - let cluster_info = KubernetesClusterInfo::new(&local_cluster_info_opts); + let cluster_info = KubernetesClusterInfo::new(&client, cluster_info_opts) + .await + .context(NewKubeletClusterInfoSnafu)?; Ok(Client::new( client, diff --git a/crates/stackable-operator/src/utils/cluster_info.rs b/crates/stackable-operator/src/utils/cluster_info.rs index d31275668..95ea7aa2b 100644 --- a/crates/stackable-operator/src/utils/cluster_info.rs +++ b/crates/stackable-operator/src/utils/cluster_info.rs @@ -1,10 +1,14 @@ -use std::str::FromStr; +use kube::Client; +use snafu::{ResultExt, Snafu}; -use crate::commons::networking::DomainName; +use crate::{commons::networking::DomainName, utils::kubelet}; -const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local"; +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("unable to fetch kubelet config"))] + KubeletConfig { source: kubelet::Error }, +} -/// Some information that we know about the Kubernetes cluster. #[derive(Debug, Clone)] pub struct KubernetesClusterInfo { /// The Kubernetes cluster domain, typically `cluster.local`. @@ -21,7 +25,10 @@ pub struct KubernetesClusterInfoOpts { } impl KubernetesClusterInfo { - pub fn new(cluster_info_opts: &KubernetesClusterInfoOpts) -> Self { + pub async fn new( + client: &Client, + cluster_info_opts: &KubernetesClusterInfoOpts, + ) -> Result { let cluster_domain = match &cluster_info_opts.kubernetes_cluster_domain { Some(cluster_domain) => { tracing::info!(%cluster_domain, "Using configured Kubernetes cluster domain"); @@ -29,17 +36,17 @@ impl KubernetesClusterInfo { cluster_domain.clone() } None => { - // TODO(sbernauer): Do some sort of advanced auto-detection, see https://github.com/stackabletech/issues/issues/436. - // There have been attempts of parsing the `/etc/resolv.conf`, but they have been - // reverted. Please read on the linked issue for details. - let cluster_domain = DomainName::from_str(KUBERNETES_CLUSTER_DOMAIN_DEFAULT) - .expect("KUBERNETES_CLUSTER_DOMAIN_DEFAULT constant must a valid domain"); - tracing::info!(%cluster_domain, "Defaulting Kubernetes cluster domain as it has not been configured"); + let kubelet_config = kubelet::KubeletConfig::fetch(client) + .await + .context(KubeletConfigSnafu)?; + + let cluster_domain = kubelet_config.cluster_domain; + tracing::info!(%cluster_domain, "Using Kubernetes cluster domain from the kubelet config"); cluster_domain } }; - Self { cluster_domain } + Ok(Self { cluster_domain }) } } From 4a07fd3155826cb044c8c432d22a6d6183d04b52 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Thu, 3 Jul 2025 08:49:53 +0200 Subject: [PATCH 10/12] review feedback --- crates/stackable-operator/src/utils/kubelet.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/stackable-operator/src/utils/kubelet.rs b/crates/stackable-operator/src/utils/kubelet.rs index 97be91d81..336312381 100644 --- a/crates/stackable-operator/src/utils/kubelet.rs +++ b/crates/stackable-operator/src/utils/kubelet.rs @@ -15,10 +15,10 @@ pub enum Error { #[snafu(display("failed to list nodes"))] ListNodes { source: kube::Error }, - #[snafu(display("failed to build \"/api/v1/nodes/{node_name}/proxy/configz\" request"))] - ConfigzRequest { + #[snafu(display("failed to build request for url path \"{url_path}\""))] + BuildConfigzRequest { source: http::Error, - node_name: String, + url_path: String, }, #[snafu(display("failed to fetch kubelet config from node {node:?}"))] @@ -59,13 +59,10 @@ impl KubeletConfig { let node = nodes.iter().next().context(EmptyKubernetesNodesListSnafu)?; let node_name = node.name_any(); - let url = format!("/api/v1/nodes/{node_name}/proxy/configz"); - let req = - http::Request::get(url) - .body(Default::default()) - .context(ConfigzRequestSnafu { - node_name: node_name.clone(), - })?; + let url_path = format!("/api/v1/nodes/{node_name}/proxy/configz"); + let req = http::Request::get(url_path.clone()) + .body(Default::default()) + .context(BuildConfigzRequestSnafu { url_path })?; let resp = client .request::(req) From a5120a2927fa81028955d50606cea34ca3b8c43a Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Thu, 3 Jul 2025 10:16:54 +0200 Subject: [PATCH 11/12] Update crates/stackable-operator/CHANGELOG.md Co-authored-by: Sebastian Bernauer --- crates/stackable-operator/CHANGELOG.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index a44dabece..5ee5a499f 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -6,7 +6,18 @@ All notable changes to this project will be documented in this file. ### Added -- The default Kubernetes cluster domain name is now fetched from the kubelet API unless explicitely configured ([#1068]). +- The default Kubernetes cluster domain name is now fetched from the kubelet API unless explicitly configured ([#1068])
+ This requires operators to have the RBAC permission to `get` `nodes/proxy` in the apiGroup "", an example RBAC rule could look like: + ```yaml + --- + apiVersion: rbac.authorization.k8s.io/v1 + kind: ClusterRole + metadata: + name: operator-cluster-role + rules: + - apiGroups: [""] + resources: [nodes/proxy] + verbs: [get] ### Changed From 3338c56a67514cb8a5006c18f83bfef918f254c3 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Thu, 3 Jul 2025 10:25:25 +0200 Subject: [PATCH 12/12] fix md lint --- crates/stackable-operator/CHANGELOG.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 5ee5a499f..e7888ce92 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -6,8 +6,9 @@ All notable changes to this project will be documented in this file. ### Added -- The default Kubernetes cluster domain name is now fetched from the kubelet API unless explicitly configured ([#1068])
+- The default Kubernetes cluster domain name is now fetched from the kubelet API unless explicitly configured ([#1068]) This requires operators to have the RBAC permission to `get` `nodes/proxy` in the apiGroup "", an example RBAC rule could look like: + ```yaml --- apiVersion: rbac.authorization.k8s.io/v1 @@ -18,6 +19,7 @@ All notable changes to this project will be documented in this file. - apiGroups: [""] resources: [nodes/proxy] verbs: [get] + ``` ### Changed @@ -164,7 +166,7 @@ All notable changes to this project will be documented in this file. ### Added - Add Deployments to `ClusterResource`s ([#992]). -- Add `DeploymentConditionBuilder` ([#993]). +- Add `DeploymentConditionBuilder` ([#993]). ### Changed @@ -385,7 +387,7 @@ All notable changes to this project will be documented in this file. ### Fixed - BREAKING: `KeyValuePairs::insert` (as well as `Labels::`/`Annotations::` via it) now overwrites - the old value if the key already exists. Previously, `iter()` would return *both* values in + the old value if the key already exists. Previously, `iter()` would return _both_ values in lexicographical order (causing further conversions like `Into` to prefer the maximum value) ([#888]). @@ -650,7 +652,7 @@ All notable changes to this project will be documented in this file. ### Changed -- Implement `PartialEq` for most *Snafu* Error enums ([#757]). +- Implement `PartialEq` for most _Snafu_ Error enums ([#757]). - Update Rust to 1.77 ([#759]) ### Fixed @@ -1401,7 +1403,7 @@ This is a rerelease of 0.25.1 which some last-minute incompatible API changes to ### Changed - Objects are now streamed rather than polled when waiting for them to be deleted ([#452]). -- serde\_yaml 0.8.26 -> 0.9.9 ([#450]) +- serde_yaml 0.8.26 -> 0.9.9 ([#450]) [#450]: https://github.com/stackabletech/operator-rs/pull/450 [#452]: https://github.com/stackabletech/operator-rs/pull/452