Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(stackable-operator)!: Reduce CRD size by omitting podOverride and affinities schemas #821

Merged
merged 10 commits into from
Aug 2, 2024
6 changes: 6 additions & 0 deletions crates/stackable-operator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ All notable changes to this project will be documented in this file.
- [kube#1494](https://github.com/kube-rs/kube/pull/1494)
- [kube#1504](https://github.com/kube-rs/kube/pull/1504)
- Upgrade opentelemetry crates ([#811]).
- BREAKING: Convert `podOverrides` and `affinity` fields to take any arbitrary YAML input, rather than using the
underlying schema. This reduces e.g. the Druid CRD size from `2.4MB` to `288K` (which is a 88% reduction). It has the
downside that the users input is not validated to be a valid `PodTemplateSpec`/affinity any more. However, this can
later be re-added by using validation webhooks if needed. This change should not be breaking for the user and is a
preparation for CRD versioning. ([#821]).
sbernauer marked this conversation as resolved.
Show resolved Hide resolved

### Fixed

Expand All @@ -30,6 +35,7 @@ All notable changes to this project will be documented in this file.
[#817]: https://github.com/stackabletech/operator-rs/pull/817
[#818]: https://github.com/stackabletech/operator-rs/pull/818
[#819]: https://github.com/stackabletech/operator-rs/pull/819
[#821]: https://github.com/stackabletech/operator-rs/pull/821

## [0.69.3] - 2024-06-12

Expand Down
12 changes: 12 additions & 0 deletions crates/stackable-operator/src/commons/affinity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use stackable_operator_derive::Fragment;
use crate::{
config::merge::{Atomic, Merge},
kvp::consts::{K8S_APP_COMPONENT_KEY, K8S_APP_INSTANCE_KEY, K8S_APP_NAME_KEY},
utils::crds::raw_object_schema,
};

pub const TOPOLOGY_KEY_HOSTNAME: &str = "kubernetes.io/hostname";
Expand All @@ -36,9 +37,19 @@ pub const TOPOLOGY_KEY_HOSTNAME: &str = "kubernetes.io/hostname";
serde(rename_all = "camelCase")
)]
pub struct StackableAffinity {
/// Same as the `spec.affinity.podAffinity` field on the Pod, see the [Kubernetes docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node)
#[fragment_attrs(schemars(schema_with = "raw_object_schema"))]
pub pod_affinity: Option<PodAffinity>,

/// Same as the `spec.affinity.podAntiAffinity` field on the Pod, see the [Kubernetes docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node)
#[fragment_attrs(schemars(schema_with = "raw_object_schema"))]
pub pod_anti_affinity: Option<PodAntiAffinity>,

/// Same as the `spec.affinity.nodeAffinity` field on the Pod, see the [Kubernetes docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node)
#[fragment_attrs(schemars(schema_with = "raw_object_schema"))]
pub node_affinity: Option<NodeAffinity>,

// This schema isn't big, so it can stay
pub node_selector: Option<StackableNodeSelector>,
}

Expand All @@ -51,6 +62,7 @@ pub struct StackableAffinity {
// FIXME: The generated JsonSchema will be wrong, so until https://github.com/GREsau/schemars/issues/259 is fixed, we
// need to use `#[schemars(deny_unknown_fields)]`.
// See https://github.com/stackabletech/operator-rs/pull/752#issuecomment-2017630433 for details.
/// Simple key-value pairs forming a nodeSelector, see the [Kubernetes docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node)
#[derive(Clone, Debug, Eq, Deserialize, JsonSchema, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
#[schemars(deny_unknown_fields)]
Expand Down
1 change: 0 additions & 1 deletion crates/stackable-operator/src/commons/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ pub mod cluster_operation;
pub mod listener;
pub mod opa;
pub mod pdb;
pub mod pod_overrides;
pub mod product_image_selection;
pub mod rbac;
pub mod resources;
Expand Down
5 changes: 3 additions & 2 deletions crates/stackable-operator/src/role_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,13 @@ use std::{
};

use crate::{
commons::{pdb::PdbConfig, pod_overrides::pod_overrides_schema},
commons::pdb::PdbConfig,
config::{
fragment::{self, FromFragment},
merge::Merge,
},
product_config_utils::Configuration,
utils::crds::raw_object_schema,
};
use derivative::Derivative;
use k8s_openapi::api::core::v1::PodTemplateSpec;
Expand Down Expand Up @@ -137,7 +138,7 @@ pub struct CommonConfiguration<T> {
/// [Pod overrides documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/overrides#pod-overrides)
/// for more information.
#[serde(default)]
#[schemars(schema_with = "pod_overrides_schema")]
#[schemars(schema_with = "raw_object_schema")]
pub pod_overrides: PodTemplateSpec,
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,56 +1,36 @@
use k8s_openapi::api::core::v1::PodTemplateSpec;
use schemars::{schema::Schema, visit::Visitor, JsonSchema};

/// Simplified schema for PodTemplateSpec without mandatory fields (e.g. `containers`) or documentation.
///
/// The normal PodTemplateSpec requires you to specify `containers` as an `Vec<Container>`.
/// Often times the user want's to overwrite/add stuff not related to a container
/// (e.g. tolerations or a ServiceAccount), so it's annoying that he always needs to
/// specify an empty array for `containers`.
///
/// Additionally all docs are removed, as the resulting Stackable CRD objects where to big for Kubernetes.
/// E.g. the HdfsCluster CRD increased to ~3.2 MB (which is over the limit of 3MB), after stripping
/// the docs it went down to ~1.3 MiB.
pub fn pod_overrides_schema(gen: &mut schemars::gen::SchemaGenerator) -> Schema {
let mut schema = PodTemplateSpec::json_schema(gen);
SimplifyOverrideSchema.visit_schema(&mut schema);

if let Schema::Object(schema) = &mut schema {
let meta = schema.metadata.get_or_insert_with(Default::default);
meta.description = Some("See PodTemplateSpec (https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#podtemplatespec-v1-core) for more details".to_string());
}

schema
use schemars::schema::Schema;

pub fn raw_object_schema(_: &mut schemars::gen::SchemaGenerator) -> Schema {
serde_json::from_value(serde_json::json!({
"type": "object",
"x-kubernetes-preserve-unknown-fields": true,
}))
.expect("Failed to parse JSON of custom raw object schema")
}

struct SimplifyOverrideSchema;
impl schemars::visit::Visitor for SimplifyOverrideSchema {
fn visit_schema_object(&mut self, schema: &mut schemars::schema::SchemaObject) {
// Strip docs to make the schema more compact
if let Some(meta) = &mut schema.metadata {
meta.description = None;
meta.examples.clear();
pub fn raw_object_list_schema(_: &mut schemars::gen::SchemaGenerator) -> Schema {
serde_json::from_value(serde_json::json!({
"type": "array",
"items": {
"type": "object",
"x-kubernetes-preserve-unknown-fields": true,
}

// Make all options optional
if let Some(object) = &mut schema.object {
object.required.clear();
}

schemars::visit::visit_schema_object(self, schema);
}
}))
.expect("Failed to parse JSON of custom raw object list schema")
}

#[cfg(test)]
mod tests {
use k8s_openapi::api::core::v1::PodTemplateSpec;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use super::*;

#[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
struct Test {
#[schemars(schema_with = "pod_overrides_schema")]
#[schemars(schema_with = "raw_object_schema")]
pub pod_overrides: PodTemplateSpec,
}

Expand Down Expand Up @@ -130,7 +110,7 @@ mod tests {
- image: docker.stackable.tech/stackable/nifi:1.23.2-stackable23.11.0
"#;

// FIXME: Ideally we would require the names of the containers to be set. We had users using podOverrides
// FIXME: Ideally we would require the names of the containers to be set. We had users using podOverrides
// without setting the name of the container and wondering why it didn't work.
serde_yaml::from_str::<Test>(input).expect("Failed to parse valid podOverride");
}
Expand Down
1 change: 1 addition & 0 deletions crates/stackable-operator/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod bash;
pub mod crds;
pub mod logging;
mod option;
mod url;
Expand Down
Loading