Skip to content

Commit 75a8b5c

Browse files
authored
refactor!: Don't parse resolv.conf for cluster domain auto-detection (#896)
* WIP spike: Store cluster domain in Client * Store cluster domain in separate KubernetesClusterInfo struct * Remove src/utils/cluster_domain.rs * Dont pass all CLI args * changelog * typo * Move CLI arg into KubernetesClusterInfoCliOpts struct * KubernetesClusterInfoCliOpts -> KubernetesClusterInfoOpts * Derive Default for KubernetesClusterInfoOpts * Fix cli leftovers * Add rust docs * Mark as breaking in changelog
1 parent 60cb372 commit 75a8b5c

File tree

10 files changed

+125
-244
lines changed

10 files changed

+125
-244
lines changed

crates/stackable-operator/CHANGELOG.md

+7
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,19 @@ All notable changes to this project will be documented in this file.
44

55
## [Unreleased]
66

7+
### Changed
8+
9+
- BREAKING: Don't parse `/etc/resolv.conf` to auto-detect the Kubernetes cluster domain in case it is not explicitly configured.
10+
Instead the operator will default to `cluster.local`. We revert this now after some concerns where raised, we will
11+
create a follow-up decision instead addressing how we will continue with this ([#896]).
12+
713
### Fixed
814

915
- Fix Kubernetes cluster domain parsing from resolv.conf, e.g. on AWS EKS.
1016
We now only consider Kubernetes services domains instead of all domains (which could include non-Kubernetes domains) ([#895]).
1117

1218
[#895]: https://github.com/stackabletech/operator-rs/pull/895
19+
[#896]: https://github.com/stackabletech/operator-rs/pull/896
1320

1421
## [0.79.0] - 2024-10-18
1522

crates/stackable-operator/fixtures/cluster_domain/fail/invalid.resolv.conf

-2
This file was deleted.

crates/stackable-operator/fixtures/cluster_domain/pass/kubernetes-multiple.resolv.conf

-4
This file was deleted.

crates/stackable-operator/fixtures/cluster_domain/pass/kubernetes.resolv.conf

-3
This file was deleted.

crates/stackable-operator/fixtures/cluster_domain/pass/openshift.resolv.conf

-3
This file was deleted.

crates/stackable-operator/src/cli.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ use clap::Args;
115115
use product_config::ProductConfigManager;
116116
use snafu::{ResultExt, Snafu};
117117

118-
use crate::{logging::TracingTarget, namespace::WatchNamespace};
118+
use crate::{
119+
logging::TracingTarget, namespace::WatchNamespace,
120+
utils::cluster_info::KubernetesClusterInfoOpts,
121+
};
119122

120123
pub const AUTHOR: &str = "Stackable GmbH - info@stackable.tech";
121124

@@ -171,15 +174,19 @@ pub enum Command<Run: Args = ProductOperatorRun> {
171174
/// common: ProductOperatorRun,
172175
/// }
173176
/// use clap::Parser;
174-
/// use stackable_operator::logging::TracingTarget;
175-
/// use stackable_operator::namespace::WatchNamespace;
177+
/// use stackable_operator::{
178+
/// logging::TracingTarget,
179+
/// namespace::WatchNamespace,
180+
/// };
181+
///
176182
/// let opts = Command::<Run>::parse_from(["foobar-operator", "run", "--name", "foo", "--product-config", "bar", "--watch-namespace", "foobar"]);
177183
/// assert_eq!(opts, Command::Run(Run {
178184
/// name: "foo".to_string(),
179185
/// common: ProductOperatorRun {
180186
/// product_config: ProductConfigPath::from("bar".as_ref()),
181187
/// watch_namespace: WatchNamespace::One("foobar".to_string()),
182-
/// tracing_target: TracingTarget::None
188+
/// tracing_target: TracingTarget::None,
189+
/// cluster_info_opts: Default::default(),
183190
/// },
184191
/// }));
185192
/// ```
@@ -205,12 +212,17 @@ pub struct ProductOperatorRun {
205212
/// Provides the path to a product-config file
206213
#[arg(long, short = 'p', value_name = "FILE", default_value = "", env)]
207214
pub product_config: ProductConfigPath,
215+
208216
/// Provides a specific namespace to watch (instead of watching all namespaces)
209217
#[arg(long, env, default_value = "")]
210218
pub watch_namespace: WatchNamespace,
219+
211220
/// Tracing log collector system
212221
#[arg(long, env, default_value_t, value_enum)]
213222
pub tracing_target: TracingTarget,
223+
224+
#[command(flatten)]
225+
pub cluster_info_opts: KubernetesClusterInfoOpts,
214226
}
215227

216228
/// A path to a [`ProductConfigManager`] spec file
@@ -384,6 +396,7 @@ mod tests {
384396
product_config: ProductConfigPath::from("bar".as_ref()),
385397
watch_namespace: WatchNamespace::One("foo".to_string()),
386398
tracing_target: TracingTarget::None,
399+
cluster_info_opts: Default::default(),
387400
}
388401
);
389402

@@ -395,6 +408,7 @@ mod tests {
395408
product_config: ProductConfigPath::from("bar".as_ref()),
396409
watch_namespace: WatchNamespace::All,
397410
tracing_target: TracingTarget::None,
411+
cluster_info_opts: Default::default(),
398412
}
399413
);
400414

@@ -407,6 +421,7 @@ mod tests {
407421
product_config: ProductConfigPath::from("bar".as_ref()),
408422
watch_namespace: WatchNamespace::One("foo".to_string()),
409423
tracing_target: TracingTarget::None,
424+
cluster_info_opts: Default::default(),
410425
}
411426
);
412427
}

crates/stackable-operator/src/client.rs

+53-37
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,29 @@
1-
use crate::kvp::LabelSelectorExt;
2-
use crate::utils::cluster_domain::{self, retrieve_cluster_domain, KUBERNETES_CLUSTER_DOMAIN};
1+
use std::{
2+
convert::TryFrom,
3+
fmt::{Debug, Display},
4+
};
35

46
use either::Either;
57
use futures::StreamExt;
6-
use k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector;
7-
use k8s_openapi::{ClusterResourceScope, NamespaceResourceScope};
8-
use kube::api::{DeleteParams, ListParams, Patch, PatchParams, PostParams, Resource, ResourceExt};
9-
use kube::client::Client as KubeClient;
10-
use kube::core::Status;
11-
use kube::runtime::wait::delete::delete_and_finalize;
12-
use kube::runtime::{watcher, WatchStreamExt};
13-
use kube::{Api, Config};
14-
use serde::de::DeserializeOwned;
15-
use serde::Serialize;
8+
use k8s_openapi::{
9+
apimachinery::pkg::apis::meta::v1::LabelSelector, ClusterResourceScope, NamespaceResourceScope,
10+
};
11+
use kube::{
12+
api::{DeleteParams, ListParams, Patch, PatchParams, PostParams, Resource, ResourceExt},
13+
client::Client as KubeClient,
14+
core::Status,
15+
runtime::{wait::delete::delete_and_finalize, watcher, WatchStreamExt},
16+
Api, Config,
17+
};
18+
use serde::{de::DeserializeOwned, Serialize};
1619
use snafu::{OptionExt, ResultExt, Snafu};
17-
use std::convert::TryFrom;
18-
use std::fmt::{Debug, Display};
1920
use tracing::trace;
2021

22+
use crate::{
23+
kvp::LabelSelectorExt,
24+
utils::cluster_info::{KubernetesClusterInfo, KubernetesClusterInfoOpts},
25+
};
26+
2127
pub type Result<T, E = Error> = std::result::Result<T, E>;
2228

2329
#[derive(Debug, Snafu)]
@@ -78,9 +84,6 @@ pub enum Error {
7884

7985
#[snafu(display("unable to create kubernetes client"))]
8086
CreateKubeClient { source: kube::Error },
81-
82-
#[snafu(display("unable to to resolve kubernetes cluster domain"))]
83-
ResolveKubernetesClusterDomain { source: cluster_domain::Error },
8487
}
8588

8689
/// This `Client` can be used to access Kubernetes.
@@ -93,13 +96,16 @@ pub struct Client {
9396
delete_params: DeleteParams,
9497
/// Default namespace as defined in the kubeconfig this client has been created from.
9598
pub default_namespace: String,
99+
100+
pub kubernetes_cluster_info: KubernetesClusterInfo,
96101
}
97102

98103
impl Client {
99104
pub fn new(
100105
client: KubeClient,
101106
field_manager: Option<String>,
102107
default_namespace: String,
108+
kubernetes_cluster_info: KubernetesClusterInfo,
103109
) -> Self {
104110
Client {
105111
client,
@@ -113,6 +119,7 @@ impl Client {
113119
},
114120
delete_params: DeleteParams::default(),
115121
default_namespace,
122+
kubernetes_cluster_info,
116123
}
117124
}
118125

@@ -517,9 +524,11 @@ impl Client {
517524
/// use stackable_operator::client::{Client, initialize_operator};
518525
///
519526
/// #[tokio::main]
520-
/// async fn main(){
527+
/// async fn main() {
521528
///
522-
/// let client: Client = initialize_operator(None).await.expect("Unable to construct client.");
529+
/// let client = initialize_operator(None, &Default::default())
530+
/// .await
531+
/// .expect("Unable to construct client.");
523532
/// let watcher_config: watcher::Config =
524533
/// watcher::Config::default().fields(&format!("metadata.name=nonexistent-pod"));
525534
///
@@ -626,38 +635,45 @@ where
626635
}
627636
}
628637

629-
pub async fn initialize_operator(field_manager: Option<String>) -> Result<Client> {
630-
let _ = KUBERNETES_CLUSTER_DOMAIN
631-
.set(retrieve_cluster_domain().context(ResolveKubernetesClusterDomainSnafu)?);
632-
create_client(field_manager).await
633-
}
634-
635-
async fn create_client(field_manager: Option<String>) -> Result<Client> {
638+
pub async fn initialize_operator(
639+
field_manager: Option<String>,
640+
cluster_info_opts: &KubernetesClusterInfoOpts,
641+
) -> Result<Client> {
636642
let kubeconfig: Config = kube::Config::infer()
637643
.await
638644
.map_err(kube::Error::InferConfig)
639645
.context(InferKubeConfigSnafu)?;
640646
let default_namespace = kubeconfig.default_namespace.clone();
641647
let client = kube::Client::try_from(kubeconfig).context(CreateKubeClientSnafu)?;
642-
Ok(Client::new(client, field_manager, default_namespace))
648+
let cluster_info = KubernetesClusterInfo::new(cluster_info_opts);
649+
650+
Ok(Client::new(
651+
client,
652+
field_manager,
653+
default_namespace,
654+
cluster_info,
655+
))
643656
}
644657

645658
#[cfg(test)]
646659
mod tests {
660+
use std::{collections::BTreeMap, time::Duration};
661+
647662
use futures::StreamExt;
648-
use k8s_openapi::api::core::v1::{Container, Pod, PodSpec};
649-
use k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector;
650-
use kube::api::{ObjectMeta, PostParams, ResourceExt};
651-
use kube::runtime::watcher;
652-
use kube::runtime::watcher::Event;
653-
use std::collections::BTreeMap;
654-
use std::time::Duration;
663+
use k8s_openapi::{
664+
api::core::v1::{Container, Pod, PodSpec},
665+
apimachinery::pkg::apis::meta::v1::LabelSelector,
666+
};
667+
use kube::{
668+
api::{ObjectMeta, PostParams, ResourceExt},
669+
runtime::watcher::{self, Event},
670+
};
655671
use tokio::time::error::Elapsed;
656672

657673
#[tokio::test]
658674
#[ignore = "Tests depending on Kubernetes are not ran by default"]
659675
async fn k8s_test_wait_created() {
660-
let client = super::create_client(None)
676+
let client = super::initialize_operator(None, &Default::default())
661677
.await
662678
.expect("KUBECONFIG variable must be configured.");
663679

@@ -735,7 +751,7 @@ mod tests {
735751
#[tokio::test]
736752
#[ignore = "Tests depending on Kubernetes are not ran by default"]
737753
async fn k8s_test_wait_created_timeout() {
738-
let client = super::create_client(None)
754+
let client = super::initialize_operator(None, &Default::default())
739755
.await
740756
.expect("KUBECONFIG variable must be configured.");
741757

@@ -755,7 +771,7 @@ mod tests {
755771
#[tokio::test]
756772
#[ignore = "Tests depending on Kubernetes are not ran by default"]
757773
async fn k8s_test_list_with_label_selector() {
758-
let client = super::create_client(None)
774+
let client = super::initialize_operator(None, &Default::default())
759775
.await
760776
.expect("KUBECONFIG variable must be configured.");
761777

0 commit comments

Comments
 (0)