From 867755f89e1de232cb329943bd601f19be220f8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 2 Aug 2022 17:44:56 +0200 Subject: [PATCH 1/3] Use a watch for waiting for deleted objects rather than polling --- Cargo.toml | 1 - src/client.rs | 52 +++++++++++++-------------------------------------- src/error.rs | 9 +++++++++ 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 46cd1b03b..94afd3498 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,6 @@ thiserror = "1.0.31" tokio = { version = "1.20.1", features = ["macros", "rt-multi-thread"] } tracing = "0.1.35" tracing-subscriber = { version = "0.3.15", features = ["env-filter"] } -backoff = "0.4.0" derivative = "2.2.0" tracing-opentelemetry = "0.17.4" opentelemetry = { version = "0.17.0", features = ["rt-tokio"] } diff --git a/src/client.rs b/src/client.rs index 48715843f..02b71fccc 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,8 +1,6 @@ use crate::error::{Error, OperatorResult}; use crate::label_selector; -use backoff::backoff::Backoff; -use backoff::ExponentialBackoff; use either::Either; use futures::StreamExt; use k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector; @@ -10,13 +8,14 @@ use kube::api::{DeleteParams, ListParams, Patch, PatchParams, PostParams, Resour use kube::client::Client as KubeClient; use kube::core::Status; use kube::error::ErrorResponse; +use kube::runtime::wait::delete::delete_and_finalize; use kube::runtime::WatchStreamExt; use kube::{Api, Config}; use serde::de::DeserializeOwned; use serde::Serialize; use std::convert::TryFrom; use std::fmt::{Debug, Display}; -use tracing::{error, info, trace}; +use tracing::trace; /// This `Client` can be used to access Kubernetes. /// It wraps an underlying [kube::client::Client] and provides some common functionality. @@ -362,44 +361,19 @@ impl Client { /// from Kubernetes pub async fn ensure_deleted(&self, resource: T) -> OperatorResult<()> where - T: Clone + Debug + DeserializeOwned + Resource, + T: Clone + Debug + DeserializeOwned + Resource + Send + 'static, ::DynamicType: Default, { - let mut backoff_strategy = ExponentialBackoff { - max_elapsed_time: None, - ..ExponentialBackoff::default() - }; - - self.delete(&resource).await?; - - loop { - if !self - .exists::(&resource.name_any(), resource.namespace().as_deref()) - .await? - { - return Ok(()); - } - - // When backoff returns `None` the timeout has expired - match backoff_strategy.next_backoff() { - Some(backoff) => { - info!( - "Waiting [{}] seconds before trying again..", - backoff.as_secs() - ); - tokio::time::sleep(backoff).await; - } - None => { - // We offer no way of specifying a timeout, so this shouldn't happen, - // if it does we'll log an error for now and continue iterating and wait for - // the last interval we saw - error!( - "Waiting for deletion timed out, but no timeout was specified, this is an error and should not happen!" - ); - tokio::time::sleep(backoff_strategy.current_interval).await; - } - } - } + Ok(delete_and_finalize( + self.get_api::(resource.namespace().as_deref()), + resource + .meta() + .name + .as_deref() + .ok_or(Error::MissingObjectName)?, + &self.delete_params, + ) + .await?) } /// Returns an [kube::Api] object which is either namespaced or not depending on whether diff --git a/src/error.rs b/src/error.rs index eb4fac511..731e31881 100644 --- a/src/error.rs +++ b/src/error.rs @@ -15,9 +15,18 @@ pub enum Error { source: kube::Error, }, + #[error("Kubernetes failed to delete object: {source}")] + KubeDeleteError { + #[from] + source: kube::runtime::wait::delete::Error, + }, + #[error("Object is missing key: {key}")] MissingObjectKey { key: &'static str }, + #[error("Object has no name")] + MissingObjectName, + #[error("Label is missing: {label}")] MissingLabel { label: &'static str }, From 48ae68530685a0fb8350eb3df9aec39c2f876b44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 2 Aug 2022 17:47:47 +0200 Subject: [PATCH 2/3] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a56fce55..75bfef41c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,10 @@ All notable changes to this project will be documented in this file. - BREAKING: The `managed_by` label must be passed explicitly to the `ObjectMetaBuilder::with_recommended_labels` function ([#436]). +- Objects are now streamed rather than polled when waiting for them to be deleted ([#452]). [#436]: https://github.com/stackabletech/operator-rs/pull/436 +[#452]: https://github.com/stackabletech/operator-rs/pull/452 ## [0.23.0] - 2022-07-26 From b30382a99447e9d3909ce97670f36837362675cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 11 Aug 2022 14:13:57 +0200 Subject: [PATCH 3/3] Switch to MissingObjectKey error --- src/client.rs | 4 +++- src/error.rs | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/client.rs b/src/client.rs index 02b71fccc..a820ddcbe 100644 --- a/src/client.rs +++ b/src/client.rs @@ -370,7 +370,9 @@ impl Client { .meta() .name .as_deref() - .ok_or(Error::MissingObjectName)?, + .ok_or(Error::MissingObjectKey { + key: "metadata.name", + })?, &self.delete_params, ) .await?) diff --git a/src/error.rs b/src/error.rs index 731e31881..9b80b031c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -24,9 +24,6 @@ pub enum Error { #[error("Object is missing key: {key}")] MissingObjectKey { key: &'static str }, - #[error("Object has no name")] - MissingObjectName, - #[error("Label is missing: {label}")] MissingLabel { label: &'static str },