vishesh92 commented on code in PR #7515:
URL: https://github.com/apache/cloudstack/pull/7515#discussion_r1191723460
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java:
##########
@@ -600,8 +609,66 @@ private void validateEndpointUrl() {
throw new InvalidParameterValueException(error);
}
}
+ private void validateUnmanagedKubernetesClusterCreateParameters(final
CreateKubernetesClusterCmd cmd) throws CloudRuntimeException {
+ final String name = cmd.getName();
+ final Long zoneId = cmd.getZoneId();
+ final Account owner =
accountService.getActiveAccountById(cmd.getEntityOwnerId());
+ final Long networkId = cmd.getNetworkId();
+ final String sshKeyPair = cmd.getSSHKeyPairName();
+ final String dockerRegistryUserName = cmd.getDockerRegistryUserName();
+ final String dockerRegistryPassword = cmd.getDockerRegistryPassword();
+ final String dockerRegistryUrl = cmd.getDockerRegistryUrl();
+ final Long nodeRootDiskSize = cmd.getNodeRootDiskSize();
+ final String externalLoadBalancerIpAddress =
cmd.getExternalLoadBalancerIpAddress();
+
+ if (name == null || name.isEmpty()) {
+ throw new InvalidParameterValueException("Invalid name for the
Kubernetes cluster name:" + name);
+ }
+
+ DataCenter zone = dataCenterDao.findById(zoneId);
+ if (zone == null) {
+ throw new InvalidParameterValueException("Unable to find zone by
ID: " + zoneId);
+ }
+
+ if (zone.getAllocationState() == Grouping.AllocationState.Disabled) {
+ throw new PermissionDeniedException(String.format("Cannot perform
this operation, zone ID: %s is currently disabled", zone.getUuid()));
+ }
+
+ if (sshKeyPair != null && !sshKeyPair.isEmpty()) {
+ SSHKeyPairVO sshKeyPairVO =
sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), sshKeyPair);
+ if (sshKeyPairVO == null) {
+ throw new InvalidParameterValueException(String.format("Given
SSH key pair with name: %s was not found for the account %s", sshKeyPair,
owner.getAccountName()));
+ }
+ }
+
+ if (nodeRootDiskSize != null && nodeRootDiskSize <= 0) {
+ throw new InvalidParameterValueException(String.format("Invalid
value for %s", ApiConstants.NODE_ROOT_DISK_SIZE));
+ }
- private void validateKubernetesClusterCreateParameters(final
CreateKubernetesClusterCmd cmd) throws CloudRuntimeException {
+ validateDockerRegistryParams(dockerRegistryUserName,
dockerRegistryPassword, dockerRegistryUrl);
+
+ Network network = null;
+ if (networkId != null) {
+ network = networkService.getNetwork(networkId);
+ if (network == null) {
+ throw new InvalidParameterValueException("Unable to find
network with given ID");
+ }
+ }
+
+ if (StringUtils.isNotEmpty(externalLoadBalancerIpAddress)) {
+ if (!NetUtils.isValidIp4(externalLoadBalancerIpAddress) &&
!NetUtils.isValidIp6(externalLoadBalancerIpAddress)) {
+ throw new InvalidParameterValueException("Invalid external
load balancer IP address");
+ }
+ if (network == null) {
+ throw new InvalidParameterValueException(String.format("%s
parameter must be specified along with %s parameter",
ApiConstants.EXTERNAL_LOAD_BALANCER_IP_ADDRESS, ApiConstants.NETWORK_ID));
+ }
+ if (Network.GuestType.Shared.equals(network.getGuestType())) {
+ throw new InvalidParameterValueException(String.format("%s
parameter must be specified along with %s type of network",
ApiConstants.EXTERNAL_LOAD_BALANCER_IP_ADDRESS,
Network.GuestType.Shared.toString()));
+ }
Review Comment:
I am removing these checks. Since this is cluster is not managed by CKS, we
can allow the user to add this information if they want.
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java:
##########
@@ -600,8 +609,66 @@ private void validateEndpointUrl() {
throw new InvalidParameterValueException(error);
}
}
+ private void validateUnmanagedKubernetesClusterCreateParameters(final
CreateKubernetesClusterCmd cmd) throws CloudRuntimeException {
+ final String name = cmd.getName();
+ final Long zoneId = cmd.getZoneId();
+ final Account owner =
accountService.getActiveAccountById(cmd.getEntityOwnerId());
+ final Long networkId = cmd.getNetworkId();
+ final String sshKeyPair = cmd.getSSHKeyPairName();
+ final String dockerRegistryUserName = cmd.getDockerRegistryUserName();
+ final String dockerRegistryPassword = cmd.getDockerRegistryPassword();
+ final String dockerRegistryUrl = cmd.getDockerRegistryUrl();
+ final Long nodeRootDiskSize = cmd.getNodeRootDiskSize();
+ final String externalLoadBalancerIpAddress =
cmd.getExternalLoadBalancerIpAddress();
+
+ if (name == null || name.isEmpty()) {
+ throw new InvalidParameterValueException("Invalid name for the
Kubernetes cluster name:" + name);
Review Comment:
Doesn't this give better information whether the `name` is `null` or an
empty string?
##########
plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/CreateKubernetesClusterCmd.java:
##########
@@ -76,13 +76,13 @@ public class CreateKubernetesClusterCmd extends
BaseAsyncCreateCmd {
description = "availability zone in which Kubernetes cluster to be
launched")
private Long zoneId;
- @Parameter(name = ApiConstants.KUBERNETES_VERSION_ID, type =
CommandType.UUID, entityType = KubernetesSupportedVersionResponse.class,
required = true,
+ @Parameter(name = ApiConstants.KUBERNETES_VERSION_ID, type =
CommandType.UUID, entityType = KubernetesSupportedVersionResponse.class,
Review Comment:
We are not adding a new parameter but making it not required. Should we
still add since here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
