diff --git a/CHANGELOG.md b/CHANGELOG.md index eb3d95c..8670694 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file. - `envOverrides` names are now validated by the shared `EnvVarName` type rather than by operator-specific validation code ([#171]). - All product containers now run with `securityContext.runAsNonRoot` set to `true` to improve security ([#172]). +- The operator now early-exits the reconcile action when the cluster is marked for deletion ([#179]). ### Fixed @@ -24,6 +25,7 @@ All notable changes to this project will be documented in this file. [#166]: https://github.com/stackabletech/opensearch-operator/pull/166 [#171]: https://github.com/stackabletech/opensearch-operator/pull/171 [#172]: https://github.com/stackabletech/opensearch-operator/pull/172 +[#179]: https://github.com/stackabletech/opensearch-operator/pull/179 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index ff3b8be..73a127d 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -442,6 +442,10 @@ pub async fn reconcile( ) -> Result { tracing::info!("Starting reconcile"); + if object.meta().deletion_timestamp.is_some() { + return Ok(Action::await_change()); + } + let cluster = object .0 .as_ref() @@ -502,17 +506,21 @@ pub async fn reconcile( #[cfg(test)] mod tests { - use std::{collections::BTreeMap, str::FromStr}; + use std::{collections::BTreeMap, str::FromStr, sync::Arc}; use stackable_operator::{ + cli::OperatorEnvironmentOptions, + client::Client, commons::{ affinity::StackableAffinity, networking::DomainName, product_image_selection::ResolvedProductImage, }, k8s_openapi::api::core::v1::PodTemplateSpec, + kube::{Client as KubeClient, Config, runtime::controller::Action}, kvp::LabelValue, product_logging::spec::AutomaticContainerLogConfig, shared::time::Duration, + utils::cluster_info::KubernetesClusterInfo, v2::{ builder::pod::container::EnvVarSet, product_logging::framework::ValidatedContainerLogConfigChoice, @@ -527,6 +535,7 @@ mod tests { use super::{ Context, NODES_ROLE_NAME, OpenSearchRoleGroupConfig, ValidatedCluster, ValidatedLogging, + reconcile, }; use crate::{ controller::{ @@ -685,4 +694,57 @@ mod tests { product_specific_common_config: GenericCommonConfig::default(), } } + + /// The client points at a closed port, so any API call would fail the reconciliation: an `Ok` + /// proves that a cluster being deleted returns before the reconciler touches the Kubernetes + /// API, and because the spec is invalid, before the `DeserializeGuard` is unwrapped. + #[test] + fn reconcile_exits_early_for_deleted_cluster() { + let cluster = serde_yaml::from_str( + r#" +apiVersion: opensearch.stackable.tech/v1alpha1 +kind: OpenSearchCluster +metadata: + name: opensearch + namespace: default + deletionTimestamp: "2026-08-14T12:00:00Z" +spec: {} +"#, + ) + .expect("YAML parses; the invalid spec is captured inside the DeserializeGuard"); + + let action = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("current-thread tokio runtime") + .block_on(async { + let client = Client::new( + KubeClient::try_from(Config::new( + "http://127.0.0.1:1".parse().expect("valid static URI"), + )) + .expect("client from static config"), + None, + "default".to_owned(), + KubernetesClusterInfo { + cluster_domain: DomainName::from_str("cluster.local") + .expect("valid cluster domain"), + }, + ); + let context = Arc::new(Context::new( + client, + OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_owned(), + operator_service_name: "opensearch-operator".to_owned(), + image_repository: "oci.stackable.tech/sdp".to_owned(), + }, + OperatorName::from_str("opensearch.stackable.tech") + .expect("valid operator name"), + )); + + reconcile(Arc::new(cluster), context).await + }) + .expect("a deleted cluster reconciles without any API call"); + + assert_eq!(action, Action::await_change()); + } } diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 725de25..55c2d45 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -195,11 +195,11 @@ async fn main() -> Result<()> { watcher::Config::default(), ) .owns( - watch_namespace.get_api::>(&client), + watch_namespace.get_api::>(&client), watcher::Config::default(), ) .owns( - watch_namespace.get_api::>(&client), + watch_namespace.get_api::>(&client), watcher::Config::default(), ) .owns( diff --git a/tests/templates/kuttl/smoke/30-assert.yaml b/tests/templates/kuttl/smoke/30-assert.yaml new file mode 100644 index 0000000..d830bb0 --- /dev/null +++ b/tests/templates/kuttl/smoke/30-assert.yaml @@ -0,0 +1,98 @@ +--- +# The recreated StatefulSets must bring the cluster back to ready, and the recreated +# objects must carry an owner reference back to the OpenSearchCluster so that garbage +# collection still works for them. +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +metadata: + name: recreate-owned-resources +# The budget is generous on purpose: the sweep recreates both StatefulSets back to +# back (after the restarter has already rolled the pods once for the recreated +# ConfigMaps), so five JVMs boot simultaneously - slower than the staggered install. +timeout: 900 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: opensearch-nodes-cluster-manager + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch +status: + readyReplicas: 3 + replicas: 3 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: opensearch-nodes-data + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch +status: + readyReplicas: 2 + replicas: 2 +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: opensearch-serviceaccount + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: opensearch-rolebinding + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: opensearch + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch +--- +apiVersion: listeners.stackable.tech/v1alpha1 +kind: Listener +metadata: + name: opensearch + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch +--- +apiVersion: listeners.stackable.tech/v1alpha1 +kind: Listener +metadata: + name: opensearch-nodes-cluster-manager + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch +--- +apiVersion: listeners.stackable.tech/v1alpha1 +kind: Listener +metadata: + name: opensearch-nodes-data + ownerReferences: + - apiVersion: opensearch.stackable.tech/v1alpha1 + controller: true + kind: OpenSearchCluster + name: opensearch diff --git a/tests/templates/kuttl/smoke/30-delete-owned-resources.yaml b/tests/templates/kuttl/smoke/30-delete-owned-resources.yaml new file mode 100644 index 0000000..80ba695 --- /dev/null +++ b/tests/templates/kuttl/smoke/30-delete-owned-resources.yaml @@ -0,0 +1,60 @@ +--- +# Every resource the operator applies carries an ownerReference and a `.owns()` watch +# (main.rs): deleting it must trigger a reconcile of the OpenSearchCluster that +# re-applies it, proving the `.owns()` routing and the ClusterRole `watch` verbs end +# to end. This lives in the smoke test because there is no cluster-operation suite. +# +# Resources are discovered by label (ClusterResources::add enforces the labels on +# everything the operator applies), so new resources and kinds are covered +# automatically. Labels over-match on derived objects, so each match must also carry +# a controller ownerReference pointing at the OpenSearchCluster; kinds that can never +# pass that gate are excluded up front. Recreation is proven by UID change, and a +# floor guard catches a selector that silently matches nothing. +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +metadata: + name: delete-owned-resources +timeout: 300 +commands: + - script: | + set -eu + + delete_and_await_recreation() { + resource=$1 + old_uid=$(kubectl get -n "$NAMESPACE" "$resource" -o jsonpath='{.metadata.uid}') + kubectl delete -n "$NAMESPACE" "$resource" --wait=false + # Recreation is a single reconcile away, so this normally succeeds on the + # first iteration; 30s is a generous upper bound well below the step timeout. + for _ in $(seq 1 30); do + new_uid=$(kubectl get -n "$NAMESPACE" "$resource" -o jsonpath='{.metadata.uid}' 2>/dev/null || true) + if [ -n "$new_uid" ] && [ "$new_uid" != "$old_uid" ]; then + return 0 + fi + sleep 1 + done + echo "$resource was not recreated (old uid: $old_uid, current: '${new_uid:-}')" >&2 + return 1 + } + + selector="app.kubernetes.io/instance=opensearch,app.kubernetes.io/managed-by=opensearch.stackable.tech_opensearchcluster" + excluded="^(pods|persistentvolumeclaims|endpoints|events)$|^endpointslices\.|^controllerrevisions\.|^events\." + + deleted=0 + for kind in $(kubectl api-resources --verbs=list --namespaced -o name | grep -Ev "$excluded" | sort); do + for resource in $(kubectl get -n "$NAMESPACE" "$kind" -l "$selector" -o name 2>/dev/null); do + owner=$(kubectl get -n "$NAMESPACE" "$resource" -o jsonpath='{.metadata.ownerReferences[?(@.controller==true)].kind}/{.metadata.ownerReferences[?(@.controller==true)].name}' 2>/dev/null || true) + if [ "$owner" != "OpenSearchCluster/opensearch" ]; then + echo "skipping $resource: controller owner is '${owner:-none}', not the OpenSearchCluster" + continue + fi + delete_and_await_recreation "$resource" + deleted=$((deleted + 1)) + done + done + + # Guard against the sweep silently matching nothing (wrong selector, renamed + # labels): the fixture is known to produce well over this many owned resources. + if [ "$deleted" -lt 8 ]; then + echo "only $deleted labelled resources were swept - the label selector is broken" >&2 + exit 1 + fi