Skip to content

Commit 2721d72

Browse files
sbernauersbernauer
committed
Improve tracing
1 parent 6fc8a97 commit 2721d72

File tree

2 files changed

+85
-67
lines changed

2 files changed

+85
-67
lines changed

‎crates/stackable-webhook/src/servers/conversion_webhook.rs‎

Lines changed: 83 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use kube::{
2020
};
2121
use snafu::{ResultExt, Snafu, ensure};
2222
use tokio::sync::oneshot;
23+
use tracing::instrument;
2324
use x509_cert::Certificate;
2425

2526
use super::{Webhook, WebhookError};
@@ -69,6 +70,84 @@ impl<H> ConversionWebhook<H> {
6970
initial_reconcile_rx,
7071
)
7172
}
73+
74+
#[instrument(
75+
skip(self, crd, crd_api),
76+
fields(
77+
name = crd.name_any(),
78+
kind = &crd.spec.names.kind
79+
)
80+
)]
81+
async fn reconcile_crd(
82+
&self,
83+
mut crd: CustomResourceDefinition,
84+
crd_api: &Api<CustomResourceDefinition>,
85+
new_ca_bundle: &ByteString,
86+
options: &WebhookServerOptions,
87+
) -> Result<(), WebhookError> {
88+
let crd_kind = &crd.spec.names.kind;
89+
let crd_name = crd.name_any();
90+
91+
tracing::info!(
92+
k8s.crd.kind = crd_kind,
93+
k8s.crd.name = crd_name,
94+
"reconciling custom resource definition"
95+
);
96+
97+
crd.spec.conversion = Some(CustomResourceConversion {
98+
strategy: "Webhook".to_owned(),
99+
webhook: Some(WebhookConversion {
100+
// conversionReviewVersions indicates what ConversionReview versions are
101+
// supported by the webhook. The first version in the list understood by the
102+
// API server is sent to the webhook. The webhook must respond with a
103+
// ConversionReview object in the same version it received. We only support
104+
// the stable v1 ConversionReview to keep the implementation as simple as
105+
// possible.
106+
conversion_review_versions: vec!["v1".to_owned()],
107+
client_config: Some(WebhookClientConfig {
108+
service: Some(ServiceReference {
109+
name: options.webhook_service_name.to_owned(),
110+
namespace: options.webhook_namespace.to_owned(),
111+
path: Some(format!("/convert/{crd_name}")),
112+
port: Some(options.socket_addr.port().into()),
113+
}),
114+
// Here, ByteString takes care of encoding the provided content as base64.
115+
ca_bundle: Some(new_ca_bundle.to_owned()),
116+
url: None,
117+
}),
118+
}),
119+
});
120+
121+
// Deploy the updated CRDs using a server-side apply.
122+
let patch = Patch::Apply(&crd);
123+
124+
// We force apply here, because we want to become the sole manager of the CRD. This
125+
// avoids any conflicts from previous deployments via helm or stackablectl which are
126+
// reported with the following error message:
127+
//
128+
// Apply failed with 2 conflicts: conflicts with "stackablectl" using apiextensions.k8s.io/v1:
129+
// - .spec.versions
130+
// - .spec.conversion.strategy: Conflict
131+
//
132+
// The official Kubernetes documentation provides three options on how to solve
133+
// these conflicts. Option 1 is used, which is described as follows:
134+
//
135+
// Overwrite value, become sole manager: If overwriting the value was intentional
136+
// (or if the applier is an automated process like a controller) the applier should
137+
// set the force query parameter to true [...], and make the request again. This
138+
// forces the operation to succeed, changes the value of the field, and removes the
139+
// field from all other managers' entries in managedFields.
140+
//
141+
// See https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts
142+
let patch_params = PatchParams::apply(&self.field_manager).force();
143+
144+
crd_api
145+
.patch(&crd_name, &patch_params, &patch)
146+
.await
147+
.with_context(|_| PatchCrdSnafu { crd_name })?;
148+
149+
Ok(())
150+
}
72151
}
73152

74153
#[async_trait]
@@ -91,6 +170,7 @@ where
91170
router
92171
}
93172

173+
#[instrument(skip(self))]
94174
async fn handle_certificate_rotation(
95175
&mut self,
96176
_new_certificate: &Certificate,
@@ -101,74 +181,10 @@ where
101181
return Ok(());
102182
}
103183

104-
tracing::info!(
105-
k8s.crd.names = ?self.crds_and_handlers.iter().map(|(crd, _)| crd.name_any()).collect::<Vec<_>>(),
106-
"reconciling custom resource definitions"
107-
);
108-
109184
let crd_api: Api<CustomResourceDefinition> = Api::all(self.client.clone());
110-
111-
for mut crd in self.crds_and_handlers.iter().map(|(crd, _)| crd).cloned() {
112-
let crd_kind = &crd.spec.names.kind;
113-
let crd_name = crd.name_any();
114-
115-
tracing::debug!(
116-
k8s.crd.kind = crd_kind,
117-
k8s.crd.name = crd_name,
118-
"reconciling custom resource definition"
119-
);
120-
121-
crd.spec.conversion = Some(CustomResourceConversion {
122-
strategy: "Webhook".to_owned(),
123-
webhook: Some(WebhookConversion {
124-
// conversionReviewVersions indicates what ConversionReview versions are
125-
// supported by the webhook. The first version in the list understood by the
126-
// API server is sent to the webhook. The webhook must respond with a
127-
// ConversionReview object in the same version it received. We only support
128-
// the stable v1 ConversionReview to keep the implementation as simple as
129-
// possible.
130-
conversion_review_versions: vec!["v1".to_owned()],
131-
client_config: Some(WebhookClientConfig {
132-
service: Some(ServiceReference {
133-
name: options.webhook_service_name.to_owned(),
134-
namespace: options.webhook_namespace.to_owned(),
135-
path: Some(format!("/convert/{crd_name}")),
136-
port: Some(options.socket_addr.port().into()),
137-
}),
138-
// Here, ByteString takes care of encoding the provided content as base64.
139-
ca_bundle: Some(new_ca_bundle.to_owned()),
140-
url: None,
141-
}),
142-
}),
143-
});
144-
145-
// Deploy the updated CRDs using a server-side apply.
146-
let patch = Patch::Apply(&crd);
147-
148-
// We force apply here, because we want to become the sole manager of the CRD. This
149-
// avoids any conflicts from previous deployments via helm or stackablectl which are
150-
// reported with the following error message:
151-
//
152-
// Apply failed with 2 conflicts: conflicts with "stackablectl" using apiextensions.k8s.io/v1:
153-
// - .spec.versions
154-
// - .spec.conversion.strategy: Conflict
155-
//
156-
// The official Kubernetes documentation provides three options on how to solve
157-
// these conflicts. Option 1 is used, which is described as follows:
158-
//
159-
// Overwrite value, become sole manager: If overwriting the value was intentional
160-
// (or if the applier is an automated process like a controller) the applier should
161-
// set the force query parameter to true [...], and make the request again. This
162-
// forces the operation to succeed, changes the value of the field, and removes the
163-
// field from all other managers' entries in managedFields.
164-
//
165-
// See https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts
166-
let patch_params = PatchParams::apply(&self.field_manager).force();
167-
168-
crd_api
169-
.patch(&crd_name, &patch_params, &patch)
170-
.await
171-
.with_context(|_| PatchCrdSnafu { crd_name })?;
185+
for (crd, _) in &self.crds_and_handlers {
186+
self.reconcile_crd(crd.clone(), &crd_api, new_ca_bundle, options)
187+
.await?;
172188
}
173189

174190
// After the reconciliation of the CRDs, the initial reconcile heartbeat is sent out

‎crates/stackable-webhook/src/servers/mutating_webhook.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use kube::{
1515
};
1616
use serde::{Serialize, de::DeserializeOwned};
1717
use snafu::{ResultExt, Snafu};
18+
use tracing::instrument;
1819
use x509_cert::Certificate;
1920

2021
use super::{Webhook, WebhookError};
@@ -178,6 +179,7 @@ where
178179
router.route(&route, post(handler_fn))
179180
}
180181

182+
#[instrument(skip(self))]
181183
async fn handle_certificate_rotation(
182184
&mut self,
183185
_new_certificate: &Certificate,

0 commit comments

Comments
 (0)