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

refactor!(stackable-operator): Fix and improve quantity parsing #936

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
7 changes: 3 additions & 4 deletions crates/stackable-operator/src/builder/pod/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use tracing::warn;

use crate::{
commons::resources::ResourceRequirementsType,
cpu::{self, CpuQuantity},
memory::{self, MemoryQuantity},
quantity::{CpuQuantity, MemoryQuantity, ParseQuantityError},
};

const RESOURCE_DENYLIST: &[&str] = &["cpu", "memory"];
Expand Down Expand Up @@ -54,7 +53,7 @@ impl<CL, MR, ML> ResourceRequirementsBuilder<(), CL, MR, ML> {
self,
request: impl Into<String>,
factor: f32,
) -> cpu::Result<ResourceRequirementsBuilder<Quantity, Quantity, MR, ML>> {
) -> Result<ResourceRequirementsBuilder<Quantity, Quantity, MR, ML>, ParseQuantityError> {
let request = CpuQuantity::from_str(&request.into())?;
let limit = request * factor;

Expand Down Expand Up @@ -124,7 +123,7 @@ impl<CR, CL, ML> ResourceRequirementsBuilder<CL, CR, (), ML> {
self,
request: impl Into<String>,
factor: f32,
) -> memory::Result<ResourceRequirementsBuilder<CL, CR, Quantity, Quantity>> {
) -> Result<ResourceRequirementsBuilder<CL, CR, Quantity, Quantity>, ParseQuantityError> {
let request = MemoryQuantity::from_str(&request.into())?;
let limit = request * factor;

Expand Down
52 changes: 28 additions & 24 deletions crates/stackable-operator/src/commons/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,30 +67,34 @@
//! shared_storage: PvcConfig,
//! }
//! ```
use std::{collections::BTreeMap, fmt::Debug};

use crate::{
config::{
fragment::{Fragment, FromFragment},
merge::Merge,
},
cpu::CpuQuantity,
memory::MemoryQuantity,
};
use educe::Educe;
use k8s_openapi::api::core::v1::{
Container, PersistentVolumeClaim, PersistentVolumeClaimSpec, PodSpec, ResourceRequirements,
VolumeResourceRequirements,
use k8s_openapi::{
api::core::v1::{
Container, PersistentVolumeClaim, PersistentVolumeClaimSpec, PodSpec, ResourceRequirements,
VolumeResourceRequirements,
},
apimachinery::pkg::{
api::resource::Quantity,
apis::meta::v1::{LabelSelector, ObjectMeta},
},
};
use k8s_openapi::apimachinery::pkg::api::resource::Quantity;
use k8s_openapi::apimachinery::pkg::apis::meta::v1::{LabelSelector, ObjectMeta};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use snafu::{ResultExt, Snafu};
use std::{collections::BTreeMap, fmt::Debug};
use strum::Display;

pub const LIMIT_REQUEST_RATIO_CPU: f32 = 5.0;
pub const LIMIT_REQUEST_RATIO_MEMORY: f32 = 1.0;
use crate::{
config::{
fragment::{Fragment, FromFragment},
merge::Merge,
},
quantity::{CpuQuantity, MemoryQuantity, ParseQuantityError},
};

pub const LIMIT_REQUEST_RATIO_CPU: f64 = 5.0;
pub const LIMIT_REQUEST_RATIO_MEMORY: f64 = 1.0;

type Result<T, E = Error> = std::result::Result<T, E>;

Expand All @@ -109,20 +113,20 @@ pub enum Error {
LimitToRequestRatioExceeded {
container_name: String,
resource_key: String,
allowed_ration: f32,
allowed_ration: f64,
},

#[snafu(display("failed to convert Quantity to CpuQuantity for cpu limit"))]
CpuLimit { source: crate::cpu::Error },
CpuLimit { source: ParseQuantityError },

#[snafu(display("failed to convert Quantity to CpuQuantity for cpu request"))]
CpuRequest { source: crate::cpu::Error },
CpuRequest { source: ParseQuantityError },

#[snafu(display("failed to convert Quantity to MemoryQuantity for memory limit"))]
MemoryLimit { source: crate::memory::Error },
MemoryLimit { source: ParseQuantityError },

#[snafu(display("failed to convert Quantity to MemoryQuantity for memory request"))]
MemoryRequest { source: crate::memory::Error },
MemoryRequest { source: ParseQuantityError },
}

/// Resource usage is configured here, this includes CPU usage, memory usage and disk storage
Expand Down Expand Up @@ -429,7 +433,7 @@ pub trait ResourceRequirementsExt {
&self,
resource: &ComputeResource,
// We did choose a f32 instead of a usize here, as LimitRange ratios can be a floating point (Quantity - e.g. 1500m)
ratio: f32,
ratio: f64,
) -> Result<()>;
}

Expand Down Expand Up @@ -459,7 +463,7 @@ impl ResourceRequirementsExt for Container {
Ok(())
}

fn check_limit_to_request_ratio(&self, resource: &ComputeResource, ratio: f32) -> Result<()> {
fn check_limit_to_request_ratio(&self, resource: &ComputeResource, ratio: f64) -> Result<()> {
let limit = self
.resources
.as_ref()
Expand Down Expand Up @@ -511,7 +515,7 @@ impl ResourceRequirementsExt for PodSpec {
Ok(())
}

fn check_limit_to_request_ratio(&self, resource: &ComputeResource, ratio: f32) -> Result<()> {
fn check_limit_to_request_ratio(&self, resource: &ComputeResource, ratio: f64) -> Result<()> {
for container in &self.containers {
container.check_limit_to_request_ratio(resource, ratio)?;
}
Expand Down
Loading
Loading