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..6d407dae 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/docs/modules/opa/pages/usage-guide/operations/pod-disruptions.adoc b/docs/modules/opa/pages/usage-guide/operations/pod-disruptions.adoc index 69915f83..4a038254 100644 --- a/docs/modules/opa/pages/usage-guide/operations/pod-disruptions.adoc +++ b/docs/modules/opa/pages/usage-guide/operations/pod-disruptions.adoc @@ -1,5 +1,35 @@ = Allowed Pod disruptions +:description: Whether the operator creates a PodDisruptionBudget for OPA depends on the workload kind, and how to configure or disable that budget. -For OPA clusters, the operator does not deploy any PodDisruptionBudgets (PDBs), as there is one instance per Kubernetes node running (Daemonset). -When a Kubernetes node gets drained to gracefully shut it down, the OPA Pod get's evicted - there is no point in blocking the eviction. -In case the OPA Pod terminated before the products depending on OPA (e.g. Trino coordinator) on the same node, the products can still use the OPA Service, as it routes to OPA Pods running on other Kubernetes nodes. +You can configure the allowed Pod disruptions as described in xref:concepts:operations/pod_disruptions.adoc[]. + +What the operator creates depends on the xref:usage-guide/workload-kind.adoc[workload kind] of the role. + +== DaemonSet + +No PodDisruptionBudget is created. +Draining a node has to be able to remove the OPA Pod on that node, and `kubectl drain` skips DaemonSet Pods, means a budget would block maintenance without adding an upside. + +Because the role Service routes node-locally in this mode, products on a drained node cannot reach OPA on another node. + +== Deployment + +The operator creates one PodDisruptionBudget per role, allowing one Pod to be unavailable at a time by default: + +[source,yaml] +---- +spec: + servers: + roleConfig: + podDisruptionBudget: + enabled: true # <1> + maxUnavailable: 1 # <2> +---- +<1> Defaults to true for a Deployment. Set it to false to create no budget. +<2> Defaults to 1. Raise it to allow more Pods to be unavailable at once. + +Keep `maxUnavailable` at 1 unless you have measured that OPA tolerates more. +Products query OPA on every request, budgets that drain too many Pods at once turn a node rotation into a platform-wide slowdown. + +NOTE: A budget only helps when there is another Pod to fall back to. +With `replicas` set to 1, the single Pod can still be evicted. diff --git a/docs/modules/opa/pages/usage-guide/operations/pod-placement.adoc b/docs/modules/opa/pages/usage-guide/operations/pod-placement.adoc new file mode 100644 index 00000000..75d9adc8 --- /dev/null +++ b/docs/modules/opa/pages/usage-guide/operations/pod-placement.adoc @@ -0,0 +1,33 @@ += Pod placement +:description: The default affinity spreads OPA Pods across Kubernetes nodes, which matters for a Deployment, and how to override it with your own affinities. + +You can configure Pod placement for OPA Pods as described in xref:concepts:operations/pod_placement.adoc[]. + +== Defaults + +The default affinity created by the operator is: + +* Distribute all Pods of the `servers` role across nodes, so that multiple Pods don't end up on the same Kubernetes node (weight 70) + +This constrains nothing for a DaemonSet, which already places exactly one Pod per node. +It matters for a Deployment, where several replicas would otherwise be free to share the same node. +See xref:usage-guide/workload-kind.adoc[]. + +[source,yaml] +---- +affinity: + podAntiAffinity: + preferredDuringSchedulingIgnoredDuringExecution: + - podAffinityTerm: + labelSelector: + matchLabels: + app.kubernetes.io/component: server + app.kubernetes.io/instance: cluster-name + app.kubernetes.io/name: opa + topologyKey: kubernetes.io/hostname + weight: 70 +---- + +NOTE: The default affinity is only preferred and not enforced, because not every setup has multiple Kubernetes nodes. +To enforce it, set your own `requiredDuringSchedulingIgnoredDuringExecution` affinity. +Be aware that a Deployment with more replicas than nodes then leaves Pods unschedulable. diff --git a/docs/modules/opa/pages/usage-guide/workload-kind.adoc b/docs/modules/opa/pages/usage-guide/workload-kind.adoc new file mode 100644 index 00000000..4b971856 --- /dev/null +++ b/docs/modules/opa/pages/usage-guide/workload-kind.adoc @@ -0,0 +1,84 @@ += Workload kind +:description: Run the OPA servers as a DaemonSet or as a Deployment, and learn how that choice affects the Pod count, Service routing and changing it later. + +By default the `servers` role runs as a DaemonSet, which places one OPA Pod on every Kubernetes node. +Set `workloadKind` to Deployment to run a fixed number of Pods instead. + +[source,yaml] +---- +spec: + servers: + roleConfig: + workloadKind: Deployment # <1> + roleGroups: + default: + replicas: 3 # <2> +---- +<1> Either DaemonSet (default) or Deployment. +<2> Only used by Deployment, DaemonSet derives Pod count from the number of nodes. + +The workload kind also decides whether the operator creates a xref:usage-guide/operations/pod-disruptions.adoc[PodDisruptionBudget], and it changes what the default xref:usage-guide/operations/pod-placement.adoc[Pod placement] achieves. + +== Choosing a workload kind + +Use a DaemonSet when every node runs products that query OPA. +Each product then queries the OPA Pod on its own node, so no policy query crosses the network. +The number of OPA Pods grows and shrinks with the node count. + +Use a Deployment when the number of OPA Pods should be fixed. +You set the count with `replicas` and queries are spread across all Pods. +This fits large clusters, and clusters where only a few nodes run products that query OPA. + +== Service routing + +The operator derives the role Service's `internalTrafficPolicy` from the workload kind. + +* A DaemonSet gets Local, so a query only reaches the OPA Pod on the client's own node. + This avoids the network hop, and a DaemonSet covers every node, thus such a Pod always exists. +* A Deployment gets Cluster, so a query reaches any OPA Pod of the role. + A Deployment's Pods do not cover every node necessarily, so node-local routing would leave products on the remaining nodes unable to reach OPA at all. + +If the derived value doesn't suit your cluster, you can override it as described in <>. + +[#override-traffic-policy] +== Override traffic policy + +In edge cases (e.g. node autoscaling under load), it is useful to use a DaemonSet with `internalTrafficPolicy: Cluster`. +This can be achieved using xref:concepts:overrides.adoc#object-overrides[object overrides]: + +[source,yaml] +---- +apiVersion: opa.stackable.tech/v1alpha2 +kind: OpaCluster +metadata: + name: simple-opa + namespace: default +spec: + image: + productVersion: 1.16.2 + objectOverrides: + - apiVersion: v1 + kind: Service + metadata: + name: simple-opa-server + namespace: default + spec: + internalTrafficPolicy: Cluster # <1> + servers: + roleGroups: + default: {} +---- +<1> Changes `internalTrafficPolicy` from Local to Cluster. + +== Changing the workload kind + +Changing `workloadKind` replaces the workload object, so policy queries can fail while the new Pods start up. +Products usually treat a failed policy query as a denied request. + +Changing to DaemonSet is the more disruptive direction. +The role Service narrows to Local as soon as you apply the change, while the running Pods still belong to the outgoing Deployment and cover only some nodes. +Products on the remaining nodes fail until the DaemonSet has rolled out everywhere. + +To ease the interruption, pin the traffic policy to Cluster as described in <>, and remove the override once the rollout has finished. +This keeps every product able to reach any OPA Pod throughout the change. +A short window in which no Pod is ready can still occur, because the outgoing workload is removed as the new one starts. diff --git a/docs/modules/opa/partials/nav.adoc b/docs/modules/opa/partials/nav.adoc index 58dfa4df..70b92a10 100644 --- a/docs/modules/opa/partials/nav.adoc +++ b/docs/modules/opa/partials/nav.adoc @@ -3,6 +3,7 @@ ** xref:opa:getting_started/first_steps.adoc[] * xref:opa:usage-guide/index.adoc[] ** xref:opa:usage-guide/listenerclass.adoc[] +** xref:opa:usage-guide/workload-kind.adoc[] ** xref:opa:usage-guide/policies.adoc[] ** xref:opa:usage-guide/user-info-fetcher.adoc[] ** xref:opa:usage-guide/resource-info-fetcher.adoc[] @@ -14,7 +15,7 @@ ** xref:opa:usage-guide/tls.adoc[] ** xref:opa:usage-guide/operations/index.adoc[] *** xref:opa:usage-guide/operations/cluster-operations.adoc[] -// *** xref:hdfs:usage-guide/operations/pod-placement.adoc[] Missing +*** xref:opa:usage-guide/operations/pod-placement.adoc[] *** xref:opa:usage-guide/operations/pod-disruptions.adoc[] *** xref:opa:usage-guide/operations/graceful-shutdown.adoc[] * xref:opa:reference/index.adoc[] 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..a68f0cea --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/workload/daemonset.rs @@ -0,0 +1,474 @@ +//! 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_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/workload/deployment.rs b/rust/operator-binary/src/controller/build/resource/workload/deployment.rs new file mode 100644 index 00000000..a2e68b0d --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/workload/deployment.rs @@ -0,0 +1,213 @@ +//! 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). +#[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 guessing + // 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_opa_operator::crd::OpaRole; + use stackable_operator::commons::networking::DomainName; + + 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..f266647b 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.as_ref(), &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..dd4eaa3b --- /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..e122aecf 100644 --- a/tests/templates/kuttl/smoke/12-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/12-assert.yaml.j2 @@ -8,8 +8,7 @@ # 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. # # `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..8d69d588 --- /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..0a5f0317 --- /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