From ddaa65f4f362f227d1db0bbf8eb44ad5c35b4547 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 25 Feb 2025 10:35:12 -0300 Subject: [PATCH] Address review comments --- .../user/userdata/DeleteCniConfigurationCmd.java | 4 ++-- .../cloudstack/api/response/TemplateResponse.java | 2 +- .../cluster/KubernetesClusterManagerImpl.java | 3 +-- .../cluster/KubernetesClusterService.java | 14 +++++++------- .../KubernetesClusterStartWorker.java | 6 +++--- .../cluster/AddNodesToKubernetesClusterCmd.java | 9 +++------ .../cluster/CreateKubernetesClusterCmd.java | 12 ++++++++---- .../RemoveNodesFromKubernetesClusterCmd.java | 5 ++--- .../cluster/ScaleKubernetesClusterCmd.java | 3 ++- .../main/java/com/cloud/api/ApiResponseHelper.java | 2 +- 10 files changed, 30 insertions(+), 30 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteCniConfigurationCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteCniConfigurationCmd.java index 819b3761ddb..8faa612d3d9 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteCniConfigurationCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteCniConfigurationCmd.java @@ -34,7 +34,7 @@ import org.apache.logging.log4j.Logger; authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class DeleteCniConfigurationCmd extends DeleteUserDataCmd { - public static final Logger logger = LogManager.getLogger(DeleteUserDataCmd.class.getName()); + public static final Logger logger = LogManager.getLogger(DeleteCniConfigurationCmd.class.getName()); ///////////////////////////////////////////////////// @@ -49,7 +49,7 @@ public class DeleteCniConfigurationCmd extends DeleteUserDataCmd { response.setSuccess(result); setResponseObject(response); } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete userdata"); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete CNI configuration"); } } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/TemplateResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/TemplateResponse.java index dac3c0554a3..a6c624e2678 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/TemplateResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/TemplateResponse.java @@ -210,7 +210,7 @@ public class TemplateResponse extends BaseResponseWithTagInformation implements @SerializedName(ApiConstants.FOR_CKS) @Param(description = "If true it indicates that the template can be used for CKS cluster deployments", - since = "4.20") + since = "4.21.0") private Boolean forCks; @SerializedName(ApiConstants.DEPLOY_AS_IS_DETAILS) diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index 052a6e8d3ab..33fd6bb1665 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -805,8 +805,6 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne response.setClusterType(kubernetesCluster.getClusterType()); response.setCreated(kubernetesCluster.getCreated()); - - return response; } @@ -2476,6 +2474,7 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne KubernetesClusterUpgradeTimeout, KubernetesClusterUpgradeRetries, KubernetesClusterAddNodeTimeout, + KubernetesClusterRemoveNodeTimeout, KubernetesClusterExperimentalFeaturesEnabled, KubernetesMaxClusterSize, KubernetesControlNodeInstallAttemptWait, diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java index ad7aef3250b..b486a5516a1 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterService.java @@ -88,13 +88,13 @@ public interface KubernetesClusterService extends PluggableService, Configurable static final ConfigKey KubernetesClusterAddNodeTimeout = new ConfigKey("Advanced", Long.class, "cloud.kubernetes.cluster.add.node.timeout", "3600", - "Timeout interval (in seconds) in which an external node(VM / baremetal host) addition to a cluster should be completed", + "Timeout interval (in seconds) in which an external node (VM / baremetal host) addition to a cluster should be completed", true, KubernetesServiceEnabled.key()); static final ConfigKey KubernetesClusterRemoveNodeTimeout = new ConfigKey("Advanced", Long.class, - "cloud.kubernetes.cluster.add.node.timeout", + "cloud.kubernetes.cluster.remove.node.timeout", "900", - "Timeout interval (in seconds) in which an external node(VM / baremetal host) removal from a cluster should be completed", + "Timeout interval (in seconds) in which an external node (VM / baremetal host) removal from a cluster should be completed", true, KubernetesServiceEnabled.key()); static final ConfigKey KubernetesClusterExperimentalFeaturesEnabled = new ConfigKey("Advanced", Boolean.class, @@ -113,25 +113,25 @@ public interface KubernetesClusterService extends PluggableService, Configurable static final ConfigKey KubernetesControlNodeInstallAttemptWait = new ConfigKey("Advanced", Long.class, "cloud.kubernetes.control.node.install.attempt.wait.duration", "15", - "Time in seconds for the installation process to wait before it re-attempts", + "Control Nodes: Time in seconds for the installation process to wait before it re-attempts", true, KubernetesServiceEnabled.key()); static final ConfigKey KubernetesControlNodeInstallReattempts = new ConfigKey("Advanced", Long.class, "cloud.kubernetes.control.node.install.reattempt.count", "100", - "Number of times the offline installation of K8S will be re-attempted", + "Control Nodes: Number of times the offline installation of K8S will be re-attempted", true, KubernetesServiceEnabled.key()); final ConfigKey KubernetesWorkerNodeInstallAttemptWait = new ConfigKey("Advanced", Long.class, "cloud.kubernetes.worker.node.install.attempt.wait.duration", "30", - "Time in seconds for the installation process to wait before it re-attempts", + "Worker Nodes: Time in seconds for the installation process to wait before it re-attempts", true, KubernetesServiceEnabled.key()); static final ConfigKey KubernetesWorkerNodeInstallReattempts = new ConfigKey("Advanced", Long.class, "cloud.kubernetes.worker.node.install.reattempt.count", "40", - "Number of times the offline installation of K8S will be re-attempted", + "Worker Nodes: Number of times the offline installation of K8S will be re-attempted", true, KubernetesServiceEnabled.key()); static final ConfigKey KubernetesEtcdNodeStartPort = new ConfigKey("Advanced", Integer.class, diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java index 48b0c12ecd2..5b0214f2095 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java @@ -334,10 +334,10 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif } private String getInitialEtcdClusterDetails(List ipAddresses, List hostnames) { - String initialCluster = "%s=http://%s:2380"; + String initialCluster = "%s=http://%s:%s"; StringBuilder clusterInfo = new StringBuilder(); for (int i = 0; i < ipAddresses.size(); i++) { - clusterInfo.append(String.format(initialCluster, hostnames.get(i), ipAddresses.get(i))); + clusterInfo.append(String.format(initialCluster, hostnames.get(i), ipAddresses.get(i), KubernetesClusterActionWorker.ETCD_NODE_PEER_COMM_PORT)); if (i < ipAddresses.size()-1) { clusterInfo.append(","); } @@ -353,7 +353,7 @@ public class KubernetesClusterStartWorker extends KubernetesClusterResourceModif private String getEtcdEndpointList(List ipAddresses) { StringBuilder endpoints = new StringBuilder(); for (int i = 0; i < ipAddresses.size(); i++) { - endpoints.append(String.format("- http://%s:2379", ipAddresses.get(i).getIp4Address())); + endpoints.append(String.format("- http://%s:%s", ipAddresses.get(i).getIp4Address(), KubernetesClusterActionWorker.ETCD_NODE_CLIENT_REQUEST_PORT)); if (i < ipAddresses.size()-1) { endpoints.append("\n "); } diff --git a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/AddNodesToKubernetesClusterCmd.java b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/AddNodesToKubernetesClusterCmd.java index 09cbefe0bf8..71cc8ffcde2 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/AddNodesToKubernetesClusterCmd.java +++ b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/AddNodesToKubernetesClusterCmd.java @@ -52,8 +52,7 @@ public class AddNodesToKubernetesClusterCmd extends BaseAsyncCmd { entityType= UserVmResponse.class, description = "comma separated list of (external) node (physical or virtual machines) IDs that need to be" + "added as worker nodes to an existing managed Kubernetes cluster (CKS)", - required = true, - since = "4.21.0") + required = true) private List nodeIds; @Parameter(name = ApiConstants.ID, type = CommandType.UUID, required = true, @@ -62,13 +61,11 @@ public class AddNodesToKubernetesClusterCmd extends BaseAsyncCmd { private Long clusterId; @Parameter(name = ApiConstants.MOUNT_CKS_ISO_ON_VR, type = CommandType.BOOLEAN, - description = "(optional) Vmware only, uses the CKS cluster network VR to mount the CKS ISO", - since = "4.21.0") + description = "(optional) Vmware only, uses the CKS cluster network VR to mount the CKS ISO") private Boolean mountCksIsoOnVr; @Parameter(name = ApiConstants.MANUAL_UPGRADE, type = CommandType.BOOLEAN, - description = "(optional) indicates if the node is marked for manual upgrade and excluded from the Kubernetes cluster upgrade operation", - since = "4.21.0") + description = "(optional) indicates if the node is marked for manual upgrade and excluded from the Kubernetes cluster upgrade operation") private Boolean manualUpgrade; ///////////////////////////////////////////////////// diff --git a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/CreateKubernetesClusterCmd.java b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/CreateKubernetesClusterCmd.java index 7dd2978a5e3..8810b9262a8 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/CreateKubernetesClusterCmd.java +++ b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/CreateKubernetesClusterCmd.java @@ -115,18 +115,21 @@ public class CreateKubernetesClusterCmd extends BaseAsyncCreateCmd { @ACL(accessType = AccessType.UseEntry) @Parameter(name = ApiConstants.NODE_TYPE_OFFERING_MAP, type = CommandType.MAP, - description = "(Optional) Node Type to Service Offering ID mapping. If provided, it overrides the serviceofferingid parameter") + description = "(Optional) Node Type to Service Offering ID mapping. If provided, it overrides the serviceofferingid parameter", + since = "4.21.0") private Map> serviceOfferingNodeTypeMap; @ACL(accessType = AccessType.UseEntry) @Parameter(name = ApiConstants.NODE_TYPE_TEMPLATE_MAP, type = CommandType.MAP, - description = "(Optional) Node Type to Template ID mapping. If provided, it overrides the default template: System VM template") + description = "(Optional) Node Type to Template ID mapping. If provided, it overrides the default template: System VM template", + since = "4.21.0") private Map> templateNodeTypeMap; @ACL(accessType = AccessType.UseEntry) @Parameter(name = ApiConstants.ETCD_NODES, type = CommandType.LONG, description = "(Optional) Number of Kubernetes cluster etcd nodes, default is 0." + - "In case the number is greater than 0, etcd nodes are separate from master nodes and are provisioned accordingly") + "In case the number is greater than 0, etcd nodes are separate from master nodes and are provisioned accordingly", + since = "4.21.0") private Long etcdNodes; @ACL(accessType = AccessType.UseEntry) @@ -200,7 +203,8 @@ public class CreateKubernetesClusterCmd extends BaseAsyncCreateCmd { @Parameter(name = ApiConstants.CNI_CONFIG_DETAILS, type = CommandType.MAP, description = "used to specify the parameters values for the variables in userdata. " + "Example: cniconfigdetails[0].key=accesskey&cniconfigdetails[0].value=s389ddssaa&" + - "cniconfigdetails[1].key=secretkey&cniconfigdetails[1].value=8dshfsss", since = "4.21.0") + "cniconfigdetails[1].key=secretkey&cniconfigdetails[1].value=8dshfsss", + since = "4.21.0") private Map cniConfigDetails; @Parameter(name=ApiConstants.AS_NUMBER, type=CommandType.LONG, description="the AS Number of the network") diff --git a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/RemoveNodesFromKubernetesClusterCmd.java b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/RemoveNodesFromKubernetesClusterCmd.java index fd089ede9ec..2ca36e83ab4 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/RemoveNodesFromKubernetesClusterCmd.java +++ b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/RemoveNodesFromKubernetesClusterCmd.java @@ -58,13 +58,12 @@ public class RemoveNodesFromKubernetesClusterCmd extends BaseAsyncCmd { entityType= UserVmResponse.class, description = "comma separated list of node (physical or virtual machines) IDs that need to be" + "removed from the Kubernetes cluster (CKS)", - required = true, - since = "4.21.0") + required = true) private List nodeIds; @Parameter(name = ApiConstants.ID, type = CommandType.UUID, required = true, entityType = KubernetesClusterResponse.class, - description = "the ID of the Kubernetes cluster", since = "4.21.0") + description = "the ID of the Kubernetes cluster") private Long clusterId; ///////////////////////////////////////////////////// diff --git a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/ScaleKubernetesClusterCmd.java b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/ScaleKubernetesClusterCmd.java index 9afd4b8c324..2fb92f60fb9 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/ScaleKubernetesClusterCmd.java +++ b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/ScaleKubernetesClusterCmd.java @@ -74,7 +74,8 @@ public class ScaleKubernetesClusterCmd extends BaseAsyncCmd { @ACL(accessType = SecurityChecker.AccessType.UseEntry) @Parameter(name = ApiConstants.NODE_TYPE_OFFERING_MAP, type = CommandType.MAP, - description = "(Optional) Node Type to Service Offering ID mapping. If provided, it overrides the serviceofferingid parameter") + description = "(Optional) Node Type to Service Offering ID mapping. If provided, it overrides the serviceofferingid parameter", + since = "4.21.0") protected Map> serviceOfferingNodeTypeMap; @Parameter(name=ApiConstants.SIZE, type = CommandType.LONG, diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 1f964fbbd42..cb63ebcd8da 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -5417,7 +5417,7 @@ public class ApiResponseHelper implements ResponseGenerator { String rangeText = String.format("%s-%s", range.getStartASNumber(), range.getEndASNumber()); response.setAsNumberRange(rangeText); } else { - logger.info("is null for as number: "+ asn.getAsNumber()); + logger.info("Range is null for AS number: "+ asn.getAsNumber()); } response.setAllocated(asn.getAllocatedTime()); response.setAllocationState(asn.isAllocated() ? "Allocated" : "Free");