From e5b87c010d69fa9a0a045621029ce30061170a58 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 8 Apr 2025 21:39:57 +0200 Subject: [PATCH 1/5] inject vector aggregator address as env into config & add watch for referenced cms --- CHANGELOG.md | 7 ++ Cargo.lock | 8 +-- Cargo.toml | 2 +- rust/operator-binary/src/controller.rs | 79 ++++++++++----------- rust/operator-binary/src/product_logging.rs | 48 +------------ 5 files changed, 53 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6171ef16..74a8030c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,15 @@ All notable changes to this project will be documented in this file. - BREAKING: user-info-fetcher: The file log directory was set by `OPA_OPERATOR_LOG_DIRECTORY`, and is now set by `ROLLING_LOGS` (or via `--rolling-logs `). - Replace stackable-operator `print_startup_string` with `tracing::info!` with fields. +- BREAKING: Inject the vector aggregator address into the vector config using the env var `VECTOR_AGGREGATOR_ADDRESS` instead + of having the operator write it to the vector config ([#XXX]). + +### Fixed + +- Fix a bug where changes to ConfigMaps that are referenced in the OpaCluster spec didn't trigger a reconciliation ([#XXX]). [#703]: https://github.com/stackabletech/opa-operator/pull/703 +[#XXX]: https://github.com/stackabletech/opa-operator/pull/XXX ## [25.3.0] - 2025-03-21 diff --git a/Cargo.lock b/Cargo.lock index 397ce312..f2390f1c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3213,8 +3213,8 @@ dependencies = [ [[package]] name = "stackable-operator" -version = "0.89.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.89.1#cd73728af410c52972b9a9a3ba1302bcdb574d04" +version = "0.90.0" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.90.0#ea063b4595caa20c82d37c595487c76476c9ab10" dependencies = [ "chrono", "clap", @@ -3249,7 +3249,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.89.1#cd73728af410c52972b9a9a3ba1302bcdb574d04" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.90.0#ea063b4595caa20c82d37c595487c76476c9ab10" dependencies = [ "darling", "proc-macro2", @@ -3260,7 +3260,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.0.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.89.1#cd73728af410c52972b9a9a3ba1302bcdb574d04" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.90.0#ea063b4595caa20c82d37c595487c76476c9ab10" dependencies = [ "kube 0.99.0", "semver", diff --git a/Cargo.toml b/Cargo.toml index 16060bd2..8e021586 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/stackabletech/opa-operator" [workspace.dependencies] product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.7.0" } -stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.89.1" } +stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.90.0" } stackable-telemetry = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-telemetry-0.4.0" } stackable-versioned = { git = "https://github.com/stackabletech/operator-rs.git", features = ["k8s"], tag = "stackable-versioned-0.6.0" } diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index a6db20fb..7f0e7fae 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -76,9 +76,7 @@ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ discovery::{self, build_discovery_configmaps}, operations::graceful_shutdown::add_graceful_shutdown_config, - product_logging::{ - BundleBuilderLogLevel, extend_role_group_config_map, resolve_vector_aggregator_address, - }, + product_logging::{BundleBuilderLogLevel, extend_role_group_config_map}, }; pub const OPA_CONTROLLER_NAME: &str = "opacluster"; @@ -251,10 +249,8 @@ pub enum Error { source: stackable_operator::builder::pod::container::Error, }, - #[snafu(display("failed to resolve the Vector aggregator address"))] - ResolveVectorAggregatorAddress { - source: crate::product_logging::Error, - }, + #[snafu(display("vector agent is enabled but vector aggregator ConfigMap is missing"))] + VectorAggregatorConfigMapMissing, #[snafu(display("failed to add the logging configuration to the ConfigMap [{cm_name}]"))] InvalidLoggingConfig { @@ -443,10 +439,6 @@ pub async fn reconcile_opa( .map(Cow::Borrowed) .unwrap_or_default(); - let vector_aggregator_address = resolve_vector_aggregator_address(opa, client) - .await - .context(ResolveVectorAggregatorAddressSnafu)?; - let server_role_service = build_server_role_service(opa, &resolved_product_image)?; // required for discovery config map later let server_role_service = cluster_resources @@ -488,7 +480,6 @@ pub async fn reconcile_opa( &resolved_product_image, &rolegroup, &merged_config, - vector_aggregator_address.as_deref(), )?; let rg_service = build_rolegroup_service(opa, &resolved_product_image, &rolegroup)?; let rg_daemonset = build_server_rolegroup_daemonset( @@ -682,7 +673,6 @@ fn build_server_rolegroup_config_map( resolved_product_image: &ResolvedProductImage, rolegroup: &RoleGroupRef, merged_config: &v1alpha1::OpaConfig, - vector_aggregator_address: Option<&str>, ) -> Result { let mut cm_builder = ConfigMapBuilder::new(); @@ -711,15 +701,11 @@ fn build_server_rolegroup_config_map( ); } - extend_role_group_config_map( - rolegroup, - vector_aggregator_address, - &merged_config.logging, - &mut cm_builder, - ) - .context(InvalidLoggingConfigSnafu { - cm_name: rolegroup.object_name(), - })?; + extend_role_group_config_map(rolegroup, &merged_config.logging, &mut cm_builder).context( + InvalidLoggingConfigSnafu { + cm_name: rolegroup.object_name(), + }, + )?; cm_builder .build() @@ -1034,24 +1020,37 @@ fn build_server_rolegroup_daemonset( } if merged_config.logging.enable_vector_agent { - pb.add_container( - product_logging::framework::vector_container( - resolved_product_image, - CONFIG_VOLUME_NAME, - LOG_VOLUME_NAME, - merged_config - .logging - .containers - .get(&v1alpha1::Container::Vector), - ResourceRequirementsBuilder::new() - .with_cpu_request("250m") - .with_cpu_limit("500m") - .with_memory_request("128Mi") - .with_memory_limit("128Mi") - .build(), - ) - .context(ConfigureLoggingSnafu)?, - ); + match opa + .spec + .cluster_config + .vector_aggregator_config_map_name + .to_owned() + { + Some(vector_aggregator_config_map_name) => { + pb.add_container( + product_logging::framework::vector_container( + resolved_product_image, + CONFIG_VOLUME_NAME, + LOG_VOLUME_NAME, + merged_config + .logging + .containers + .get(&v1alpha1::Container::Vector), + ResourceRequirementsBuilder::new() + .with_cpu_request("250m") + .with_cpu_limit("500m") + .with_memory_request("128Mi") + .with_memory_limit("128Mi") + .build(), + &vector_aggregator_config_map_name, + ) + .context(ConfigureLoggingSnafu)?, + ); + } + None => { + VectorAggregatorConfigMapMissingSnafu.fail()?; + } + } } add_graceful_shutdown_config(merged_config, &mut pb).context(GracefulShutdownSnafu)?; diff --git a/rust/operator-binary/src/product_logging.rs b/rust/operator-binary/src/product_logging.rs index bf58e831..842829c2 100644 --- a/rust/operator-binary/src/product_logging.rs +++ b/rust/operator-binary/src/product_logging.rs @@ -1,10 +1,7 @@ -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::Snafu; use stackable_opa_operator::crd::v1alpha1; use stackable_operator::{ builder::configmap::ConfigMapBuilder, - client::Client, - k8s_openapi::api::core::v1::ConfigMap, - kube::ResourceExt, product_logging::{ self, spec::{ContainerLogConfig, ContainerLogConfigChoice, LogLevel, Logging}, @@ -32,8 +29,6 @@ pub enum Error { type Result = std::result::Result; -const VECTOR_AGGREGATOR_CM_ENTRY: &str = "ADDRESS"; - #[derive(strum::Display)] #[strum(serialize_all = "lowercase")] pub enum OpaLogLevel { @@ -74,44 +69,9 @@ impl From for BundleBuilderLogLevel { } } -/// Return the address of the Vector aggregator if the corresponding ConfigMap name is given in the -/// cluster spec -pub async fn resolve_vector_aggregator_address( - opa: &v1alpha1::OpaCluster, - client: &Client, -) -> Result> { - let vector_aggregator_address = if let Some(vector_aggregator_config_map_name) = - &opa.spec.cluster_config.vector_aggregator_config_map_name - { - let vector_aggregator_address = client - .get::( - vector_aggregator_config_map_name, - opa.namespace() - .as_deref() - .context(ObjectHasNoNamespaceSnafu)?, - ) - .await - .context(ConfigMapNotFoundSnafu { - cm_name: vector_aggregator_config_map_name.to_string(), - })? - .data - .and_then(|mut data| data.remove(VECTOR_AGGREGATOR_CM_ENTRY)) - .context(MissingConfigMapEntrySnafu { - entry: VECTOR_AGGREGATOR_CM_ENTRY, - cm_name: vector_aggregator_config_map_name.to_string(), - })?; - Some(vector_aggregator_address) - } else { - None - }; - - Ok(vector_aggregator_address) -} - /// Extend the role group ConfigMap with logging and Vector configurations pub fn extend_role_group_config_map( rolegroup: &RoleGroupRef, - vector_aggregator_address: Option<&str>, logging: &Logging, cm_builder: &mut ConfigMapBuilder, ) -> Result<()> { @@ -127,11 +87,7 @@ pub fn extend_role_group_config_map( if logging.enable_vector_agent { cm_builder.add_data( product_logging::framework::VECTOR_CONFIG_FILE, - product_logging::framework::create_vector_config( - rolegroup, - vector_aggregator_address.context(MissingVectorAggregatorAddressSnafu)?, - vector_log_config, - ), + product_logging::framework::create_vector_config(rolegroup, vector_log_config), ); } From 766c4f169d3d2280cd53f7afc1ef8b2699a9cc87 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 8 Apr 2025 21:46:08 +0200 Subject: [PATCH 2/5] add pr number to changelog --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74a8030c..9d16ef32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,14 +19,14 @@ All notable changes to this project will be documented in this file. and is now set by `ROLLING_LOGS` (or via `--rolling-logs `). - Replace stackable-operator `print_startup_string` with `tracing::info!` with fields. - BREAKING: Inject the vector aggregator address into the vector config using the env var `VECTOR_AGGREGATOR_ADDRESS` instead - of having the operator write it to the vector config ([#XXX]). + of having the operator write it to the vector config ([#707]). ### Fixed -- Fix a bug where changes to ConfigMaps that are referenced in the OpaCluster spec didn't trigger a reconciliation ([#XXX]). +- Fix a bug where changes to ConfigMaps that are referenced in the OpaCluster spec didn't trigger a reconciliation ([#707]). [#703]: https://github.com/stackabletech/opa-operator/pull/703 -[#XXX]: https://github.com/stackabletech/opa-operator/pull/XXX +[#707]: https://github.com/stackabletech/opa-operator/pull/707 ## [25.3.0] - 2025-03-21 From f8e144435288d540bad7e3b0d3d2e589b220fa97 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 8 Apr 2025 21:46:37 +0200 Subject: [PATCH 3/5] regenerate nix --- Cargo.nix | 14 +++++++------- crate-hashes.json | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.nix b/Cargo.nix index 8bcafffb..698a6913 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -10769,13 +10769,13 @@ rec { }; "stackable-operator" = rec { crateName = "stackable-operator"; - version = "0.89.1"; + version = "0.90.0"; edition = "2024"; workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "cd73728af410c52972b9a9a3ba1302bcdb574d04"; - sha256 = "1hrxrybc6197ibx0m2wfxlg5pdg4hanf6xvslzrhsp77a04pb0y9"; + rev = "ea063b4595caa20c82d37c595487c76476c9ab10"; + sha256 = "0fclvpxhchykqd7bl8hscr4v06mbs2v5vjp0xv27nvqr94j63xs2"; }; libName = "stackable_operator"; authors = [ @@ -10920,8 +10920,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "cd73728af410c52972b9a9a3ba1302bcdb574d04"; - sha256 = "1hrxrybc6197ibx0m2wfxlg5pdg4hanf6xvslzrhsp77a04pb0y9"; + rev = "ea063b4595caa20c82d37c595487c76476c9ab10"; + sha256 = "0fclvpxhchykqd7bl8hscr4v06mbs2v5vjp0xv27nvqr94j63xs2"; }; procMacro = true; libName = "stackable_operator_derive"; @@ -10955,8 +10955,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "cd73728af410c52972b9a9a3ba1302bcdb574d04"; - sha256 = "1hrxrybc6197ibx0m2wfxlg5pdg4hanf6xvslzrhsp77a04pb0y9"; + rev = "ea063b4595caa20c82d37c595487c76476c9ab10"; + sha256 = "0fclvpxhchykqd7bl8hscr4v06mbs2v5vjp0xv27nvqr94j63xs2"; }; libName = "stackable_shared"; authors = [ diff --git a/crate-hashes.json b/crate-hashes.json index 6161c677..69e0f985 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,7 +1,7 @@ { - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.89.1#stackable-operator-derive@0.3.1": "1hrxrybc6197ibx0m2wfxlg5pdg4hanf6xvslzrhsp77a04pb0y9", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.89.1#stackable-operator@0.89.1": "1hrxrybc6197ibx0m2wfxlg5pdg4hanf6xvslzrhsp77a04pb0y9", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.89.1#stackable-shared@0.0.1": "1hrxrybc6197ibx0m2wfxlg5pdg4hanf6xvslzrhsp77a04pb0y9", + "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.90.0#stackable-operator-derive@0.3.1": "0fclvpxhchykqd7bl8hscr4v06mbs2v5vjp0xv27nvqr94j63xs2", + "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.90.0#stackable-operator@0.90.0": "0fclvpxhchykqd7bl8hscr4v06mbs2v5vjp0xv27nvqr94j63xs2", + "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.90.0#stackable-shared@0.0.1": "0fclvpxhchykqd7bl8hscr4v06mbs2v5vjp0xv27nvqr94j63xs2", "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-telemetry-0.4.0#stackable-telemetry@0.4.0": "0hcm64fb2ngyalq8rci5lrr881prg023pq9cd1sfr79iynbr6a26", "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-versioned-0.6.0#k8s-version@0.1.2": "0asgwj93drwvqsgd5c563qawjc3avb42nav0i5dgs4zv8bldx6x0", "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-versioned-0.6.0#stackable-versioned-macros@0.6.0": "0asgwj93drwvqsgd5c563qawjc3avb42nav0i5dgs4zv8bldx6x0", From db46043d5bb8fdf81d92e3bc18a10edea8bf2115 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 8 Apr 2025 22:07:57 +0200 Subject: [PATCH 4/5] fix changelog --- CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d16ef32..8433cf27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,10 +21,6 @@ All notable changes to this project will be documented in this file. - BREAKING: Inject the vector aggregator address into the vector config using the env var `VECTOR_AGGREGATOR_ADDRESS` instead of having the operator write it to the vector config ([#707]). -### Fixed - -- Fix a bug where changes to ConfigMaps that are referenced in the OpaCluster spec didn't trigger a reconciliation ([#707]). - [#703]: https://github.com/stackabletech/opa-operator/pull/703 [#707]: https://github.com/stackabletech/opa-operator/pull/707 From 9ae3ff2eadfa7baedbfd8b480b127aff0ae6beac Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 9 Apr 2025 18:27:36 +0200 Subject: [PATCH 5/5] chore: Use borrows --- rust/operator-binary/src/controller.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 7f0e7fae..7e2d098f 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1020,12 +1020,7 @@ fn build_server_rolegroup_daemonset( } if merged_config.logging.enable_vector_agent { - match opa - .spec - .cluster_config - .vector_aggregator_config_map_name - .to_owned() - { + match &opa.spec.cluster_config.vector_aggregator_config_map_name { Some(vector_aggregator_config_map_name) => { pb.add_container( product_logging::framework::vector_container( @@ -1042,7 +1037,7 @@ fn build_server_rolegroup_daemonset( .with_memory_request("128Mi") .with_memory_limit("128Mi") .build(), - &vector_aggregator_config_map_name, + vector_aggregator_config_map_name, ) .context(ConfigureLoggingSnafu)?, );