diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java index 56c818f832b..efccb5c09b0 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.api.command.user.firewall; import java.util.ArrayList; import java.util.List; +import org.apache.commons.collections.CollectionUtils; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; @@ -40,6 +41,7 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IpAddress; import com.cloud.network.rules.FirewallRule; import com.cloud.user.Account; +import com.cloud.utils.StringUtils; import com.cloud.utils.net.NetUtils; @APICommand(name = "createFirewallRule", description = "Creates a firewall rule for a given IP address", responseObject = FirewallResponse.class, entityType = {FirewallRule.class}, @@ -125,14 +127,13 @@ public class CreateFirewallRuleCmd extends BaseAsyncCreateCmd implements Firewal @Override public List getSourceCidrList() { - if (cidrlist != null) { + if (CollectionUtils.isNotEmpty(cidrlist) && !(cidrlist.size() == 1 && StringUtils.isBlank(cidrlist.get(0)))) { return cidrlist; } else { - List oneCidrList = new ArrayList(); + List oneCidrList = new ArrayList<>(); oneCidrList.add(NetUtils.ALL_IP4_CIDRS); return oneCidrList; } - } // /////////////////////////////////////////////////// diff --git a/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java new file mode 100644 index 00000000000..c905974b2be --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java @@ -0,0 +1,91 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.firewall; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.apache.commons.collections.CollectionUtils; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.utils.net.NetUtils; + +@RunWith(MockitoJUnitRunner.class) +public class CreateFirewallRuleCmdTest { + + private void validateAllIp4Cidr(final CreateFirewallRuleCmd cmd) { + Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList())); + Assert.assertEquals(1, cmd.getSourceCidrList().size()); + Assert.assertEquals(NetUtils.ALL_IP4_CIDRS, cmd.getSourceCidrList().get(0)); + } + + @Test + public void testGetSourceCidrList_Null() { + final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd(); + ReflectionTestUtils.setField(cmd, "cidrlist", null); + validateAllIp4Cidr(cmd); + } + + @Test + public void testGetSourceCidrList_Empty() { + final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd(); + ReflectionTestUtils.setField(cmd, "cidrlist", new ArrayList<>()); + validateAllIp4Cidr(cmd); + } + + @Test + public void testGetSourceCidrList_NullFirstElement() { + final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd(); + List list = new ArrayList<>(); + list.add(null); + ReflectionTestUtils.setField(cmd, "cidrlist", list); + validateAllIp4Cidr(cmd); + } + + @Test + public void testGetSourceCidrList_EmptyFirstElement() { + final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd(); + ReflectionTestUtils.setField(cmd, "cidrlist", Collections.singletonList(" ")); + validateAllIp4Cidr(cmd); + } + + @Test + public void testGetSourceCidrList_Valid() { + final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd(); + String cidr = "10.1.1.1/22"; + ReflectionTestUtils.setField(cmd, "cidrlist", Collections.singletonList(cidr)); + Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList())); + Assert.assertEquals(1, cmd.getSourceCidrList().size()); + Assert.assertEquals(cidr, cmd.getSourceCidrList().get(0)); + } + + @Test + public void testGetSourceCidrList_EmptyFirstElementButMore() { + final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd(); + String cidr = "10.1.1.1/22"; + ReflectionTestUtils.setField(cmd, "cidrlist", Arrays.asList(" ", cidr)); + Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList())); + Assert.assertEquals(2, cmd.getSourceCidrList().size()); + Assert.assertEquals(cidr, cmd.getSourceCidrList().get(1)); + } +} diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index d8e97f0277b..8463d9cee98 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -82,6 +82,9 @@ public interface NetworkOrchestrationService { ConfigKey NetworkLockTimeout = new ConfigKey(Integer.class, NetworkLockTimeoutCK, "Network", "600", "Lock wait timeout (seconds) while implementing network", true, Scope.Global, null); + ConfigKey DeniedRoutes = new ConfigKey(String.class, "denied.routes", "Network", "", + "Routes that are denied, can not be used for Static Routes creation for the VPC Private Gateway", true, ConfigKey.Scope.Zone, null); + ConfigKey GuestDomainSuffix = new ConfigKey(String.class, GuestDomainSuffixCK, "Network", "cloud.internal", "Default domain name for vms inside virtualized networks fronted by router", true, ConfigKey.Scope.Zone, null); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 7efc29b02a6..5106fd824c7 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -4872,7 +4872,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{NetworkGcWait, NetworkGcInterval, NetworkLockTimeout, + return new ConfigKey[]{NetworkGcWait, NetworkGcInterval, NetworkLockTimeout, DeniedRoutes, GuestDomainSuffix, NetworkThrottlingRate, MinVRVersion, PromiscuousMode, MacAddressChanges, ForgedTransmits, MacLearning, RollingRestartEnabled, TUNGSTEN_ENABLED, NSX_ENABLED }; diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java index e5584239a32..94a16497e87 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java @@ -61,6 +61,7 @@ import com.cloud.org.Managed; import com.cloud.resource.ResourceState; import com.cloud.utils.DateUtil; import com.cloud.utils.Pair; +import com.cloud.utils.StringUtils; import com.cloud.utils.db.Attribute; import com.cloud.utils.db.DB; import com.cloud.utils.db.Filter; @@ -80,13 +81,13 @@ import com.cloud.utils.exception.CloudRuntimeException; @TableGenerator(name = "host_req_sq", table = "op_host", pkColumnName = "id", valueColumnName = "sequence", allocationSize = 1) public class HostDaoImpl extends GenericDaoBase implements HostDao { //FIXME: , ExternalIdDao { - private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT filtered.host_id, COUNT(filtered.tag) AS tag_count " - + "FROM (SELECT host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag) AS filtered " - + "WHERE tag IN(%s) AND is_tag_a_rule = 0 " + private static final String LIST_HOST_IDS_BY_HOST_TAGS = "SELECT filtered.host_id, COUNT(filtered.tag) AS tag_count " + + "FROM (SELECT host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag,is_tag_a_rule) AS filtered " + + "WHERE tag IN (%s) AND (is_tag_a_rule = 0 OR is_tag_a_rule IS NULL) " + "GROUP BY host_id " + "HAVING tag_count = %s "; private static final String SEPARATOR = ","; - private static final String LIST_CLUSTERID_FOR_HOST_TAG = "select distinct cluster_id from host join ( %s ) AS selected_hosts ON host.id = selected_hosts.host_id"; + private static final String LIST_CLUSTER_IDS_FOR_HOST_TAGS = "select distinct cluster_id from host join ( %s ) AS selected_hosts ON host.id = selected_hosts.host_id"; private static final String GET_HOSTS_OF_ACTIVE_VMS = "select h.id " + "from vm_instance vm " + "join host h on (vm.host_id=h.id) " + @@ -625,9 +626,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao sb.append(" "); } - if (logger.isTraceEnabled()) { - logger.trace("Following hosts got reset: " + sb.toString()); - } + logger.trace("Following hosts got reset: {}", sb); } /* @@ -637,8 +636,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao SearchCriteria sc = ClustersOwnedByMSSearch.create(); sc.setParameters("server", managementServerId); - List clusters = customSearch(sc, null); - return clusters; + return customSearch(sc, null); } /* @@ -648,13 +646,11 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao SearchCriteria sc = ClustersForHostsNotOwnedByAnyMSSearch.create(); sc.setJoinParameters("ClusterManagedSearch", "managed", Managed.ManagedState.Managed); - List clusters = customSearch(sc, null); - return clusters; + return customSearch(sc, null); } /** * This determines if hosts belonging to cluster(@clusterId) are up for grabs - * * This is used for handling following cases: * 1. First host added in cluster * 2. During MS restart all hosts in a cluster are without any MS @@ -664,9 +660,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao sc.setParameters("cluster", clusterId); List hosts = search(sc, null); - boolean ownCluster = (hosts == null || hosts.size() == 0); - - return ownCluster; + return (hosts == null || hosts.isEmpty()); } @Override @@ -683,14 +677,14 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao logger.debug("Completed resetting hosts suitable for reconnect"); } - List assignedHosts = new ArrayList(); + List assignedHosts = new ArrayList<>(); if (logger.isDebugEnabled()) { logger.debug("Acquiring hosts for clusters already owned by this management server"); } List clusters = findClustersOwnedByManagementServer(managementServerId); txn.start(); - if (clusters.size() > 0) { + if (!clusters.isEmpty()) { // handle clusters already owned by @managementServerId SearchCriteria sc = UnmanagedDirectConnectSearch.create(); sc.setParameters("lastPinged", lastPingSecondsAfter); @@ -705,13 +699,9 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao sb.append(host.getId()); sb.append(" "); } - if (logger.isTraceEnabled()) { - logger.trace("Following hosts got acquired for clusters already owned: " + sb.toString()); - } - } - if (logger.isDebugEnabled()) { - logger.debug("Completed acquiring hosts for clusters already owned by this management server"); + logger.trace("Following hosts got acquired for clusters already owned: {}", sb); } + logger.debug("Completed acquiring hosts for clusters already owned by this management server"); if (assignedHosts.size() < limit) { if (logger.isDebugEnabled()) { @@ -723,7 +713,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao if (clusters.size() > limit) { updatedClusters = clusters.subList(0, limit.intValue()); } - if (updatedClusters.size() > 0) { + if (!updatedClusters.isEmpty()) { SearchCriteria sc = UnmanagedDirectConnectSearch.create(); sc.setParameters("lastPinged", lastPingSecondsAfter); sc.setJoinParameters("ClusterManagedSearch", "managed", Managed.ManagedState.Managed); @@ -731,10 +721,10 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao List unmanagedHosts = lockRows(sc, null, true); // group hosts based on cluster - Map> hostMap = new HashMap>(); + Map> hostMap = new HashMap<>(); for (HostVO host : unmanagedHosts) { if (hostMap.get(host.getClusterId()) == null) { - hostMap.put(host.getClusterId(), new ArrayList()); + hostMap.put(host.getClusterId(), new ArrayList<>()); } hostMap.get(host.getClusterId()).add(host); } @@ -755,13 +745,9 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao break; } } - if (logger.isTraceEnabled()) { - logger.trace("Following hosts got acquired from newly owned clusters: " + sb.toString()); - } - } - if (logger.isDebugEnabled()) { - logger.debug("Completed acquiring hosts for clusters not owned by any management server"); + logger.trace("Following hosts got acquired from newly owned clusters: {}", sb); } + logger.debug("Completed acquiring hosts for clusters not owned by any management server"); } txn.commit(); @@ -816,6 +802,15 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao @Override public List listByHostTag(Host.Type type, Long clusterId, Long podId, Long dcId, String hostTag) { + return listHostsWithOrWithoutHostTags(type, clusterId, podId, dcId, hostTag, true); + } + + private List listHostsWithOrWithoutHostTags(Host.Type type, Long clusterId, Long podId, Long dcId, String hostTags, boolean withHostTags) { + if (StringUtils.isEmpty(hostTags)) { + logger.debug("Host tags not specified, to list hosts"); + return new ArrayList<>(); + } + SearchBuilder hostSearch = createSearchBuilder(); HostVO entity = hostSearch.entity(); hostSearch.and("type", entity.getType(), SearchCriteria.Op.EQ); @@ -826,7 +821,9 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao hostSearch.and("resourceState", entity.getResourceState(), SearchCriteria.Op.EQ); SearchCriteria sc = hostSearch.create(); - sc.setParameters("type", type.toString()); + if (type != null) { + sc.setParameters("type", type.toString()); + } if (podId != null) { sc.setParameters("pod", podId); } @@ -839,27 +836,38 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao sc.setParameters("status", Status.Up.toString()); sc.setParameters("resourceState", ResourceState.Enabled.toString()); - List tmpHosts = listBy(sc); - List correctHostsByHostTags = new ArrayList(); - List hostIdsByComputeOffTags = findHostByComputeOfferings(hostTag); + List upAndEnabledHosts = listBy(sc); + if (CollectionUtils.isEmpty(upAndEnabledHosts)) { + return new ArrayList<>(); + } - tmpHosts.forEach((host) -> { if(hostIdsByComputeOffTags.contains(host.getId())) correctHostsByHostTags.add(host);}); + List hostIdsByHostTags = findHostIdsByHostTags(hostTags); + if (CollectionUtils.isEmpty(hostIdsByHostTags)) { + return withHostTags ? new ArrayList<>() : upAndEnabledHosts; + } - return correctHostsByHostTags; + if (withHostTags) { + List upAndEnabledHostsWithHostTags = new ArrayList<>(); + upAndEnabledHosts.forEach((host) -> { if (hostIdsByHostTags.contains(host.getId())) upAndEnabledHostsWithHostTags.add(host);}); + return upAndEnabledHostsWithHostTags; + } else { + List upAndEnabledHostsWithoutHostTags = new ArrayList<>(); + upAndEnabledHosts.forEach((host) -> { if (!hostIdsByHostTags.contains(host.getId())) upAndEnabledHostsWithoutHostTags.add(host);}); + return upAndEnabledHostsWithoutHostTags; + } } @Override public List listAllUpAndEnabledNonHAHosts(Type type, Long clusterId, Long podId, long dcId, String haTag) { + if (StringUtils.isNotEmpty(haTag)) { + return listHostsWithOrWithoutHostTags(type, clusterId, podId, dcId, haTag, false); + } + SearchBuilder hostTagSearch = _hostTagsDao.createSearchBuilder(); hostTagSearch.and(); hostTagSearch.op("isTagARule", hostTagSearch.entity().getIsTagARule(), Op.EQ); hostTagSearch.or("tagDoesNotExist", hostTagSearch.entity().getIsTagARule(), Op.NULL); hostTagSearch.cp(); - if (haTag != null && !haTag.isEmpty()) { - hostTagSearch.and().op("tag", hostTagSearch.entity().getTag(), SearchCriteria.Op.NEQ); - hostTagSearch.or("tagNull", hostTagSearch.entity().getTag(), SearchCriteria.Op.NULL); - hostTagSearch.cp(); - } SearchBuilder hostSearch = createSearchBuilder(); @@ -870,18 +878,12 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao hostSearch.and("status", hostSearch.entity().getStatus(), SearchCriteria.Op.EQ); hostSearch.and("resourceState", hostSearch.entity().getResourceState(), SearchCriteria.Op.EQ); - hostSearch.join("hostTagSearch", hostTagSearch, hostSearch.entity().getId(), hostTagSearch.entity().getHostId(), JoinBuilder.JoinType.LEFTOUTER); - SearchCriteria sc = hostSearch.create(); sc.setJoinParameters("hostTagSearch", "isTagARule", false); - if (haTag != null && !haTag.isEmpty()) { - sc.setJoinParameters("hostTagSearch", "tag", haTag); - } - if (type != null) { sc.setParameters("type", type); } @@ -921,12 +923,12 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao @DB @Override public List findLostHosts(long timeout) { - List result = new ArrayList(); + List result = new ArrayList<>(); String sql = "select h.id from host h left join cluster c on h.cluster_id=c.id where h.mgmt_server_id is not null and h.last_ping < ? and h.status in ('Up', 'Updating', 'Disconnected', 'Connecting') and h.type not in ('ExternalFirewall', 'ExternalLoadBalancer', 'TrafficMonitor', 'SecondaryStorage', 'LocalSecondaryStorage', 'L2Networking') and (h.cluster_id is null or c.managed_state = 'Managed') ;"; try (TransactionLegacy txn = TransactionLegacy.currentTxn(); - PreparedStatement pstmt = txn.prepareStatement(sql);) { + PreparedStatement pstmt = txn.prepareStatement(sql)) { pstmt.setLong(1, timeout); - try (ResultSet rs = pstmt.executeQuery();) { + try (ResultSet rs = pstmt.executeQuery()) { while (rs.next()) { long id = rs.getLong(1); //ID column result.add(findById(id)); @@ -959,7 +961,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao HashMap> groupDetails = host.getGpuGroupDetails(); if (groupDetails != null) { // Create/Update GPU group entries - _hostGpuGroupsDao.persist(host.getId(), new ArrayList(groupDetails.keySet())); + _hostGpuGroupsDao.persist(host.getId(), new ArrayList<>(groupDetails.keySet())); // Create/Update VGPU types entries _vgpuTypesDao.persist(host.getId(), groupDetails); } @@ -1002,7 +1004,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao boolean persisted = super.update(hostId, host); if (!persisted) { - return persisted; + return false; } saveDetails(host); @@ -1011,7 +1013,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao txn.commit(); - return persisted; + return true; } @Override @@ -1022,11 +1024,10 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao + "select h.data_center_id, h.type, count(*) as count from host as h INNER JOIN mshost as m ON h.mgmt_server_id=m.msid " + "where h.status='Up' and h.type='Routing' and m.last_update > ? " + "group by h.data_center_id, h.type) as t " + "ORDER by t.data_center_id, t.type"; - ArrayList l = new ArrayList(); + ArrayList l = new ArrayList<>(); TransactionLegacy txn = TransactionLegacy.currentTxn(); - ; - PreparedStatement pstmt = null; + PreparedStatement pstmt; try { pstmt = txn.prepareAutoCloseStatement(sql); String gmtCutTime = DateUtil.getDateDisplayString(TimeZone.getTimeZone("GMT"), cutTime); @@ -1050,9 +1051,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao @Override public long getNextSequence(long hostId) { - if (logger.isTraceEnabled()) { - logger.trace("getNextSequence(), hostId: " + hostId); - } + logger.trace("getNextSequence(), hostId: {}", hostId); TableGenerator tg = _tgs.get("host_req_sq"); assert tg != null : "how can this be wrong!"; @@ -1121,31 +1120,30 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao HostVO ho = findById(host.getId()); assert ho != null : "How how how? : " + host.getId(); + // TODO handle this if(debug){}else{log.debug} it makes no sense if (logger.isDebugEnabled()) { - - StringBuilder str = new StringBuilder("Unable to update host for event:").append(event.toString()); - str.append(". Name=").append(host.getName()); - str.append("; New=[status=").append(newStatus.toString()).append(":msid=").append(newStatus.lostConnection() ? "null" : host.getManagementServerId()) - .append(":lastpinged=").append(host.getLastPinged()).append("]"); - str.append("; Old=[status=").append(oldStatus.toString()).append(":msid=").append(host.getManagementServerId()).append(":lastpinged=").append(oldPingTime) - .append("]"); - str.append("; DB=[status=").append(vo.getStatus().toString()).append(":msid=").append(vo.getManagementServerId()).append(":lastpinged=").append(vo.getLastPinged()) - .append(":old update count=").append(oldUpdateCount).append("]"); - logger.debug(str.toString()); + String str = "Unable to update host for event:" + event + + ". Name=" + host.getName() + + "; New=[status=" + newStatus + ":msid=" + (newStatus.lostConnection() ? "null" : host.getManagementServerId()) + + ":lastpinged=" + host.getLastPinged() + "]" + + "; Old=[status=" + oldStatus.toString() + ":msid=" + host.getManagementServerId() + ":lastpinged=" + oldPingTime + + "]" + + "; DB=[status=" + vo.getStatus().toString() + ":msid=" + vo.getManagementServerId() + ":lastpinged=" + vo.getLastPinged() + + ":old update count=" + oldUpdateCount + "]"; + logger.debug(str); } else { - StringBuilder msg = new StringBuilder("Agent status update: ["); - msg.append("id = " + host.getId()); - msg.append("; name = " + host.getName()); - msg.append("; old status = " + oldStatus); - msg.append("; event = " + event); - msg.append("; new status = " + newStatus); - msg.append("; old update count = " + oldUpdateCount); - msg.append("; new update count = " + newUpdateCount + "]"); - logger.debug(msg.toString()); + String msg = "Agent status update: [" + "id = " + host.getId() + + "; name = " + host.getName() + + "; old status = " + oldStatus + + "; event = " + event + + "; new status = " + newStatus + + "; old update count = " + oldUpdateCount + + "; new update count = " + newUpdateCount + "]"; + logger.debug(msg); } if (ho.getState() == newStatus) { - logger.debug("Host " + ho.getName() + " state has already been updated to " + newStatus); + logger.debug("Host {} state has already been updated to {}", ho.getName(), newStatus); return true; } } @@ -1171,25 +1169,24 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao int result = update(ub, sc, null); assert result <= 1 : "How can this update " + result + " rows? "; + // TODO handle this if(debug){}else{log.debug} it makes no sense if (logger.isDebugEnabled() && result == 0) { HostVO ho = findById(host.getId()); assert ho != null : "How how how? : " + host.getId(); - StringBuilder str = new StringBuilder("Unable to update resource state: ["); - str.append("m = " + host.getId()); - str.append("; name = " + host.getName()); - str.append("; old state = " + oldState); - str.append("; event = " + event); - str.append("; new state = " + newState + "]"); - logger.debug(str.toString()); + String str = "Unable to update resource state: [" + "m = " + host.getId() + + "; name = " + host.getName() + + "; old state = " + oldState + + "; event = " + event + + "; new state = " + newState + "]"; + logger.debug(str); } else { - StringBuilder msg = new StringBuilder("Resource state update: ["); - msg.append("id = " + host.getId()); - msg.append("; name = " + host.getName()); - msg.append("; old state = " + oldState); - msg.append("; event = " + event); - msg.append("; new state = " + newState + "]"); - logger.debug(msg.toString()); + String msg = "Resource state update: [" + "id = " + host.getId() + + "; name = " + host.getName() + + "; old state = " + oldState + + "; event = " + event + + "; new state = " + newState + "]"; + logger.debug(msg); } return result > 0; @@ -1364,7 +1361,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao Filter orderByFilter = new Filter(HostVO.class, "created", true, null, null); List hosts = search(sc, orderByFilter, null, false); - if (hosts != null && hosts.size() > 0) { + if (hosts != null && !hosts.isEmpty()) { return hosts.get(0); } @@ -1407,19 +1404,19 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao } @Override - public List listClustersByHostTag(String computeOfferingTags) { + public List listClustersByHostTag(String hostTags) { TransactionLegacy txn = TransactionLegacy.currentTxn(); - String sql = this.LIST_CLUSTERID_FOR_HOST_TAG; - PreparedStatement pstmt = null; - List result = new ArrayList(); - List tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR)); - String subselect = getHostIdsByComputeTags(tags); - sql = String.format(sql, subselect); + String selectStmtToListClusterIdsByHostTags = LIST_CLUSTER_IDS_FOR_HOST_TAGS; + PreparedStatement pstmt; + List result = new ArrayList<>(); + List tags = Arrays.asList(hostTags.split(SEPARATOR)); + String selectStmtToListHostIdsByHostTags = getSelectStmtToListHostIdsByHostTags(tags); + selectStmtToListClusterIdsByHostTags = String.format(selectStmtToListClusterIdsByHostTags, selectStmtToListHostIdsByHostTags); try { - pstmt = txn.prepareStatement(sql); + pstmt = txn.prepareStatement(selectStmtToListClusterIdsByHostTags); - for(int i = 0; i < tags.size(); i++){ + for (int i = 0; i < tags.size(); i++){ pstmt.setString(i+1, tags.get(i)); } @@ -1430,20 +1427,20 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao pstmt.close(); return result; } catch (SQLException e) { - throw new CloudRuntimeException("DB Exception on: " + sql, e); + throw new CloudRuntimeException("DB Exception on: " + selectStmtToListClusterIdsByHostTags, e); } } - private List findHostByComputeOfferings(String computeOfferingTags){ + private List findHostIdsByHostTags(String hostTags){ TransactionLegacy txn = TransactionLegacy.currentTxn(); - PreparedStatement pstmt = null; - List result = new ArrayList(); - List tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR)); - String select = getHostIdsByComputeTags(tags); + PreparedStatement pstmt; + List result = new ArrayList<>(); + List tags = Arrays.asList(hostTags.split(SEPARATOR)); + String selectStmtToListHostIdsByHostTags = getSelectStmtToListHostIdsByHostTags(tags); try { - pstmt = txn.prepareStatement(select); + pstmt = txn.prepareStatement(selectStmtToListHostIdsByHostTags); - for(int i = 0; i < tags.size(); i++){ + for (int i = 0; i < tags.size(); i++){ pstmt.setString(i+1, tags.get(i)); } @@ -1454,7 +1451,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao pstmt.close(); return result; } catch (SQLException e) { - throw new CloudRuntimeException("DB Exception on: " + select, e); + throw new CloudRuntimeException("DB Exception on: " + selectStmtToListHostIdsByHostTags, e); } } @@ -1504,16 +1501,16 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao return result; } - private String getHostIdsByComputeTags(List offeringTags){ - List questionMarks = new ArrayList(); - offeringTags.forEach((tag) -> { questionMarks.add("?"); }); - return String.format(this.LIST_HOST_IDS_BY_COMPUTETAGS, String.join(",", questionMarks),questionMarks.size()); + private String getSelectStmtToListHostIdsByHostTags(List hostTags){ + List questionMarks = new ArrayList<>(); + hostTags.forEach((tag) -> questionMarks.add("?")); + return String.format(LIST_HOST_IDS_BY_HOST_TAGS, String.join(SEPARATOR, questionMarks), questionMarks.size()); } @Override public List listHostsWithActiveVMs(long offeringId) { TransactionLegacy txn = TransactionLegacy.currentTxn(); - PreparedStatement pstmt = null; + PreparedStatement pstmt; List result = new ArrayList<>(); StringBuilder sql = new StringBuilder(GET_HOSTS_OF_ACTIVE_VMS); try { @@ -1540,7 +1537,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao @Override public List listOrderedHostsHypervisorVersionsInDatacenter(long datacenterId, HypervisorType hypervisorType) { - PreparedStatement pstmt = null; + PreparedStatement pstmt; List result = new ArrayList<>(); try { TransactionLegacy txn = TransactionLegacy.currentTxn(); diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 2b08bd25eba..a5356375db7 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -1774,6 +1774,10 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C throwInvalidIdException("Network offering with specified id doesn't support adding multiple ip ranges", ntwkOff.getUuid(), NETWORK_OFFERING_ID); } + if (GuestType.Shared == ntwkOff.getGuestType() && !ntwkOff.isSpecifyVlan() && Objects.isNull(associatedNetworkId)) { + throw new CloudRuntimeException("Associated network must be provided when creating Shared networks when specifyVlan is false"); + } + Pair interfaceMTUs = validateMtuConfig(publicMtu, privateMtu, zone.getId()); mtuCheckForVpcNetwork(vpcId, interfaceMTUs, publicMtu, privateMtu); diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 399abbfad17..d542ffc1346 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -2973,7 +2973,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } // 2) CIDR should be outside of link-local cidr - if (NetUtils.isNetworksOverlap(vpc.getCidr(), NetUtils.getLinkLocalCIDR())) { + if (NetUtils.isNetworksOverlap(cidr, NetUtils.getLinkLocalCIDR())) { throw new InvalidParameterValueException("CIDR should be outside of link local cidr " + NetUtils.getLinkLocalCIDR()); } @@ -3002,7 +3002,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } protected boolean isCidrDenylisted(final String cidr, final long zoneId) { - final String routesStr = NetworkOrchestrationService.GuestDomainSuffix.valueIn(zoneId); + final String routesStr = NetworkOrchestrationService.DeniedRoutes.valueIn(zoneId); if (routesStr != null && !routesStr.isEmpty()) { final String[] cidrDenyList = routesStr.split(","); diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index f54fd96bfdf..9ffad8b8418 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -1924,7 +1924,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe final String haTag = _haMgr.getHaTag(); SearchBuilder hostTagSearch = null; - if (haHosts != null && haTag != null && !haTag.isEmpty()) { + if (haHosts != null && StringUtils.isNotEmpty(haTag)) { hostTagSearch = _hostTagsDao.createSearchBuilder(); if ((Boolean)haHosts) { hostTagSearch.and().op("tag", hostTagSearch.entity().getTag(), SearchCriteria.Op.EQ); @@ -1985,7 +1985,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe sc.setParameters("resourceState", resourceState); } - if (haHosts != null && haTag != null && !haTag.isEmpty()) { + if (haHosts != null && StringUtils.isNotEmpty(haTag)) { sc.setJoinParameters("hostTagSearch", "tag", haTag); } diff --git a/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java b/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java index 981649758cb..7df4857f4fd 100644 --- a/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java +++ b/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java @@ -219,8 +219,6 @@ public class FirstFitPlannerTest { } private List initializeForClusterListBasedOnHostTag(ServiceOffering offering) { - - when(offering.getHostTag()).thenReturn("hosttag1"); initializeForClusterThresholdDisabled(); List matchingClusters = new ArrayList<>(); diff --git a/ui/src/views/network/FirewallRules.vue b/ui/src/views/network/FirewallRules.vue index 43ee9536be5..a0ecb2042ae 100644 --- a/ui/src/views/network/FirewallRules.vue +++ b/ui/src/views/network/FirewallRules.vue @@ -457,6 +457,9 @@ export default { addRule () { if (this.loading) return this.loading = true + if (this.newRule.cidrlist == null || this.newRule.cidrlist.trim?.() === '') { + delete this.newRule.cidrlist + } api('createFirewallRule', { ...this.newRule }).then(response => { this.$pollJob({ jobId: response.createfirewallruleresponse.jobid,