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 73bbc02fa..df4ad5198 100644 --- a/control-plane/agents/src/bin/core/controller/resources/operations.rs +++ b/control-plane/agents/src/bin/core/controller/resources/operations.rs @@ -1,5 +1,6 @@ use crate::controller::{registry::Registry, resources::OperationGuardArc}; use agents::errors::SvcError; +use std::collections::HashMap; use stor_port::types::v0::store::volume::VolumeSpec; /// Resource Cordon Operations. @@ -22,6 +23,27 @@ pub(crate) trait ResourceCordon { ) -> Result; } +/// Resource Label Operations. +#[async_trait::async_trait] +pub(crate) trait ResourceLabel { + type LabelOutput: Sync + Send + Sized; + type UnlabelOutput: Sync + Send + Sized; + + /// Label the resource. + async fn label( + &mut self, + registry: &Registry, + label: HashMap, + overwrite: bool, + ) -> Result; + /// Remove label from the resource. + async fn unlabel( + &mut self, + registry: &Registry, + label: String, + ) -> Result; +} + /// Resource Drain Operations. #[async_trait::async_trait] pub(crate) trait ResourceDrain { diff --git a/control-plane/agents/src/bin/core/node/operations.rs b/control-plane/agents/src/bin/core/node/operations.rs index 743fd2fed..40d9703a7 100644 --- a/control-plane/agents/src/bin/core/node/operations.rs +++ b/control-plane/agents/src/bin/core/node/operations.rs @@ -5,11 +5,12 @@ use stor_port::types::v0::store::node::{DrainingVolumes, NodeOperation, NodeSpec use crate::controller::{ registry::Registry, resources::{ - operations::{ResourceCordon, ResourceDrain}, + operations::{ResourceCordon, ResourceDrain, ResourceLabel}, operations_helper::GuardedOperationsHelper, OperationGuardArc, }, }; +use std::collections::HashMap; /// Resource Cordon Operations. #[async_trait::async_trait] @@ -48,6 +49,48 @@ impl ResourceCordon for OperationGuardArc { } } +/// Resource Label Operations. +#[async_trait::async_trait] +impl ResourceLabel for OperationGuardArc { + type LabelOutput = NodeSpec; + type UnlabelOutput = NodeSpec; + + /// Label a node via operation guard functions. + async fn label( + &mut self, + registry: &Registry, + label: HashMap, + overwrite: bool, + ) -> Result { + let cloned_node_spec = self.lock().clone(); + let spec_clone = self + .start_update( + registry, + &cloned_node_spec, + NodeOperation::Label(label, overwrite), + ) + .await?; + + self.complete_update(registry, Ok(()), spec_clone).await?; + Ok(self.as_ref().clone()) + } + + /// Unlabel a node via operation guard functions. + async fn unlabel( + &mut self, + registry: &Registry, + label: String, + ) -> Result { + let cloned_node_spec = self.lock().clone(); + let spec_clone = self + .start_update(registry, &cloned_node_spec, NodeOperation::Unlabel(label)) + .await?; + + self.complete_update(registry, Ok(()), spec_clone).await?; + Ok(self.as_ref().clone()) + } +} + /// Resource Drain Operations. #[async_trait::async_trait] impl ResourceDrain for OperationGuardArc { diff --git a/control-plane/agents/src/bin/core/node/service.rs b/control-plane/agents/src/bin/core/node/service.rs index 8eb3fa40c..1b07d1512 100644 --- a/control-plane/agents/src/bin/core/node/service.rs +++ b/control-plane/agents/src/bin/core/node/service.rs @@ -3,7 +3,7 @@ use crate::controller::{ reconciler::PollTriggerEvent, registry::Registry, resources::{ - operations::{ResourceCordon, ResourceDrain}, + operations::{ResourceCordon, ResourceDrain, ResourceLabel}, operations_helper::ResourceSpecsLocked, }, wrapper::NodeWrapper, @@ -119,6 +119,29 @@ impl NodeOperations for Service { let node = self.drain(id, label).await?; Ok(node) } + + /// Apply the label to node. + async fn label( + &self, + id: NodeId, + label: HashMap, + overwrite: bool, + ) -> Result { + 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() { + return Err(SvcError::InvalidLabel { + labels: label, + resource_kind: ResourceKind::Node, + } + .into()); + } + let node = self.unlabel(id, label).await?; + Ok(node) + } } #[tonic::async_trait] @@ -375,4 +398,25 @@ impl Service { Ok(Node::new(id, Some(spec), state)) } + + /// Label the specified node. + async fn label( + &self, + id: NodeId, + label: HashMap, + overwrite: bool, + ) -> Result { + let mut guarded_node = self.specs().guarded_node(&id).await?; + let spec = guarded_node.label(&self.registry, label, overwrite).await?; + let state = self.registry.node_state(&id).await.ok(); + Ok(Node::new(id, Some(spec), state)) + } + + /// Remove the specified label from the specified node. + async fn unlabel(&self, id: NodeId, label: String) -> Result { + let mut guarded_node = self.specs().guarded_node(&id).await?; + let spec = guarded_node.unlabel(&self.registry, label).await?; + let state = self.registry.node_state(&id).await.ok(); + Ok(Node::new(id, Some(spec), state)) + } } diff --git a/control-plane/agents/src/bin/core/node/specs.rs b/control-plane/agents/src/bin/core/node/specs.rs index 88613ad12..61fec5fdd 100644 --- a/control-plane/agents/src/bin/core/node/specs.rs +++ b/control-plane/agents/src/bin/core/node/specs.rs @@ -168,6 +168,34 @@ impl SpecOperationsHelper for NodeSpec { Ok(()) } } + NodeOperation::Label(label, overwrite) => { + // Check that the label is present. + if !overwrite && self.has_labels_key(label.keys().collect()) { + Err(SvcError::LabelExists { + node_id: self.id().to_string(), + label: label + .into_iter() + .map(|(key, value)| format!("{key}: {value}")) + .collect::>() + .join(", "), + }) + } else { + self.start_op(op); + Ok(()) + } + } + NodeOperation::Unlabel(label) => { + // Check that the label is present. + if !self.has_labels_key(vec![&label]) { + Err(SvcError::LabelNotFound { + node_id: self.id().to_string(), + label, + }) + } else { + self.start_op(op); + Ok(()) + } + } _ => { self.start_op(op); Ok(()) diff --git a/control-plane/agents/src/common/errors.rs b/control-plane/agents/src/common/errors.rs index 0de3f8876..f96b4e5f8 100644 --- a/control-plane/agents/src/common/errors.rs +++ b/control-plane/agents/src/common/errors.rs @@ -31,6 +31,10 @@ 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))] CordonLabel { node_id: String, label: String }, #[snafu(display("Node {} does not have a cordon label '{}'", node_id, label))] @@ -181,6 +185,11 @@ pub enum SvcError { Internal { details: String }, #[snafu(display("Invalid Arguments"))] InvalidArguments {}, + #[snafu(display("Invalid {}, labels: {} ", resource_kind, labels))] + InvalidLabel { + labels: String, + resource_kind: ResourceKind, + }, #[snafu(display("Multiple nexuses not supported"))] MultipleNexuses {}, #[snafu(display("Storage Error: {}", source))] @@ -545,6 +554,20 @@ impl From for ReplyError { extra, }, + SvcError::LabelExists { .. } => ReplyError { + kind: ReplyErrorKind::AlreadyExists, + resource: ResourceKind::Node, + source, + extra, + }, + + SvcError::LabelNotFound { .. } => ReplyError { + kind: ReplyErrorKind::NotFound, + resource: ResourceKind::Node, + source, + extra, + }, + SvcError::UncordonLabel { .. } => ReplyError { kind: ReplyErrorKind::FailedPrecondition, resource: ResourceKind::Node, @@ -717,6 +740,12 @@ impl From for ReplyError { source, extra, }, + SvcError::InvalidLabel { resource_kind, .. } => ReplyError { + kind: ReplyErrorKind::InvalidArgument, + resource: resource_kind, + source, + extra, + }, SvcError::MultipleNexuses { .. } => ReplyError { kind: ReplyErrorKind::InvalidArgument, resource: ResourceKind::Unknown, diff --git a/control-plane/grpc/proto/v1/node/target_node.proto b/control-plane/grpc/proto/v1/node/target_node.proto index cc31eeb55..1208c3a69 100644 --- a/control-plane/grpc/proto/v1/node/target_node.proto +++ b/control-plane/grpc/proto/v1/node/target_node.proto @@ -146,6 +146,36 @@ message DrainState { repeated string drain_labels = 2; } +message LabelNodeRequest { + // Node identification + string node_id = 1; + // Node label map + common.StringMapValue label = 2; + // Overwrite an existing key + bool overwrite = 3; +} + +message LabelNodeReply { + oneof reply { + Node node = 1; + common.ReplyError error = 2; + } +} + +message UnlabelNodeRequest { + // Node identification + string node_id = 1; + // Node cordon label + string label = 2; +} + +message UnlabelNodeReply { + oneof reply { + Node node = 1; + common.ReplyError error = 2; + } +} + service NodeGrpc { rpc GetNodes (GetNodesRequest) returns (GetNodesReply) {} rpc GetBlockDevices (blockdevice.GetBlockDevicesRequest) returns (blockdevice.GetBlockDevicesReply) {} @@ -153,4 +183,6 @@ service NodeGrpc { rpc CordonNode (CordonNodeRequest) returns (CordonNodeReply) {} rpc UncordonNode (UncordonNodeRequest) returns (UncordonNodeReply) {} rpc DrainNode (DrainNodeRequest) returns (DrainNodeReply) {} + rpc LabelNode (LabelNodeRequest) returns (LabelNodeReply) {} + rpc UnlabelNode (UnlabelNodeRequest) returns (UnlabelNodeReply) {} } diff --git a/control-plane/grpc/src/operations/node/client.rs b/control-plane/grpc/src/operations/node/client.rs index 0b5b918c0..67dc69c2e 100644 --- a/control-plane/grpc/src/operations/node/client.rs +++ b/control-plane/grpc/src/operations/node/client.rs @@ -3,13 +3,14 @@ use crate::{ common::NodeFilter, context::{Client, Context, TracedChannel}, node::{ - cordon_node_reply, drain_node_reply, get_nodes_reply, get_nodes_request, - node_grpc_client::NodeGrpcClient, uncordon_node_reply, CordonNodeRequest, DrainNodeRequest, - GetNodesRequest, ProbeRequest, UncordonNodeRequest, + cordon_node_reply, drain_node_reply, get_nodes_reply, get_nodes_request, label_node_reply, + node_grpc_client::NodeGrpcClient, uncordon_node_reply, unlabel_node_reply, + CordonNodeRequest, DrainNodeRequest, GetNodesRequest, LabelNodeRequest, ProbeRequest, + UncordonNodeRequest, UnlabelNodeRequest, }, operations::node::traits::{GetBlockDeviceInfo, NodeOperations}, }; -use std::{convert::TryFrom, ops::Deref}; +use std::{collections::HashMap, convert::TryFrom, ops::Deref}; use stor_port::{ transport_api::{ v0::{BlockDevices, Nodes}, @@ -141,4 +142,42 @@ impl NodeOperations for NodeClient { None => Err(ReplyError::invalid_response(ResourceKind::Node)), } } + + #[tracing::instrument(name = "NodeClient::label", level = "debug", skip(self), err)] + async fn label( + &self, + id: NodeId, + label: HashMap, + overwrite: bool, + ) -> Result { + let req = LabelNodeRequest { + node_id: id.to_string(), + label: Some(crate::common::StringMapValue { value: label }), + overwrite, + }; + let response = self.client().label_node(req).await?.into_inner(); + match response.reply { + Some(label_node_reply) => match label_node_reply { + label_node_reply::Reply::Node(node) => Ok(Node::try_from(node)?), + label_node_reply::Reply::Error(err) => Err(err.into()), + }, + None => Err(ReplyError::invalid_response(ResourceKind::Node)), + } + } + + #[tracing::instrument(name = "NodeClient::unlabel", level = "debug", skip(self), err)] + async fn unlabel(&self, id: NodeId, label: String) -> Result { + let req = UnlabelNodeRequest { + node_id: id.to_string(), + label, + }; + let response = self.client().unlabel_node(req).await?.into_inner(); + match response.reply { + Some(unlabel_node_reply) => match unlabel_node_reply { + unlabel_node_reply::Reply::Node(node) => Ok(Node::try_from(node)?), + unlabel_node_reply::Reply::Error(err) => Err(err.into()), + }, + None => Err(ReplyError::invalid_response(ResourceKind::Node)), + } + } } diff --git a/control-plane/grpc/src/operations/node/server.rs b/control-plane/grpc/src/operations/node/server.rs index f64bace79..e6fd4c7f8 100644 --- a/control-plane/grpc/src/operations/node/server.rs +++ b/control-plane/grpc/src/operations/node/server.rs @@ -2,11 +2,12 @@ use crate::{ blockdevice::{get_block_devices_reply, GetBlockDevicesReply, GetBlockDevicesRequest}, node, node::{ - cordon_node_reply, drain_node_reply, get_nodes_reply, + cordon_node_reply, drain_node_reply, get_nodes_reply, label_node_reply, node_grpc_server::{NodeGrpc, NodeGrpcServer}, - uncordon_node_reply, CordonNodeReply, CordonNodeRequest, DrainNodeReply, DrainNodeRequest, - GetNodesReply, GetNodesRequest, ProbeRequest, ProbeResponse, UncordonNodeReply, - UncordonNodeRequest, + uncordon_node_reply, unlabel_node_reply, CordonNodeReply, CordonNodeRequest, + DrainNodeReply, DrainNodeRequest, GetNodesReply, GetNodesRequest, LabelNodeReply, + LabelNodeRequest, ProbeRequest, ProbeResponse, UncordonNodeReply, UncordonNodeRequest, + UnlabelNodeReply, UnlabelNodeRequest, }, operations::node::traits::NodeOperations, }; @@ -117,4 +118,43 @@ impl NodeGrpc for NodeServer { })), } } + + async fn label_node( + &self, + request: tonic::Request, + ) -> Result, tonic::Status> { + let req: LabelNodeRequest = request.into_inner(); + + let label_map = match req.label { + Some(labels) => labels.value, + None => return Err(tonic::Status::invalid_argument("Label is required")), + }; + match self + .service + .label(req.node_id.into(), label_map, req.overwrite) + .await + { + Ok(node) => Ok(Response::new(LabelNodeReply { + reply: Some(label_node_reply::Reply::Node(node.into())), + })), + Err(err) => Ok(Response::new(LabelNodeReply { + reply: Some(label_node_reply::Reply::Error(err.into())), + })), + } + } + + async fn unlabel_node( + &self, + request: tonic::Request, + ) -> Result, tonic::Status> { + let req: UnlabelNodeRequest = request.into_inner(); + match self.service.unlabel(req.node_id.into(), req.label).await { + Ok(node) => Ok(Response::new(UnlabelNodeReply { + reply: Some(unlabel_node_reply::Reply::Node(node.into())), + })), + Err(err) => Ok(Response::new(UnlabelNodeReply { + reply: Some(unlabel_node_reply::Reply::Error(err.into())), + })), + } + } } diff --git a/control-plane/grpc/src/operations/node/traits.rs b/control-plane/grpc/src/operations/node/traits.rs index 8520f13ba..342f4a0dc 100644 --- a/control-plane/grpc/src/operations/node/traits.rs +++ b/control-plane/grpc/src/operations/node/traits.rs @@ -2,7 +2,7 @@ use crate::{ blockdevice, blockdevice::GetBlockDevicesRequest, context::Context, node, node::get_nodes_request, }; -use std::{convert::TryFrom, str::FromStr}; +use std::{collections::HashMap, convert::TryFrom, str::FromStr}; use stor_port::{ transport_api::{ v0::{BlockDevices, Nodes}, @@ -42,6 +42,15 @@ pub trait NodeOperations: Send + Sync { async fn uncordon(&self, id: NodeId, label: String) -> Result; /// Drain the node with the given ID and associate the label with the draining node. async fn drain(&self, id: NodeId, label: String) -> Result; + /// Associate the labels with the given node. + async fn label( + &self, + id: NodeId, + label: HashMap, + overwrite: bool, + ) -> Result; + /// Remove label from the a given node. + async fn unlabel(&self, id: NodeId, label: String) -> Result; } impl TryFrom for Node { diff --git a/control-plane/plugin/src/lib.rs b/control-plane/plugin/src/lib.rs index 418f73322..b992300f5 100644 --- a/control-plane/plugin/src/lib.rs +++ b/control-plane/plugin/src/lib.rs @@ -3,6 +3,9 @@ extern crate prettytable; #[macro_use] extern crate lazy_static; +use operations::Label; +use resources::LabelResources; + use crate::{ operations::{ Cordoning, Drain, Get, GetBlockDevices, GetSnapshots, List, ListExt, Operations, @@ -84,6 +87,7 @@ impl ExecuteOperation for Operations { Operations::Scale(resource) => resource.execute(cli_args).await, Operations::Cordon(resource) => resource.execute(cli_args).await, Operations::Uncordon(resource) => resource.execute(cli_args).await, + Operations::Label(resource) => resource.execute(cli_args).await, } } } @@ -200,3 +204,18 @@ impl ExecuteOperation for UnCordonResources { } } } + +#[async_trait::async_trait(?Send)] +impl ExecuteOperation for LabelResources { + type Args = CliArgs; + type Error = crate::resources::Error; + async fn execute(&self, cli_args: &CliArgs) -> PluginResult { + match self { + LabelResources::Node { + id, + label, + overwrite, + } => node::Node::label(id, label.to_string(), *overwrite, &cli_args.output).await, + } + } +} diff --git a/control-plane/plugin/src/operations.rs b/control-plane/plugin/src/operations.rs index 6efda4c9c..645cf1eb8 100644 --- a/control-plane/plugin/src/operations.rs +++ b/control-plane/plugin/src/operations.rs @@ -1,6 +1,6 @@ use crate::resources::{ - error::Error, utils, CordonResources, DrainResources, GetResources, ScaleResources, - UnCordonResources, + error::Error, utils, CordonResources, DrainResources, GetResources, LabelResources, + ScaleResources, UnCordonResources, }; use async_trait::async_trait; @@ -25,6 +25,9 @@ pub enum Operations { /// 'Uncordon' resources. #[clap(subcommand)] Uncordon(UnCordonResources), + /// 'Label' resources. + #[clap(subcommand)] + Label(LabelResources), } /// Drain trait. @@ -40,6 +43,19 @@ pub trait Drain { ) -> PluginResult; } +/// Label trait. +/// To be implemented by resources which support the 'label' operation. +#[async_trait(?Send)] +pub trait Label { + type ID; + async fn label( + id: &Self::ID, + label: String, + overwrite: bool, + output: &utils::OutputFormat, + ) -> PluginResult; +} + /// List trait. /// To be implemented by resources which support the 'list' operation. #[async_trait(?Send)] diff --git a/control-plane/plugin/src/resources/error.rs b/control-plane/plugin/src/resources/error.rs index fd60ee084..932df628e 100644 --- a/control-plane/plugin/src/resources/error.rs +++ b/control-plane/plugin/src/resources/error.rs @@ -23,6 +23,14 @@ 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 }, /// 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 e91a74b26..af16bfffb 100644 --- a/control-plane/plugin/src/resources/mod.rs +++ b/control-plane/plugin/src/resources/mod.rs @@ -101,6 +101,17 @@ pub enum DrainResources { Node(DrainNodeArgs), } +/// The types of resources that support the 'label' operation. +#[derive(clap::Subcommand, Debug)] +pub enum LabelResources { + /// Label node with the given ID. + Node { + id: NodeId, + label: String, + overwrite: bool, + }, +} + #[derive(clap::Subcommand, Debug)] pub enum GetDrainArgs { /// Get the drain for the node with the given ID. diff --git a/control-plane/plugin/src/resources/node.rs b/control-plane/plugin/src/resources/node.rs index 8b828f2bc..3172dd9e2 100644 --- a/control-plane/plugin/src/resources/node.rs +++ b/control-plane/plugin/src/resources/node.rs @@ -1,5 +1,5 @@ use crate::{ - operations::{Cordoning, Drain, Get, List, PluginResult}, + operations::{Cordoning, Drain, Get, Label, List, PluginResult}, resources::{ error::Error, utils, @@ -554,3 +554,55 @@ impl Drain for Node { Ok(()) } } + +#[async_trait(?Send)] +impl Label for Node { + type ID = NodeId; + async fn label( + id: &Self::ID, + label: String, + 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 + } + } + None => { + return Err(Error::EmptyNodeLabelError { id: id.to_string() }); + } + }; + + 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, + }); + } + } + Ok(()) + } +} diff --git a/control-plane/rest/openapi-specs/v0_api_spec.yaml b/control-plane/rest/openapi-specs/v0_api_spec.yaml index 661e7c090..a4938096d 100644 --- a/control-plane/rest/openapi-specs/v0_api_spec.yaml +++ b/control-plane/rest/openapi-specs/v0_api_spec.yaml @@ -319,6 +319,79 @@ paths: $ref: '#/components/responses/ServerError' security: - JWT: [] + '/nodes/{id}/label/{label}': + put: + tags: + - Nodes + operationId: put_node_label + description: |- + Add labels to node. + parameters: + - in: path + name: id + required: true + schema: + type: string + example: io-engine-1 + - in: path + name: label + required: true + schema: + type: string + description: |- + The label to the node. + - in: query + name: overwrite + description: |- + Overwrite existing label if the label key exists. + required: false + schema: + type: boolean + default: false + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Node' + '4XX': + $ref: '#/components/responses/ClientError' + '5XX': + $ref: '#/components/responses/ServerError' + security: + - JWT: [] + delete: + tags: + - Nodes + operationId: delete_node_label + parameters: + - in: path + name: id + required: true + schema: + type: string + example: io-engine-1 + - in: path + name: label + required: true + schema: + type: string + description: |- + The key of the label to the node. + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Node' + '4XX': + $ref: '#/components/responses/ClientError' + '5XX': + $ref: '#/components/responses/ServerError' + security: + - JWT: [] '/nodes/{id}/nexuses': get: tags: @@ -2636,6 +2709,11 @@ components: id: description: node ID $ref: '#/components/schemas/NodeId' + labels: + description: labels to be set on the node + type: object + additionalProperties: + type: string cordondrainstate: description: the drain state allOf: diff --git a/control-plane/rest/service/src/v0/nodes.rs b/control-plane/rest/service/src/v0/nodes.rs index e1b1d6873..4cb6fbe70 100644 --- a/control-plane/rest/service/src/v0/nodes.rs +++ b/control-plane/rest/service/src/v0/nodes.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use super::*; use grpc::operations::node::traits::NodeOperations; @@ -56,6 +58,35 @@ impl apis::actix_server::Nodes for RestApi { let node = client().drain(id.into(), label).await?; Ok(node.into()) } + + async fn put_node_label( + Path((id, label)): Path<(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?; + Ok(node.into()) + } + + async fn delete_node_label( + Path((id, label)): Path<(String, String)>, + ) -> Result> { + let node = client().unlabel(id.into(), label).await?; + Ok(node.into()) + } } /// returns node from node option and returns an error on non existence diff --git a/control-plane/rest/tests/v0_test.rs b/control-plane/rest/tests/v0_test.rs index 765f53c96..8200f18f5 100644 --- a/control-plane/rest/tests/v0_test.rs +++ b/control-plane/rest/tests/v0_test.rs @@ -97,6 +97,7 @@ async fn client_test(cluster: &Cluster, auth: &bool) { "{}:10124", cluster.composer().container_ip(cluster.node(0).as_str()) ), + labels: None, cordondrainstate: None, node_nqn: Some(HostNqn::from_nodename(&io_engine1.to_string()).to_string()), }), 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 3da44165a..9c80aefb0 100644 --- a/control-plane/stor-port/src/types/v0/store/node.rs +++ b/control-plane/stor-port/src/types/v0/store/node.rs @@ -267,6 +267,16 @@ impl NodeSpec { self.resolve(); } + /// Label node by applying the label. + pub fn label(&mut self, label: HashMap) { + self.labels.extend(label); + } + + /// Remove label from node. + pub fn unlabel(&mut self, label: String) { + self.labels.remove(&label); + } + /// Drain node by applying the drain label. pub fn set_drain(&mut self, label: String) { // the the node has the label, return with an error @@ -338,6 +348,17 @@ 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; + } + } + found + } + /// Returns true if it has the label in the cordon list. pub fn has_cordon_only_label(&self, label: &str) -> bool { match &self.cordon_drain_state { @@ -417,9 +438,15 @@ impl NodeSpec { impl From for models::NodeSpec { fn from(src: NodeSpec) -> Self { + let labels = if src.labels.is_empty() { + None + } else { + Some(src.labels) + }; Self::new_all( src.endpoint.to_string(), - src.id, + src.id.clone(), + labels, src.cordon_drain_state.into_opt(), src.node_nqn.into_opt(), ) @@ -518,6 +545,8 @@ pub enum NodeOperation { RemoveDrainingVolumes(DrainingVolumes), RemoveAllDrainingVolumes(), SetDrained(), + Label(HashMap, bool), + Unlabel(String), } /// Operation State for a Node spec resource. @@ -558,6 +587,12 @@ impl SpecTransaction for NodeSpec { NodeOperation::SetDrained() => { self.set_drained(); } + NodeOperation::Label(label, _) => { + self.label(label); + } + NodeOperation::Unlabel(label) => { + self.unlabel(label); + } } } self.clear_op(); @@ -589,6 +624,8 @@ impl SpecTransaction for NodeSpec { NodeOperation::RemoveDrainingVolumes(_) => (false, true), NodeOperation::RemoveAllDrainingVolumes() => (false, true), NodeOperation::SetDrained() => (false, true), + NodeOperation::Label(_, _) => (false, true), + NodeOperation::Unlabel(_) => (false, true), } } diff --git a/tests/bdd/features/drain/node/__init__.py b/tests/bdd/features/node/drain/__init__.py similarity index 100% rename from tests/bdd/features/drain/node/__init__.py rename to tests/bdd/features/node/drain/__init__.py diff --git a/tests/bdd/features/drain/node/feature.feature b/tests/bdd/features/node/drain/feature.feature similarity index 100% rename from tests/bdd/features/drain/node/feature.feature rename to tests/bdd/features/node/drain/feature.feature diff --git a/tests/bdd/features/node/label/feature.feature b/tests/bdd/features/node/label/feature.feature new file mode 100644 index 000000000..dcdaabe36 --- /dev/null +++ b/tests/bdd/features/node/label/feature.feature @@ -0,0 +1,30 @@ +Feature: Label a node, this will be used while scheduling replica of volume considering the + node topology + + Background: + Given a control plane, two Io-Engine instances, two pools + + Scenario: Label a node + Given an unlabeled node + When the user issues a label command with a label to the node + Then the given node should be labeled with the given label + + 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" + + Scenario: Label a node when label key already exist and overwrite is true + Given a labeled node + When the user attempts to label the same node with the same label key and overwrite as true + Then the given node should be labeled with the new given label + + Scenario: Unlabel a node + Given a labeled node + When the user issues a unlabel command with a label key present as label for the node + Then the given node should remove the label with the given key + + 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 diff --git a/tests/bdd/features/node/label/test_label_unlabel_node.py b/tests/bdd/features/node/label/test_label_unlabel_node.py new file mode 100644 index 000000000..c605c5034 --- /dev/null +++ b/tests/bdd/features/node/label/test_label_unlabel_node.py @@ -0,0 +1,199 @@ +"""Label a node, this will be used while scheduling replica of volume considering the feature tests.""" + +import docker +import pytest +import sys +import http + +from pytest_bdd import given, scenario, then, when +from common.deployer import Deployer +from common.apiclient import ApiClient +from common.docker import Docker +from openapi.model.node import Node +from openapi.exceptions import ApiException + +NUM_IO_ENGINES = 2 +LABEL1 = "KEY1=VALUE1" +LABEL1_NEW = "KEY1=NEW_LABEL" +LABEL1_MAP = "{'KEY1': 'VALUE1'}" +LABEL2 = "KEY2=VALUE2" +LABEL_1_NEW = "KEY1=NEW_LABEL" +LABEL_KEY_TO_DELETE = "KEY1" +LABEL_KEY_TO_DELETE_ABSENT = "ABSENT_KEY" +NODE_ID_1 = "io-engine-1" + + +# Fixtures +@pytest.fixture(autouse=True) +def init(): + Deployer.start(NUM_IO_ENGINES) + 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') +def test_label_a_node(): + """Label a node.""" + +@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') +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') +def test_unlabel_a_node(): + """Unlabel a node.""" + +@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.""" + docker_client = docker.from_env() + + # The control plane comprises the core agents, rest server and etcd instance. + for component in ["core", "rest", "etcd"]: + Docker.check_container_running(component) + + # Check all Io-Engine instances are running + try: + io_engines = docker_client.containers.list( + all=True, filters={"name": "io-engine"} + ) + + except docker.errors.NotFound: + raise Exception("No Io-Engine instances") + + for io_engine in io_engines: + Docker.check_container_running(io_engine.attrs["Name"]) + + # Check for a nodes + nodes = ApiClient.nodes_api().get_nodes() + assert len(nodes) == 2 + + +@given("an unlabeled node") +def an_unlabeled_node(): + """an unlabeled node.""" + node = ApiClient.nodes_api().get_node(NODE_ID_1) + assert not "labels" in node.spec + +@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') +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): + """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): + """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): + """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): + """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): + """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): + """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): + """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" + +@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") + 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) + context["node"] = node + return node + except ApiException as exception: + 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 +