diff --git a/Jenkinsfile b/Jenkinsfile index d52ae9fa7..b570ec667 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -117,7 +117,7 @@ pipeline { sh 'nix-shell --run "./scripts/rust/generate-openapi-bindings.sh"' sh 'nix-shell --run "cargo fmt --all -- --check"' sh 'nix-shell --run "cargo clippy --all-targets -- -D warnings"' - sh 'nix-shell --run "black tests/bdd"' + sh 'nix-shell --run "black --check tests/bdd"' sh 'nix-shell --run "./scripts/git/check-submodule-branches.sh"' } } diff --git a/control-plane/agents/src/bin/core/controller/resources/operations.rs b/control-plane/agents/src/bin/core/controller/resources/operations.rs index df4ad5198..3999e8055 100644 --- a/control-plane/agents/src/bin/core/controller/resources/operations.rs +++ b/control-plane/agents/src/bin/core/controller/resources/operations.rs @@ -36,11 +36,11 @@ pub(crate) trait ResourceLabel { label: HashMap, overwrite: bool, ) -> Result; - /// Remove label from the resource. + /// Remove the label key from the resource. async fn unlabel( &mut self, registry: &Registry, - label: String, + label_key: String, ) -> Result; } diff --git a/control-plane/agents/src/bin/core/node/operations.rs b/control-plane/agents/src/bin/core/node/operations.rs index 40d9703a7..fa9ae309d 100644 --- a/control-plane/agents/src/bin/core/node/operations.rs +++ b/control-plane/agents/src/bin/core/node/operations.rs @@ -67,7 +67,7 @@ impl ResourceLabel for OperationGuardArc { .start_update( registry, &cloned_node_spec, - NodeOperation::Label(label, overwrite), + NodeOperation::Label((label, overwrite).into()), ) .await?; @@ -79,11 +79,15 @@ impl ResourceLabel for OperationGuardArc { async fn unlabel( &mut self, registry: &Registry, - label: String, + label_key: String, ) -> Result { let cloned_node_spec = self.lock().clone(); let spec_clone = self - .start_update(registry, &cloned_node_spec, NodeOperation::Unlabel(label)) + .start_update( + registry, + &cloned_node_spec, + NodeOperation::Unlabel(label_key.into()), + ) .await?; self.complete_update(registry, Ok(()), spec_clone).await?; diff --git a/control-plane/agents/src/bin/core/node/service.rs b/control-plane/agents/src/bin/core/node/service.rs index 1b07d1512..b18bbb182 100644 --- a/control-plane/agents/src/bin/core/node/service.rs +++ b/control-plane/agents/src/bin/core/node/service.rs @@ -130,16 +130,16 @@ impl NodeOperations for Service { let node = self.label(id, label, overwrite).await?; Ok(node) } - /// Remove the specified label from the node. - async fn unlabel(&self, id: NodeId, label: String) -> Result { - if label.is_empty() { + /// Remove the specified label key from the node. + async fn unlabel(&self, id: NodeId, label_key: String) -> Result { + if label_key.is_empty() { return Err(SvcError::InvalidLabel { - labels: label, + labels: label_key, resource_kind: ResourceKind::Node, } .into()); } - let node = self.unlabel(id, label).await?; + let node = self.unlabel(id, label_key).await?; Ok(node) } } diff --git a/control-plane/agents/src/bin/core/node/specs.rs b/control-plane/agents/src/bin/core/node/specs.rs index 61fec5fdd..fda03be1a 100644 --- a/control-plane/agents/src/bin/core/node/specs.rs +++ b/control-plane/agents/src/bin/core/node/specs.rs @@ -11,7 +11,7 @@ use stor_port::{ transport_api::ResourceKind, types::v0::{ store::{ - node::{NodeLabels, NodeOperation, NodeSpec}, + node::{NodeLabelOp, NodeLabels, NodeOperation, NodeSpec, NodeUnLabelOp}, SpecStatus, SpecTransaction, }, transport::{NodeId, Register}, @@ -143,13 +143,13 @@ impl SpecOperationsHelper for NodeSpec { _state: &Self::State, op: Self::UpdateOp, ) -> Result<(), SvcError> { - match op.clone() { + match &op { NodeOperation::Cordon(label) | NodeOperation::Drain(label) => { // Do not allow the same label to be applied more than once. - if self.has_cordon_label(&label) { + if self.has_cordon_label(label) { Err(SvcError::CordonLabel { node_id: self.id().to_string(), - label, + label: label.clone(), }) } else { self.start_op(op); @@ -158,38 +158,35 @@ impl SpecOperationsHelper for NodeSpec { } NodeOperation::Uncordon(label) => { // Check that the label is present. - if !self.has_cordon_label(&label) { + if !self.has_cordon_label(label) { Err(SvcError::UncordonLabel { node_id: self.id().to_string(), - label, + label: label.clone(), }) } else { self.start_op(op); Ok(()) } } - NodeOperation::Label(label, overwrite) => { - // Check that the label is present. - if !overwrite && self.has_labels_key(label.keys().collect()) { - Err(SvcError::LabelExists { + NodeOperation::Label(NodeLabelOp { labels, overwrite }) => { + let (existing, conflict) = self.label_collisions(labels); + if !*overwrite && !existing.is_empty() { + Err(SvcError::LabelsExists { node_id: self.id().to_string(), - label: label - .into_iter() - .map(|(key, value)| format!("{key}: {value}")) - .collect::>() - .join(", "), + labels: format!("{existing:?}"), + conflict, }) } else { self.start_op(op); Ok(()) } } - NodeOperation::Unlabel(label) => { + NodeOperation::Unlabel(NodeUnLabelOp { label_key }) => { // Check that the label is present. - if !self.has_labels_key(vec![&label]) { + if !self.has_labels_key(label_key) { Err(SvcError::LabelNotFound { node_id: self.id().to_string(), - label, + label_key: label_key.to_string(), }) } else { self.start_op(op); diff --git a/control-plane/agents/src/common/errors.rs b/control-plane/agents/src/common/errors.rs index f96b4e5f8..309175b48 100644 --- a/control-plane/agents/src/common/errors.rs +++ b/control-plane/agents/src/common/errors.rs @@ -31,13 +31,17 @@ pub enum SvcError { NoNodes {}, #[snafu(display("Node {} is cordoned", node_id))] CordonedNode { node_id: String }, - #[snafu(display("Node {} is already labelled with label '{}'", node_id, label))] - LabelExists { node_id: String, label: String }, - #[snafu(display("Node {} doesn't have the label '{}'", node_id, label))] - LabelNotFound { node_id: String, label: String }, - #[snafu(display("Node {} is already cordoned with label '{}'", node_id, label))] + #[snafu(display("Node {node_id} is already labelled with labels '{labels}'"))] + LabelsExists { + node_id: String, + labels: String, + conflict: bool, + }, + #[snafu(display("Node {node_id} doesn't have the label key '{label_key}'"))] + LabelNotFound { node_id: String, label_key: String }, + #[snafu(display("Node {node_id} is already cordoned with label '{label}'"))] CordonLabel { node_id: String, label: String }, - #[snafu(display("Node {} does not have a cordon label '{}'", node_id, label))] + #[snafu(display("Node {node_id} does not have a cordon label '{label}'"))] UncordonLabel { node_id: String, label: String }, #[snafu(display( "Timed out after '{:?}' attempting to connect to node '{}' via gRPC endpoint '{}'", @@ -554,15 +558,19 @@ impl From for ReplyError { extra, }, - SvcError::LabelExists { .. } => ReplyError { - kind: ReplyErrorKind::AlreadyExists, + SvcError::LabelsExists { conflict, .. } => ReplyError { + kind: if conflict { + ReplyErrorKind::FailedPrecondition + } else { + ReplyErrorKind::AlreadyExists + }, resource: ResourceKind::Node, source, extra, }, SvcError::LabelNotFound { .. } => ReplyError { - kind: ReplyErrorKind::NotFound, + kind: ReplyErrorKind::FailedPrecondition, resource: ResourceKind::Node, source, extra, diff --git a/control-plane/grpc/proto/v1/node/target_node.proto b/control-plane/grpc/proto/v1/node/target_node.proto index 1208c3a69..23da3dd6b 100644 --- a/control-plane/grpc/proto/v1/node/target_node.proto +++ b/control-plane/grpc/proto/v1/node/target_node.proto @@ -165,8 +165,8 @@ message LabelNodeReply { message UnlabelNodeRequest { // Node identification string node_id = 1; - // Node cordon label - string label = 2; + // Node label key to remove + string label_key = 2; } message UnlabelNodeReply { diff --git a/control-plane/grpc/src/operations/node/client.rs b/control-plane/grpc/src/operations/node/client.rs index 67dc69c2e..badf36009 100644 --- a/control-plane/grpc/src/operations/node/client.rs +++ b/control-plane/grpc/src/operations/node/client.rs @@ -166,10 +166,10 @@ impl NodeOperations for NodeClient { } #[tracing::instrument(name = "NodeClient::unlabel", level = "debug", skip(self), err)] - async fn unlabel(&self, id: NodeId, label: String) -> Result { + async fn unlabel(&self, id: NodeId, label_key: String) -> Result { let req = UnlabelNodeRequest { node_id: id.to_string(), - label, + label_key, }; let response = self.client().unlabel_node(req).await?.into_inner(); match response.reply { diff --git a/control-plane/grpc/src/operations/node/server.rs b/control-plane/grpc/src/operations/node/server.rs index e6fd4c7f8..44bda7eb1 100644 --- a/control-plane/grpc/src/operations/node/server.rs +++ b/control-plane/grpc/src/operations/node/server.rs @@ -148,7 +148,11 @@ impl NodeGrpc for NodeServer { request: tonic::Request, ) -> Result, tonic::Status> { let req: UnlabelNodeRequest = request.into_inner(); - match self.service.unlabel(req.node_id.into(), req.label).await { + match self + .service + .unlabel(req.node_id.into(), req.label_key) + .await + { Ok(node) => Ok(Response::new(UnlabelNodeReply { reply: Some(unlabel_node_reply::Reply::Node(node.into())), })), diff --git a/control-plane/plugin/src/resources/error.rs b/control-plane/plugin/src/resources/error.rs index 932df628e..c22e26b62 100644 --- a/control-plane/plugin/src/resources/error.rs +++ b/control-plane/plugin/src/resources/error.rs @@ -1,3 +1,4 @@ +use crate::resources::node; use snafu::Snafu; /// All errors returned when resources command fails. @@ -23,14 +24,10 @@ pub enum Error { id: String, source: openapi::tower::client::Error, }, - /// Error when node label request fails. - #[snafu(display("Failed to get node {id}. Error {source}"))] - NodeLabelError { - id: String, - source: openapi::tower::client::Error, - }, - #[snafu(display("Empty node label {id}."))] - EmptyNodeLabelError { id: String }, + #[snafu(display("Invalid label format: {source}"))] + NodeLabelFormat { source: node::TopologyError }, + #[snafu(display("{source}"))] + NodeLabel { source: node::OpError }, /// Error when node uncordon request fails. #[snafu(display("Failed to uncordon node {id}. Error {source}"))] NodeUncordonError { diff --git a/control-plane/plugin/src/resources/mod.rs b/control-plane/plugin/src/resources/mod.rs index af16bfffb..4747ab129 100644 --- a/control-plane/plugin/src/resources/mod.rs +++ b/control-plane/plugin/src/resources/mod.rs @@ -104,10 +104,22 @@ pub enum DrainResources { /// The types of resources that support the 'label' operation. #[derive(clap::Subcommand, Debug)] pub enum LabelResources { - /// Label node with the given ID. + /// Adds or removes a label to or from the specified node. Node { + /// The id of the node to label/unlabel. id: NodeId, + /// The label to be added or removed from the node. + /// To add a label, please use the following format: + /// ${key}=${value} + /// To remove a label, please use the following format: + /// ${key}- + /// A label key and value must begin with a letter or number, and may contain letters, + /// numbers, hyphens, dots, and underscores, up to 63 characters each. + /// The key may contain a single slash. label: String, + /// Allow labels to be overwritten, otherwise reject label updates that overwrite existing + /// labels. + #[clap(long)] overwrite: bool, }, } @@ -116,6 +128,7 @@ pub enum LabelResources { pub enum GetDrainArgs { /// Get the drain for the node with the given ID. Node { + /// The id of the node to get the drain labels from. id: NodeId, }, Nodes, diff --git a/control-plane/plugin/src/resources/node.rs b/control-plane/plugin/src/resources/node.rs index 3172dd9e2..c01e816c0 100644 --- a/control-plane/plugin/src/resources/node.rs +++ b/control-plane/plugin/src/resources/node.rs @@ -9,9 +9,10 @@ use crate::{ rest_wrapper::RestClient, }; use async_trait::async_trait; -use openapi::models::CordonDrainState; +use openapi::{apis::StatusCode, models::CordonDrainState}; use prettytable::{Cell, Row}; use serde::Serialize; +use snafu::ResultExt; use std::time; use strum_macros::{AsRefStr, Display, EnumString}; use tokio::time::Duration; @@ -555,6 +556,109 @@ impl Drain for Node { } } +/// Errors related to node label topology formats. +#[derive(Debug, snafu::Snafu)] +pub enum TopologyError { + #[snafu(display("key must not be an empty string"))] + KeyIsEmpty {}, + #[snafu(display("value must not be an empty string"))] + ValueIsEmpty {}, + #[snafu(display("key part must no more than 63 characters"))] + KeyTooLong {}, + #[snafu(display("value part must no more than 63 characters"))] + ValueTooLong {}, + #[snafu(display("both key and value parts must start with an ascii alphanumeric character"))] + EdgesNotAlphaNum {}, + #[snafu(display("key can contain at most one / character"))] + KeySlashCount {}, + #[snafu(display( + "only ascii alphanumeric characters and (/ - _ .) are allowed for the key part" + ))] + KeyIsNotAlphaNumericPlus {}, + #[snafu(display( + "only ascii alphanumeric characters and (- _ .) are allowed for the label part" + ))] + ValueIsNotAlphaNumericPlus {}, + #[snafu(display("only a single assignment key=value is allowed"))] + LabelMultiAssign {}, + #[snafu(display( + "the supported formats are: \ + key=value for adding (example: group=a) \ + and key- for removing (example: group-)" + ))] + LabelAssign {}, +} + +/// Errors related to node label topology operation execution. +#[derive(Debug, snafu::Snafu)] +pub enum OpError { + #[snafu(display("Node {id} not unlabelled as it did not contain the label"))] + LabelNotFound { id: String }, + #[snafu(display("Node {id} not labelled as the same label already exists"))] + LabelExists { id: String }, + #[snafu(display("Node {id} not found"))] + NodeNotFound { id: String }, + #[snafu(display( + "Node {id} not labelled as the label key already exists, but with a different value and --overwrite is false" + ))] + LabelConflict { id: String }, + #[snafu(display("Failed to label node {id}. Error {source}"))] + Generic { + id: String, + source: openapi::tower::client::Error, + }, +} + +impl From for Error { + fn from(source: TopologyError) -> Self { + Self::NodeLabelFormat { source } + } +} +impl From for Error { + fn from(source: OpError) -> Self { + Self::NodeLabel { source } + } +} + +fn allowed_topology_chars(key: char) -> bool { + key.is_ascii_alphanumeric() || matches!(key, '_' | '-' | '.') +} +fn allowed_topology_tips(label: &str) -> bool { + fn allowed_topology_tips_chars(char: Option) -> bool { + char.map(|c| c.is_ascii_alphanumeric()).unwrap_or(true) + } + + allowed_topology_tips_chars(label.chars().next()) + && allowed_topology_tips_chars(label.chars().last()) +} +fn validate_topology_key(key: &str) -> Result<(), TopologyError> { + snafu::ensure!(!key.is_empty(), KeyIsEmptySnafu); + snafu::ensure!(key.len() <= 63, KeyTooLongSnafu); + snafu::ensure!(allowed_topology_tips(key), EdgesNotAlphaNumSnafu); + + snafu::ensure!( + key.chars().filter(|c| c == &'/').count() <= 1, + KeySlashCountSnafu + ); + + snafu::ensure!( + key.chars().all(|c| allowed_topology_chars(c) || c == '/'), + KeyIsNotAlphaNumericPlusSnafu + ); + + Ok(()) +} +fn validate_topology_value(value: &str) -> Result<(), TopologyError> { + snafu::ensure!(!value.is_empty(), ValueIsEmptySnafu); + snafu::ensure!(value.len() <= 63, ValueTooLongSnafu); + snafu::ensure!(allowed_topology_tips(value), EdgesNotAlphaNumSnafu); + snafu::ensure!( + value.chars().all(allowed_topology_chars), + ValueIsNotAlphaNumericPlusSnafu + ); + Ok(()) +} + #[async_trait(?Send)] impl Label for Node { type ID = NodeId; @@ -564,43 +668,69 @@ impl Label for Node { overwrite: bool, output: &utils::OutputFormat, ) -> PluginResult { - let result = match label.chars().last() { - Some(last_char) => { - // If the last character is a hyphen the it signifies that the lable needs to be - // removed. - if last_char == '-' { - RestClient::client() - .nodes_api() - .delete_node_label(id, &label[.. label.len() - 1]) - .await - } else { - RestClient::client() - .nodes_api() - .put_node_label(id, &label, Some(overwrite)) - .await - } + let result = if label.contains('=') { + let [key, value] = label.split('=').collect::>()[..] else { + return Err(TopologyError::LabelMultiAssign {}.into()); + }; + + validate_topology_key(key).context(super::error::NodeLabelFormatSnafu)?; + validate_topology_value(value).context(super::error::NodeLabelFormatSnafu)?; + match RestClient::client() + .nodes_api() + .put_node_label(id, key, value, Some(overwrite)) + .await + { + Err(source) => match source.status() { + Some(StatusCode::UNPROCESSABLE_ENTITY) if output.none() => { + Err(OpError::LabelExists { id: id.to_string() }) + } + Some(StatusCode::PRECONDITION_FAILED) if output.none() => { + Err(OpError::LabelConflict { id: id.to_string() }) + } + Some(StatusCode::NOT_FOUND) if output.none() => { + Err(OpError::NodeNotFound { id: id.to_string() }) + } + _ => Err(OpError::Generic { + id: id.to_string(), + source, + }), + }, + Ok(node) => Ok(node), } - None => { - return Err(Error::EmptyNodeLabelError { id: id.to_string() }); + } else { + snafu::ensure!(label.len() >= 2 && label.ends_with('-'), LabelAssignSnafu); + let key = &label[.. label.len() - 1]; + validate_topology_key(key)?; + match RestClient::client() + .nodes_api() + .delete_node_label(id, key) + .await + { + Err(source) => match source.status() { + Some(StatusCode::PRECONDITION_FAILED) if output.none() => { + Err(OpError::LabelNotFound { id: id.to_string() }) + } + Some(StatusCode::NOT_FOUND) if output.none() => { + Err(OpError::NodeNotFound { id: id.to_string() }) + } + _ => Err(OpError::Generic { + id: id.to_string(), + source, + }), + }, + Ok(node) => Ok(node), } - }; - - match result { - Ok(node) => match output { - OutputFormat::Yaml | OutputFormat::Json => { - // Print json or yaml based on output format. - utils::print_table(output, node.into_body()); - } - OutputFormat::None => { - // In case the output format is not specified, show a success message. - println!("Node {id} labelled successfully") - } - }, - Err(e) => { - return Err(Error::NodeLabelError { - id: id.to_string(), - source: e, - }); + }?; + let node = result.into_body(); + match output { + OutputFormat::Yaml | OutputFormat::Json => { + // Print json or yaml based on output format. + print_table(output, node); + } + OutputFormat::None => { + // In case the output format is not specified, show a success message. + let labels = node.spec.unwrap().labels.unwrap_or_default(); + println!("Node {id} labelled successfully. Current labels: {labels:?}"); } } Ok(()) diff --git a/control-plane/plugin/src/resources/utils.rs b/control-plane/plugin/src/resources/utils.rs index 264e9d488..263377696 100644 --- a/control-plane/plugin/src/resources/utils.rs +++ b/control-plane/plugin/src/resources/utils.rs @@ -132,6 +132,12 @@ pub enum OutputFormat { Yaml, Json, } +impl OutputFormat { + /// Check for non output format. + pub fn none(&self) -> bool { + matches!(self, Self::None) + } +} impl CreateRows for Vec where diff --git a/control-plane/rest/openapi-specs/v0_api_spec.yaml b/control-plane/rest/openapi-specs/v0_api_spec.yaml index a4938096d..c8dcd1fad 100644 --- a/control-plane/rest/openapi-specs/v0_api_spec.yaml +++ b/control-plane/rest/openapi-specs/v0_api_spec.yaml @@ -319,7 +319,7 @@ paths: $ref: '#/components/responses/ServerError' security: - JWT: [] - '/nodes/{id}/label/{label}': + '/nodes/{id}/label/{key}={value}': put: tags: - Nodes @@ -334,12 +334,19 @@ paths: type: string example: io-engine-1 - in: path - name: label + name: key required: true schema: type: string description: |- - The label to the node. + The key of the label to be added. + - in: path + name: value + required: true + schema: + type: string + description: |- + The value of the label to be added. - in: query name: overwrite description: |- @@ -361,6 +368,7 @@ paths: $ref: '#/components/responses/ServerError' security: - JWT: [] + '/nodes/{id}/label/{key}': delete: tags: - Nodes @@ -373,12 +381,12 @@ paths: type: string example: io-engine-1 - in: path - name: label + name: key required: true schema: type: string description: |- - The key of the label to the node. + The key of the label to be removed. responses: '200': description: OK diff --git a/control-plane/rest/service/src/v0/nodes.rs b/control-plane/rest/service/src/v0/nodes.rs index 4cb6fbe70..f1c60c92a 100644 --- a/control-plane/rest/service/src/v0/nodes.rs +++ b/control-plane/rest/service/src/v0/nodes.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; - use super::*; use grpc::operations::node::traits::NodeOperations; @@ -60,31 +58,20 @@ impl apis::actix_server::Nodes for RestApi { } async fn put_node_label( - Path((id, label)): Path<(String, String)>, + Path((id, key, value)): Path<(String, String, String)>, Query(overwrite): Query>, ) -> Result> { - let mut label_map = HashMap::new(); - let mut iter = label.split('='); let overwrite = overwrite.unwrap_or(false); - - if let (Some(key), Some(value)) = (iter.next(), iter.next()) { - label_map.insert(key.trim().to_string(), value.trim().to_string()); - } else { - return Err(RestError::from(ReplyError { - kind: ReplyErrorKind::Internal, - resource: ResourceKind::Node, - source: "put_node_label".to_string(), - extra: format!("invalid label for node resource {}", label), - })); - } - let node = client().label(id.into(), label_map, overwrite).await?; + let node = client() + .label(id.into(), [(key, value)].into(), overwrite) + .await?; Ok(node.into()) } async fn delete_node_label( - Path((id, label)): Path<(String, String)>, + Path((id, label_key)): Path<(String, String)>, ) -> Result> { - let node = client().unlabel(id.into(), label).await?; + let node = client().unlabel(id.into(), label_key).await?; Ok(node.into()) } } diff --git a/control-plane/stor-port/src/types/v0/store/node.rs b/control-plane/stor-port/src/types/v0/store/node.rs index 9c80aefb0..6a1e25996 100644 --- a/control-plane/stor-port/src/types/v0/store/node.rs +++ b/control-plane/stor-port/src/types/v0/store/node.rs @@ -273,8 +273,8 @@ impl NodeSpec { } /// Remove label from node. - pub fn unlabel(&mut self, label: String) { - self.labels.remove(&label); + pub fn unlabel(&mut self, label_key: &str) { + self.labels.remove(label_key); } /// Drain node by applying the drain label. @@ -348,15 +348,26 @@ impl NodeSpec { self.has_cordon_only_label(label) || self.has_drain_label(label) } - pub fn has_labels_key(&self, key: Vec<&String>) -> bool { - let mut found = false; - for k in key { - if self.labels.contains_key(k) { - found = true; - break; + /// Check if the node has the given topology label key. + pub fn has_labels_key(&self, key: &str) -> bool { + self.labels.contains_key(key) + } + /// Check if there are key collisions between current topology labels and the given labels. + pub fn label_collisions<'a>( + &self, + labels: &'a HashMap, + ) -> (HashMap<&'a String, &'a String>, bool) { + let mut conflict = false; + let existing = labels.iter().filter(|(key, value)| { + let Some(existing) = self.labels.get(*key) else { + return false; + }; + if &existing != value { + conflict = true; } - } - found + true + }); + (existing.collect(), conflict) } /// Returns true if it has the label in the cordon list. @@ -545,8 +556,30 @@ pub enum NodeOperation { RemoveDrainingVolumes(DrainingVolumes), RemoveAllDrainingVolumes(), SetDrained(), - Label(HashMap, bool), - Unlabel(String), + Label(NodeLabelOp), + Unlabel(NodeUnLabelOp), +} + +/// Parameter for adding node labels. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +pub struct NodeLabelOp { + pub labels: HashMap, + pub overwrite: bool, +} +impl From<(HashMap, bool)> for NodeLabelOp { + fn from((labels, overwrite): (HashMap, bool)) -> Self { + Self { labels, overwrite } + } +} +/// Parameter for removing node labels. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +pub struct NodeUnLabelOp { + pub label_key: String, +} +impl From for NodeUnLabelOp { + fn from(label_key: String) -> Self { + Self { label_key } + } } /// Operation State for a Node spec resource. @@ -587,11 +620,11 @@ impl SpecTransaction for NodeSpec { NodeOperation::SetDrained() => { self.set_drained(); } - NodeOperation::Label(label, _) => { - self.label(label); + NodeOperation::Label(NodeLabelOp { labels, .. }) => { + self.label(labels); } - NodeOperation::Unlabel(label) => { - self.unlabel(label); + NodeOperation::Unlabel(NodeUnLabelOp { label_key }) => { + self.unlabel(&label_key); } } } @@ -624,7 +657,7 @@ impl SpecTransaction for NodeSpec { NodeOperation::RemoveDrainingVolumes(_) => (false, true), NodeOperation::RemoveAllDrainingVolumes() => (false, true), NodeOperation::SetDrained() => (false, true), - NodeOperation::Label(_, _) => (false, true), + NodeOperation::Label(_) => (false, true), NodeOperation::Unlabel(_) => (false, true), } } diff --git a/nix/pkgs/openapi-generator/default.nix b/nix/pkgs/openapi-generator/default.nix index b7abaa186..0acd7d5de 100644 --- a/nix/pkgs/openapi-generator/default.nix +++ b/nix/pkgs/openapi-generator/default.nix @@ -6,7 +6,7 @@ let name = "${src_json.repo}-${src_json.rev}-src"; inherit (src_json) owner repo rev hash; }; - version = "6.1.0-${src.rev}"; + version = "6.4.0-${src.rev}"; # perform fake build to make a fixed-output derivation out of the files downloaded from maven central deps = stdenv.mkDerivation { @@ -17,7 +17,7 @@ let buildPhase = '' runHook preBuild - while mvn package -Dmaven.test.skip=true -Dmaven.repo.local=$out/.m2; [ $? = 1 ]; do + while mvn package -Dmaven.test.skip=true -Dmaven.javadoc.skip=true -Dmaven.repo.local=$out/.m2; [ $? = 1 ]; do echo "timeout, restart maven to continue downloading" done @@ -28,7 +28,7 @@ let "find $out/.m2 -type f -regex '.+\\(\\.lastUpdated\\|resolver-status\\.properties\\|_remote\\.repositories\\)' -delete"; outputHashAlgo = "sha256"; outputHashMode = "recursive"; - outputHash = if stdenv.hostPlatform.isDarwin then "sha256-9Li0uSD39ZwptIRgOXeBkLeZvfy/9w69faNDm75zdws=" else "sha256-MieSA5Y8u35H1xdP27A+YDekyyQ6CThNXOjQ82ArM7U="; + outputHash = if stdenv.hostPlatform.isDarwin then "sha256-8fk0lC3zBFgcGeBSASEDk0Zgz1xXLhJw6CSWszeSGf4=" else "sha256-mk6opj5FzELc/NambThC0K2ADJG2aRV4f8F2ZteMvMc="; }; in stdenv.mkDerivation rec { diff --git a/nix/pkgs/openapi-generator/source.json b/nix/pkgs/openapi-generator/source.json index 894cdafa7..8e4cc74bf 100644 --- a/nix/pkgs/openapi-generator/source.json +++ b/nix/pkgs/openapi-generator/source.json @@ -1,6 +1,6 @@ { "owner": "openebs", "repo": "openapi-generator", - "rev": "e67a78f33fa4f6cced0a0f90d75c9630af8a54c0", - "hash": "sha256:03wai8siap7rk3hb1pfndi08d0ch2znh650iaglvmh20qpddxy3y" + "rev": "4ccb259371ebe642a20ceeedb940589476e52aac", + "hash": "sha256-htKo1G+pvrkgZgpA62UIX+KvLkN1RTjdRO3uR8cQMhQ=" } diff --git a/scripts/python/generate-openapi-bindings.sh b/scripts/python/generate-openapi-bindings.sh index c8d71f7e9..5c20addc4 100755 --- a/scripts/python/generate-openapi-bindings.sh +++ b/scripts/python/generate-openapi-bindings.sh @@ -14,8 +14,11 @@ mkdir -p "$TARGET" tmpd=$(mktemp -d /tmp/openapi-gen-bdd-XXXXXXX) +# Work around bug: https://github.com/OpenAPITools/openapi-generator/issues/11763 +# export _JAVA_OPTIONS="--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED" +# But only required for the new python generator, not for python-prior # Generate a new openapi python client for use by the BDD tests -openapi-generator-cli generate -i "$SPEC" -g python -o "$tmpd" --additional-properties packageName="openapi" +openapi-generator-cli generate -i "$SPEC" -g python-prior -o "$tmpd" --additional-properties packageName="openapi" mv "$tmpd"/openapi/* "$TARGET" rm -rf "$tmpd" diff --git a/shell.nix b/shell.nix index cef2f4817..b1dc71953 100644 --- a/shell.nix +++ b/shell.nix @@ -71,8 +71,11 @@ mkShell { shellHook = '' ./scripts/nix/git-submodule-init.sh - pre-commit install - pre-commit install --hook commit-msg + if [ -z "$CI" ]; then + echo + pre-commit install + pre-commit install --hook commit-msg + fi ${pkgs.lib.optionalString (norust) "cowsay ${norust_moth}"} ${pkgs.lib.optionalString (norust) "echo"} diff --git a/tests/bdd/features/node/label/feature.feature b/tests/bdd/features/node/label/feature.feature index dcdaabe36..32006b95d 100644 --- a/tests/bdd/features/node/label/feature.feature +++ b/tests/bdd/features/node/label/feature.feature @@ -12,7 +12,7 @@ Feature: Label a node, this will be used while scheduling replica of volume cons Scenario: Label a node when label key already exist and overwrite is false Given a labeled node When the user attempts to label the same node with the same label key with overwrite as false - Then the node label should fail with error "AlreadyExists" + Then the node label should fail with error "PRECONDITION_FAILED" Scenario: Label a node when label key already exist and overwrite is true Given a labeled node @@ -27,4 +27,4 @@ Feature: Label a node, this will be used while scheduling replica of volume cons Scenario: Unlabel a node when the label key is not present Given a labeled node When the user issues an unlabel command with a label key that is not currently associated with the node - Then the unlabel operation for the node should fail with error NotPresent + Then the unlabel operation for the node should fail with error PRECONDITION_FAILED diff --git a/tests/bdd/features/node/label/test_label_unlabel_node.py b/tests/bdd/features/node/label/test_label_unlabel_node.py index c605c5034..0d21638cb 100644 --- a/tests/bdd/features/node/label/test_label_unlabel_node.py +++ b/tests/bdd/features/node/label/test_label_unlabel_node.py @@ -30,34 +30,47 @@ def init(): yield Deployer.stop() + @pytest.fixture(scope="function") def context(): return {} + @pytest.fixture(scope="function") def node(context): yield context["node"] -@scenario('feature.feature', 'Label a node') + +@scenario("feature.feature", "Label a node") def test_label_a_node(): """Label a node.""" -@scenario('feature.feature', 'Label a node when label key already exist and overwrite is false') + +@scenario( + "feature.feature", + "Label a node when label key already exist and overwrite is false", +) def test_label_a_node_when_label_key_already_exist_and_overwrite_is_false(): """Label a node when label key already exist and overwrite is false.""" -@scenario('feature.feature', 'Label a node when label key already exist and overwrite is true') + +@scenario( + "feature.feature", "Label a node when label key already exist and overwrite is true" +) def test_label_a_node_when_label_key_already_exist_and_overwrite_is_true(): """Label a node when label key already exist and overwrite is true.""" -@scenario('feature.feature', 'Unlabel a node') + +@scenario("feature.feature", "Unlabel a node") def test_unlabel_a_node(): """Unlabel a node.""" -@scenario('feature.feature', 'Unlabel a node when the label key is not present') + +@scenario("feature.feature", "Unlabel a node when the label key is not present") def test_unlabel_a_node_when_the_label_key_is_not_present(): """Unlabel a node when the label key is not present.""" + @given("a control plane, two Io-Engine instances, two pools") def a_control_plane_two_ioengine_instances_two_pools(): """a control plane, two Io-Engine instances, two pools.""" @@ -90,98 +103,137 @@ def an_unlabeled_node(): node = ApiClient.nodes_api().get_node(NODE_ID_1) assert not "labels" in node.spec -@given('a labeled node') + +@given("a labeled node") def a_labeled_node(a_node_labeled_1): """a labeled node.""" -@when('the user issues a label command with a label to the node') +@when("the user issues a label command with a label to the node") def the_user_issues_a_label_command_with_a_label_to_the_node(attempt_add_label_one): """the user issues a label command with a label to the node.""" -@when('the user attempts to label the same node with the same label key with overwrite as false') -def the_user_attempts_to_label_the_same_node_with_the_same_label_key_with_overwrite_as_false(context, node): +@when( + "the user attempts to label the same node with the same label key with overwrite as false" +) +def the_user_attempts_to_label_the_same_node_with_the_same_label_key_with_overwrite_as_false( + context, node +): """the user attempts to label the same node with the same label key with overwrite as false.""" attempt_add_label(node.id, LABEL1_NEW, False, context) -@when('the user attempts to label the same node with the same label key and overwrite as true') -def the_user_attempts_to_label_the_same_node_with_the_same_label_key_and_overwrite_as_true(context, node): + +@when( + "the user attempts to label the same node with the same label key and overwrite as true" +) +def the_user_attempts_to_label_the_same_node_with_the_same_label_key_and_overwrite_as_true( + context, node +): """the user attempts to label the same node with the same label key and overwrite as true.""" attempt_add_label(node.id, LABEL1_NEW, True, context) -@when('the user issues a unlabel command with a label key present as label for the node') -def the_user_issues_a_unlabel_command_with_a_label_key_present_as_label_for_the_node(context, node): + +@when( + "the user issues a unlabel command with a label key present as label for the node" +) +def the_user_issues_a_unlabel_command_with_a_label_key_present_as_label_for_the_node( + context, node +): """the user issues a unlabel command with a label key present as label for the node.""" -@when('the user issues an unlabel command with a label key that is not currently associated with the node') -def the_user_issues_an_unlabel_command_with_a_label_key_that_is_not_currently_associated_with_the_node(context, node): + +@when( + "the user issues an unlabel command with a label key that is not currently associated with the node" +) +def the_user_issues_an_unlabel_command_with_a_label_key_that_is_not_currently_associated_with_the_node( + context, node +): """the user issues an unlabel command with a label key that is not currently associated with the node.""" attempt_delete_label(node.id, LABEL_KEY_TO_DELETE_ABSENT, context) -@then('the given node should be labeled with the given label') -def the_given_node_should_be_labeled_with_the_given_label(attempt_add_label_one, context): +@then("the given node should be labeled with the given label") +def the_given_node_should_be_labeled_with_the_given_label( + attempt_add_label_one, context +): """the given node should be labeled with the given label.""" labelling_succeeds(attempt_add_label_one, context) -@then('the node label should fail with error "AlreadyExists"') -def the_node_label_should_fail_with_error_alreadyexists(node_attempt): - """the node label should fail with error "AlreadyExists".""" - assert node_attempt.status == http.HTTPStatus.UNPROCESSABLE_ENTITY - assert ApiClient.exception_to_error(node_attempt).kind == "AlreadyExists" -@then('the given node should be labeled with the new given label') -def the_given_node_should_be_labeled_with_the_new_given_label(attempt_add_label_one_with_overwrite, context): +@then('the node label should fail with error "PRECONDITION_FAILED"') +def the_node_label_should_fail_with_error_precondition_failed(node_attempt): + """the node label should fail with error "PRECONDITION_FAILED".""" + assert node_attempt.status == http.HTTPStatus.PRECONDITION_FAILED + assert ApiClient.exception_to_error(node_attempt).kind == "FailedPrecondition" + + +@then("the given node should be labeled with the new given label") +def the_given_node_should_be_labeled_with_the_new_given_label( + attempt_add_label_one_with_overwrite, context +): """the given node should be labeled with the new given label.""" labelling_succeeds(attempt_add_label_one_with_overwrite, context) -@then('the given node should remove the label with the given key') -def the_given_node_should_remove_the_label_with_the_given_key(attempt_delete_label_of_node, context): + +@then("the given node should remove the label with the given key") +def the_given_node_should_remove_the_label_with_the_given_key( + attempt_delete_label_of_node, context +): """the given node should remove the label with the given key.""" labelling_succeeds(attempt_delete_label_of_node, context) -@then('the unlabel operation for the node should fail with error NotPresent') -def the_unlabel_operation_for_the_node_should_fail_with_error_notpresent(node_attempt): - """the unlabel operation for the node should fail with error NotPresent.""" - assert node_attempt.status == http.HTTPStatus.NOT_FOUND - assert ApiClient.exception_to_error(node_attempt).kind == "NotFound" + +@then("the unlabel operation for the node should fail with error PRECONDITION_FAILED") +def the_unlabel_operation_for_the_node_should_fail_with_error_precondition_failed( + node_attempt, +): + """the unlabel operation for the node should fail with error PRECONDITION_FAILED.""" + assert node_attempt.status == http.HTTPStatus.PRECONDITION_FAILED + assert ApiClient.exception_to_error(node_attempt).kind == "FailedPrecondition" + @pytest.fixture(scope="function") def node_attempt(context): return context["attempt_result"] + @pytest.fixture def a_node_labeled_1(context): node = attempt_add_label(NODE_ID_1, LABEL1, False, context) labelling_succeeds(node, context) yield node + @pytest.fixture def attempt_add_label_one(context): yield attempt_add_label(NODE_ID_1, LABEL1, False, context) + @pytest.fixture def attempt_add_label_one_with_overwrite(context): yield attempt_add_label(NODE_ID_1, LABEL1_NEW, True, context) + def attempt_add_label(node_name, label, overwrite, context): try: - if overwrite: - node = ApiClient.nodes_api().put_node_label(node_name, label, overwrite="true") - else: - node = ApiClient.nodes_api().put_node_label(node_name, label, overwrite="false") + [key, value] = label.split("=") + overwrite = "true" if overwrite else "false" + node = ApiClient.nodes_api().put_node_label( + node_name, key, value, overwrite=overwrite + ) context["node"] = node return node except ApiException as exception: context["attempt_result"] = exception return exception - + + @pytest.fixture def attempt_delete_label_of_node(context): yield attempt_delete_label(NODE_ID_1, LABEL_KEY_TO_DELETE, context) - + def attempt_delete_label(node_name, label, context): try: node = ApiClient.nodes_api().delete_node_label(node_name, label) @@ -191,9 +243,9 @@ def attempt_delete_label(node_name, label, context): context["attempt_result"] = exception return exception + def labelling_succeeds(result, context): # raise result for exception information assert isinstance(result, Node) ApiClient.nodes_api().get_node(result.id) context["node"] = result - diff --git a/tests/bdd/features/volume/topology/test_pool_affinity_spread.py b/tests/bdd/features/volume/topology/test_pool_affinity_spread.py index 9217c6dd8..e3ffa9b16 100644 --- a/tests/bdd/features/volume/topology/test_pool_affinity_spread.py +++ b/tests/bdd/features/volume/topology/test_pool_affinity_spread.py @@ -29,7 +29,6 @@ from openapi.model.volume_spec import VolumeSpec - NODE_1_NAME = "io-engine-1" NODE_2_NAME = "io-engine-2" NODE_3_NAME = "io-engine-3" @@ -73,7 +72,7 @@ def init(): CreatePoolBody( ["malloc:///disk?size_mb=50"], labels={ - "pool2-specific-key": "pool2-specific-value", + "pool2-specific-key": "pool2-specific-value", "openebs.io/created-by": "operator-diskpool", "node": "io-engine-2", }, @@ -106,6 +105,7 @@ def init(): yield Deployer.stop() + # Fixture used to pass the volume create request between test steps. @pytest.fixture(scope="function") def create_request(): @@ -117,32 +117,55 @@ def create_request(): def replica_ctx(): return {} -@scenario('pool-affinity-spread.feature', 'suitable pools which caters volume exclusion topology labels key with empty value') + +@scenario( + "pool-affinity-spread.feature", + "suitable pools which caters volume exclusion topology labels key with empty value", +) def test_suitable_pools_which_caters_volume_exclusion_topology_labels_key_with_empty_value(): """suitable pools which caters volume exclusion topology labels key with empty value.""" -@scenario('pool-affinity-spread.feature', 'desired number of replicas cannot be created if topology inclusion labels and exclusion label as same') + +@scenario( + "pool-affinity-spread.feature", + "desired number of replicas cannot be created if topology inclusion labels and exclusion label as same", +) def test_desired_number_of_replicas_cannot_be_created_if_topology_inclusion_labels_and_exclusion_label_as_same(): """desired number of replicas cannot be created if topology inclusion labels and exclusion label as same.""" -@scenario('pool-affinity-spread.feature', 'suitable pools which caters volume exclusion topology labels') + +@scenario( + "pool-affinity-spread.feature", + "suitable pools which caters volume exclusion topology labels", +) def test_suitable_pools_which_caters_volume_exclusion_topology_labels(): """suitable pools which caters volume exclusion topology labels.""" -@scenario('pool-affinity-spread.feature', 'desired number of replicas cannot be created') + +@scenario( + "pool-affinity-spread.feature", "desired number of replicas cannot be created" +) def test_desired_number_of_replicas_cannot_be_created(): """desired number of replicas cannot be created.""" -@scenario('pool-affinity-spread.feature', 'suitable pools which contain volume topology labels') + +@scenario( + "pool-affinity-spread.feature", + "suitable pools which contain volume topology labels", +) def test_suitable_pools_which_contain_volume_topology_labels(): """suitable pools which contain volume topology labels.""" -@scenario('pool-affinity-spread.feature', 'suitable pools which contain volume topology labels key with empty value') + +@scenario( + "pool-affinity-spread.feature", + "suitable pools which contain volume topology labels key with empty value", +) def test_suitable_pools_which_contain_volume_topology_labels_key_with_empty_value(): """suitable pools which contain volume topology labels key with empty value.""" -@given('a control plane, three Io-Engine instances, three pools') +@given("a control plane, three Io-Engine instances, three pools") def a_control_plane_three_ioengine_instances_three_pools(): """a control plane, three Io-Engine instances, three pools.""" docker_client = docker.from_env() @@ -167,8 +190,13 @@ def a_control_plane_three_ioengine_instances_three_pools(): pools = ApiClient.pools_api().get_pools() assert len(pools) == 4 -@given('a request for a volume with its topology exclusion labels key with empty value matching with any pools with labels') -def a_request_for_a_volume_with_its_topology_exclusion_labels_key_with_empty_value_matching_with_any_pools_with_labels(create_request): + +@given( + "a request for a volume with its topology exclusion labels key with empty value matching with any pools with labels" +) +def a_request_for_a_volume_with_its_topology_exclusion_labels_key_with_empty_value_matching_with_any_pools_with_labels( + create_request, +): """a request for a volume with its topology exclusion labels key with empty value matching with any pools with labels.""" request = CreateVolumeBody( VolumePolicy(False), @@ -187,8 +215,12 @@ def a_request_for_a_volume_with_its_topology_exclusion_labels_key_with_empty_val create_request[CREATE_REQUEST_KEY] = request -@given('a request for a volume with its topology inclusion labels and exclusion label as same') -def a_request_for_a_volume_with_its_topology_inclusion_labels_and_exclusion_label_as_same(create_request): +@given( + "a request for a volume with its topology inclusion labels and exclusion label as same" +) +def a_request_for_a_volume_with_its_topology_inclusion_labels_and_exclusion_label_as_same( + create_request, +): """a request for a volume with its topology inclusion labels and exclusion label as same.""" request = CreateVolumeBody( VolumePolicy(False), @@ -199,7 +231,10 @@ def a_request_for_a_volume_with_its_topology_inclusion_labels_and_exclusion_labe pool_topology=PoolTopology( labelled=LabelledTopology( exclusion={"pool1-specific-key": "pool1-specific-value"}, - inclusion={"openebs.io/created-by": "operator-diskpool", "pool1-specific-key": "pool1-specific-value"}, + inclusion={ + "openebs.io/created-by": "operator-diskpool", + "pool1-specific-key": "pool1-specific-value", + }, ) ) ), @@ -207,8 +242,12 @@ def a_request_for_a_volume_with_its_topology_inclusion_labels_and_exclusion_labe create_request[CREATE_REQUEST_KEY] = request -@given('a request for a volume with its topology exclusion labels key matching with any pools with labels') -def a_request_for_a_volume_with_its_topology_exclusion_labels_key_matching_with_any_pools_with_labels(create_request): +@given( + "a request for a volume with its topology exclusion labels key matching with any pools with labels" +) +def a_request_for_a_volume_with_its_topology_exclusion_labels_key_matching_with_any_pools_with_labels( + create_request, +): """a request for a volume with its topology exclusion labels key matching with any pools with labels.""" request = CreateVolumeBody( VolumePolicy(False), @@ -227,8 +266,12 @@ def a_request_for_a_volume_with_its_topology_exclusion_labels_key_matching_with_ create_request[CREATE_REQUEST_KEY] = request -@given('a request for a volume with its topology inclusion labels not matching with any pools with labels') -def a_request_for_a_volume_with_its_topology_inclusion_labels_not_matching_with_any_pools_with_labels(create_request): +@given( + "a request for a volume with its topology inclusion labels not matching with any pools with labels" +) +def a_request_for_a_volume_with_its_topology_inclusion_labels_not_matching_with_any_pools_with_labels( + create_request, +): """a request for a volume with its topology inclusion labels not matching with any pools with labels.""" request = CreateVolumeBody( VolumePolicy(False), @@ -239,17 +282,23 @@ def a_request_for_a_volume_with_its_topology_inclusion_labels_not_matching_with_ pool_topology=PoolTopology( labelled=LabelledTopology( exclusion={}, - inclusion={"openebs.io/created-by": "operator-diskpool", - "fake-label-key": "fake-label-value", - }, + inclusion={ + "openebs.io/created-by": "operator-diskpool", + "fake-label-key": "fake-label-value", + }, ) ) ), ) create_request[CREATE_REQUEST_KEY] = request -@given('a request for a volume with its topology inclusion labels matching certain pools with same labels') -def a_request_for_a_volume_with_its_topology_inclusion_labels_matching_certain_pools_with_same_labels(create_request): + +@given( + "a request for a volume with its topology inclusion labels matching certain pools with same labels" +) +def a_request_for_a_volume_with_its_topology_inclusion_labels_matching_certain_pools_with_same_labels( + create_request, +): """a request for a volume with its topology inclusion labels matching certain pools with same labels.""" request = CreateVolumeBody( VolumePolicy(False), @@ -260,17 +309,23 @@ def a_request_for_a_volume_with_its_topology_inclusion_labels_matching_certain_p pool_topology=PoolTopology( labelled=LabelledTopology( exclusion={}, - inclusion={"openebs.io/created-by": "operator-diskpool", - "pool1-specific-key": "pool1-specific-value", - }, + inclusion={ + "openebs.io/created-by": "operator-diskpool", + "pool1-specific-key": "pool1-specific-value", + }, ) ) ), ) create_request[CREATE_REQUEST_KEY] = request -@given('a request for a volume with its topology inclusion labels key with empty value matching certain pools with same labels') -def a_request_for_a_volume_with_its_topology_inclusion_labels_key_with_empty_value_matching_certain_pools_with_same_labels(create_request): + +@given( + "a request for a volume with its topology inclusion labels key with empty value matching certain pools with same labels" +) +def a_request_for_a_volume_with_its_topology_inclusion_labels_key_with_empty_value_matching_certain_pools_with_same_labels( + create_request, +): """a request for a volume with its topology inclusion labels key with empty value matching certain pools with same labels.""" request = CreateVolumeBody( VolumePolicy(False), @@ -281,9 +336,10 @@ def a_request_for_a_volume_with_its_topology_inclusion_labels_key_with_empty_val pool_topology=PoolTopology( labelled=LabelledTopology( exclusion={}, - inclusion={"openebs.io/created-by": "operator-diskpool", - "pool3-specific-key": "", - }, + inclusion={ + "openebs.io/created-by": "operator-diskpool", + "pool3-specific-key": "", + }, ) ) ), @@ -291,9 +347,10 @@ def a_request_for_a_volume_with_its_topology_inclusion_labels_key_with_empty_val create_request[CREATE_REQUEST_KEY] = request - -@when('the volume replicas has topology inclusion labels and exclusion labels same') -def the_volume_replicas_has_topology_inclusion_labels_and_exclusion_labels_same(create_request): +@when("the volume replicas has topology inclusion labels and exclusion labels same") +def the_volume_replicas_has_topology_inclusion_labels_and_exclusion_labels_same( + create_request, +): """the volume replicas has topology inclusion labels and exclusion labels same.""" num_volume_replicas = create_request[CREATE_REQUEST_KEY]["replicas"] no_of_pools = no_of_suitable_pools( @@ -302,12 +359,17 @@ def the_volume_replicas_has_topology_inclusion_labels_and_exclusion_labels_same( ], create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "exclusion" - ] + ], ) assert num_volume_replicas > no_of_pools -@when('the volume replicas has topology exclusion labels key with empty value not matching with any pools with labels key') -def the_volume_replicas_has_topology_exclusion_labels_key_with_empty_value_not_matching_with_any_pools_with_labels_key(create_request): + +@when( + "the volume replicas has topology exclusion labels key with empty value not matching with any pools with labels key" +) +def the_volume_replicas_has_topology_exclusion_labels_key_with_empty_value_not_matching_with_any_pools_with_labels_key( + create_request, +): """the volume replicas has topology exclusion labels key with empty value not matching with any pools with labels key.""" num_volume_replicas = create_request[CREATE_REQUEST_KEY]["replicas"] if ( @@ -317,10 +379,10 @@ def the_volume_replicas_has_topology_exclusion_labels_key_with_empty_value_not_m no_of_pools = no_of_suitable_pools( create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "inclusion" - ], + ], create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "exclusion" - ] + ], ) else: # Here we are fetching all pools and comparing its length, because if we reach this part of code @@ -328,8 +390,13 @@ def the_volume_replicas_has_topology_exclusion_labels_key_with_empty_value_not_m no_of_pools = len(ApiClient.pools_api().get_pools()) assert num_volume_replicas < no_of_pools -@when('the volume replicas has topology exclusion labels key matching with any pools with labels key and not value') -def the_volume_replicas_has_topology_exclusion_labels_key_matching_with_any_pools_with_labels_key_and_not_value(create_request): + +@when( + "the volume replicas has topology exclusion labels key matching with any pools with labels key and not value" +) +def the_volume_replicas_has_topology_exclusion_labels_key_matching_with_any_pools_with_labels_key_and_not_value( + create_request, +): """the volume replicas has topology exclusion labels key matching with any pools with labels key and not value.""" num_volume_replicas = create_request[CREATE_REQUEST_KEY]["replicas"] if ( @@ -339,10 +406,10 @@ def the_volume_replicas_has_topology_exclusion_labels_key_matching_with_any_pool no_of_pools = no_of_suitable_pools( create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "inclusion" - ], + ], create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "exclusion" - ] + ], ) else: # Here we are fetching all pools and comparing its length, because if we reach this part of code @@ -351,23 +418,31 @@ def the_volume_replicas_has_topology_exclusion_labels_key_matching_with_any_pool assert num_volume_replicas == no_of_pools -@when('the volume replicas has topology inclusion labels not matching with any pools with labels') -def the_volume_replicas_has_topology_inclusion_labels_not_matching_with_any_pools_with_labels(create_request): +@when( + "the volume replicas has topology inclusion labels not matching with any pools with labels" +) +def the_volume_replicas_has_topology_inclusion_labels_not_matching_with_any_pools_with_labels( + create_request, +): """the volume replicas has topology inclusion labels not matching with any pools with labels.""" num_volume_replicas = create_request[CREATE_REQUEST_KEY]["replicas"] no_of_pools = no_of_suitable_pools( create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "inclusion" - ], + ], create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "exclusion" - ] + ], ) assert num_volume_replicas > no_of_pools -@when('the volume replicas has topology inclusion labels key with empty value matching to the labels as that of certain pools') -def the_volume_replicas_has_topology_inclusion_labels_key_with_empty_value_matching_to_the_labels_as_that_of_certain_pools(create_request): +@when( + "the volume replicas has topology inclusion labels key with empty value matching to the labels as that of certain pools" +) +def the_volume_replicas_has_topology_inclusion_labels_key_with_empty_value_matching_to_the_labels_as_that_of_certain_pools( + create_request, +): """the volume replicas has topology inclusion labels key with empty value matching to the labels as that of certain pools.""" num_volume_replicas = create_request[CREATE_REQUEST_KEY]["replicas"] if ( @@ -377,10 +452,10 @@ def the_volume_replicas_has_topology_inclusion_labels_key_with_empty_value_match no_of_pools = no_of_suitable_pools( create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "inclusion" - ], + ], create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "exclusion" - ] + ], ) else: # Here we are fetching all pools and comparing its length, because if we reach this part of code @@ -389,8 +464,12 @@ def the_volume_replicas_has_topology_inclusion_labels_key_with_empty_value_match assert num_volume_replicas < no_of_pools -@when('the volume replicas has topology inclusion labels matching to the labels as that of certain pools') -def the_volume_replicas_has_topology_inclusion_labels_matching_to_the_labels_as_that_of_certain_pools(create_request): +@when( + "the volume replicas has topology inclusion labels matching to the labels as that of certain pools" +) +def the_volume_replicas_has_topology_inclusion_labels_matching_to_the_labels_as_that_of_certain_pools( + create_request, +): """the volume replicas has topology inclusion labels matching to the labels as that of certain pools.""" num_volume_replicas = create_request[CREATE_REQUEST_KEY]["replicas"] if ( @@ -400,10 +479,10 @@ def the_volume_replicas_has_topology_inclusion_labels_matching_to_the_labels_as_ no_of_pools = no_of_suitable_pools( create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "inclusion" - ], + ], create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "exclusion" - ] + ], ) else: # Here we are fetching all pools and comparing its length, because if we reach this part of code @@ -412,7 +491,7 @@ def the_volume_replicas_has_topology_inclusion_labels_matching_to_the_labels_as_ assert num_volume_replicas == no_of_pools -@then('replica creation should fail with an insufficient storage error') +@then("replica creation should fail with an insufficient storage error") def replica_creation_should_fail_with_an_insufficient_storage_error(create_request): """replica creation should fail with an insufficient storage error.""" request = create_request[CREATE_REQUEST_KEY] @@ -427,7 +506,7 @@ def replica_creation_should_fail_with_an_insufficient_storage_error(create_reque assert len(volumes) == 0 -@then('pool labels must contain all the volume request topology labels') +@then("pool labels must contain all the volume request topology labels") def pool_labels_must_contain_all_the_volume_request_topology_labels(create_request): """pool labels must contain all the volume request topology labels.""" assert ( @@ -443,7 +522,8 @@ def pool_labels_must_contain_all_the_volume_request_topology_labels(create_reque ] ) -@then('volume creation should fail with an insufficient storage error') + +@then("volume creation should fail with an insufficient storage error") def volume_creation_should_fail_with_an_insufficient_storage_error(create_request): """volume creation should fail with an insufficient storage error.""" request = create_request[CREATE_REQUEST_KEY] @@ -458,8 +538,7 @@ def volume_creation_should_fail_with_an_insufficient_storage_error(create_reques assert len(volumes) == 0 - -@then('pool labels must not contain the volume request topology labels') +@then("pool labels must not contain the volume request topology labels") def pool_labels_must_not_contain_the_volume_request_topology_labels(create_request): """pool labels must not contain the volume request topology labels.""" assert ( @@ -469,13 +548,15 @@ def pool_labels_must_not_contain_the_volume_request_topology_labels(create_reque ], create_request[CREATE_REQUEST_KEY]["topology"]["pool_topology"]["labelled"][ "exclusion" - ] + ], ) ) == 0 -@then('volume creation should succeed with a returned volume object with topology') -def volume_creation_should_succeed_with_a_returned_volume_object_with_topology(create_request): +@then("volume creation should succeed with a returned volume object with topology") +def volume_creation_should_succeed_with_a_returned_volume_object_with_topology( + create_request, +): """volume creation should succeed with a returned volume object with topology.""" expected_spec = VolumeSpec( 1, @@ -489,9 +570,10 @@ def volume_creation_should_succeed_with_a_returned_volume_object_with_topology(c pool_topology=PoolTopology( labelled=LabelledTopology( exclusion={}, - inclusion={"openebs.io/created-by": "operator-diskpool", - "pool1-specific-key": "pool1-specific-value" - }, + inclusion={ + "openebs.io/created-by": "operator-diskpool", + "pool1-specific-key": "pool1-specific-value", + }, ) ) ), @@ -503,10 +585,15 @@ def volume_creation_should_succeed_with_a_returned_volume_object_with_topology(c assert str(volume.spec) == str(expected_spec) assert str(volume.state["status"]) == "Online" for key, value in volume.state.replica_topology.items(): - assert str( value["pool"] == POOL_1_UUID) + assert str(value["pool"] == POOL_1_UUID) + -@then('volume creation should succeed with a returned volume object with exclusion topology key same as pool key and different value') -def volume_creation_should_succeed_with_a_returned_volume_object_with_exclusion_topology_key_same_as_pool_key_and_different_value(create_request): +@then( + "volume creation should succeed with a returned volume object with exclusion topology key same as pool key and different value" +) +def volume_creation_should_succeed_with_a_returned_volume_object_with_exclusion_topology_key_same_as_pool_key_and_different_value( + create_request, +): """volume creation should succeed with a returned volume object with exclusion topology key same as pool key and different value.""" expected_spec = VolumeSpec( 1, @@ -532,11 +619,15 @@ def volume_creation_should_succeed_with_a_returned_volume_object_with_exclusion_ assert str(volume.spec) == str(expected_spec) assert str(volume.state["status"]) == "Online" for key, value in volume.state.replica_topology.items(): - assert str( value["pool"] == POOL_4_UUID) + assert str(value["pool"] == POOL_4_UUID) -@then('volume creation should succeed with a returned volume object with topology label key with empty value') -def volume_creation_should_succeed_with_a_returned_volume_object_with_topology_label_key_with_empty_value(create_request): +@then( + "volume creation should succeed with a returned volume object with topology label key with empty value" +) +def volume_creation_should_succeed_with_a_returned_volume_object_with_topology_label_key_with_empty_value( + create_request, +): """volume creation should succeed with a returned volume object with topology label key with empty value.""" expected_spec = VolumeSpec( 1, @@ -550,7 +641,10 @@ def volume_creation_should_succeed_with_a_returned_volume_object_with_topology_l pool_topology=PoolTopology( labelled=LabelledTopology( exclusion={}, - inclusion={"openebs.io/created-by": "operator-diskpool", "pool3-specific-key": ""}, + inclusion={ + "openebs.io/created-by": "operator-diskpool", + "pool3-specific-key": "", + }, ) ) ), @@ -562,11 +656,15 @@ def volume_creation_should_succeed_with_a_returned_volume_object_with_topology_l assert str(volume.spec) == str(expected_spec) assert str(volume.state["status"]) == "Online" for key, value in volume.state.replica_topology.items(): - assert str( value["pool"] == POOL_4_UUID) or str( value["pool"] == POOL_3_UUID) + assert str(value["pool"] == POOL_4_UUID) or str(value["pool"] == POOL_3_UUID) -@then('volume creation should succeed with a returned volume object with exclusion topology key different as pool key') -def volume_creation_should_succeed_with_a_returned_volume_object_with_exclusion_topology_key_different_as_pool_key(create_request): +@then( + "volume creation should succeed with a returned volume object with exclusion topology key different as pool key" +) +def volume_creation_should_succeed_with_a_returned_volume_object_with_exclusion_topology_key_different_as_pool_key( + create_request, +): """volume creation should succeed with a returned volume object with exclusion topology key different as pool key.""" expected_spec = VolumeSpec( 1, @@ -592,9 +690,12 @@ def volume_creation_should_succeed_with_a_returned_volume_object_with_exclusion_ assert str(volume.spec) == str(expected_spec) assert str(volume.state["status"]) == "Online" for key, value in volume.state.replica_topology.items(): - assert str( value["pool"] == POOL_1_UUID) or str( value["pool"] == POOL_2_UUID) + assert str(value["pool"] == POOL_1_UUID) or str(value["pool"] == POOL_2_UUID) -def no_of_suitable_pools(volume_pool_topology_inclusion_labels, volume_pool_topology_exclusion_labels): + +def no_of_suitable_pools( + volume_pool_topology_inclusion_labels, volume_pool_topology_exclusion_labels +): pool_with_labels = { POOL_1_UUID: ApiClient.pools_api().get_pool(POOL_1_UUID)["spec"]["labels"], POOL_2_UUID: ApiClient.pools_api().get_pool(POOL_2_UUID)["spec"]["labels"], @@ -603,12 +704,18 @@ def no_of_suitable_pools(volume_pool_topology_inclusion_labels, volume_pool_topo } qualified_pools = list() for pool_id, labels in pool_with_labels.items(): - if does_pool_qualify_inclusion_labels(volume_pool_topology_inclusion_labels, labels ) and does_pool_qualify_exclusion_labels(volume_pool_topology_exclusion_labels, labels ): + if does_pool_qualify_inclusion_labels( + volume_pool_topology_inclusion_labels, labels + ) and does_pool_qualify_exclusion_labels( + volume_pool_topology_exclusion_labels, labels + ): qualified_pools.append(pool_id) - return len(qualified_pools) + return len(qualified_pools) -def does_pool_qualify_inclusion_labels(volume_pool_topology_inclusion_labels, pool_labels): +def does_pool_qualify_inclusion_labels( + volume_pool_topology_inclusion_labels, pool_labels +): inc_match = True for key, value in volume_pool_topology_inclusion_labels.items(): if key in pool_labels: @@ -619,10 +726,13 @@ def does_pool_qualify_inclusion_labels(volume_pool_topology_inclusion_labels, po inc_match = False break else: - inc_match = False + inc_match = False return inc_match -def does_pool_qualify_exclusion_labels(volume_pool_topology_exclusion_labels, pool_labels): + +def does_pool_qualify_exclusion_labels( + volume_pool_topology_exclusion_labels, pool_labels +): if len(volume_pool_topology_exclusion_labels) == 0: return True for key in volume_pool_topology_exclusion_labels: @@ -633,6 +743,7 @@ def does_pool_qualify_exclusion_labels(volume_pool_topology_exclusion_labels, po return False return True + def common_labels(volume_pool_topology_labels, pool): pool_labels = pool["spec"]["labels"] count = 0