fix: Prevent Pod 0 restart by utilizing mutating webhook

Author: sbernauer

Cargo.lock

@@ -10,7 +10,7 @@ edition = "2021"
 repository = "https://github.com/stackabletech/commons-operator"
 
 [workspace.dependencies]
-stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", features = ["telemetry"], tag = "stackable-operator-0.100.1" }
+stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", features = ["telemetry", "webhook"], tag = "stackable-operator-0.100.1" }
 
 anyhow = "1.0"
 built = { version = "0.8", features = ["chrono", "git2"] }
@@ -24,5 +24,6 @@ strum = { version = "0.27", features = ["derive"] }
 tokio = { version = "1.40", features = ["full"] }
 tracing = "0.1"
 
-# [patch."https://github.com/stackabletech/operator-rs.git"]
-# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" }
+[patch."https://github.com/stackabletech/operator-rs.git"]
+# stackable-operator = { path = "../operator-rs/crates/stackable-operator" }
+stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "feat/mutating-webhook" }

Cargo.nix

@@ -0,0 +1,62 @@
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: web
+  labels:
+    restarter.stackable.tech/enabled: "true"
+spec:
+  selector:
+    matchLabels:
+      app: nginx
+  serviceName: nginx
+  replicas: 3
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      terminationGracePeriodSeconds: 10
+      containers:
+        - name: nginx
+          image: registry.k8s.io/nginx-slim:0.24
+          ports:
+          - containerPort: 80
+            name: web
+          volumeMounts:
+          - name: config
+            mountPath: "/config"
+          envFrom:
+          - configMapRef:
+              name: web-config-2
+      volumes:
+        - name: config
+          configMap:
+            name: web-config
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: nginx
+  labels:
+    app: nginx
+spec:
+  ports:
+  - port: 80
+    name: web
+  clusterIP: None
+  selector:
+    app: nginx
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: web-config
+data:
+  foo: bar
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: web-config-2
+data:
+  foo: bar

Cargo.toml

@@ -46,3 +46,10 @@ rules:
       - pods/eviction
     verbs:
       - create
+  # Required to maintain MutatingWebhookConfigurations. The operator needs to do this, as it needs
+  # to enter e.g. it's generated certificate in the webhooks.
+  - apiGroups: [admissionregistration.k8s.io]
+    resources: [mutatingwebhookconfigurations]
+    verbs:
+      - create
+      - patch

_TEST.yaml

@@ -2,10 +2,9 @@
 // This will need changes in our and upstream error types.
 #![allow(clippy::large_enum_variant)]
 
-mod restart_controller;
-
+use anyhow::anyhow;
 use clap::Parser;
-use futures::FutureExt;
+use futures::{FutureExt, TryFutureExt};
 use stackable_operator::{
     YamlSchema as _,
     cli::{Command, RunArguments},
@@ -17,11 +16,18 @@ use stackable_operator::{
     shared::yaml::SerializeOptions,
     telemetry::Tracing,
 };
+use webhooks::create_webhook;
+
+mod restart_controller;
+mod webhooks;
 
 mod built_info {
     include!(concat!(env!("OUT_DIR"), "/built.rs"));
 }
 
+pub const OPERATOR_NAME: &str = "commons.stackable.tech";
+pub const FIELD_MANAGER: &str = "commons-operator";
+
 #[derive(Parser)]
 #[clap(about, author)]
 struct Opts {
@@ -44,7 +50,7 @@ async fn main() -> anyhow::Result<()> {
         Command::Run(RunArguments {
             product_config: _,
             watch_namespace,
-            operator_environment: _,
+            operator_environment,
             maintenance,
             common,
         }) => {
@@ -76,12 +82,28 @@ async fn main() -> anyhow::Result<()> {
             )
             .await?;
 
+            let webhook = create_webhook(
+                &operator_environment,
+                // TODO: Make user configurable
+                false,
+                client.as_kube_client(),
+            )
+            .await?;
+            let webhook = webhook
+                .run()
+                .map_err(|err| anyhow!(err).context("failed to run webhook"));
+
             let sts_restart_controller =
                 restart_controller::statefulset::start(&client, &watch_namespace).map(anyhow::Ok);
             let pod_restart_controller =
                 restart_controller::pod::start(&client, &watch_namespace).map(anyhow::Ok);
 
-            futures::try_join!(sts_restart_controller, pod_restart_controller, eos_checker)?;
+            futures::try_join!(
+                sts_restart_controller,
+                pod_restart_controller,
+                eos_checker,
+                webhook
+            )?;
         }
     }
 

crate-hashes.json

@@ -13,9 +13,8 @@ use stackable_operator::{
         apps::v1::StatefulSet,
         core::v1::{ConfigMap, EnvFromSource, EnvVar, PodSpec, Secret, Volume},
     },
-    kube,
     kube::{
-        Resource, ResourceExt,
+        self, Resource, ResourceExt,
         api::{PartialObjectMeta, Patch, PatchParams},
         core::{DeserializeGuard, DynamicObject, error_boundary},
         runtime::{

deploy/helm/commons-operator/templates/roles.yaml

@@ -0,0 +1,116 @@
+use std::collections::BTreeMap;
+
+use snafu::{ResultExt, Snafu};
+use stackable_operator::{
+    builder::meta::ObjectMetaBuilder,
+    cli::OperatorEnvironmentOptions,
+    k8s_openapi::{
+        api::{
+            admissionregistration::v1::{
+                MutatingWebhook, MutatingWebhookConfiguration, RuleWithOperations,
+                WebhookClientConfig,
+            },
+            apps::v1::StatefulSet,
+        },
+        apimachinery::pkg::apis::meta::v1::LabelSelector,
+    },
+    kube::{
+        Client,
+        core::admission::{AdmissionRequest, AdmissionResponse},
+    },
+    kvp::Label,
+    webhook::{WebhookError, WebhookOptions, WebhookServer, servers::MutatingWebhookServer},
+};
+
+use crate::{FIELD_MANAGER, OPERATOR_NAME};
+
+#[derive(Debug, Snafu)]
+pub enum Error {
+    #[snafu(display("failed to create webhook server"))]
+    CreateWebhookServer { source: WebhookError },
+}
+
+pub async fn create_webhook<'a>(
+    operator_environment: &'a OperatorEnvironmentOptions,
+    disable_mutating_webhook_configuration_maintenance: bool,
+    client: Client,
+) -> Result<WebhookServer, Error> {
+    let mutating_webhook_server = MutatingWebhookServer::new(
+        get_mutating_webhook_configuration(),
+        foo,
+        disable_mutating_webhook_configuration_maintenance,
+        client,
+        FIELD_MANAGER.to_owned(),
+    );
+
+    let webhook_options = WebhookOptions {
+        socket_addr: WebhookServer::DEFAULT_SOCKET_ADDRESS,
+        operator_namespace: operator_environment.operator_namespace.to_owned(),
+        operator_service_name: operator_environment.operator_service_name.to_owned(),
+    };
+    WebhookServer::new(webhook_options, vec![Box::new(mutating_webhook_server)])
+        .await
+        .context(CreateWebhookServerSnafu)
+}
+
+fn get_mutating_webhook_configuration() -> MutatingWebhookConfiguration {
+    let webhook_name = "restarter-sts-enricher.stackable.tech";
+    let metadata = ObjectMetaBuilder::new()
+        .name(webhook_name)
+        .with_label(Label::stackable_vendor())
+        .with_label(
+            Label::managed_by(OPERATOR_NAME, webhook_name).expect("static label is always valid"),
+        )
+        .build();
+
+    MutatingWebhookConfiguration {
+        metadata,
+        webhooks: Some(vec![MutatingWebhook {
+            name: webhook_name.to_owned(),
+            // This is checked by the stackable_webhook code
+            admission_review_versions: vec!["v1".to_owned()],
+            rules: Some(vec![RuleWithOperations {
+                api_groups: Some(vec!["apps".to_owned()]),
+                api_versions: Some(vec!["v1".to_owned()]),
+                resources: Some(vec!["statefulsets".to_owned()]),
+                operations: Some(vec!["CREATE".to_owned()]),
+                scope: Some("Namespaced".to_owned()),
+            }]),
+            // We only need to care about StatefulSets with the `restarter.stackable.tech/enabled``
+            // label set to `true`.
+            object_selector: Some(LabelSelector {
+                match_labels: Some(BTreeMap::from([(
+                    "restarter.stackable.tech/enabled".to_owned(),
+                    "true".to_owned(),
+                )])),
+                match_expressions: None,
+            }),
+            // Will be set by the stackable_webhook code
+            client_config: WebhookClientConfig::default(),
+            // Worst case the annotations are missing cause a restart of Pod 0, basically the same
+            // behavior which we had for years.
+            // See https://github.com/stackabletech/commons-operator/issues/111 for details
+            // failure_policy: Some("Ignore".to_owned()),
+            // TEMP for testing
+            failure_policy: Some("Fail".to_owned()),
+            // It could be the case that other mutating webhooks add more ConfigMpa/Secret mounts,
+            // in which case it would be nice if we detect that.
+            reinvocation_policy: Some("IfNeeded".to_owned()),
+            // We don't have side effects
+            side_effects: "None".to_owned(),
+            ..Default::default()
+        }]),
+    }
+}
+
+fn foo(request: AdmissionRequest<StatefulSet>) -> AdmissionResponse {
+    let Some(sts) = &request.object else {
+        return AdmissionResponse::invalid(
+            "object (of type StatefulSet) missing - for operation CREATE it must be always present",
+        );
+    };
+
+    dbg!(&request);
+
+    AdmissionResponse::from(&request)
+}