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

[COR-205] Add resources to CoreDB spec #122

Merged
merged 18 commits into from
Feb 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions coredb-operator/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion coredb-operator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ telemetry = ["tonic", "opentelemetry-otlp"]
actix-web = "4.2.1"
futures = "0.3.25"
tokio = { version = "1.22.0", features = ["macros", "rt-multi-thread"] }
k8s-openapi = { version = "0.16.0", features = ["v1_24"], default-features = false }
k8s-openapi = { version = "0.16.0", features = ["v1_24", "schemars"], default-features = false }
schemars = { version = "0.8.11", features = ["chrono"] }
serde = { version = "1.0.148", features = ["derive"] }
serde_json = "1.0.89"
Expand Down
4 changes: 3 additions & 1 deletion coredb-operator/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
extensions::create_extensions, postgres_exporter_role::create_postgres_exporter_role,
secret::reconcile_secret,
};
use k8s_openapi::api::core::v1::{Namespace, Pod};
use k8s_openapi::api::core::v1::{Namespace, Pod, ResourceRequirements};
use kube::runtime::wait::Condition;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand All @@ -48,6 +48,8 @@ pub static COREDB_FINALIZER: &str = "coredbs.coredb.io";
pub struct CoreDBSpec {
#[serde(default = "defaults::default_replicas")]
pub replicas: i32,
#[serde(default = "defaults::default_resources")]
pub resources: ResourceRequirements,
#[serde(default = "defaults::default_postgres_exporter_enabled")]
pub postgresExporterEnabled: bool,
#[serde(default = "defaults::default_image")]
Expand Down
18 changes: 18 additions & 0 deletions coredb-operator/src/defaults.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
use k8s_openapi::{api::core::v1::ResourceRequirements, apimachinery::pkg::api::resource::Quantity};
use std::collections::BTreeMap;

pub fn default_replicas() -> i32 {
1
}

pub fn default_resources() -> ResourceRequirements {
let limits: BTreeMap<String, Quantity> = BTreeMap::from([
("cpu".to_owned(), Quantity("2".to_string())),
("memory".to_owned(), Quantity("2Gi".to_string())),
]);
let requests: BTreeMap<String, Quantity> = BTreeMap::from([
("cpu".to_owned(), Quantity("500m".to_string())),
("memory".to_owned(), Quantity("512Mi".to_string())),
Comment on lines +9 to +15
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjmiller609 let me know your thoughts on these default values

]);
ResourceRequirements {
limits: Some(limits),
requests: Some(requests),
}
}

pub fn default_postgres_exporter_enabled() -> bool {
true
}
Expand Down
2 changes: 1 addition & 1 deletion coredb-operator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub mod telemetry;
mod metrics;
pub use metrics::Metrics;

mod defaults;
pub mod defaults;
mod extensions;
#[cfg(test)] pub mod fixtures;
mod postgres_exporter_role;
Expand Down
1 change: 1 addition & 0 deletions coredb-operator/src/statefulset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ pub fn stateful_set_from_cdb(cdb: &CoreDB) -> StatefulSet {
}),
name: "postgres".to_string(),
image: Some(cdb.spec.image.clone()),
resources: Some(cdb.spec.resources.clone()),
ports: Some(vec![ContainerPort {
container_port: 5432,
..ContainerPort::default()
Expand Down
40 changes: 22 additions & 18 deletions coredb-operator/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
#[cfg(test)]
mod test {

use controller::{is_pod_ready, CoreDB};
use controller::{defaults, is_pod_ready, CoreDB};
use defaults::default_resources;
use k8s_openapi::{
api::core::v1::{Container, Namespace, Pod, PodSpec, Secret},
api::core::v1::{Container, Namespace, Pod, PodSpec, ResourceRequirements, Secret},
apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition,
apimachinery::pkg::apis::meta::v1::ObjectMeta,
};
Expand Down Expand Up @@ -125,23 +126,20 @@ mod test {
let secret_name = format!("{}-connection", name);
println!("Waiting for secret to be created: {}", secret_name);
let establish = await_condition(secret_api.clone(), &secret_name, wait_for_secret());
let _ = tokio::time::timeout(
std::time::Duration::from_secs(timeout_seconds_secret_present),
establish,
)
.await
.expect(&format!(
"Did not find the secret {} present after waiting {} seconds",
secret_name, timeout_seconds_secret_present
));
let _ = tokio::time::timeout(Duration::from_secs(timeout_seconds_secret_present), establish)
.await
.expect(&format!(
"Did not find the secret {} present after waiting {} seconds",
secret_name, timeout_seconds_secret_present
));
println!("Found secret: {}", secret_name);

// Wait for Pod to be created
let pod_name = format!("{}-0", name);

println!("Waiting for pod to be running: {}", pod_name);
let _check_for_pod = tokio::time::timeout(
std::time::Duration::from_secs(timeout_seconds_start_pod),
Duration::from_secs(timeout_seconds_start_pod),
await_condition(pods.clone(), &pod_name, conditions::is_pod_running()),
)
.await
Expand All @@ -151,7 +149,7 @@ mod test {
));
println!("Waiting for pod to be ready: {}", pod_name);
let _check_for_pod_ready = tokio::time::timeout(
std::time::Duration::from_secs(timeout_seconds_pod_ready),
Duration::from_secs(timeout_seconds_pod_ready),
await_condition(pods.clone(), &pod_name, is_pod_ready()),
)
.await
Expand All @@ -161,6 +159,12 @@ mod test {
));
println!("Found pod ready: {}", pod_name);

// Assert default resource values are applied to postgres container
let default_resources: ResourceRequirements = default_resources();
let pg_pod = pods.get(&pod_name).await.unwrap();
let resources = pg_pod.spec.unwrap().containers[0].clone().resources;
assert_eq!(default_resources, resources.unwrap());

// Assert no tables found
let result = coredb_resource
.psql("\\dt".to_string(), "postgres".to_string(), client.clone())
Expand Down Expand Up @@ -293,7 +297,7 @@ mod test {
let pods: Api<Pod> = Api::namespaced(client.clone(), namespace);
println!("Waiting for pod to be running: {}", pod_name);
let _check_for_pod = tokio::time::timeout(
std::time::Duration::from_secs(timeout_seconds_start_pod),
Duration::from_secs(timeout_seconds_start_pod),
await_condition(pods.clone(), &pod_name, conditions::is_pod_running()),
)
.await
Expand All @@ -303,7 +307,7 @@ mod test {
));
println!("Waiting for pod to be ready: {}", pod_name);
let _check_for_pod_ready = tokio::time::timeout(
std::time::Duration::from_secs(timeout_seconds_pod_ready),
Duration::from_secs(timeout_seconds_pod_ready),
await_condition(pods.clone(), &pod_name, is_pod_ready()),
)
.await
Expand All @@ -321,7 +325,7 @@ mod test {
// similar to the loop used in namespace delete assertion, but received a comparison error.
println!("Waiting for CoreDB to be deleted: {}", &name);
let _assert_coredb_deleted = tokio::time::timeout(
std::time::Duration::from_secs(timeout_seconds_coredb_deleted),
Duration::from_secs(timeout_seconds_coredb_deleted),
await_condition(coredbs.clone(), &name, conditions::is_deleted("")),
)
.await
Expand All @@ -348,7 +352,7 @@ mod test {
));
}

async fn kube_client() -> kube::Client {
async fn kube_client() -> Client {
// Get the name of the currently selected namespace
let kube_config = Config::infer()
.await
Expand Down Expand Up @@ -378,7 +382,7 @@ mod test {
let custom_resource_definitions: Api<CustomResourceDefinition> = Api::all(client.clone());

let _check_for_crd = tokio::time::timeout(
std::time::Duration::from_secs(2),
Duration::from_secs(2),
await_condition(
custom_resource_definitions,
"coredbs.coredb.io",
Expand Down
85 changes: 85 additions & 0 deletions coredb-operator/yaml/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,91 @@ spec:
default: 1
format: int32
type: integer
resources:
default:
limits:
cpu: '2'
memory: 2Gi
requests:
cpu: 500m
memory: 512Mi
description: ResourceRequirements describes the compute resource requirements.
properties:
limits:
additionalProperties:
description: |-
Quantity is a fixed-point representation of a number. It provides convenient marshaling/unmarshaling in JSON and YAML, in addition to String() and AsInt64() accessors.

The serialization format is:

<quantity> ::= <signedNumber><suffix>
(Note that <suffix> may be empty, from the "" case in <decimalSI>.)
<digit> ::= 0 | 1 | ... | 9 <digits> ::= <digit> | <digit><digits> <number> ::= <digits> | <digits>.<digits> | <digits>. | .<digits> <sign> ::= "+" | "-" <signedNumber> ::= <number> | <sign><number> <suffix> ::= <binarySI> | <decimalExponent> | <decimalSI> <binarySI> ::= Ki | Mi | Gi | Ti | Pi | Ei
(International System of units; See: http://physics.nist.gov/cuu/Units/binary.html)
<decimalSI> ::= m | "" | k | M | G | T | P | E
(Note that 1024 = 1Ki but 1000 = 1k; I didn't choose the capitalization.)
<decimalExponent> ::= "e" <signedNumber> | "E" <signedNumber>

No matter which of the three exponent forms is used, no quantity may represent a number greater than 2^63-1 in magnitude, nor may it have more than 3 decimal places. Numbers larger or more precise will be capped or rounded up. (E.g.: 0.1m will rounded up to 1m.) This may be extended in the future if we require larger or smaller quantities.

When a Quantity is parsed from a string, it will remember the type of suffix it had, and will use the same type again when it is serialized.

Before serializing, Quantity will be put in "canonical form". This means that Exponent/suffix will be adjusted up or down (with a corresponding increase or decrease in Mantissa) such that:
a. No precision is lost
b. No fractional digits will be emitted
c. The exponent (or suffix) is as large as possible.
The sign will be omitted unless the number is negative.

Examples:
1.5 will be serialized as "1500m"
1.5Gi will be serialized as "1536Mi"

Note that the quantity will NEVER be internally represented by a floating point number. That is the whole point of this exercise.

Non-canonical values will still parse as long as they are well formed, but will be re-emitted in their canonical form. (So always use canonical form, or don't diff.)

This format is intended to make it difficult to use these numbers without writing some sort of special handling code in the hopes that that will cause implementors to also use a fixed point implementation.
type: string
description: 'Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
requests:
additionalProperties:
description: |-
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These descriptions can cause unnecessary bloat in our CRD. Go's controller-gen has a maxDescLen flag that you can set to 0 to disable these descriptions. There doesn't seem to be an equivalent in kube-derive at the moment.

It's fine to include them at the moment, but wanted to call this out in case we need to trim things down in the future.

Quantity is a fixed-point representation of a number. It provides convenient marshaling/unmarshaling in JSON and YAML, in addition to String() and AsInt64() accessors.

The serialization format is:

<quantity> ::= <signedNumber><suffix>
(Note that <suffix> may be empty, from the "" case in <decimalSI>.)
<digit> ::= 0 | 1 | ... | 9 <digits> ::= <digit> | <digit><digits> <number> ::= <digits> | <digits>.<digits> | <digits>. | .<digits> <sign> ::= "+" | "-" <signedNumber> ::= <number> | <sign><number> <suffix> ::= <binarySI> | <decimalExponent> | <decimalSI> <binarySI> ::= Ki | Mi | Gi | Ti | Pi | Ei
(International System of units; See: http://physics.nist.gov/cuu/Units/binary.html)
<decimalSI> ::= m | "" | k | M | G | T | P | E
(Note that 1024 = 1Ki but 1000 = 1k; I didn't choose the capitalization.)
<decimalExponent> ::= "e" <signedNumber> | "E" <signedNumber>

No matter which of the three exponent forms is used, no quantity may represent a number greater than 2^63-1 in magnitude, nor may it have more than 3 decimal places. Numbers larger or more precise will be capped or rounded up. (E.g.: 0.1m will rounded up to 1m.) This may be extended in the future if we require larger or smaller quantities.

When a Quantity is parsed from a string, it will remember the type of suffix it had, and will use the same type again when it is serialized.

Before serializing, Quantity will be put in "canonical form". This means that Exponent/suffix will be adjusted up or down (with a corresponding increase or decrease in Mantissa) such that:
a. No precision is lost
b. No fractional digits will be emitted
c. The exponent (or suffix) is as large as possible.
The sign will be omitted unless the number is negative.

Examples:
1.5 will be serialized as "1500m"
1.5Gi will be serialized as "1536Mi"

Note that the quantity will NEVER be internally represented by a floating point number. That is the whole point of this exercise.

Non-canonical values will still parse as long as they are well formed, but will be re-emitted in their canonical form. (So always use canonical form, or don't diff.)

This format is intended to make it difficult to use these numbers without writing some sort of special handling code in the hopes that that will cause implementors to also use a fixed point implementation.
type: string
description: 'Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
uid:
default: 999
format: int32
Expand Down