Skip to content

Commit 5e8d4d8

Browse files
authored
chore: Avoid unnecessary k8s::Client creations (#295)
* chore: Avoid unnecessary k8s::Client creations * changelog * Use expect * Fix clippy
1 parent 439c248 commit 5e8d4d8

File tree

19 files changed

+174
-117
lines changed

19 files changed

+174
-117
lines changed

rust/stackable-cockpit/src/platform/cluster/resource_request.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,8 @@ impl ResourceRequests {
9191
/// Validates the struct [`ResourceRequests`] by comparing the required
9292
/// resources to the available ones in the current cluster. `object_name`
9393
/// should be `stack` or `demo`.
94-
pub async fn validate_cluster_size(&self, object_name: &str) -> Result<()> {
95-
let kube_client = Client::new().await.context(KubeClientCreateSnafu)?;
96-
let cluster_info = kube_client
97-
.get_cluster_info()
98-
.await
99-
.context(ClusterInfoSnafu)?;
94+
pub async fn validate_cluster_size(&self, client: &Client, object_name: &str) -> Result<()> {
95+
let cluster_info = client.get_cluster_info().await.context(ClusterInfoSnafu)?;
10096

10197
let stack_cpu =
10298
CpuQuantity::try_from(&self.cpu).context(ParseCpuResourceRequirementsSnafu)?;

rust/stackable-cockpit/src/platform/credentials.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use kube::{core::DynamicObject, ResourceExt};
44
use serde::Serialize;
55
use snafu::{OptionExt, ResultExt, Snafu};
66

7-
use crate::utils::k8s;
7+
use crate::utils::k8s::{self, Client};
88

99
pub type Result<T, E = Error> = std::result::Result<T, E>;
1010

@@ -34,7 +34,7 @@ impl Display for Credentials {
3434
/// and/or `password_key` are not found or the product does not provide
3535
/// any credentials.
3636
pub async fn get(
37-
kube_client: &k8s::Client,
37+
client: &Client,
3838
product_name: &str,
3939
stacklet: &DynamicObject,
4040
) -> Result<Option<Credentials>> {
@@ -56,7 +56,7 @@ pub async fn get(
5656
.as_str()
5757
.context(NoSecretSnafu)?;
5858

59-
kube_client
59+
client
6060
.get_credentials_from_secret(
6161
secret_name,
6262
&stacklet.namespace().unwrap(),
@@ -71,7 +71,7 @@ pub async fn get(
7171
.as_str()
7272
.context(NoSecretSnafu)?;
7373

74-
kube_client
74+
client
7575
.get_credentials_from_secret(
7676
secret_name,
7777
&stacklet.namespace().unwrap(),

rust/stackable-cockpit/src/platform/demo/spec.rs

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,13 @@ use crate::{
1414
release::ReleaseList,
1515
stack::{self, StackInstallParameters, StackList},
1616
},
17-
utils::params::{
18-
IntoParameters, IntoParametersError, Parameter, RawParameter, RawParameterParseError,
17+
utils::{
18+
k8s::Client,
19+
params::{
20+
IntoParameters, IntoParametersError, Parameter, RawParameter, RawParameterParseError,
21+
},
1922
},
20-
xfer::{self, Client},
23+
xfer,
2124
};
2225

2326
pub type RawDemoParameterParseError = RawParameterParseError;
@@ -94,7 +97,11 @@ impl DemoSpec {
9497
/// - Does the demo support to be installed in the requested namespace?
9598
/// - Does the cluster have enough resources available to run this demo?
9699
#[instrument(skip_all)]
97-
pub async fn check_prerequisites(&self, product_namespace: &str) -> Result<(), Error> {
100+
pub async fn check_prerequisites(
101+
&self,
102+
client: &Client,
103+
product_namespace: &str,
104+
) -> Result<(), Error> {
98105
debug!("Checking prerequisites before installing demo");
99106

100107
// Returns an error if the demo doesn't support to be installed in the
@@ -109,7 +116,10 @@ impl DemoSpec {
109116
// Checks if the available cluster resources are sufficient to deploy
110117
// the demo.
111118
if let Some(resource_requests) = &self.resource_requests {
112-
if let Err(err) = resource_requests.validate_cluster_size("demo").await {
119+
if let Err(err) = resource_requests
120+
.validate_cluster_size(client, "demo")
121+
.await
122+
{
113123
match err {
114124
ResourceRequestsError::ValidationErrors { errors } => {
115125
for error in errors {
@@ -129,15 +139,16 @@ impl DemoSpec {
129139
stack_list: StackList,
130140
release_list: ReleaseList,
131141
install_parameters: DemoInstallParameters,
132-
transfer_client: &Client,
142+
client: &Client,
143+
transfer_client: &xfer::Client,
133144
) -> Result<(), Error> {
134145
// Get the stack spec based on the name defined in the demo spec
135146
let stack = stack_list.get(&self.stack).context(NoSuchStackSnafu {
136147
name: self.stack.clone(),
137148
})?;
138149

139150
// Check demo prerequisites
140-
self.check_prerequisites(&install_parameters.product_namespace)
151+
self.check_prerequisites(client, &install_parameters.product_namespace)
141152
.await?;
142153

143154
let stack_install_parameters = StackInstallParameters {
@@ -151,19 +162,25 @@ impl DemoSpec {
151162
};
152163

153164
stack
154-
.install(release_list, stack_install_parameters, transfer_client)
165+
.install(
166+
release_list,
167+
stack_install_parameters,
168+
client,
169+
transfer_client,
170+
)
155171
.await
156172
.context(InstallStackSnafu)?;
157173

158174
// Install demo manifests
159-
self.prepare_manifests(install_parameters, transfer_client)
175+
self.prepare_manifests(install_parameters, client, transfer_client)
160176
.await
161177
}
162178

163179
#[instrument(skip_all)]
164180
async fn prepare_manifests(
165181
&self,
166182
install_params: DemoInstallParameters,
183+
client: &Client,
167184
transfer_client: &xfer::Client,
168185
) -> Result<(), Error> {
169186
info!("Installing demo manifests");
@@ -179,6 +196,7 @@ impl DemoSpec {
179196
&params,
180197
&install_params.product_namespace,
181198
install_params.labels,
199+
client,
182200
transfer_client,
183201
)
184202
.await

rust/stackable-cockpit/src/platform/manifests.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
common::manifest::ManifestSpec,
99
helm,
1010
utils::{
11-
k8s,
11+
k8s::{self, Client},
1212
path::{IntoPathOrUrl, PathOrUrlParseError},
1313
},
1414
xfer::{
@@ -61,20 +61,19 @@ pub enum Error {
6161
}
6262

6363
pub trait InstallManifestsExt {
64-
// TODO (Techassi): This step shouldn't care about templating the manifests nor fecthing them from remote
64+
// TODO (Techassi): This step shouldn't care about templating the manifests nor fetching them from remote
6565
#[instrument(skip_all)]
6666
#[allow(async_fn_in_trait)]
6767
async fn install_manifests(
6868
manifests: &[ManifestSpec],
6969
parameters: &HashMap<String, String>,
7070
product_namespace: &str,
7171
labels: Labels,
72+
client: &Client,
7273
transfer_client: &xfer::Client,
7374
) -> Result<(), Error> {
7475
debug!("Installing demo / stack manifests");
7576

76-
let kube_client = k8s::Client::new().await.context(CreateKubeClientSnafu)?;
77-
7877
for manifest in manifests {
7978
match manifest {
8079
ManifestSpec::HelmChart(helm_file) => {
@@ -137,7 +136,7 @@ pub trait InstallManifestsExt {
137136
.await
138137
.context(FileTransferSnafu)?;
139138

140-
kube_client
139+
client
141140
.deploy_manifests(&manifests, product_namespace, labels.clone())
142141
.await
143142
.context(DeployManifestSnafu)?

rust/stackable-cockpit/src/platform/namespace.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use snafu::{ResultExt, Snafu};
1+
use snafu::Snafu;
22

3-
use crate::utils::k8s;
3+
use crate::utils::k8s::{self, Client};
44

55
#[derive(Debug, Snafu)]
66
pub enum Error {
@@ -13,9 +13,7 @@ pub enum Error {
1313

1414
/// Creates a namespace with `name` if needed (not already present in the
1515
/// cluster).
16-
pub async fn create_if_needed(name: String) -> Result<(), Error> {
17-
let client = k8s::Client::new().await.context(KubeClientCreateSnafu)?;
18-
16+
pub async fn create_if_needed(client: &Client, name: String) -> Result<(), Error> {
1917
client
2018
.create_namespace_if_needed(name)
2119
.await

rust/stackable-cockpit/src/platform/service.rs

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use kube::{api::ListParams, ResourceExt};
1313
use snafu::{OptionExt, ResultExt, Snafu};
1414
use tracing::{debug, warn};
1515

16-
use crate::utils::k8s::{self, ListParamsExt};
16+
use crate::utils::k8s::{self, Client, ListParamsExt};
1717

1818
#[derive(Debug, Snafu)]
1919
pub enum Error {
@@ -40,15 +40,15 @@ pub enum Error {
4040
}
4141

4242
pub async fn get_endpoints(
43-
kube_client: &k8s::Client,
43+
client: &Client,
4444
product_name: &str,
4545
object_name: &str,
4646
object_namespace: &str,
4747
) -> Result<IndexMap<String, String>, Error> {
4848
let list_params =
4949
ListParams::from_product(product_name, Some(object_name), k8s::ProductLabel::Name);
5050

51-
let listeners = kube_client
51+
let listeners = client
5252
.list_listeners(Some(object_namespace), &list_params)
5353
.await;
5454
let listeners = match listeners {
@@ -92,13 +92,13 @@ pub async fn get_endpoints(
9292
return Ok(endpoints);
9393
}
9494

95-
let services = kube_client
95+
let services = client
9696
.list_services(Some(object_namespace), &list_params)
9797
.await
9898
.context(KubeClientFetchSnafu)?;
9999

100100
for service in services {
101-
match get_endpoint_urls(kube_client, &service, object_name).await {
101+
match get_endpoint_urls(client, &service, object_name).await {
102102
Ok(urls) => endpoints.extend(urls),
103103
Err(err) => warn!(
104104
"Failed to get endpoint_urls of service {service_name}: {err}",
@@ -111,7 +111,7 @@ pub async fn get_endpoints(
111111
}
112112

113113
pub async fn get_endpoint_urls(
114-
kube_client: &k8s::Client,
114+
client: &Client,
115115
service: &Service,
116116
referenced_object_name: &str,
117117
) -> Result<IndexMap<String, String>, Error> {
@@ -128,7 +128,7 @@ pub async fn get_endpoint_urls(
128128
let endpoints = match service_spec.type_.as_deref() {
129129
Some("NodePort") => {
130130
get_endpoint_urls_for_nodeport(
131-
kube_client,
131+
client,
132132
&service_name,
133133
service_spec,
134134
&service_namespace,
@@ -152,13 +152,13 @@ pub async fn get_endpoint_urls(
152152
}
153153

154154
pub async fn get_endpoint_urls_for_nodeport(
155-
kube_client: &k8s::Client,
155+
client: &Client,
156156
service_name: &str,
157157
service_spec: &ServiceSpec,
158158
service_namespace: &str,
159159
referenced_object_name: &str,
160160
) -> Result<IndexMap<String, String>, Error> {
161-
let endpoints = kube_client
161+
let endpoints = client
162162
.get_endpoints(service_namespace, service_name)
163163
.await
164164
.context(KubeClientFetchSnafu)?;
@@ -191,7 +191,7 @@ pub async fn get_endpoint_urls_for_nodeport(
191191
}
192192
};
193193

194-
let node_ip = get_node_ip(kube_client, node_name).await?;
194+
let node_ip = get_node_ip(client, node_name).await?;
195195

196196
let mut endpoints = IndexMap::new();
197197
for service_port in service_spec.ports.iter().flatten() {
@@ -265,8 +265,8 @@ pub async fn get_endpoint_urls_for_loadbalancer(
265265
Ok(endpoints)
266266
}
267267

268-
async fn get_node_ip(kube_client: &k8s::Client, node_name: &str) -> Result<String, Error> {
269-
let node_name_ip_mapping = get_node_name_ip_mapping(kube_client).await?;
268+
async fn get_node_ip(client: &Client, node_name: &str) -> Result<String, Error> {
269+
let node_name_ip_mapping = get_node_name_ip_mapping(client).await?;
270270

271271
match node_name_ip_mapping.get(node_name) {
272272
Some(node_ip) => Ok(node_ip.to_string()),
@@ -276,13 +276,8 @@ async fn get_node_ip(kube_client: &k8s::Client, node_name: &str) -> Result<Strin
276276

277277
// TODO(sbernauer): Add caching. Not going to do so now, as listener-op
278278
// will replace this code entirely anyway.
279-
async fn get_node_name_ip_mapping(
280-
kube_client: &k8s::Client,
281-
) -> Result<HashMap<String, String>, Error> {
282-
let nodes = kube_client
283-
.list_nodes()
284-
.await
285-
.context(KubeClientFetchSnafu)?;
279+
async fn get_node_name_ip_mapping(client: &Client) -> Result<HashMap<String, String>, Error> {
280+
let nodes = client.list_nodes().await.context(KubeClientFetchSnafu)?;
286281

287282
let mut result = HashMap::new();
288283
for node in nodes {

0 commit comments

Comments
 (0)