diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e5c6889..d7a346df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,15 @@ All notable changes to this project will be documented in this file. Also, a rego-rule library has been added to make it easier to call resource-info-fetcher from within OPA. The API (especially the response) might change in the future once more data catalogs are supported ([#863]). - Allow specifying the maximum number of cached entries in the user-info-fetcher ([#863]). +- The `servers` role can now run as a `Deployment` instead of a `DaemonSet`, selected via + `spec.servers.roleConfig.workloadKind`. ([#873]). +- A `PodDisruptionBudget` is now written out for the `servers` role when it runs as a `Deployment`, + with `maxUnavailable: 1`. Configurable via `spec.servers.roleConfig.podDisruptionBudget` ([#873]). ### Changed +- OPA Pods now default to a soft anti-affinity that spreads them across nodes. This is a no-op for a + `DaemonSet`, which already runs one Pod per node, but keeps a `Deployment`'s replicas from being deployed together ([#873]). - Internal operator refactoring: introduce a build() step in the reconciler that assembles all relevant Kubernetes resources before anything is applied ([#852]). - Bump `stackable-operator` to 0.114.0 ([#867]). @@ -27,6 +33,7 @@ All notable changes to this project will be documented in this file. which could cause problems with GitOps tools (e.g. ArgoCD) reporting a diff in the custom resources. See [our internal issue](https://github.com/stackabletech/hdfs-operator/issues/626) and [the fix](https://github.com/kube-rs/kube/pull/2042) for details ([#871]). +[#873]: https://github.com/stackabletech/opa-operator/pull/873 [#852]: https://github.com/stackabletech/opa-operator/pull/852 [#861]: https://github.com/stackabletech/opa-operator/pull/861 [#863]: https://github.com/stackabletech/opa-operator/pull/863 diff --git a/deploy/helm/opa-operator/templates/clusterrole-operator.yaml b/deploy/helm/opa-operator/templates/clusterrole-operator.yaml index 5805576d..f44eea59 100644 --- a/deploy/helm/opa-operator/templates/clusterrole-operator.yaml +++ b/deploy/helm/opa-operator/templates/clusterrole-operator.yaml @@ -63,12 +63,26 @@ rules: - bind resourceNames: - {{ include "operator.name" . }}-clusterrole - # DaemonSet created per role group. Applied via SSA, tracked for orphan cleanup, and - # owned by the controller. + # DaemonSet or Deployment created per role group, depending on the role's `workloadKind`. + # Applied via SSA, tracked for orphan cleanup, and owned by the controller. - apiGroups: - apps resources: - daemonsets + - deployments + verbs: + - create + - delete + - get + - list + - patch + - watch + # PodDisruptionBudget created per role, when the role has it enabled. Also needs `delete`, because + # disabling it (or switching to a DaemonSet) must clean the existing budget up. + - apiGroups: + - policy + resources: + - poddisruptionbudgets verbs: - create - delete diff --git a/extra/crds.yaml b/extra/crds.yaml index 17d51505..2276f537 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -1490,10 +1490,54 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true roleConfig: - default: {} - description: |- - This is a product-agnostic RoleConfig, with nothing in it. It is used e.g. by products that have - nothing configurable at role level. + default: + podDisruptionBudget: + enabled: null + maxUnavailable: null + workloadKind: DaemonSet + description: Role-level configuration for the OPA servers. + properties: + podDisruptionBudget: + default: + enabled: null + maxUnavailable: null + description: |- + This struct is used to configure: + + 1. If PodDisruptionBudgets are created by the operator + 2. The allowed number of Pods to be unavailable (`maxUnavailable`) + + Documentation: + [allowed Pod disruptions documentation](https://docs.stackable.tech/home/nightly/concepts/operations/pod_disruptions). + properties: + enabled: + description: |- + Whether a PodDisruptionBudget should be written out for this role. + + Defaults to `true` when `workloadKind` is `Deployment` and to `false` when it is + `DaemonSet`, since a PodDisruptionBudget doesn't make sense for a DaemonSet. + nullable: true + type: boolean + maxUnavailable: + description: The number of Pods that are allowed to be down simultaneous. + format: uint16 + maximum: 65535.0 + minimum: 0.0 + nullable: true + type: integer + type: object + workloadKind: + default: DaemonSet + description: |- + The Kubernetes workload the OPA servers run as. + + * `DaemonSet`: one Pod per node. `replicas` is ignored. + + * `Deployment`: fixed number of Pods, configured by `replicas`. + enum: + - DaemonSet + - Deployment + type: string type: object roleGroups: additionalProperties: @@ -3747,10 +3791,54 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true roleConfig: - default: {} - description: |- - This is a product-agnostic RoleConfig, with nothing in it. It is used e.g. by products that have - nothing configurable at role level. + default: + podDisruptionBudget: + enabled: null + maxUnavailable: null + workloadKind: DaemonSet + description: Role-level configuration for the OPA servers. + properties: + podDisruptionBudget: + default: + enabled: null + maxUnavailable: null + description: |- + This struct is used to configure: + + 1. If PodDisruptionBudgets are created by the operator + 2. The allowed number of Pods to be unavailable (`maxUnavailable`) + + Documentation: + [allowed Pod disruptions documentation](https://docs.stackable.tech/home/nightly/concepts/operations/pod_disruptions). + properties: + enabled: + description: |- + Whether a PodDisruptionBudget should be written out for this role. + + Defaults to `true` when `workloadKind` is `Deployment` and to `false` when it is + `DaemonSet`, since a PodDisruptionBudget doesn't make sense for a DaemonSet. + nullable: true + type: boolean + maxUnavailable: + description: The number of Pods that are allowed to be down simultaneous. + format: uint16 + maximum: 65535.0 + minimum: 0.0 + nullable: true + type: integer + type: object + workloadKind: + default: DaemonSet + description: |- + The Kubernetes workload the OPA servers run as. + + * `DaemonSet`: one Pod per node. `replicas` is ignored. + + * `Deployment`: fixed number of Pods, configured by `replicas`. + enum: + - DaemonSet + - Deployment + type: string type: object roleGroups: additionalProperties: diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index d97844f8..ebe8791a 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -4,6 +4,7 @@ use std::str::FromStr; use snafu::{ResultExt, Snafu}; +use stackable_opa_operator::crd::v1alpha2; use stackable_operator::{ builder::meta::ObjectMetaBuilder, utils::cluster_info::KubernetesClusterInfo, @@ -14,13 +15,17 @@ use crate::controller::{ KubernetesResources, RoleGroupName, ValidatedCluster, build::resource::{ config_map::build_rolegroup_config_map, - daemonset::build_server_rolegroup_daemonset, discovery::build_discovery_config_map, + pdb::build_role_pod_disruption_budget, rbac::{build_role_binding, build_service_account}, service::{ build_rolegroup_headless_service, build_rolegroup_metrics_service, build_server_role_service, }, + workload::{ + daemonset::build_server_rolegroup_daemonset, + deployment::build_server_rolegroup_deployment, + }, }, }; @@ -37,7 +42,13 @@ pub enum Error { #[snafu(display("failed to build DaemonSet for role group {role_group}"))] DaemonSet { - source: resource::daemonset::Error, + source: resource::workload::Error, + role_group: RoleGroupName, + }, + + #[snafu(display("failed to build Deployment for role group {role_group}"))] + Deployment { + source: resource::workload::Error, role_group: RoleGroupName, }, @@ -62,13 +73,25 @@ pub fn build( cluster_info: &KubernetesClusterInfo, ) -> Result { let mut daemon_sets = vec![]; + let mut deployments = vec![]; let mut services = vec![]; let mut config_maps = vec![]; + let mut pod_disruption_budgets = vec![]; // The role-level load-balanced Service, which is not bound to a single role group. services.push(build_server_role_service(cluster)); - for role_group_configs in cluster.role_group_configs.values() { + // Iterating with the role key, because the workload kind is configured per role. + for (opa_role, role_group_configs) in &cluster.role_group_configs { + let role_config = cluster.role_config(opa_role); + let workload_kind = &role_config.workload_kind; + + pod_disruption_budgets.extend(build_role_pod_disruption_budget( + cluster, + opa_role, + role_config, + )); + for (role_group_name, role_group) in role_group_configs { config_maps.push( build_rolegroup_config_map(cluster, role_group_name, role_group).context( @@ -79,20 +102,37 @@ pub fn build( ); services.push(build_rolegroup_headless_service(cluster, role_group_name)); services.push(build_rolegroup_metrics_service(cluster, role_group_name)); - daemon_sets.push( - build_server_rolegroup_daemonset( - cluster, - role_group_name, - role_group, - opa_bundle_builder_image, - user_info_fetcher_image, - resource_info_fetcher_image, - cluster_info, - ) - .context(DaemonSetSnafu { - role_group: role_group_name.clone(), - })?, - ); + // Exactly one workload object per role group, of the kind its role asks for. + match workload_kind { + v1alpha2::WorkloadKind::DaemonSet => daemon_sets.push( + build_server_rolegroup_daemonset( + cluster, + role_group_name, + role_group, + opa_bundle_builder_image, + user_info_fetcher_image, + resource_info_fetcher_image, + cluster_info, + ) + .context(DaemonSetSnafu { + role_group: role_group_name.clone(), + })?, + ), + v1alpha2::WorkloadKind::Deployment => deployments.push( + build_server_rolegroup_deployment( + cluster, + role_group_name, + role_group, + opa_bundle_builder_image, + user_info_fetcher_image, + resource_info_fetcher_image, + cluster_info, + ) + .context(DeploymentSnafu { + role_group: role_group_name.clone(), + })?, + ), + } } } @@ -101,10 +141,12 @@ pub fn build( Ok(KubernetesResources { daemon_sets, + deployments, services, config_maps, service_accounts: vec![build_service_account(cluster)], role_bindings: vec![build_role_binding(cluster)], + pod_disruption_budgets, }) } @@ -185,11 +227,12 @@ mod tests { ) .expect("build succeeds"); - // One DaemonSet per role group. + // One DaemonSet per role group, as `workloadKind` defaults to `DaemonSet`. assert_eq!( sorted_names(&resources.daemon_sets), ["test-opa-server-default"] ); + assert!(resources.deployments.is_empty()); // The role-level Service plus a headless and a metrics Service per role group. assert_eq!( sorted_names(&resources.services), @@ -213,5 +256,59 @@ mod tests { sorted_names(&resources.role_bindings), ["test-opa-rolebinding"] ); + // The default `DaemonSet` gets no PodDisruptionBudget, so existing installations gain no + // new object on upgrade. + assert!(resources.pod_disruption_budgets.is_empty()); + } + + /// `workloadKind` decides which workload object a role group gets. Exactly one kind is built, so + /// the other list stays empty and `ClusterResources` sweeps the workload that is no longer + /// wanted when the administrator switches modes. + #[test] + fn build_dispatches_on_workload_kind() { + let build_with = |workload_kind| { + build( + &validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": workload_kind }, + "roleGroups": { "default": {} }, + }, + })), + "bundle-builder-image", + "user-info-fetcher-image", + "resource-info-fetcher-image", + &cluster_info(), + ) + .expect("build succeeds") + }; + + let daemon_set_mode = build_with("DaemonSet"); + assert_eq!( + sorted_names(&daemon_set_mode.daemon_sets), + ["test-opa-server-default"] + ); + assert!(daemon_set_mode.deployments.is_empty()); + + let deployment_mode = build_with("Deployment"); + assert_eq!( + sorted_names(&deployment_mode.deployments), + ["test-opa-server-default"] + ); + assert!(deployment_mode.daemon_sets.is_empty()); + + // One role-level PodDisruptionBudget for a Deployment, none for a DaemonSet whose Pods + // `kubectl drain` skips anyway. + assert!(daemon_set_mode.pod_disruption_budgets.is_empty()); + assert_eq!( + sorted_names(&deployment_mode.pod_disruption_budgets), + ["test-opa-server"] + ); + + // Products consume the discovery ConfigMap, so it must not depend on the workload kind. + assert_eq!( + sorted_names(&daemon_set_mode.config_maps), + sorted_names(&deployment_mode.config_maps) + ); } } diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index a921b543..baefdf6e 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -2,7 +2,8 @@ //! Kubernetes resources, one module per resource kind. pub mod config_map; -pub mod daemonset; pub mod discovery; +pub mod pdb; pub mod rbac; pub mod service; +pub mod workload; diff --git a/rust/operator-binary/src/controller/build/resource/pdb.rs b/rust/operator-binary/src/controller/build/resource/pdb.rs new file mode 100644 index 00000000..fbeb5b72 --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/pdb.rs @@ -0,0 +1,170 @@ +//! Builds the [`PodDisruptionBudget`] that limits how many OPA Pods of a role a voluntary +//! disruption (a node drain, say) may take down at once. + +use stackable_opa_operator::crd::{OpaRole, v1alpha2}; +use stackable_operator::{ + k8s_openapi::api::policy::v1::PodDisruptionBudget, + v2::builder::pdb::pod_disruption_budget_builder_with_role, +}; + +use crate::controller::{ValidatedCluster, controller_name, operator_name, product_name}; + +/// How many Pods of a role may be unavailable when the administrator configured no +/// `maxUnavailable`. +const DEFAULT_MAX_UNAVAILABLE: u16 = 1; + +/// The role-level [`PodDisruptionBudget`], or `None` when the role has it disabled. +/// +/// One per role rather than per role group, because the budget selects on the role's labels and so +/// covers every role group of that role at once. +pub fn build_role_pod_disruption_budget( + cluster: &ValidatedCluster, + opa_role: &OpaRole, + role_config: &v1alpha2::OpaRoleConfig, +) -> Option { + if !role_config.pod_disruption_budget_enabled() { + return None; + } + + let max_unavailable = role_config + .pod_disruption_budget + .max_unavailable + .unwrap_or(DEFAULT_MAX_UNAVAILABLE); + + Some( + pod_disruption_budget_builder_with_role( + cluster, + &product_name(), + &opa_role.clone().into(), + &operator_name(), + &controller_name(), + ) + .with_max_unavailable(max_unavailable) + .build(), + ) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + use crate::controller::build::properties::test_support::validated_cluster_from_spec; + + fn build(spec: serde_json::Value) -> Option { + let cluster = validated_cluster_from_spec(spec); + build_role_pod_disruption_budget( + &cluster, + &OpaRole::Server, + cluster.role_config(&OpaRole::Server), + ) + } + + /// A DaemonSet covers every node and `kubectl drain` skips its Pods, so a budget would protect + /// nothing. This is the default, so existing installations gain no new object. + #[test] + fn daemonset_gets_no_pod_disruption_budget() { + assert!( + build(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { "roleGroups": { "default": {} } }, + })) + .is_none() + ); + } + + /// A Deployment's Pods are evictable, so the budget is created by default. + #[test] + fn deployment_gets_a_pod_disruption_budget_selecting_the_whole_role() { + let pdb = build(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": {}, "other": {} }, + }, + })) + .expect("a Deployment is protected by default"); + + assert_eq!(pdb.metadata.name.as_deref(), Some("test-opa-server")); + let spec = pdb.spec.expect("the builder always sets a spec"); + assert_eq!( + spec.max_unavailable, + Some( + stackable_operator::k8s_openapi::apimachinery::pkg::util::intstr::IntOrString::Int( + 1 + ) + ) + ); + // Selecting the role rather than a role group is what lets one budget cover both role + // groups configured above. + let match_labels = spec + .selector + .and_then(|selector| selector.match_labels) + .expect("the builder always sets a role selector"); + assert_eq!( + match_labels + .get("app.kubernetes.io/component") + .map(String::as_str), + Some("server") + ); + assert!(!match_labels.contains_key("app.kubernetes.io/role-group")); + } + + /// `enabled` is an explicit override, so a DaemonSet gets a budget when the administrator asks + /// for one, even though it will not do much. + #[test] + fn explicitly_enabling_it_wins_over_the_workload_kind_default() { + assert!( + build(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "podDisruptionBudget": { "enabled": true } }, + "roleGroups": { "default": {} }, + }, + })) + .is_some() + ); + } + + /// ...and disabling it opts a Deployment out. + #[test] + fn explicitly_disabling_it_opts_a_deployment_out() { + assert!( + build(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { + "workloadKind": "Deployment", + "podDisruptionBudget": { "enabled": false }, + }, + "roleGroups": { "default": {} }, + }, + })) + .is_none() + ); + } + + #[test] + fn configured_max_unavailable_overrides_the_default() { + let pdb = build(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { + "workloadKind": "Deployment", + "podDisruptionBudget": { "maxUnavailable": 2 }, + }, + "roleGroups": { "default": {} }, + }, + })) + .expect("a Deployment is protected by default"); + + assert_eq!( + pdb.spec.unwrap().max_unavailable, + Some( + stackable_operator::k8s_openapi::apimachinery::pkg::util::intstr::IntOrString::Int( + 2 + ) + ) + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index bc49acef..1dfc5f29 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; +use stackable_opa_operator::crd::OpaRole; use stackable_operator::{ k8s_openapi::api::core::v1::{Service, ServicePort, ServiceSpec}, v2::{ @@ -33,13 +34,24 @@ pub(crate) fn build_server_role_service(cluster: &ValidatedCluster) -> Service { type_: Some(cluster.cluster_config.listener_class.k8s_service_type()), ports: Some(data_service_ports(cluster.is_tls_enabled())), selector: Some(cluster.role_selector().into()), - // This ensures that products (e.g. Trino) on a node always talk to the OPA pod on the - // same node, avoiding cross-node latency. The downside is that if the local OPA pod is - // unavailable, requests fail instead of falling back to another node. - // TODO: Once our minimum supported Kubernetes version is 1.35, use + // Derived from the role's `workloadKind`: + // + // * `Local` for a DaemonSet, so that products (e.g. Trino) on a node always talk to the OPA + // Pod on the same node, avoiding cross-node latency. The downside is that if the local OPA + // Pod is unavailable, requests fail instead of falling back to another node. + // + // * `Cluster` for a Deployment, whose Pods do not cover every node, so node-local routing + // would leave products on Pod-less nodes unable to reach OPA at all. + // + // TODO: In the DaemonSet case, once our minimum supported Kubernetes version is 1.35, use // `trafficDistribution: PreferSameNode` instead, which prefers the local node but // gracefully falls back to other nodes if the local pod is unavailable. - internal_traffic_policy: Some("Local".to_string()), + internal_traffic_policy: Some( + cluster + .role_config(&OpaRole::Server) + .internal_traffic_policy() + .to_string(), + ), ..ServiceSpec::default() }; @@ -223,6 +235,22 @@ mod tests { assert!(!spec.selector.unwrap().contains_key(ROLE_GROUP_LABEL)); } + /// In `Deployment` mode the Pods do not cover every node, so node-local routing would strand + /// products running on Pod-less nodes. The policy has to follow `workloadKind`. + #[test] + fn role_service_traffic_policy_follows_workload_kind() { + let deployment_mode = validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": {} }, + }, + })); + + let spec = build_server_role_service(&deployment_mode).spec.unwrap(); + assert_eq!(spec.internal_traffic_policy.as_deref(), Some("Cluster")); + } + #[test] fn role_service_port_follows_tls() { assert_eq!( diff --git a/rust/operator-binary/src/controller/build/resource/workload/daemonset.rs b/rust/operator-binary/src/controller/build/resource/workload/daemonset.rs new file mode 100644 index 00000000..8f6de324 --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/workload/daemonset.rs @@ -0,0 +1,475 @@ +//! Builds the rolegroup [`DaemonSet`] that runs OPA on every node. + +use stackable_operator::k8s_openapi::{ + api::apps::v1::{DaemonSet, DaemonSetSpec, DaemonSetUpdateStrategy, RollingUpdateDaemonSet}, + apimachinery::pkg::apis::meta::v1::LabelSelector, +}; + +use super::*; + +/// The rolegroup [`DaemonSet`] runs the rolegroup, as configured by the administrator. +/// +/// The [`Pod`](`stackable_operator::k8s_openapi::api::core::v1::Pod`)s are accessible through the +/// corresponding [`Service`](`stackable_operator::k8s_openapi::api::core::v1::Service`) (from +/// [`build_server_role_service`](super::super::service::build_server_role_service)). +/// +/// We run an OPA on each node, because we want to avoid requiring network roundtrips for services making +/// policy queries (which are often chained in serial, and block other tasks in the products). +#[allow(clippy::too_many_arguments)] +pub fn build_server_rolegroup_daemonset( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + role_group: &OpaRoleGroupConfig, + opa_bundle_builder_image: &str, + user_info_fetcher_image: &str, + resource_info_fetcher_image: &str, + cluster_info: &KubernetesClusterInfo, +) -> Result { + let pod_template = build_server_rolegroup_pod_template( + cluster, + role_group_name, + role_group, + opa_bundle_builder_image, + user_info_fetcher_image, + resource_info_fetcher_image, + cluster_info, + )?; + + let metadata = build::object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .daemon_set_name() + .to_string(), + role_group_name, + ) + .build(); + + let daemonset_spec = DaemonSetSpec { + selector: LabelSelector { + match_labels: Some(cluster.role_group_selector(role_group_name).into()), + ..LabelSelector::default() + }, + template: pod_template, + update_strategy: Some(DaemonSetUpdateStrategy { + type_: Some("RollingUpdate".to_string()), + rolling_update: Some(RollingUpdateDaemonSet { + max_surge: Some(IntOrString::Int(1)), + max_unavailable: Some(IntOrString::Int(0)), + }), + }), + ..DaemonSetSpec::default() + }; + + Ok(DaemonSet { + metadata, + spec: Some(daemonset_spec), + status: None, + }) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + use stackable_operator::{ + commons::networking::DomainName, k8s_openapi::api::core::v1::Container, + }; + + use stackable_opa_operator::crd::OpaRole; + + use super::*; + use crate::controller::build::properties::test_support::validated_cluster_from_spec; + + fn cluster_info() -> KubernetesClusterInfo { + KubernetesClusterInfo { + cluster_domain: DomainName::try_from("cluster.local").unwrap(), + } + } + + fn build(cluster: &ValidatedCluster) -> DaemonSet { + let (role_group_name, role_group) = cluster.role_group_configs[&OpaRole::Server] + .iter() + .next() + .expect("the default role group should exist"); + build_server_rolegroup_daemonset( + cluster, + role_group_name, + role_group, + "bundle-builder-image", + "user-info-fetcher-image", + "resource-info-fetcher-image", + &cluster_info(), + ) + .expect("the daemonset should build") + } + + fn container_names(ds: &DaemonSet) -> Vec { + ds.spec + .as_ref() + .unwrap() + .template + .spec + .as_ref() + .unwrap() + .containers + .iter() + .map(|c| c.name.clone()) + .collect() + } + + fn volume_names(ds: &DaemonSet) -> Vec { + ds.spec + .as_ref() + .unwrap() + .template + .spec + .as_ref() + .unwrap() + .volumes + .as_ref() + .unwrap() + .iter() + .map(|v| v.name.clone()) + .collect() + } + + #[test] + fn daemonset_has_expected_name_and_rolling_update_strategy() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + assert_eq!(ds.metadata.name.as_deref(), Some("test-opa-server-default")); + let strategy = ds.spec.as_ref().unwrap().update_strategy.as_ref().unwrap(); + assert_eq!(strategy.type_.as_deref(), Some("RollingUpdate")); + let rolling_update = strategy.rolling_update.as_ref().unwrap(); + // A DaemonSet must never take an OPA pod down before the replacement is ready. + assert_eq!(rolling_update.max_unavailable, Some(IntOrString::Int(0))); + } + + #[test] + fn daemonset_runs_opa_and_bundle_builder_with_prepare_init_container() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + let containers = container_names(&ds); + assert!(containers.contains(&"opa".to_owned())); + assert!(containers.contains(&"bundle-builder".to_owned())); + // No sidecars without the corresponding cluster config. + assert!(!containers.contains(&"user-info-fetcher".to_owned())); + assert!(!containers.contains(&"vector".to_owned())); + + let pod_spec = ds.spec.as_ref().unwrap().template.spec.as_ref().unwrap(); + let init_containers: Vec<_> = pod_spec + .init_containers + .as_ref() + .unwrap() + .iter() + .map(|c| c.name.clone()) + .collect(); + assert_eq!(init_containers, vec!["prepare".to_owned()]); + + // The standard volumes are always present; the TLS volume is not (no TLS configured). + let volumes = volume_names(&ds); + for expected in ["config", "bundles", "log"] { + assert!( + volumes.contains(&expected.to_owned()), + "missing volume {expected}" + ); + } + assert!(!volumes.contains(&"tls".to_owned())); + } + + #[test] + fn daemonset_adds_vector_container_when_agent_enabled() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { "vectorAggregatorConfigMapName": "vector-aggregator-discovery" }, + "servers": { + "config": { "logging": { "enableVectorAgent": true } }, + "roleGroups": { "default": {} }, + }, + }))); + + assert!(container_names(&ds).contains(&"vector".to_owned())); + } + + #[test] + fn daemonset_adds_user_info_fetcher_container_when_configured() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { + "userInfo": { + "backend": { + "experimentalXfscAas": { + "hostname": "aas.default.svc.cluster.local", + "port": 5000, + } + } + } + }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + assert!(container_names(&ds).contains(&"user-info-fetcher".to_owned())); + } + + #[test] + fn opa_probes_root_and_bundle_builder_probes_status() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { "roleGroups": { "default": {} } }, + }))); + let pod_spec = ds.spec.as_ref().unwrap().template.spec.as_ref().unwrap(); + let liveness_path = |container: &str| -> String { + pod_spec + .containers + .iter() + .find(|c| c.name == container) + .unwrap_or_else(|| panic!("container {container} should exist")) + .liveness_probe + .as_ref() + .unwrap() + .http_get + .as_ref() + .unwrap() + .path + .clone() + .unwrap() + }; + // OPA's HTTP server answers `/`; only the bundle-builder exposes `/status`. A wrong path + // here makes the liveness probe fail and the OPA container CrashLoop. + assert_eq!(liveness_path("opa"), "/"); + assert_eq!(liveness_path("bundle-builder"), "/status"); + } + + #[test] + fn daemonset_adds_tls_volume_when_tls_enabled() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { "tls": { "serverSecretClass": "tls" } }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + assert!(volume_names(&ds).contains(&"tls".to_owned())); + } + + #[test] + fn opa_container_serves_https_when_tls_enabled() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { "tls": { "serverSecretClass": "tls" } }, + "servers": { "roleGroups": { "default": {} } }, + }))); + let pod_spec = ds.spec.as_ref().unwrap().template.spec.as_ref().unwrap(); + let opa = pod_spec + .containers + .iter() + .find(|c| c.name == "opa") + .expect("opa container should exist"); + + // The single container port is the HTTPS data port. + let ports = opa.ports.as_ref().unwrap(); + assert_eq!(ports.len(), 1); + assert_eq!(ports[0].name.as_deref(), Some("https")); + assert_eq!(ports[0].container_port, 8443); + + // The probe must speak HTTPS, otherwise it would fail against the TLS-only server. + let scheme = opa + .liveness_probe + .as_ref() + .unwrap() + .http_get + .as_ref() + .unwrap() + .scheme + .clone(); + assert_eq!(scheme.as_deref(), Some("HTTPS")); + + // The start command binds the HTTPS port and passes the TLS cert/key flags. + let args = opa.args.as_ref().unwrap(); + assert!(args[0].contains("-a 0.0.0.0:8443")); + assert!(args[0].contains("--tls-cert-file")); + } + + #[test] + fn bundle_builder_start_command_silences_console_only_when_none() { + let role_group_config = |spec: serde_json::Value| { + let cluster = validated_cluster_from_spec(spec); + cluster.role_group_configs[&OpaRole::Server] + .values() + .next() + .expect("the default role group should exist") + .config + .clone() + }; + + // Console level NONE redirects bundle-builder output to /dev/null (no `tee`). + let silenced = role_group_config(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "config": { "logging": { "containers": { + "bundle-builder": { "console": { "level": "NONE" } } + } } }, + "roleGroups": { "default": {} }, + }, + })); + // The redirect is appended directly after the bundle-builder invocation. (`/dev/null` also + // appears in the shared bash trap helpers, so match the specific redirect.) + assert!( + build_bundle_builder_start_command(&silenced, "bundle-builder") + .contains("stackable-opa-bundle-builder > /dev/null") + ); + + // With a console level above NONE, output is not discarded. + let logging = role_group_config(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "config": { "logging": { "containers": { + "bundle-builder": { "console": { "level": "INFO" } } + } } }, + "roleGroups": { "default": {} }, + }, + })); + assert!( + build_bundle_builder_start_command(&logging, "bundle-builder") + .contains("stackable-opa-bundle-builder &") + ); + } + + fn uif_container(ds: &DaemonSet) -> Container { + ds.spec + .as_ref() + .unwrap() + .template + .spec + .as_ref() + .unwrap() + .containers + .iter() + .find(|c| c.name == "user-info-fetcher") + .expect("the user-info-fetcher container should exist") + .clone() + } + + fn env_var(container: &Container, name: &str) -> String { + container + .env + .as_ref() + .expect("the container should have env vars") + .iter() + .find(|e| e.name == name) + .unwrap_or_else(|| panic!("env var {name} should be set")) + .value + .clone() + .unwrap_or_else(|| panic!("env var {name} should have a literal value")) + } + + fn mount_path(container: &Container, volume_name: &str) -> String { + container + .volume_mounts + .as_ref() + .expect("the container should have volume mounts") + .iter() + .find(|m| m.name == volume_name) + .unwrap_or_else(|| panic!("volume mount {volume_name} should exist")) + .mount_path + .clone() + } + + #[test] + fn user_info_fetcher_container_has_expected_command_and_config_wiring() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { + "userInfo": { + "backend": { + "experimentalXfscAas": { + "hostname": "aas.default.svc.cluster.local", + "port": 5000, + } + } + } + }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + let uif = uif_container(&ds); + assert_eq!( + uif.command, + Some(vec!["stackable-opa-user-info-fetcher".to_owned()]) + ); + // The sidecar reads its config from the shared config volume, and looks for backend + // credentials in a fixed directory (populated by the backend-specific arms below). + assert_eq!( + env_var(&uif, "CONFIG"), + "/stackable/config/user-info-fetcher.json" + ); + assert_eq!(env_var(&uif, "CREDENTIALS_DIR"), "/stackable/credentials"); + assert_eq!(mount_path(&uif, "config"), "/stackable/config"); + } + + #[test] + fn user_info_fetcher_active_directory_backend_mounts_kerberos_and_sets_krb5_env() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { + "userInfo": { + "backend": { + "experimentalActiveDirectory": { + "ldapServer": "ad.example.com", + "baseDistinguishedName": "dc=example,dc=com", + "kerberosSecretClassName": "kerberos", + } + } + } + }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + // A Kerberos secret volume is provisioned and mounted for the sidecar. + assert!(volume_names(&ds).contains(&"kerberos".to_owned())); + let uif = uif_container(&ds); + assert_eq!(mount_path(&uif, "kerberos"), "/stackable/kerberos"); + // The krb5 client must find the config and keytab, and keep tickets in memory only. + assert_eq!( + env_var(&uif, "KRB5_CONFIG"), + "/stackable/kerberos/krb5.conf" + ); + assert_eq!( + env_var(&uif, "KRB5_CLIENT_KTNAME"), + "/stackable/kerberos/keytab" + ); + assert_eq!(env_var(&uif, "KRB5CCNAME"), "MEMORY:"); + } + + #[test] + fn user_info_fetcher_keycloak_backend_mounts_client_credentials_secret() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { + "userInfo": { + "backend": { + "keycloak": { + "hostname": "keycloak.example.com", + "clientCredentialsSecret": "keycloak-credentials", + "adminRealm": "master", + "userRealm": "my-realm", + } + } + } + }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + // The client credentials secret is projected into the sidecar's credentials dir. + assert!(volume_names(&ds).contains(&"user-info-fetcher-credentials".to_owned())); + assert_eq!( + mount_path(&uif_container(&ds), "user-info-fetcher-credentials"), + "/stackable/credentials" + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/workload/deployment.rs b/rust/operator-binary/src/controller/build/resource/workload/deployment.rs new file mode 100644 index 00000000..fa78a746 --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/workload/deployment.rs @@ -0,0 +1,216 @@ +//! Builds the rolegroup [`Deployment`] that runs a fixed number of OPA replicas. + +use stackable_operator::k8s_openapi::{ + api::apps::v1::{Deployment, DeploymentSpec, DeploymentStrategy, RollingUpdateDeployment}, + apimachinery::pkg::apis::meta::v1::LabelSelector, +}; + +use super::*; + +/// Runs a fixed number of replicas, unlike [`daemonset`](super::daemonset), which covers every +/// node. The Pods therefore do not cover every node and the role Service has to route to any of +/// them rather than to a node-local one. +#[allow(clippy::too_many_arguments)] +pub fn build_server_rolegroup_deployment( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + role_group: &OpaRoleGroupConfig, + opa_bundle_builder_image: &str, + user_info_fetcher_image: &str, + resource_info_fetcher_image: &str, + cluster_info: &KubernetesClusterInfo, +) -> Result { + let pod_template = build_server_rolegroup_pod_template( + cluster, + role_group_name, + role_group, + opa_bundle_builder_image, + user_info_fetcher_image, + resource_info_fetcher_image, + cluster_info, + )?; + + let metadata = build::object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .deployment_name() + .to_string(), + role_group_name, + ) + .build(); + + let deployment_spec = DeploymentSpec { + // Left unset so Kubernetes applies its default of one, rather than the operator inventing + // a replica count. + replicas: role_group.replicas.map(i32::from), + selector: LabelSelector { + match_labels: Some(cluster.role_group_selector(role_group_name).into()), + ..LabelSelector::default() + }, + template: pod_template, + strategy: Some(DeploymentStrategy { + type_: Some("RollingUpdate".to_string()), + rolling_update: Some(RollingUpdateDeployment { + max_surge: Some(IntOrString::Int(1)), + max_unavailable: Some(IntOrString::Int(0)), + }), + }), + ..DeploymentSpec::default() + }; + + Ok(Deployment { + metadata, + spec: Some(deployment_spec), + status: None, + }) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + use stackable_operator::commons::networking::DomainName; + + use stackable_opa_operator::crd::OpaRole; + + use super::*; + use crate::controller::build::properties::test_support::validated_cluster_from_spec; + + fn cluster_info() -> KubernetesClusterInfo { + KubernetesClusterInfo { + cluster_domain: DomainName::try_from("cluster.local").unwrap(), + } + } + + fn build(cluster: &ValidatedCluster) -> Deployment { + let (role_group_name, role_group) = cluster.role_group_configs[&OpaRole::Server] + .iter() + .next() + .expect("the default role group should exist"); + build_server_rolegroup_deployment( + cluster, + role_group_name, + role_group, + "bundle-builder-image", + "user-info-fetcher-image", + "resource-info-fetcher-image", + &cluster_info(), + ) + .expect("the deployment should build") + } + + /// Named like the DaemonSet it replaces, so switching `workloadKind` swaps like for like. + #[test] + fn deployment_has_expected_name_and_rolling_update_strategy() { + let deployment = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": {} }, + }, + }))); + + assert_eq!( + deployment.metadata.name.as_deref(), + Some("test-opa-server-default") + ); + let strategy = deployment.spec.as_ref().unwrap().strategy.as_ref().unwrap(); + assert_eq!(strategy.type_.as_deref(), Some("RollingUpdate")); + let rolling_update = strategy.rolling_update.as_ref().unwrap(); + // OPA sits in the products' hot path, so a rollout must never reduce the ready Pod count. + assert_eq!(rolling_update.max_unavailable, Some(IntOrString::Int(0))); + assert_eq!(rolling_update.max_surge, Some(IntOrString::Int(1))); + } + + /// `replicas` is what a Deployment adds over a DaemonSet, so it has to reach the spec. + #[test] + fn deployment_takes_the_replicas_of_its_role_group() { + let deployment = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": { "replicas": 3 } }, + }, + }))); + + assert_eq!(deployment.spec.as_ref().unwrap().replicas, Some(3)); + } + + /// An unset `replicas` stays unset, leaving the Kubernetes default of one in place. + #[test] + fn deployment_without_replicas_leaves_them_unset() { + let deployment = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": {} }, + }, + }))); + + assert_eq!(deployment.spec.as_ref().unwrap().replicas, None); + } + + /// The Pod template is shared with the DaemonSet and covered by its tests; this only checks that + /// it is wrapped and selected the same way. + #[test] + fn deployment_wraps_the_shared_pod_template() { + let deployment = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": {} }, + }, + }))); + + let spec = deployment.spec.as_ref().unwrap(); + let containers: Vec<&str> = spec + .template + .spec + .as_ref() + .unwrap() + .containers + .iter() + .map(|container| container.name.as_str()) + .collect(); + assert!(containers.contains(&"opa")); + assert!(containers.contains(&"bundle-builder")); + + let match_labels = spec.selector.match_labels.as_ref().unwrap(); + assert_eq!( + match_labels + .get("app.kubernetes.io/role-group") + .map(String::as_str), + Some("default") + ); + } + + /// Replicas are only worth having if they are deployed on different nodes, so the default anti-affinity + /// has to survive the config merge into the Pod template. + #[test] + fn deployment_pods_are_spread_across_nodes_by_default() { + let deployment = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": { "replicas": 3 } }, + }, + }))); + + let anti_affinity = deployment + .spec + .and_then(|spec| spec.template.spec) + .and_then(|pod_spec| pod_spec.affinity) + .and_then(|affinity| affinity.pod_anti_affinity) + .expect("the default affinity spreads the role's Pods"); + + let preferred = anti_affinity + .preferred_during_scheduling_ignored_during_execution + .expect("the spread is a soft term"); + assert_eq!(preferred.len(), 1); + assert_eq!(preferred[0].weight, 70); + assert_eq!( + preferred[0].pod_affinity_term.topology_key, + "kubernetes.io/hostname" + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs b/rust/operator-binary/src/controller/build/resource/workload/mod.rs similarity index 60% rename from rust/operator-binary/src/controller/build/resource/daemonset/mod.rs rename to rust/operator-binary/src/controller/build/resource/workload/mod.rs index e6ab8c44..6bbc4735 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/workload/mod.rs @@ -1,5 +1,8 @@ -//! Builds the rolegroup [`DaemonSet`] that runs OPA (plus its bundle-builder, optional -//! user-info-fetcher, and Vector sidecars) on every node. +//! Building blocks shared by the rolegroup workload objects that run OPA (plus its +//! bundle-builder, optional user-info-fetcher, and Vector sidecars). +//! +//! The Pod template is identical regardless of how the rolegroup is deployed, so it is built here +//! and wrapped by the workload-specific submodules ([`daemonset`], [`deployment`]). use std::{collections::BTreeMap, str::FromStr}; @@ -21,14 +24,11 @@ use stackable_operator::{ commons::secret_class::SecretClassVolumeProvisionParts, k8s_openapi::{ DeepMerge, - api::{ - apps::v1::{DaemonSet, DaemonSetSpec, DaemonSetUpdateStrategy, RollingUpdateDaemonSet}, - core::v1::{ - EmptyDirVolumeSource, EnvVarSource, HTTPGetAction, ObjectFieldSelector, Probe, - ResourceRequirements, - }, + api::core::v1::{ + EmptyDirVolumeSource, EnvVarSource, HTTPGetAction, ObjectFieldSelector, + PodTemplateSpec, Probe, ResourceRequirements, }, - apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString}, + apimachinery::pkg::util::intstr::IntOrString, }, memory::{BinaryMultiple, MemoryQuantity}, product_logging::{ @@ -52,7 +52,7 @@ use crate::{ OpaRoleGroupConfig, RoleGroupName, ValidatedCluster, ValidatedOpaConfig, build::{ self, - resource::daemonset::{ + resource::workload::{ resource_info_fetcher::add_resource_info_fetcher_sidecar, user_info_fetcher::add_user_info_fetcher_sidecar, }, @@ -61,6 +61,8 @@ use crate::{ operations::graceful_shutdown::add_graceful_shutdown_config, }; +pub mod daemonset; +pub mod deployment; mod resource_info_fetcher; mod user_info_fetcher; @@ -221,16 +223,14 @@ fn http_liveness_probe(path: &str, port: IntOrString, scheme: Option) -> } } -/// The rolegroup [`DaemonSet`] runs the rolegroup, as configured by the administrator. -/// -/// The [`Pod`](`stackable_operator::k8s_openapi::api::core::v1::Pod`)s are accessible through the -/// corresponding [`Service`](`stackable_operator::k8s_openapi::api::core::v1::Service`) (from -/// [`build_server_role_service`](super::service::build_server_role_service)). +/// Builds the [`PodTemplateSpec`] for a rolegroup, shared by every deployment mode. /// -/// We run an OPA on each node, because we want to avoid requiring network roundtrips for services making -/// policy queries (which are often chained in serial, and block other tasks in the products). +/// The template carries the `prepare` init container, the OPA and bundle-builder containers, the +/// optional user-info-fetcher and Vector sidecars, and all volumes they mount. Callers wrap it in +/// the workload object of their choice; see [`daemonset::build_server_rolegroup_daemonset`] and +/// [`deployment::build_server_rolegroup_deployment`]. #[allow(clippy::too_many_arguments)] -pub fn build_server_rolegroup_daemonset( +pub(crate) fn build_server_rolegroup_pod_template( cluster: &ValidatedCluster, role_group_name: &RoleGroupName, role_group: &OpaRoleGroupConfig, @@ -238,7 +238,7 @@ pub fn build_server_rolegroup_daemonset( user_info_fetcher_image: &str, resource_info_fetcher_image: &str, cluster_info: &KubernetesClusterInfo, -) -> Result { +) -> Result { let resolved_product_image = &cluster.image; let rolegroup_config = role_group; // All overrides were already merged (role group over role over defaults) in the validate step. @@ -474,37 +474,7 @@ pub fn build_server_rolegroup_daemonset( let mut pod_template = pb.build_template(); pod_template.merge_from(rolegroup_config.pod_overrides.clone()); - let metadata = build::object_meta( - cluster, - cluster - .role_group_resource_names(role_group_name) - .daemon_set_name() - .to_string(), - role_group_name, - ) - .build(); - - let daemonset_spec = DaemonSetSpec { - selector: LabelSelector { - match_labels: Some(cluster.role_group_selector(role_group_name).into()), - ..LabelSelector::default() - }, - template: pod_template, - update_strategy: Some(DaemonSetUpdateStrategy { - type_: Some("RollingUpdate".to_string()), - rolling_update: Some(RollingUpdateDaemonSet { - max_surge: Some(IntOrString::Int(1)), - max_unavailable: Some(IntOrString::Int(0)), - }), - }), - ..DaemonSetSpec::default() - }; - - Ok(DaemonSet { - metadata, - spec: Some(daemonset_spec), - status: None, - }) + Ok(pod_template) } /// Env variables that are need to run stackable Rust binaries, such as @@ -736,408 +706,3 @@ fn build_prepare_start_command( prepare_container_args } - -#[cfg(test)] -mod tests { - use serde_json::json; - use stackable_opa_operator::crd::OpaRole; - use stackable_operator::{ - commons::networking::DomainName, k8s_openapi::api::core::v1::Container, - }; - - use super::*; - use crate::controller::build::properties::test_support::validated_cluster_from_spec; - - fn cluster_info() -> KubernetesClusterInfo { - KubernetesClusterInfo { - cluster_domain: DomainName::try_from("cluster.local").unwrap(), - } - } - - fn build(cluster: &ValidatedCluster) -> DaemonSet { - let (role_group_name, role_group) = cluster.role_group_configs[&OpaRole::Server] - .iter() - .next() - .expect("the default role group should exist"); - build_server_rolegroup_daemonset( - cluster, - role_group_name, - role_group, - "bundle-builder-image", - "user-info-fetcher-image", - "resource-info-fetcher-image", - &cluster_info(), - ) - .expect("the daemonset should build") - } - - fn container_names(ds: &DaemonSet) -> Vec { - ds.spec - .as_ref() - .unwrap() - .template - .spec - .as_ref() - .unwrap() - .containers - .iter() - .map(|c| c.name.clone()) - .collect() - } - - fn volume_names(ds: &DaemonSet) -> Vec { - ds.spec - .as_ref() - .unwrap() - .template - .spec - .as_ref() - .unwrap() - .volumes - .as_ref() - .unwrap() - .iter() - .map(|v| v.name.clone()) - .collect() - } - - #[test] - fn daemonset_has_expected_name_and_rolling_update_strategy() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "servers": { "roleGroups": { "default": {} } }, - }))); - - assert_eq!(ds.metadata.name.as_deref(), Some("test-opa-server-default")); - let strategy = ds.spec.as_ref().unwrap().update_strategy.as_ref().unwrap(); - assert_eq!(strategy.type_.as_deref(), Some("RollingUpdate")); - let rolling_update = strategy.rolling_update.as_ref().unwrap(); - // A DaemonSet must never take an OPA pod down before the replacement is ready. - assert_eq!(rolling_update.max_unavailable, Some(IntOrString::Int(0))); - } - - #[test] - fn daemonset_runs_opa_and_bundle_builder_with_prepare_init_container() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "servers": { "roleGroups": { "default": {} } }, - }))); - - let containers = container_names(&ds); - assert!(containers.contains(&"opa".to_owned())); - assert!(containers.contains(&"bundle-builder".to_owned())); - // No sidecars without the corresponding cluster config. - assert!(!containers.contains(&"user-info-fetcher".to_owned())); - assert!(!containers.contains(&"vector".to_owned())); - - let pod_spec = ds.spec.as_ref().unwrap().template.spec.as_ref().unwrap(); - let init_containers: Vec<_> = pod_spec - .init_containers - .as_ref() - .unwrap() - .iter() - .map(|c| c.name.clone()) - .collect(); - assert_eq!(init_containers, vec!["prepare".to_owned()]); - - // The standard volumes are always present; the TLS volume is not (no TLS configured). - let volumes = volume_names(&ds); - for expected in ["config", "bundles", "log"] { - assert!( - volumes.contains(&expected.to_owned()), - "missing volume {expected}" - ); - } - assert!(!volumes.contains(&"tls".to_owned())); - } - - #[test] - fn daemonset_adds_vector_container_when_agent_enabled() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "clusterConfig": { "vectorAggregatorConfigMapName": "vector-aggregator-discovery" }, - "servers": { - "config": { "logging": { "enableVectorAgent": true } }, - "roleGroups": { "default": {} }, - }, - }))); - - assert!(container_names(&ds).contains(&"vector".to_owned())); - } - - #[test] - fn daemonset_adds_user_info_fetcher_container_when_configured() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "clusterConfig": { - "userInfo": { - "backend": { - "experimentalXfscAas": { - "hostname": "aas.default.svc.cluster.local", - "port": 5000, - } - } - } - }, - "servers": { "roleGroups": { "default": {} } }, - }))); - - assert!(container_names(&ds).contains(&"user-info-fetcher".to_owned())); - } - - #[test] - fn opa_probes_root_and_bundle_builder_probes_status() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "servers": { "roleGroups": { "default": {} } }, - }))); - let pod_spec = ds.spec.as_ref().unwrap().template.spec.as_ref().unwrap(); - let liveness_path = |container: &str| -> String { - pod_spec - .containers - .iter() - .find(|c| c.name == container) - .unwrap_or_else(|| panic!("container {container} should exist")) - .liveness_probe - .as_ref() - .unwrap() - .http_get - .as_ref() - .unwrap() - .path - .clone() - .unwrap() - }; - // OPA's HTTP server answers `/`; only the bundle-builder exposes `/status`. A wrong path - // here makes the liveness probe fail and the OPA container CrashLoop. - assert_eq!(liveness_path("opa"), "/"); - assert_eq!(liveness_path("bundle-builder"), "/status"); - } - - #[test] - fn daemonset_adds_tls_volume_when_tls_enabled() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "clusterConfig": { "tls": { "serverSecretClass": "tls" } }, - "servers": { "roleGroups": { "default": {} } }, - }))); - - assert!(volume_names(&ds).contains(&"tls".to_owned())); - } - - #[test] - fn opa_container_serves_https_when_tls_enabled() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "clusterConfig": { "tls": { "serverSecretClass": "tls" } }, - "servers": { "roleGroups": { "default": {} } }, - }))); - let pod_spec = ds.spec.as_ref().unwrap().template.spec.as_ref().unwrap(); - let opa = pod_spec - .containers - .iter() - .find(|c| c.name == "opa") - .expect("opa container should exist"); - - // The single container port is the HTTPS data port. - let ports = opa.ports.as_ref().unwrap(); - assert_eq!(ports.len(), 1); - assert_eq!(ports[0].name.as_deref(), Some("https")); - assert_eq!(ports[0].container_port, 8443); - - // The probe must speak HTTPS, otherwise it would fail against the TLS-only server. - let scheme = opa - .liveness_probe - .as_ref() - .unwrap() - .http_get - .as_ref() - .unwrap() - .scheme - .clone(); - assert_eq!(scheme.as_deref(), Some("HTTPS")); - - // The start command binds the HTTPS port and passes the TLS cert/key flags. - let args = opa.args.as_ref().unwrap(); - assert!(args[0].contains("-a 0.0.0.0:8443")); - assert!(args[0].contains("--tls-cert-file")); - } - - #[test] - fn bundle_builder_start_command_silences_console_only_when_none() { - let role_group_config = |spec: serde_json::Value| { - let cluster = validated_cluster_from_spec(spec); - cluster.role_group_configs[&OpaRole::Server] - .values() - .next() - .expect("the default role group should exist") - .config - .clone() - }; - - // Console level NONE redirects bundle-builder output to /dev/null (no `tee`). - let silenced = role_group_config(json!({ - "image": { "productVersion": "1.2.3" }, - "servers": { - "config": { "logging": { "containers": { - "bundle-builder": { "console": { "level": "NONE" } } - } } }, - "roleGroups": { "default": {} }, - }, - })); - // The redirect is appended directly after the bundle-builder invocation. (`/dev/null` also - // appears in the shared bash trap helpers, so match the specific redirect.) - assert!( - build_bundle_builder_start_command(&silenced, "bundle-builder") - .contains("stackable-opa-bundle-builder > /dev/null") - ); - - // With a console level above NONE, output is not discarded. - let logging = role_group_config(json!({ - "image": { "productVersion": "1.2.3" }, - "servers": { - "config": { "logging": { "containers": { - "bundle-builder": { "console": { "level": "INFO" } } - } } }, - "roleGroups": { "default": {} }, - }, - })); - assert!( - build_bundle_builder_start_command(&logging, "bundle-builder") - .contains("stackable-opa-bundle-builder &") - ); - } - - fn uif_container(ds: &DaemonSet) -> Container { - ds.spec - .as_ref() - .unwrap() - .template - .spec - .as_ref() - .unwrap() - .containers - .iter() - .find(|c| c.name == "user-info-fetcher") - .expect("the user-info-fetcher container should exist") - .clone() - } - - fn env_var(container: &Container, name: &str) -> String { - container - .env - .as_ref() - .expect("the container should have env vars") - .iter() - .find(|e| e.name == name) - .unwrap_or_else(|| panic!("env var {name} should be set")) - .value - .clone() - .unwrap_or_else(|| panic!("env var {name} should have a literal value")) - } - - fn mount_path(container: &Container, volume_name: &str) -> String { - container - .volume_mounts - .as_ref() - .expect("the container should have volume mounts") - .iter() - .find(|m| m.name == volume_name) - .unwrap_or_else(|| panic!("volume mount {volume_name} should exist")) - .mount_path - .clone() - } - - #[test] - fn user_info_fetcher_container_has_expected_command_and_config_wiring() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "clusterConfig": { - "userInfo": { - "backend": { - "experimentalXfscAas": { - "hostname": "aas.default.svc.cluster.local", - "port": 5000, - } - } - } - }, - "servers": { "roleGroups": { "default": {} } }, - }))); - - let uif = uif_container(&ds); - assert_eq!( - uif.command, - Some(vec!["stackable-opa-user-info-fetcher".to_owned()]) - ); - // The sidecar reads its config from the shared config volume, and looks for backend - // credentials in a fixed directory (populated by the backend-specific arms below). - assert_eq!( - env_var(&uif, "CONFIG"), - "/stackable/config/user-info-fetcher.json" - ); - assert_eq!(env_var(&uif, "CREDENTIALS_DIR"), "/stackable/credentials"); - assert_eq!(mount_path(&uif, "config"), "/stackable/config"); - } - - #[test] - fn user_info_fetcher_active_directory_backend_mounts_kerberos_and_sets_krb5_env() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "clusterConfig": { - "userInfo": { - "backend": { - "experimentalActiveDirectory": { - "ldapServer": "ad.example.com", - "baseDistinguishedName": "dc=example,dc=com", - "kerberosSecretClassName": "kerberos", - } - } - } - }, - "servers": { "roleGroups": { "default": {} } }, - }))); - - // A Kerberos secret volume is provisioned and mounted for the sidecar. - assert!(volume_names(&ds).contains(&"kerberos".to_owned())); - let uif = uif_container(&ds); - assert_eq!(mount_path(&uif, "kerberos"), "/stackable/kerberos"); - // The krb5 client must find the config and keytab, and keep tickets in memory only. - assert_eq!( - env_var(&uif, "KRB5_CONFIG"), - "/stackable/kerberos/krb5.conf" - ); - assert_eq!( - env_var(&uif, "KRB5_CLIENT_KTNAME"), - "/stackable/kerberos/keytab" - ); - assert_eq!(env_var(&uif, "KRB5CCNAME"), "MEMORY:"); - } - - #[test] - fn user_info_fetcher_keycloak_backend_mounts_client_credentials_secret() { - let ds = build(&validated_cluster_from_spec(json!({ - "image": { "productVersion": "1.2.3" }, - "clusterConfig": { - "userInfo": { - "backend": { - "keycloak": { - "hostname": "keycloak.example.com", - "clientCredentialsSecret": "keycloak-credentials", - "adminRealm": "master", - "userRealm": "my-realm", - } - } - } - }, - "servers": { "roleGroups": { "default": {} } }, - }))); - - // The client credentials secret is projected into the sidecar's credentials dir. - assert!(volume_names(&ds).contains(&"user-info-fetcher-credentials".to_owned())); - assert_eq!( - mount_path(&uif_container(&ds), "user-info-fetcher-credentials"), - "/stackable/credentials" - ); - } -} diff --git a/rust/operator-binary/src/controller/build/resource/daemonset/resource_info_fetcher.rs b/rust/operator-binary/src/controller/build/resource/workload/resource_info_fetcher.rs similarity index 99% rename from rust/operator-binary/src/controller/build/resource/daemonset/resource_info_fetcher.rs rename to rust/operator-binary/src/controller/build/resource/workload/resource_info_fetcher.rs index 21f64222..508500c1 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset/resource_info_fetcher.rs +++ b/rust/operator-binary/src/controller/build/resource/workload/resource_info_fetcher.rs @@ -15,7 +15,7 @@ use crate::controller::{ ValidatedCluster, ValidatedOpaConfig, build::{ self, - resource::daemonset::{ + resource::workload::{ CONFIG_DIR, CONFIG_VOLUME_NAME, RESOURCE_INFO_FETCHER_CREDENTIALS_DIR, RESOURCE_INFO_FETCHER_CREDENTIALS_VOLUME_NAME, add_stackable_rust_cli_env_vars, container_name, sidecar_container_log_level, sidecar_resource_requirements, diff --git a/rust/operator-binary/src/controller/build/resource/daemonset/user_info_fetcher.rs b/rust/operator-binary/src/controller/build/resource/workload/user_info_fetcher.rs similarity index 99% rename from rust/operator-binary/src/controller/build/resource/daemonset/user_info_fetcher.rs rename to rust/operator-binary/src/controller/build/resource/workload/user_info_fetcher.rs index 31b4f542..45efe36b 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset/user_info_fetcher.rs +++ b/rust/operator-binary/src/controller/build/resource/workload/user_info_fetcher.rs @@ -21,7 +21,7 @@ use crate::controller::{ ValidatedCluster, ValidatedOpaConfig, build::{ self, - resource::daemonset::{ + resource::workload::{ CONFIG_DIR, CONFIG_VOLUME_NAME, USER_INFO_FETCHER_CREDENTIALS_DIR, USER_INFO_FETCHER_CREDENTIALS_VOLUME_NAME, USER_INFO_FETCHER_KERBEROS_DIR, USER_INFO_FETCHER_KERBEROS_VOLUME_NAME, add_stackable_rust_cli_env_vars, diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 1aa2769e..37e283ba 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -16,8 +16,9 @@ use stackable_operator::{ resources::{NoRuntimeLimits, Resources}, }, k8s_openapi::api::{ - apps::v1::DaemonSet, + apps::v1::{DaemonSet, Deployment}, core::v1::{ConfigMap, Service, ServiceAccount}, + policy::v1::PodDisruptionBudget, rbac::v1::RoleBinding, }, kube::{Resource as KubeResource, api::ObjectMeta}, @@ -59,6 +60,11 @@ pub struct ValidatedCluster { pub product_version: ProductVersion, pub image: ResolvedProductImage, pub cluster_config: ValidatedClusterConfig, + /// The role-level configuration of every role, keyed the same way as `role_group_configs`. + /// + /// Role-level rather than role-group-level, because `workloadKind` decides the shape of the + /// role Service, which selects across all of a role's role groups. + pub role_configs: BTreeMap, pub role_group_configs: BTreeMap>, } @@ -69,6 +75,7 @@ impl ValidatedCluster { uid: Uid, image: ResolvedProductImage, cluster_config: ValidatedClusterConfig, + role_configs: BTreeMap, role_group_configs: BTreeMap>, ) -> Self { let product_version = ProductVersion::from_str(&image.app_version_label_value) @@ -88,10 +95,21 @@ impl ValidatedCluster { product_version, image, cluster_config, + role_configs, role_group_configs, } } + /// The role-level configuration of `role`. + /// + /// The validate step inserts an entry for every [`OpaRole`], falling back to the + /// `OpaRoleConfig` default for roles the user did not configure. + pub fn role_config(&self, role: &OpaRole) -> &v1alpha2::OpaRoleConfig { + self.role_configs + .get(role) + .expect("the validate step inserts a role config for every role") + } + /// Whether the cluster serves HTTPS, derived from the validated cluster config. pub fn is_tls_enabled(&self) -> bool { self.cluster_config.tls.is_some() @@ -235,16 +253,22 @@ impl KubeResource for ValidatedCluster { /// Every Kubernetes resource produced by the [`build`](build::build) step. /// -/// OPA runs as a `DaemonSet` (one Pod per node), so there are no `StatefulSet`s, PDBs or -/// `Listener`s. `services` holds the role-level `Service` and the per-role-group headless and -/// metrics `Service`s; `config_maps` holds the per-role-group `ConfigMap`s and the cluster-level -/// discovery `ConfigMap`. +/// Each role group might run as either a `DaemonSet` or a `Deployment`, depending on its role's +/// `workloadKind`, so exactly one of `daemon_sets` and `deployments` holds an entry for it. There +/// are no `StatefulSet`s or `Listener`s. `services` holds the role-level `Service` and the +/// per-role-group headless and metrics `Service`s; `config_maps` holds the per-role-group +/// `ConfigMap`s and the cluster-level discovery `ConfigMap`. +/// +/// `pod_disruption_budgets` holds at most one entry per role, and is empty for roles that have it +/// disabled (the default for a `DaemonSet`). pub struct KubernetesResources { pub daemon_sets: Vec, + pub deployments: Vec, pub services: Vec, pub config_maps: Vec, pub service_accounts: Vec, pub role_bindings: Vec, + pub pod_disruption_budgets: Vec, } /// Cluster-wide settings resolved once during validation, so the build steps no longer need the diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 954abb19..4844c981 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -165,21 +165,29 @@ pub fn validate( .vector_aggregator_config_map_name .clone(); + let mut role_configs = BTreeMap::new(); let mut role_group_configs = BTreeMap::new(); for opa_role in OpaRole::iter() { let role = opa.role(&opa_role); + // Carried per role rather than per cluster, so a second role could pick its own + // `workloadKind`. `serde(default)` on `Role::role_config` means this is the + // `OpaRoleConfig` default. + role_configs.insert(opa_role.clone(), role.role_config.clone()); + let mut group_configs = BTreeMap::new(); for (role_group_name, role_group) in &role.role_groups { // Merge default <- role <- role group and validate the config fragment, plus merge all // four override kinds (config/env/cli/pod) in one shot. Role group wins over role wins // over defaults. - let merged: RoleGroup = - with_validated_config(role_group, role, &OpaConfig::default_config()).context( - ValidateRoleGroupConfigSnafu { - role_group: role_group_name.clone(), - }, - )?; + let merged: RoleGroup = with_validated_config( + role_group, + role, + &OpaConfig::default_config(&name.to_string(), &opa_role), + ) + .context(ValidateRoleGroupConfigSnafu { + role_group: role_group_name.clone(), + })?; // `envOverrides` is kept as a `HashMap`; lift it into the type-safe // `EnvVarSet` consumed by the build step. @@ -209,7 +217,8 @@ pub fn validate( group_configs.insert( role_group_name, OpaRoleGroupConfig { - // Unused for a DaemonSet, but the `RoleGroupConfig` type requires it. + // Only used in `Deployment` mode; a DaemonSet derives its Pod count from the + // number of nodes. replicas: merged.replicas, config: ValidatedOpaConfig::from_merged(merged.config.config, logging), config_overrides: merged.config.config_overrides, @@ -235,6 +244,7 @@ pub fn validate( tls: opa.spec.cluster_config.tls.clone(), listener_class: opa.spec.cluster_config.listener_class.clone(), }, + role_configs, role_group_configs, )) } @@ -305,6 +315,12 @@ mod tests { v1alpha2::CurrentlySupportedListenerClasses::ClusterInternal ); + // The fixture sets no `roleConfig`, so the role falls back to the `OpaRoleConfig` default. + assert_eq!( + cluster.role_config(&OpaRole::Server), + &v1alpha2::OpaRoleConfig::default() + ); + // A single `server` role with the single `default` role group; the Vector agent is off. assert_eq!(cluster.role_group_configs.len(), 1); let role_groups = &cluster.role_group_configs[&OpaRole::Server]; @@ -317,6 +333,46 @@ mod tests { assert_eq!(role_group.config.logging.vector_container, None); } + /// A configured `roleConfig` reaches the build step, and every role gets an entry so + /// `ValidatedCluster::role_config` cannot panic. + #[test] + fn validate_carries_the_role_config_of_every_role() { + let opa: v1alpha2::OpaCluster = serde_json::from_value(json!({ + "apiVersion": "opa.stackable.tech/v1alpha2", + "kind": "OpaCluster", + "metadata": { + "name": "test-opa", + "namespace": "default", + "uid": "c27b3971-ca72-42c1-80a4-abdfc1db0ddd", + }, + "spec": { + "image": { "productVersion": "1.2.3" }, + "servers": { + "roleConfig": { "workloadKind": "Deployment" }, + "roleGroups": { "default": {} }, + }, + }, + })) + .expect("valid test input"); + let operator_environment = OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_string(), + operator_service_name: "opa-operator".to_string(), + image_repository: "oci.example.org".to_string(), + }; + + let cluster = validate(&opa, &operator_environment).expect("the fixture validates"); + + assert_eq!( + cluster.role_config(&OpaRole::Server).workload_kind, + v1alpha2::WorkloadKind::Deployment + ); + // Every role is present, whether or not the user configured it. + assert_eq!(cluster.role_configs.len(), OpaRole::iter().count()); + for opa_role in OpaRole::iter() { + cluster.role_config(&opa_role); + } + } + /// A [`Logging`] with an automatic log config for every container, as the (defaulted) merged /// config provides at runtime. `validate_logging` validates all containers, so all must be /// present. diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs new file mode 100644 index 00000000..ac934ba2 --- /dev/null +++ b/rust/operator-binary/src/crd/affinity.rs @@ -0,0 +1,109 @@ +//! The default [`StackableAffinityFragment`] of an OPA role. + +use stackable_operator::{ + commons::affinity::{StackableAffinityFragment, affinity_between_role_pods}, + k8s_openapi::api::core::v1::PodAntiAffinity, +}; + +use crate::crd::{APP_NAME, OpaRole}; + +/// Weight of the anti-affinity that spreads the Pods of a role across nodes. +/// +/// The absolute value only matters once a second, competing term exists; see the `PreferSameNode` +/// note on [`get_affinity`]. +const ANTI_AFFINITY_BETWEEN_ROLE_PODS_WEIGHT: i32 = 70; + +/// The default affinity of `role`: prefer to spread its Pods across nodes. +/// +/// Soft (`preferred`), so it can never leave a Pod unschedulable, and inert for a `DaemonSet`, which +/// already places exactly one Pod per node. It matters in `Deployment` mode only. +// +// TODO: Revisit once our minimum supported Kubernetes version is 1.35 and the role Service can use +// `trafficDistribution: PreferSameNode` instead of `internalTrafficPolicy` (see +// `controller::build::resource::service`). +// +// The chance: with node-local routing that degrades gracefully, an affinity attracting OPA Pods +// towards the products that query them would genuinely pay off, because traffic would prefer a +// node-local OPA Pod without the current risk of failing outright when there is none. +// +// The concerns: +// +// * `PreferSameNode` falls back to other nodes only when there is no *ready* local endpoint, never +// because the local one is busy. A request-heavy client (Trino, Kafka, depending on their +// config) would keep hitting its local Pod while the others idle. +// +// * Field experience points the other way: spreading the load across Pods outperformed avoiding the +// network hop by a wide margin. +// +// * The scheduler scores `podAffinity` and `podAntiAffinity` on one scale, so the two weights would +// compete. Keeping this one at 70 above a lower attraction weight encodes "spreading wins", where +// equal weights would cancel out. +pub fn get_affinity(cluster_name: &str, role: &OpaRole) -> StackableAffinityFragment { + StackableAffinityFragment { + pod_affinity: None, + pod_anti_affinity: Some(PodAntiAffinity { + preferred_during_scheduling_ignored_during_execution: Some(vec![ + affinity_between_role_pods( + APP_NAME, + cluster_name, + &role.to_string(), + ANTI_AFFINITY_BETWEEN_ROLE_PODS_WEIGHT, + ), + ]), + required_during_scheduling_ignored_during_execution: None, + }), + node_affinity: None, + node_selector: None, + } +} + +#[cfg(test)] +mod tests { + use stackable_operator::k8s_openapi::{ + api::core::v1::{PodAffinityTerm, WeightedPodAffinityTerm}, + apimachinery::pkg::apis::meta::v1::LabelSelector, + }; + + use super::*; + + /// Locks the shape of the default: a soft, per-node anti-affinity selecting the whole role + /// (so across every role group), which is what makes replicas spread instead of piling up. + #[test] + fn default_affinity_spreads_the_role_across_nodes() { + let affinity = get_affinity("simple-opa", &OpaRole::Server); + + assert_eq!(affinity.pod_affinity, None); + assert_eq!(affinity.node_affinity, None); + assert_eq!(affinity.node_selector, None); + + let anti_affinity = affinity.pod_anti_affinity.expect("is always set"); + // Soft only: a `required` term would leave Pods Pending once the replica count exceeds the + // number of schedulable nodes. + assert_eq!( + anti_affinity.required_during_scheduling_ignored_during_execution, + None + ); + assert_eq!( + anti_affinity.preferred_during_scheduling_ignored_during_execution, + Some(vec![WeightedPodAffinityTerm { + weight: ANTI_AFFINITY_BETWEEN_ROLE_PODS_WEIGHT, + pod_affinity_term: PodAffinityTerm { + label_selector: Some(LabelSelector { + match_expressions: None, + match_labels: Some( + [ + ("app.kubernetes.io/name", "opa"), + ("app.kubernetes.io/instance", "simple-opa"), + ("app.kubernetes.io/component", "server"), + ] + .map(|(key, value)| (key.to_string(), value.to_string())) + .into() + ), + }), + topology_key: "kubernetes.io/hostname".to_string(), + ..PodAffinityTerm::default() + }, + }]) + ); + } +} diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index ed3f32f2..d2bd1d4b 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -16,7 +16,7 @@ use stackable_operator::{ k8s_openapi::apimachinery::pkg::api::resource::Quantity, kube::CustomResource, product_logging::{self, spec::Logging}, - role_utils::{EmptyRoleConfig, Role}, + role_utils::Role, schemars::{self, JsonSchema}, shared::time::Duration, status::condition::{ClusterCondition, HasStatusCondition}, @@ -32,6 +32,7 @@ use stackable_operator::{ }; use strum::{Display, EnumIter, EnumString}; +pub mod affinity; pub mod cache; pub mod resource_info_fetcher; pub mod user_info_fetcher; @@ -45,7 +46,7 @@ pub const DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_mi pub const SERVER_GRACEFUL_SHUTDOWN_SAFETY_OVERHEAD: Duration = Duration::from_secs(5); pub type OpaRoleType = - Role; + Role; #[versioned( version(name = "v1alpha1"), @@ -142,6 +143,76 @@ pub mod versioned { pub tls: Option, } + /// Role-level configuration for the OPA servers. + #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] + #[serde(rename_all = "camelCase")] + pub struct OpaRoleConfig { + /// The Kubernetes workload the OPA servers run as. + /// + /// * `DaemonSet`: one Pod per node. `replicas` is ignored. + /// + /// * `Deployment`: fixed number of Pods, configured by `replicas`. + #[serde(default)] + pub workload_kind: WorkloadKind, + + // `internalTrafficPolicy` is deliberately not a field here: the operator derives it from + // `workloadKind` in `OpaRoleConfig::internal_traffic_policy`. Exposing it as a user + // override means adding an `Option` field back and falling back to + // that helper's `match`. + // + // We can not #[serde(flatten)] a `GenericRoleConfig` here, as we need a PodDisruptionBudget + // default that depends on `workloadKind`. + #[serde(default)] + pub pod_disruption_budget: OpaPdbConfig, + } + + /// The Kubernetes Kind currently supported. + #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] + #[serde(rename_all = "PascalCase")] + pub enum WorkloadKind { + #[default] + DaemonSet, + Deployment, + } + + /// The `internalTrafficPolicy` of a Kubernetes Service. + /// + /// The variants are spelled as Kubernetes spells them, so the value can be passed through to + /// `Service.spec.internalTrafficPolicy` unchanged. + /// + /// TODO: Not yet part of the CRD: the operator derives the policy from [`WorkloadKind`]. + #[derive(Clone, Debug, Deserialize, Display, Eq, JsonSchema, PartialEq, Serialize)] + #[serde(rename_all = "PascalCase")] + pub enum InternalTrafficPolicy { + Local, + Cluster, + } + + // A copy of `PdbConfig` from stackable-operator, but with `enabled` as an `Option`. The + // default depends on `workloadKind` and can therefore not be hard-coded. + // + /// This struct is used to configure: + /// + /// 1. If PodDisruptionBudgets are created by the operator + /// 2. The allowed number of Pods to be unavailable (`maxUnavailable`) + /// + /// Documentation: + /// [allowed Pod disruptions documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/operations/pod_disruptions). + #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] + #[serde(rename_all = "camelCase")] + pub struct OpaPdbConfig { + /// Whether a PodDisruptionBudget should be written out for this role. + /// + /// Defaults to `true` when `workloadKind` is `Deployment` and to `false` when it is + /// `DaemonSet`, since a PodDisruptionBudget doesn't make sense for a DaemonSet. + #[serde(default)] + pub enabled: Option, + + /// The number of Pods that are allowed to be down simultaneous. + #[serde(default)] + pub max_unavailable: Option, + } + #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct OpaTls { @@ -302,8 +373,36 @@ impl v1alpha2::CurrentlySupportedListenerClasses { } } +impl v1alpha2::OpaRoleConfig { + /// The `internalTrafficPolicy` to write into the role Service. + /// + /// Derived from the [`v1alpha2::WorkloadKind`]: `Local` for a DaemonSet, which covers every + /// node, and `Cluster` for a Deployment, whose Pods do not. + /// + /// This is the single place the policy is decided, so exposing a user override later means + /// adding the CRD field back and wrapping this `match` in an `unwrap_or`; no call site changes. + pub fn internal_traffic_policy(&self) -> v1alpha2::InternalTrafficPolicy { + match self.workload_kind { + v1alpha2::WorkloadKind::DaemonSet => v1alpha2::InternalTrafficPolicy::Local, + v1alpha2::WorkloadKind::Deployment => v1alpha2::InternalTrafficPolicy::Cluster, + } + } + + /// Whether a PodDisruptionBudget should be written out for this role. + /// + /// Falls back to `true` for a Deployment only: `kubectl drain` requires `--ignore-daemonsets` + /// and then leaves those Pods alone, so a PDB would protect nothing in DaemonSet mode. + pub fn pod_disruption_budget_enabled(&self) -> bool { + self.pod_disruption_budget + .enabled + .unwrap_or(self.workload_kind == v1alpha2::WorkloadKind::Deployment) + } +} + impl OpaConfig { - pub fn default_config() -> OpaConfigFragment { + /// `cluster_name` and `role` are needed for the default affinity, whose selector is specific to + /// this cluster's role rather than a static value. + pub fn default_config(cluster_name: &str, role: &OpaRole) -> OpaConfigFragment { OpaConfigFragment { logging: product_logging::spec::default_logging(), resources: ResourcesFragment { @@ -317,9 +416,10 @@ impl OpaConfig { }, storage: OpaStorageConfigFragment {}, }, - // There is no point in having a default affinity, as exactly one OPA Pods should run on every node. - // We only have the affinity configurable to let users limit the nodes the OPA Pods run on. - affinity: Default::default(), + // Spreads the role's Pods across nodes. A no-op for a DaemonSet, which already runs + // exactly one Pod per node, but it is what keeps a Deployment's replicas from landing + // together. See `affinity::get_affinity`. + affinity: affinity::get_affinity(cluster_name, role), graceful_shutdown_timeout: Some(DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT), } } @@ -346,10 +446,111 @@ impl HasStatusCondition for v1alpha2::OpaCluster { #[cfg(test)] mod tests { use indoc::formatdoc; + use serde_json::json; use stackable_operator::versioned::test_utils::RoundtripTestData; use super::{v1alpha1, v1alpha2}; + /// The values the operator derives from `workloadKind`, which an OpenAPI schema default cannot + /// express. `internalTrafficPolicy` is derived outright; `podDisruptionBudget.enabled` is a + /// default the user can override. + #[test] + fn role_config_defaults_follow_workload_kind() { + let role_config = |workload_kind| v1alpha2::OpaRoleConfig { + workload_kind, + ..v1alpha2::OpaRoleConfig::default() + }; + + let daemon_set = role_config(v1alpha2::WorkloadKind::DaemonSet); + assert_eq!( + daemon_set.internal_traffic_policy(), + v1alpha2::InternalTrafficPolicy::Local + ); + // `kubectl drain` skips DaemonSet Pods, so a PDB would protect nothing. + assert!(!daemon_set.pod_disruption_budget_enabled()); + + let deployment = role_config(v1alpha2::WorkloadKind::Deployment); + assert_eq!( + deployment.internal_traffic_policy(), + v1alpha2::InternalTrafficPolicy::Cluster + ); + assert!(deployment.pod_disruption_budget_enabled()); + } + + /// An explicitly configured `podDisruptionBudget.enabled` wins over the `workloadKind`-derived + /// default, which is the point of exposing the field as an `Option` at all. + #[test] + fn explicit_role_config_overrides_the_derived_defaults() { + let role_config = v1alpha2::OpaRoleConfig { + workload_kind: v1alpha2::WorkloadKind::DaemonSet, + pod_disruption_budget: v1alpha2::OpaPdbConfig { + enabled: Some(true), + max_unavailable: None, + }, + }; + + assert!(role_config.pod_disruption_budget_enabled()); + // `internalTrafficPolicy` is not yet user-configurable, so it stays at the DaemonSet default. + assert_eq!( + role_config.internal_traffic_policy(), + v1alpha2::InternalTrafficPolicy::Local + ); + } + + /// Leaving the PDB fields out and writing them as an explicit `null` must resolve to the same + /// unset state, as the derived default is applied by the operator rather than by the schema. + /// + /// Only covers what serde does; substituting the `roleConfig` default for an entirely absent + /// `roleConfig` is the apiserver's job and is not exercised here. + #[test] + fn unset_role_config_fields_deserialise_to_none() { + let unset = v1alpha2::OpaRoleConfig::default(); + + for value in [ + json!({}), + json!({ "workloadKind": "DaemonSet" }), + json!({ + "workloadKind": "DaemonSet", + "podDisruptionBudget": { "enabled": null, "maxUnavailable": null }, + }), + ] { + let role_config: v1alpha2::OpaRoleConfig = + serde_json::from_value(value.clone()).expect("a valid role config"); + assert_eq!(role_config, unset, "unexpected role config for {value}"); + } + } + + /// The two enums must serialise the way Kubernetes spells them: `workloadKind` names the + /// workload API kinds, and `internalTrafficPolicy` is passed through to `Service.spec`. + #[test] + fn enums_use_the_kubernetes_spelling() { + assert_eq!( + serde_json::to_value(v1alpha2::WorkloadKind::DaemonSet).unwrap(), + json!("DaemonSet") + ); + assert_eq!( + serde_json::to_value(v1alpha2::WorkloadKind::Deployment).unwrap(), + json!("Deployment") + ); + assert_eq!( + serde_json::to_value(v1alpha2::InternalTrafficPolicy::Local).unwrap(), + json!("Local") + ); + assert_eq!( + serde_json::to_value(v1alpha2::InternalTrafficPolicy::Cluster).unwrap(), + json!("Cluster") + ); + + // The Service builder writes the policy via `Display`, which is derived by strum and does + // not honour `#[serde(rename_all)]`. Asserted separately, so renaming a variant cannot + // leave serde green while the Service gets a value Kubernetes rejects. + assert_eq!(v1alpha2::InternalTrafficPolicy::Local.to_string(), "Local"); + assert_eq!( + v1alpha2::InternalTrafficPolicy::Cluster.to_string(), + "Cluster" + ); + } + impl RoundtripTestData for v1alpha1::OpaClusterSpec { fn roundtrip_test_data() -> Vec { let user_info_fetcher_sections = vec![ diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index f15de158..e1469214 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -13,7 +13,7 @@ use stackable_operator::{ client, eos::EndOfSupportChecker, k8s_openapi::api::{ - apps::v1::DaemonSet, + apps::v1::{DaemonSet, Deployment}, core::v1::{ConfigMap, Service}, }, kube::{ @@ -147,6 +147,13 @@ async fn main() -> anyhow::Result<()> { watch_namespace.get_api::>(&client), watcher::Config::default(), ) + // Watched alongside DaemonSets, because a role group runs as either kind. Without + // this the cluster's `Available` condition would not follow a Deployment's Pods + // becoming ready or unready. + .owns( + watch_namespace.get_api::>(&client), + watcher::Config::default(), + ) .owns( watch_namespace.get_api::>(&client), watcher::Config::default(), diff --git a/rust/operator-binary/src/opa_controller.rs b/rust/operator-binary/src/opa_controller.rs index 374ad100..01c05556 100644 --- a/rust/operator-binary/src/opa_controller.rs +++ b/rust/operator-binary/src/opa_controller.rs @@ -16,7 +16,7 @@ use stackable_operator::{ shared::time::Duration, status::condition::{ compute_conditions, daemonset::DaemonSetConditionBuilder, - operations::ClusterOperationsConditionBuilder, + deployment::DeploymentConditionBuilder, operations::ClusterOperationsConditionBuilder, }, utils::cluster_info::KubernetesClusterInfo, v2::cluster_resources::cluster_resources_new, @@ -123,9 +123,10 @@ pub async fn reconcile_opa( .context(BuildResourcesSnafu)?; let mut ds_cond_builder = DaemonSetConditionBuilder::default(); + let mut deployment_cond_builder = DeploymentConditionBuilder::default(); - // Apply order: DaemonSets last, so a changed mounted ConfigMap already exists before the Pods - // (that would otherwise restart) are updated (commons-operator#111). + // Apply order: the workload objects last, so a changed mounted ConfigMap already exists before + // the Pods (that would otherwise restart) are updated (commons-operator#111). for service_account in resources.service_accounts { cluster_resources .add(client, service_account) @@ -150,6 +151,12 @@ pub async fn reconcile_opa( .await .context(ApplyResourceSnafu)?; } + for pod_disruption_budget in resources.pod_disruption_budgets { + cluster_resources + .add(client, pod_disruption_budget) + .await + .context(ApplyResourceSnafu)?; + } for daemon_set in resources.daemon_sets { ds_cond_builder.add( cluster_resources @@ -182,11 +189,27 @@ pub async fn reconcile_opa( })?; } + for deployment in resources.deployments { + deployment_cond_builder.add( + cluster_resources + .add(client, deployment) + .await + .context(ApplyResourceSnafu)?, + ); + } + let cluster_operation_cond_builder = ClusterOperationsConditionBuilder::new(&opa.spec.cluster_operation); let status = OpaClusterStatus { - conditions: compute_conditions(opa, &[&ds_cond_builder, &cluster_operation_cond_builder]), + conditions: compute_conditions( + opa, + &[ + &ds_cond_builder, + &deployment_cond_builder, + &cluster_operation_cond_builder, + ], + ), }; client diff --git a/rust/resource-info-fetcher/src/api.rs b/rust/resource-info-fetcher/src/api.rs index b270c2ed..159fd68b 100644 --- a/rust/resource-info-fetcher/src/api.rs +++ b/rust/resource-info-fetcher/src/api.rs @@ -91,7 +91,7 @@ pub struct RawIdentifier { } /// Generates the trivial `From for ResourceInfoRequest` conversions, so each HTTP handler -/// can turn its deserialized query parameters into a [`ResourceInfoRequest`] via `.into()`. Adding a +/// can turn its deserialized query parameters into a [`ResourceInfoRequest`] via `.from()`. Adding a /// resource type means adding its struct above and one entry here — no hand-written conversion. macro_rules! impl_into_resource_info_request { ($($variant:ident),+ $(,)?) => { diff --git a/tests/templates/kuttl/smoke/12-assert.yaml.j2 b/tests/templates/kuttl/smoke/12-assert.yaml.j2 index 12971976..452084f5 100644 --- a/tests/templates/kuttl/smoke/12-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/12-assert.yaml.j2 @@ -8,8 +8,10 @@ # only here — `.data` is asserted in 13-assert), ServiceAccount, RoleBinding. # # Catches drift in labels, owner references, selectors, ports, probe schemes, -# update strategy, container resources and TLS-dependent fields. The operator -# does not create a PodDisruptionBudget for OPA, so none is asserted here. +# update strategy, container resources and TLS-dependent fields. This cluster +# uses the default `workloadKind: DaemonSet`, for which the operator writes no +# PodDisruptionBudget, so none is asserted here; the `workload-kind` test covers +# the `Deployment` case. # # `app.kubernetes.io/version` is intentionally omitted from label matchers so # that product-version bumps in test-definition.yaml don't force snapshot diff --git a/tests/templates/kuttl/workload-kind/00-assert.yaml.j2 b/tests/templates/kuttl/workload-kind/00-assert.yaml.j2 new file mode 100644 index 00000000..50b1d4c3 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/00-assert.yaml.j2 @@ -0,0 +1,10 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +{% if lookup('env', 'VECTOR_AGGREGATOR') %} +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: vector-aggregator-discovery +{% endif %} diff --git a/tests/templates/kuttl/workload-kind/00-install-vector-aggregator-discovery-configmap.yaml.j2 b/tests/templates/kuttl/workload-kind/00-install-vector-aggregator-discovery-configmap.yaml.j2 new file mode 100644 index 00000000..2d6a0df5 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/00-install-vector-aggregator-discovery-configmap.yaml.j2 @@ -0,0 +1,9 @@ +{% if lookup('env', 'VECTOR_AGGREGATOR') %} +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: vector-aggregator-discovery +data: + ADDRESS: {{ lookup('env', 'VECTOR_AGGREGATOR') }} +{% endif %} diff --git a/tests/templates/kuttl/workload-kind/00-patch-ns.yaml.j2 b/tests/templates/kuttl/workload-kind/00-patch-ns.yaml.j2 new file mode 100644 index 00000000..67185acf --- /dev/null +++ b/tests/templates/kuttl/workload-kind/00-patch-ns.yaml.j2 @@ -0,0 +1,9 @@ +{% if test_scenario['values']['openshift'] == 'true' %} +# see https://github.com/stackabletech/issues/issues/566 +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: kubectl patch namespace $NAMESPACE -p '{"metadata":{"labels":{"pod-security.kubernetes.io/enforce":"privileged"}}}' + timeout: 120 +{% endif %} diff --git a/tests/templates/kuttl/workload-kind/10-assert.yaml b/tests/templates/kuttl/workload-kind/10-assert.yaml new file mode 100644 index 00000000..746ba3a2 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/10-assert.yaml @@ -0,0 +1,21 @@ +--- +# The `DaemonSet` default: unchanged behaviour for existing installations. +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 600 +commands: + - script: kubectl -n $NAMESPACE rollout status daemonset test-opa-server-default --timeout 600s + - script: kubectl -n $NAMESPACE wait --for=condition=available opaclusters.opa.stackable.tech/test-opa --timeout 600s +--- +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: test-opa-server-default +--- +# Node-local routing, which is only safe because a DaemonSet covers every node. +apiVersion: v1 +kind: Service +metadata: + name: test-opa-server +spec: + internalTrafficPolicy: Local diff --git a/tests/templates/kuttl/workload-kind/10-errors.yaml b/tests/templates/kuttl/workload-kind/10-errors.yaml new file mode 100644 index 00000000..2185d571 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/10-errors.yaml @@ -0,0 +1,13 @@ +--- +# A DaemonSet's Pods are skipped by `kubectl drain`, so a PodDisruptionBudget would protect nothing +# and none is written out. +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: test-opa-server +--- +# Nothing has asked for a Deployment yet. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-opa-server-default diff --git a/tests/templates/kuttl/workload-kind/10-install-opa.yaml.j2 b/tests/templates/kuttl/workload-kind/10-install-opa.yaml.j2 new file mode 100644 index 00000000..37e89006 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/10-install-opa.yaml.j2 @@ -0,0 +1,30 @@ +--- +apiVersion: opa.stackable.tech/v1alpha2 +kind: OpaCluster +metadata: + name: test-opa +spec: + image: +{% if test_scenario['values']['opa-latest'].find(",") > 0 %} + custom: "{{ test_scenario['values']['opa-latest'].split(',')[1] }}" + productVersion: "{{ test_scenario['values']['opa-latest'].split(',')[0] }}" +{% else %} + productVersion: "{{ test_scenario['values']['opa-latest'] }}" +{% endif %} + pullPolicy: IfNotPresent +{% if lookup('env', 'VECTOR_AGGREGATOR') %} + clusterConfig: + vectorAggregatorConfigMapName: vector-aggregator-discovery +{% endif %} + servers: + # No `roleConfig`, so `workloadKind` falls back to its `DaemonSet` default. This is the + # pre-upgrade shape, and the following steps switch away from and back to it. + config: + logging: + enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} + roleGroups: + default: + # Set from the start so that switching `workloadKind` below is the only change between the + # steps. A DaemonSet has no `spec.replicas` at all, so this is inert here -- it becomes + # meaningful in `20-switch-to-deployment`. + replicas: 2 diff --git a/tests/templates/kuttl/workload-kind/20-assert.yaml b/tests/templates/kuttl/workload-kind/20-assert.yaml new file mode 100644 index 00000000..49042d86 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/20-assert.yaml @@ -0,0 +1,49 @@ +--- +# Switching `workloadKind` to `Deployment` swaps the workload object, widens the role Service's +# traffic policy and adds a PodDisruptionBudget. +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 180 +commands: + - script: kubectl -n $NAMESPACE rollout status deployment test-opa-server-default --timeout 181s + - script: kubectl -n $NAMESPACE wait --for=condition=available opaclusters.opa.stackable.tech/test-opa --timeout 181s +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-opa-server-default +spec: + # Unlike a DaemonSet, a Deployment takes the role group's `replicas` verbatim. + replicas: 2 +status: + readyReplicas: 2 +--- +# `Local` would strand products on nodes without an OPA Pod, because a Deployment's Pods do not +# cover every node. +apiVersion: v1 +kind: Service +metadata: + name: test-opa-server +spec: + internalTrafficPolicy: Cluster +--- +# Role-level, so it is named after the role rather than the role group. +# +# `status` is asserted, not just `spec`: `currentHealthy` and `disruptionsAllowed` are only populated +# once the disruption controller matches the budget's selector against real Pods. +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: test-opa-server +spec: + maxUnavailable: 1 + selector: + matchLabels: + app.kubernetes.io/component: server + app.kubernetes.io/instance: test-opa + app.kubernetes.io/name: opa +status: + currentHealthy: 2 + desiredHealthy: 1 + expectedPods: 2 + disruptionsAllowed: 1 diff --git a/tests/templates/kuttl/workload-kind/20-errors.yaml b/tests/templates/kuttl/workload-kind/20-errors.yaml new file mode 100644 index 00000000..3e9b24d9 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/20-errors.yaml @@ -0,0 +1,8 @@ +--- +# The DaemonSet must be swept once the Deployment takes over. Both workload objects carry the same +# name and the same role labels, so leaving it behind would mean the role Service selects the Pods +# of both. +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: test-opa-server-default diff --git a/tests/templates/kuttl/workload-kind/20-switch-to-deployment.yaml.j2 b/tests/templates/kuttl/workload-kind/20-switch-to-deployment.yaml.j2 new file mode 100644 index 00000000..8cacf5e6 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/20-switch-to-deployment.yaml.j2 @@ -0,0 +1,28 @@ +--- +apiVersion: opa.stackable.tech/v1alpha2 +kind: OpaCluster +metadata: + name: test-opa +spec: + image: +{% if test_scenario['values']['opa-latest'].find(",") > 0 %} + custom: "{{ test_scenario['values']['opa-latest'].split(',')[1] }}" + productVersion: "{{ test_scenario['values']['opa-latest'].split(',')[0] }}" +{% else %} + productVersion: "{{ test_scenario['values']['opa-latest'] }}" +{% endif %} + pullPolicy: IfNotPresent +{% if lookup('env', 'VECTOR_AGGREGATOR') %} + clusterConfig: + vectorAggregatorConfigMapName: vector-aggregator-discovery +{% endif %} + servers: + roleConfig: + workloadKind: Deployment + config: + logging: + enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} + roleGroups: + default: + # Now honoured, unlike in the DaemonSet step above. + replicas: 2 diff --git a/tests/templates/kuttl/workload-kind/30-assert.yaml b/tests/templates/kuttl/workload-kind/30-assert.yaml new file mode 100644 index 00000000..6ea4ce49 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/30-assert.yaml @@ -0,0 +1,23 @@ +--- +# Switching back has to be possible, which means every object the Deployment mode added is removed +# again. This is the regression test for the orphan cleanup covering both `Deployment` and +# `PodDisruptionBudget`; an operator-rs that only sweeps DaemonSets would leave them behind and the +# `30-errors.yaml` next to this file would fail. +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 180 +commands: + - script: kubectl -n $NAMESPACE rollout status daemonset test-opa-server-default --timeout 181s + - script: kubectl -n $NAMESPACE wait --for=condition=available opaclusters.opa.stackable.tech/test-opa --timeout 181s +--- +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: test-opa-server-default +--- +apiVersion: v1 +kind: Service +metadata: + name: test-opa-server +spec: + internalTrafficPolicy: Local diff --git a/tests/templates/kuttl/workload-kind/30-errors.yaml b/tests/templates/kuttl/workload-kind/30-errors.yaml new file mode 100644 index 00000000..c55450c0 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/30-errors.yaml @@ -0,0 +1,14 @@ +--- +# Left behind, the Deployment's Pods would still be selected by the role Service -- which has just +# narrowed back to `internalTrafficPolicy: Local`. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-opa-server-default +--- +# A budget over DaemonSet Pods would block node drains while protecting nothing, since `kubectl +# drain` skips them anyway. +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: test-opa-server diff --git a/tests/templates/kuttl/workload-kind/30-switch-back-to-daemonset.yaml.j2 b/tests/templates/kuttl/workload-kind/30-switch-back-to-daemonset.yaml.j2 new file mode 100644 index 00000000..74cda7d2 --- /dev/null +++ b/tests/templates/kuttl/workload-kind/30-switch-back-to-daemonset.yaml.j2 @@ -0,0 +1,30 @@ +--- +apiVersion: opa.stackable.tech/v1alpha2 +kind: OpaCluster +metadata: + name: test-opa +spec: + image: +{% if test_scenario['values']['opa-latest'].find(",") > 0 %} + custom: "{{ test_scenario['values']['opa-latest'].split(',')[1] }}" + productVersion: "{{ test_scenario['values']['opa-latest'].split(',')[0] }}" +{% else %} + productVersion: "{{ test_scenario['values']['opa-latest'] }}" +{% endif %} + pullPolicy: IfNotPresent +{% if lookup('env', 'VECTOR_AGGREGATOR') %} + clusterConfig: + vectorAggregatorConfigMapName: vector-aggregator-discovery +{% endif %} + servers: + # Must be spelled out rather than omitted: kuttl applies a step as a merge patch, so an absent + # field means "leave it alone", not "remove it". Dropping this block would keep the + # `workloadKind: Deployment`. + roleConfig: + workloadKind: DaemonSet + config: + logging: + enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} + roleGroups: + default: + replicas: 2 diff --git a/tests/test-definition.yaml b/tests/test-definition.yaml index a25b1a32..b0933aba 100644 --- a/tests/test-definition.yaml +++ b/tests/test-definition.yaml @@ -60,6 +60,12 @@ tests: dimensions: - opa-latest - openshift + # Deliberately not a dimension of `smoke`: the Pod template is identical for both workload kinds, + # so re-running the whole smoke matrix would double it for no extra coverage. + - name: workload-kind + dimensions: + - opa-latest + - openshift - name: config-overrides dimensions: - opa-latest