Skip to content

[fix](cloud) Some resource APIs should not check if cluster_name is empty #50148

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
gavinchou merged 2 commits into from
Apr 22, 2025
Merged

[fix](cloud) Some resource APIs should not check if cluster_name is empty #50148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75a28d6
[fix](cloud) Some APIs cannot check if cluster_name is empty
deardeng Apr 17, 2025
f464449
fix review
deardeng Apr 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cloud/src/meta-service/meta_service_resource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@ void MetaServiceImpl::alter_cluster(google::protobuf::RpcController* controller,
});
} break;
case AlterClusterRequest::ADD_NODE: {
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false);
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false, false);
if (msg != "") {
LOG(WARNING) << msg;
break;
Expand All @@ -2059,7 +2059,7 @@ void MetaServiceImpl::alter_cluster(google::protobuf::RpcController* controller,
msg = resource_mgr_->modify_nodes(instance_id, to_add, to_del);
} break;
case AlterClusterRequest::DROP_NODE: {
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false);
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false, false);
if (msg != "") {
LOG(WARNING) << msg;
break;
Expand All @@ -2082,7 +2082,7 @@ void MetaServiceImpl::alter_cluster(google::protobuf::RpcController* controller,
msg = resource_mgr_->modify_nodes(instance_id, to_add, to_del);
} break;
case AlterClusterRequest::DECOMMISSION_NODE: {
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false);
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false, false);
if (msg != "") {
LOG(WARNING) << msg;
break;
Expand Down Expand Up @@ -2144,7 +2144,7 @@ void MetaServiceImpl::alter_cluster(google::protobuf::RpcController* controller,
}
} break;
case AlterClusterRequest::NOTIFY_DECOMMISSIONED: {
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false);
resource_mgr_->check_cluster_params_valid(request->cluster(), &msg, false, false);
if (msg != "") {
LOG(WARNING) << msg;
break;
Expand Down
7 changes: 4 additions & 3 deletions cloud/src/resource-manager/resource_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ std::string ResourceManager::get_node(const std::string& cloud_unique_id,
}

bool ResourceManager::check_cluster_params_valid(const ClusterPB& cluster, std::string* err,
bool check_master_num) {
bool check_master_num, bool check_cluster_name) {
// check
if (!cluster.has_type()) {
*err = "cluster must have type arg";
Expand All @@ -155,7 +155,7 @@ bool ResourceManager::check_cluster_params_valid(const ClusterPB& cluster, std::
return false;
}

if (!cluster.has_cluster_name() || cluster.cluster_name() == "") {
if (check_cluster_name && (!cluster.has_cluster_name() || cluster.cluster_name() == "")) {
*err = "not have cluster name";
return false;
}
Expand Down Expand Up @@ -315,7 +315,8 @@ std::pair<MetaServiceCode, std::string> ResourceManager::add_cluster(const std::
std::unique_ptr<int, std::function<void(int*)>> defer(
(int*)0x01, [&msg](int*) { LOG(INFO) << "add_cluster err=" << msg; });

if (!check_cluster_params_valid(cluster.cluster, &msg, true)) {
// just check cluster_name not empty in add_cluster
if (!check_cluster_params_valid(cluster.cluster, &msg, true, true)) {
LOG(WARNING) << msg;
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT, msg);
}
Expand Down
22 changes: 20 additions & 2 deletions cloud/src/resource-manager/resource_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,31 @@ class ResourceManager {
virtual std::pair<TxnErrorCode, std::string> get_instance(std::shared_ptr<Transaction> txn,
const std::string& instance_id,
InstanceInfoPB* inst_pb);
// return err msg
/**
* Modifies the nodes associated with a given instance.
* This function allows adding and removing nodes from the instance.
*
* @param instance_id The ID of the instance to modify nodes for.
* @param to_add A vector of NodeInfo structures representing nodes to be added.
* @param to_del A vector of NodeInfo structures representing nodes to be removed.
* @return An error message if the operation fails, or an empty string for success.
*/
virtual std::string modify_nodes(const std::string& instance_id,
const std::vector<NodeInfo>& to_add,
const std::vector<NodeInfo>& to_del);

/**
* Checks the validity of the parameters for a cluster.
* This function verifies if the provided cluster parameters meet the required conditions.
*
* @param cluster The ClusterPB structure containing the cluster parameters to validate.
* @param err Output parameter to store any error message if validation fails.
* @param check_master_num Flag indicating whether to check the number of master nodes.
* @param check_cluster_name Flag indicating whether to check the cluster name is empty, just add_cluster need.
* @return True if the parameters are valid, false otherwise.
*/
bool check_cluster_params_valid(const ClusterPB& cluster, std::string* err,
bool check_master_num);
bool check_master_num, bool check_cluster_name);

/**
* Check cloud_unique_id is degraded format, and get instance_id from cloud_unique_id
Expand Down
Loading