Skip to content

Commit 7328943

Browse files
authored
Use the local kubelet's cluster domain, rather than a random one (#1071)
* Use the local kubelet's cluster domain, rather than a random one * Changelog * Fix tests * Fix doctests too
1 parent 903249c commit 7328943

File tree

5 files changed

+93
-47
lines changed

5 files changed

+93
-47
lines changed

crates/stackable-operator/CHANGELOG.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file.
66

77
### Added
88

9-
- The default Kubernetes cluster domain name is now fetched from the kubelet API unless explicitly configured ([#1068])
9+
- The default Kubernetes cluster domain name is now fetched from the kubelet API unless explicitly configured ([#1068], [#1071])
1010
This requires operators to have the RBAC permission to `get` `nodes/proxy` in the apiGroup "", an example RBAC rule could look like:
1111

1212
```yaml
@@ -21,6 +21,16 @@ All notable changes to this project will be documented in this file.
2121
verbs: [get]
2222
```
2323
24+
In addition, they must be provided the environment variable `KUBERNETES_NODE_NAME` like this:
25+
26+
```yaml
27+
env:
28+
- name: KUBERNETES_NODE_NAME
29+
valueFrom:
30+
fieldRef:
31+
fieldPath: spec.nodeName
32+
```
33+
2434
### Changed
2535

2636
- Update `kube` to `1.1.0` ([#1049]).
@@ -41,6 +51,7 @@ All notable changes to this project will be documented in this file.
4151
[#1060]: https://github.com/stackabletech/operator-rs/pull/1060
4252
[#1064]: https://github.com/stackabletech/operator-rs/pull/1064
4353
[#1068]: https://github.com/stackabletech/operator-rs/pull/1068
54+
[#1071]: https://github.com/stackabletech/operator-rs/pull/1071
4455

4556
## [0.93.2] - 2025-05-26
4657

crates/stackable-operator/src/cli.rs

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ pub enum Command<Run: Args = ProductOperatorRun> {
165165
/// ```rust
166166
/// # use stackable_operator::cli::{Command, ProductOperatorRun, ProductConfigPath};
167167
/// use clap::Parser;
168-
/// use stackable_operator::namespace::WatchNamespace;
168+
/// use stackable_operator::{namespace::WatchNamespace, utils::cluster_info::KubernetesClusterInfoOpts};
169169
/// use stackable_telemetry::tracing::TelemetryOptions;
170170
///
171171
/// #[derive(clap::Parser, Debug, PartialEq, Eq)]
@@ -176,14 +176,17 @@ pub enum Command<Run: Args = ProductOperatorRun> {
176176
/// common: ProductOperatorRun,
177177
/// }
178178
///
179-
/// let opts = Command::<Run>::parse_from(["foobar-operator", "run", "--name", "foo", "--product-config", "bar", "--watch-namespace", "foobar"]);
179+
/// let opts = Command::<Run>::parse_from(["foobar-operator", "run", "--name", "foo", "--product-config", "bar", "--watch-namespace", "foobar", "--kubernetes-node-name", "baz"]);
180180
/// assert_eq!(opts, Command::Run(Run {
181181
/// name: "foo".to_string(),
182182
/// common: ProductOperatorRun {
183183
/// product_config: ProductConfigPath::from("bar".as_ref()),
184184
/// watch_namespace: WatchNamespace::One("foobar".to_string()),
185185
/// telemetry_arguments: TelemetryOptions::default(),
186-
/// cluster_info_opts: Default::default(),
186+
/// cluster_info_opts: KubernetesClusterInfoOpts {
187+
/// kubernetes_cluster_domain: None,
188+
/// kubernetes_node_name: "baz".to_string(),
189+
/// },
187190
/// },
188191
/// }));
189192
/// ```
@@ -388,38 +391,61 @@ mod tests {
388391
"bar",
389392
"--watch-namespace",
390393
"foo",
394+
"--kubernetes-node-name",
395+
"baz",
391396
]);
392397
assert_eq!(
393398
opts,
394399
ProductOperatorRun {
395400
product_config: ProductConfigPath::from("bar".as_ref()),
396401
watch_namespace: WatchNamespace::One("foo".to_string()),
397-
cluster_info_opts: Default::default(),
402+
cluster_info_opts: KubernetesClusterInfoOpts {
403+
kubernetes_cluster_domain: None,
404+
kubernetes_node_name: "baz".to_string()
405+
},
398406
telemetry_arguments: Default::default(),
399407
}
400408
);
401409

402410
// no cli / no env
403-
let opts = ProductOperatorRun::parse_from(["run", "--product-config", "bar"]);
411+
let opts = ProductOperatorRun::parse_from([
412+
"run",
413+
"--product-config",
414+
"bar",
415+
"--kubernetes-node-name",
416+
"baz",
417+
]);
404418
assert_eq!(
405419
opts,
406420
ProductOperatorRun {
407421
product_config: ProductConfigPath::from("bar".as_ref()),
408422
watch_namespace: WatchNamespace::All,
409-
cluster_info_opts: Default::default(),
423+
cluster_info_opts: KubernetesClusterInfoOpts {
424+
kubernetes_cluster_domain: None,
425+
kubernetes_node_name: "baz".to_string()
426+
},
410427
telemetry_arguments: Default::default(),
411428
}
412429
);
413430

414431
// env with namespace
415432
unsafe { env::set_var(WATCH_NAMESPACE, "foo") };
416-
let opts = ProductOperatorRun::parse_from(["run", "--product-config", "bar"]);
433+
let opts = ProductOperatorRun::parse_from([
434+
"run",
435+
"--product-config",
436+
"bar",
437+
"--kubernetes-node-name",
438+
"baz",
439+
]);
417440
assert_eq!(
418441
opts,
419442
ProductOperatorRun {
420443
product_config: ProductConfigPath::from("bar".as_ref()),
421444
watch_namespace: WatchNamespace::One("foo".to_string()),
422-
cluster_info_opts: Default::default(),
445+
cluster_info_opts: KubernetesClusterInfoOpts {
446+
kubernetes_cluster_domain: None,
447+
kubernetes_node_name: "baz".to_string()
448+
},
423449
telemetry_arguments: Default::default(),
424450
}
425451
);

crates/stackable-operator/src/client.rs

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -523,15 +523,19 @@ impl Client {
523523
///
524524
/// ```no_run
525525
/// use std::time::Duration;
526+
/// use clap::Parser;
526527
/// use tokio::time::error::Elapsed;
527528
/// use kube::runtime::watcher;
528529
/// use k8s_openapi::api::core::v1::Pod;
529-
/// use stackable_operator::client::{Client, initialize_operator};
530+
/// use stackable_operator::{
531+
/// client::{Client, initialize_operator},
532+
/// utils::cluster_info::KubernetesClusterInfoOpts,
533+
/// };
530534
///
531535
/// #[tokio::main]
532536
/// async fn main() {
533-
///
534-
/// let client = initialize_operator(None, &Default::default())
537+
/// let cluster_info_opts = KubernetesClusterInfoOpts::parse();
538+
/// let client = initialize_operator(None, &cluster_info_opts)
535539
/// .await
536540
/// .expect("Unable to construct client.");
537541
/// let watcher_config: watcher::Config =
@@ -683,10 +687,26 @@ mod tests {
683687
};
684688
use tokio::time::error::Elapsed;
685689

690+
use crate::utils::cluster_info::KubernetesClusterInfoOpts;
691+
692+
async fn test_cluster_info_opts() -> KubernetesClusterInfoOpts {
693+
KubernetesClusterInfoOpts {
694+
// We have to hard-code a made-up cluster domain,
695+
// since kubernetes_node_name (probably) won't be a valid Node that we can query.
696+
kubernetes_cluster_domain: Some(
697+
"fake-cluster.local"
698+
.parse()
699+
.expect("hard-coded cluster domain must be valid"),
700+
),
701+
// Tests aren't running in a kubelet, so make up a name of one.
702+
kubernetes_node_name: "fake-node-name".to_string(),
703+
}
704+
}
705+
686706
#[tokio::test]
687707
#[ignore = "Tests depending on Kubernetes are not ran by default"]
688708
async fn k8s_test_wait_created() {
689-
let client = super::initialize_operator(None, &Default::default())
709+
let client = super::initialize_operator(None, &test_cluster_info_opts().await)
690710
.await
691711
.expect("KUBECONFIG variable must be configured.");
692712

@@ -764,7 +784,7 @@ mod tests {
764784
#[tokio::test]
765785
#[ignore = "Tests depending on Kubernetes are not ran by default"]
766786
async fn k8s_test_wait_created_timeout() {
767-
let client = super::initialize_operator(None, &Default::default())
787+
let client = super::initialize_operator(None, &test_cluster_info_opts().await)
768788
.await
769789
.expect("KUBECONFIG variable must be configured.");
770790

@@ -784,7 +804,7 @@ mod tests {
784804
#[tokio::test]
785805
#[ignore = "Tests depending on Kubernetes are not ran by default"]
786806
async fn k8s_test_list_with_label_selector() {
787-
let client = super::initialize_operator(None, &Default::default())
807+
let client = super::initialize_operator(None, &test_cluster_info_opts().await)
788808
.await
789809
.expect("KUBECONFIG variable must be configured.");
790810

crates/stackable-operator/src/utils/cluster_info.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,38 @@ pub struct KubernetesClusterInfo {
1515
pub cluster_domain: DomainName,
1616
}
1717

18-
#[derive(clap::Parser, Debug, Default, PartialEq, Eq)]
18+
#[derive(clap::Parser, Debug, PartialEq, Eq)]
1919
pub struct KubernetesClusterInfoOpts {
2020
/// Kubernetes cluster domain, usually this is `cluster.local`.
21-
// We are not using a default value here, as operators will probably do an more advanced
22-
// auto-detection of the cluster domain in case it is not specified in the future.
21+
// We are not using a default value here, as we query the cluster if it is not specified.
2322
#[arg(long, env)]
2423
pub kubernetes_cluster_domain: Option<DomainName>,
24+
25+
/// Name of the Kubernetes Node that the operator is running on.
26+
#[arg(long, env)]
27+
pub kubernetes_node_name: String,
2528
}
2629

2730
impl KubernetesClusterInfo {
2831
pub async fn new(
2932
client: &Client,
3033
cluster_info_opts: &KubernetesClusterInfoOpts,
3134
) -> Result<Self, Error> {
32-
let cluster_domain = match &cluster_info_opts.kubernetes_cluster_domain {
33-
Some(cluster_domain) => {
35+
let cluster_domain = match cluster_info_opts {
36+
KubernetesClusterInfoOpts {
37+
kubernetes_cluster_domain: Some(cluster_domain),
38+
..
39+
} => {
3440
tracing::info!(%cluster_domain, "Using configured Kubernetes cluster domain");
3541

3642
cluster_domain.clone()
3743
}
38-
None => {
39-
let kubelet_config = kubelet::KubeletConfig::fetch(client)
44+
KubernetesClusterInfoOpts {
45+
kubernetes_node_name: node_name,
46+
..
47+
} => {
48+
tracing::info!(%node_name, "Fetching Kubernetes cluster domain from the local kubelet");
49+
let kubelet_config = kubelet::KubeletConfig::fetch(client, node_name)
4050
.await
4151
.context(KubeletConfigSnafu)?;
4252

crates/stackable-operator/src/utils/kubelet.rs

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,12 @@
11
use http;
2-
use k8s_openapi::api::core::v1::Node;
3-
use kube::{
4-
Api,
5-
api::{ListParams, ResourceExt},
6-
client::Client,
7-
};
2+
use kube::client::Client;
83
use serde::Deserialize;
9-
use snafu::{OptionExt, ResultExt, Snafu};
4+
use snafu::{ResultExt, Snafu};
105

116
use crate::commons::networking::DomainName;
127

138
#[derive(Debug, Snafu)]
149
pub enum Error {
15-
#[snafu(display("failed to list nodes"))]
16-
ListNodes { source: kube::Error },
17-
1810
#[snafu(display("failed to build request for url path \"{url_path}\""))]
1911
BuildConfigzRequest {
2012
source: http::Error,
@@ -29,11 +21,6 @@ pub enum Error {
2921

3022
#[snafu(display("failed to deserialize kubelet config JSON"))]
3123
KubeletConfigJson { source: serde_json::Error },
32-
33-
#[snafu(display(
34-
"empty Kubernetes nodes list. At least one node is required to fetch the cluster domain from the kubelet config"
35-
))]
36-
EmptyKubernetesNodesList,
3724
}
3825

3926
#[derive(Debug, Deserialize)]
@@ -49,16 +36,8 @@ pub struct KubeletConfig {
4936
}
5037

5138
impl KubeletConfig {
52-
/// Fetches the kubelet configuration from the "first" node in the Kubernetes cluster.
53-
pub async fn fetch(client: &Client) -> Result<Self, Error> {
54-
let api: Api<Node> = Api::all(client.clone());
55-
let nodes = api
56-
.list(&ListParams::default())
57-
.await
58-
.context(ListNodesSnafu)?;
59-
let node = nodes.iter().next().context(EmptyKubernetesNodesListSnafu)?;
60-
let node_name = node.name_any();
61-
39+
/// Fetches the kubelet configuration from the specified node in the Kubernetes cluster.
40+
pub async fn fetch(client: &Client, node_name: &str) -> Result<Self, Error> {
6241
let url_path = format!("/api/v1/nodes/{node_name}/proxy/configz");
6342
let req = http::Request::get(url_path.clone())
6443
.body(Default::default())

0 commit comments

Comments
 (0)