address code smells - part 1

This commit is contained in:
Pearl Dsilva 2023-09-07 09:42:31 -04:00
parent 7dcf2d50cb
commit 9d9c334bd2
17 changed files with 179 additions and 72 deletions

View File

@ -205,7 +205,6 @@ public interface Network extends ControlledEntity, StateObject<Network.State>, I
//Add Tungsten Fabric provider
public static final Provider Tungsten = new Provider("Tungsten", false);
//TODO: should it be true?
public static final Provider Nsx = new Provider("Nsx", false);
private final String name;

View File

@ -343,7 +343,6 @@ public class Networks {
}
},
Vswitch("vs", String.class), Undecided(null, null), Vnet("vnet", Long.class);
// NSX("nsx", String.class);
private final String scheme;
private final Class<?> type;

View File

@ -29,7 +29,7 @@ public interface VpcOffering extends InternalIdentity, Identity {
public static final String defaultVPCOfferingName = "Default VPC offering";
public static final String defaultVPCNSOfferingName = "Default VPC offering with Netscaler";
public static final String redundantVPCOfferingName = "Redundant VPC offering";
public static final String defaultVPCNSXSOfferingName = "VPC offering with NSX";
public static final String DEFAULT_VPC_NSX_OFFERING_NAME = "VPC offering with NSX";
/**
*

View File

@ -71,18 +71,6 @@ public class NsxProviderVO implements NsxProvider {
this.uuid = UUID.randomUUID().toString();
}
public NsxProviderVO( long zoneId, long hostId, String providerName, String hostname, String username, String password, String tier0Gateway, String edgeCluster) {
this.zoneId = zoneId;
this.hostId = hostId;
this.uuid = UUID.randomUUID().toString();
this.providerName = providerName;
this.hostname = hostname;
this.username = username;
this.password = password;
this.tier0Gateway = tier0Gateway;
this.edgeCluster = edgeCluster;
}
@Override
public long getId() {
return id;
@ -177,4 +165,78 @@ public class NsxProviderVO implements NsxProvider {
public void setEdgeCluster(String edgeCluster) {
this.edgeCluster = edgeCluster;
}
public static final class Builder {
private long zoneId;
private long hostId;
private String providerName;
private String hostname;
private String port;
private String username;
private String password;
private String tier0Gateway;
private String edgeCluster;
public Builder() {
}
public Builder setZoneId(long zoneId) {
this.zoneId = zoneId;
return this;
}
public Builder setHostId(long hostId) {
this.hostId = hostId;
return this;
}
public Builder setProviderName(String providerName) {
this.providerName = providerName;
return this;
}
public Builder setHostname(String hostname) {
this.hostname = hostname;
return this;
}
public Builder setPort(String port) {
this.port = port;
return this;
}
public Builder setUsername(String username) {
this.username = username;
return this;
}
public Builder setPassword(String password) {
this.password = password;
return this;
}
public Builder setTier0Gateway(String tier0Gateway) {
this.tier0Gateway = tier0Gateway;
return this;
}
public Builder setEdgeCluster(String edgeCluster) {
this.edgeCluster = edgeCluster;
return this;
}
public NsxProviderVO build() {
NsxProviderVO provider = new NsxProviderVO();
provider.setZoneId(this.zoneId);
provider.setHostId(this.hostId);
provider.setUuid(UUID.randomUUID().toString());
provider.setProviderName(this.providerName);
provider.setHostname(this.hostname);
provider.setPort(this.port);
provider.setUsername(this.username);
provider.setPassword(this.password);
provider.setTier0Gateway(this.tier0Gateway);
provider.setEdgeCluster(this.edgeCluster);
return provider;
}
}
}

View File

@ -193,6 +193,8 @@ CREATE TABLE `cloud`.`nsx_providers` (
`password` varchar(255) NOT NULL,
`tier0_gateway` varchar(255),
`edge_cluster` varchar(255),
`created` datetime NOT NULL COMMENT 'date created',
`removed` datetime COMMENT 'date removed if not null',
PRIMARY KEY (`id`),
CONSTRAINT `fk_nsx_providers__zone_id` FOREIGN KEY `fk_nsx_providers__zone_id` (`zone_id`) REFERENCES `data_center`(`id`) ON DELETE CASCADE,
INDEX `i_nsx_providers__zone_id`(`zone_id`)

View File

@ -18,6 +18,8 @@ package org.apache.cloudstack.agent.api;
import com.cloud.network.dao.NetworkVO;
import java.util.Objects;
public class CreateNsxSegmentCommand extends CreateNsxTier1GatewayCommand {
private NetworkVO tierNetwork;
public CreateNsxSegmentCommand(String zoneName, Long zoneId, String accountName, Long accountId, String vpcName, NetworkVO tierNetwork) {
@ -32,4 +34,18 @@ public class CreateNsxSegmentCommand extends CreateNsxTier1GatewayCommand {
public void setTierNetwork(NetworkVO tierNetwork) {
this.tierNetwork = tierNetwork;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
CreateNsxSegmentCommand command = (CreateNsxSegmentCommand) o;
return Objects.equals(tierNetwork, command.tierNetwork);
}
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), tierNetwork);
}
}

View File

@ -16,6 +16,8 @@
// under the License.
package org.apache.cloudstack.agent.api;
import java.util.Objects;
public class CreateNsxTier1GatewayCommand extends NsxCommand {
private String vpcName;
@ -31,4 +33,18 @@ public class CreateNsxTier1GatewayCommand extends NsxCommand {
public void setVpcName(String vpcName) {
this.vpcName = vpcName;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
CreateNsxTier1GatewayCommand that = (CreateNsxTier1GatewayCommand) o;
return Objects.equals(vpcName, that.vpcName);
}
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), vpcName);
}
}

View File

@ -18,6 +18,8 @@ package org.apache.cloudstack.agent.api;
import com.cloud.agent.api.Command;
import java.util.Objects;
public class NsxCommand extends Command {
private String zoneName;
private Long zoneId;
@ -66,4 +68,18 @@ public class NsxCommand extends Command {
public boolean executeInSequence() {
return false;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
NsxCommand that = (NsxCommand) o;
return Objects.equals(zoneName, that.zoneName) && Objects.equals(zoneId, that.zoneId) && Objects.equals(accountName, that.accountName) && Objects.equals(accountId, that.accountId);
}
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), zoneName, zoneId, accountName, accountId);
}
}

View File

@ -53,7 +53,6 @@ public class AddNsxControllerCmd extends BaseCmd {
@Parameter(name = ApiConstants.NSX_PROVIDER_HOSTNAME, type = CommandType.STRING, required = true, description = "NSX controller hostname / IP address")
private String hostname;
// TODO: May not be required
@Parameter(name = ApiConstants.NSX_PROVIDER_PORT, type = CommandType.STRING, description = "NSX controller port")
private String port;
@Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, required = true, description = "Username to log into NSX controller")

View File

@ -48,8 +48,6 @@ public class NsxControllerResponse extends BaseResponse {
@Param(description = "NSX controller port")
private String port;
// TODO: Should Password be returned?
@SerializedName(ApiConstants.TIER0_GATEWAY)
@Param(description = "The tier-0 gateway network. Tier-0 gateway is responsible for handling" +
" traffic between logical and physical networks"

View File

@ -53,6 +53,7 @@ import org.apache.cloudstack.utils.NsxApiClientUtils;
import org.apache.log4j.Logger;
import javax.naming.ConfigurationException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
@ -124,7 +125,7 @@ public class NsxResource implements ServerResource {
@Override
public void disconnected() {
// Do nothing
}
@Override
@ -134,6 +135,7 @@ public class NsxResource implements ServerResource {
@Override
public void setAgentControl(IAgentControl agentControl) {
// Do nothing
}
@Override
@ -148,12 +150,12 @@ public class NsxResource implements ServerResource {
@Override
public void setConfigParams(Map<String, Object> params) {
// Do nothing
}
@Override
public Map<String, Object> getConfigParams() {
return null;
return new HashMap<>();
}
@Override
@ -163,7 +165,7 @@ public class NsxResource implements ServerResource {
@Override
public void setRunLevel(int level) {
// Do nothing
}
@Override
@ -222,7 +224,7 @@ public class NsxResource implements ServerResource {
return new ReadyAnswer(cmd);
}
private Function<Class, Service> nsxService = svcClass -> { return nsxApi.getApiClient().createStub(svcClass); };
private Function<Class<? extends Service>, Service> nsxService = svcClass -> nsxApi.getApiClient().createStub(svcClass);
private Answer executeRequest(CreateNsxTier1GatewayCommand cmd) {
String name = getTier1GatewayName(cmd);
Tier1 tier1 = getTier1Gateway(name);
@ -315,24 +317,6 @@ public class NsxResource implements ServerResource {
}
}
private EdgeCluster getEdgeClusterDetails(String edgeClusterName) {
try {
EdgeClusters edgeClusterService = (EdgeClusters) nsxService.apply(EdgeClusters.class);
return edgeClusterService.get(edgeClusterName);
} catch (Exception e) {
throw new CloudRuntimeException(String.format("Failed to fetch details of edge cluster: %s, due to: %s", edgeClusterName, e.getMessage()));
}
}
private ServiceSegmentListResult listServiceSegments() {
try {
ServiceSegments serviceSegmentSvc = (ServiceSegments) nsxService.apply(ServiceSegments.class);
return serviceSegmentSvc.list(null, null, null, true, null);
} catch (Exception e) {
throw new CloudRuntimeException(String.format("Failed to fetch service segment list due to %s", e.getMessage()));
}
}
private Tier1 getTier1Gateway(String tier1GatewayId) {
try {
Tier1s tier1service = (Tier1s) nsxService.apply(Tier1s.class);

View File

@ -21,8 +21,6 @@ import org.apache.log4j.Logger;
public class NsxApi {
private static final Logger S_LOGGER = Logger.getLogger(NsxApi.class);
ApiClient apiClient;
public ApiClient getApiClient() {

View File

@ -75,6 +75,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.function.LongFunction;
@Component
public class NsxElement extends AdapterBase implements DhcpServiceProvider, DnsServiceProvider, VpcProvider,
@ -258,10 +259,10 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, DnsS
public boolean implementVpc(Vpc vpc, DeployDestination dest, ReservationContext context) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException {
DataCenterVO zone = zoneFunction.apply(vpc.getZoneId());
Pair<Boolean, Account> isNsxAndAccount = validateVpcConfigurationAndGetAccount(zone, vpc);
if (!isNsxAndAccount.first()) {
if (Boolean.FALSE.equals(isNsxAndAccount.first())) {
return true;
}
if (isNsxAndAccount.first() && Objects.isNull(isNsxAndAccount.second())) {
if (Boolean.TRUE.equals(isNsxAndAccount.first()) && Objects.isNull(isNsxAndAccount.second())) {
throw new InvalidParameterValueException(String.format("Failed to find account with id %s", vpc.getAccountId()));
}
Account account = isNsxAndAccount.second();
@ -272,10 +273,10 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, DnsS
public boolean shutdownVpc(Vpc vpc, ReservationContext context) throws ConcurrentOperationException {
DataCenterVO zone = zoneFunction.apply(vpc.getZoneId());
Pair<Boolean, Account> isNsxAndAccount = validateVpcConfigurationAndGetAccount(zone, vpc);
if (!isNsxAndAccount.first()) {
if (Boolean.FALSE.equals(isNsxAndAccount.first())) {
return true;
}
if (isNsxAndAccount.first() && Objects.isNull(isNsxAndAccount.second())) {
if (Boolean.TRUE.equals(isNsxAndAccount.first()) && Objects.isNull(isNsxAndAccount.second())) {
throw new InvalidParameterValueException(String.format("Failed to find account with id %s", vpc.getAccountId()));
}
Account account = isNsxAndAccount.second();
@ -337,12 +338,12 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, DnsS
@Override
public void processHostAdded(long hostId) {
// Do nothing
}
@Override
public void processConnect(Host host, StartupCommand cmd, boolean forRebalance) throws ConnectionException {
// Do nothing
}
@Override
@ -352,12 +353,12 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, DnsS
@Override
public void processHostAboutToBeRemoved(long hostId) {
// Do nothing
}
@Override
public void processHostRemoved(long hostId, long clusterId) {
// Do nothing
}
@Override
@ -387,5 +388,5 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, DnsS
return true;
}
private final Function<Long, DataCenterVO> zoneFunction = zoneId -> { return dataCenterDao.findById(zoneId); };
private final LongFunction<DataCenterVO> zoneFunction = zoneId -> dataCenterDao.findById(zoneId);
}

View File

@ -62,8 +62,6 @@ public class NsxGuestNetworkGuru extends GuestNetworkGuru implements NetworkMigr
@Inject
AccountDao accountDao;
private static final Networks.TrafficType[] TrafficTypes = {Networks.TrafficType.Guest};
public NsxGuestNetworkGuru() {
super();
_isolationMethods = new PhysicalNetwork.IsolationMethod[] {new PhysicalNetwork.IsolationMethod("NSX")};
@ -112,7 +110,7 @@ public class NsxGuestNetworkGuru extends GuestNetworkGuru implements NetworkMigr
@Override
@DB
public void deallocate(Network config, NicProfile nic, VirtualMachineProfile vm) {
// Do nothing
}
@Override
@ -156,7 +154,7 @@ public class NsxGuestNetworkGuru extends GuestNetworkGuru implements NetworkMigr
public void reserve(final NicProfile nic, final Network network, final VirtualMachineProfile vm,
final DeployDestination dest, final ReservationContext context)
throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException {
// Do nothing
}
@Override
@ -166,7 +164,7 @@ public class NsxGuestNetworkGuru extends GuestNetworkGuru implements NetworkMigr
@Override
public void shutdown(final NetworkProfile profile, final NetworkOffering offering) {
// Do nothing
}
@Override
@ -181,12 +179,12 @@ public class NsxGuestNetworkGuru extends GuestNetworkGuru implements NetworkMigr
@Override
public void rollbackMigration(NicProfile nic, Network network, VirtualMachineProfile vm, ReservationContext src, ReservationContext dst) {
// Do nothing
}
@Override
public void commitMigration(NicProfile nic, Network network, VirtualMachineProfile vm, ReservationContext src, ReservationContext dst) {
// Do nothing
}
private void createNsxSegment(NetworkVO networkVO, DataCenter zone) {

View File

@ -102,8 +102,18 @@ public class NsxProviderServiceImpl implements NsxProviderService {
final Host host = resourceManager.addHost(zoneId, nsxResource, nsxResource.getType(), params);
if (host != null) {
nsxProvider = Transaction.execute((TransactionCallback<NsxProviderVO>) status -> {
NsxProviderVO nsxProviderVO = new NsxProviderVO(zoneId, host.getId(), name, hostname,
username, password, tier0Gateway, edgeCluster);
NsxProviderVO nsxProviderVO = new NsxProviderVO.Builder()
.setZoneId(zoneId)
.setHostId(host.getId())
.setProviderName(name)
.setHostname(hostname)
.setPort(port)
.setUsername(username)
.setPassword(password)
.setTier0Gateway(tier0Gateway)
.setEdgeCluster(edgeCluster)
.build();
nsxProviderDao.persist(nsxProviderVO);
DetailVO detail = new DetailVO(host.getId(), "nsxcontrollerid",
@ -170,14 +180,7 @@ public class NsxProviderServiceImpl implements NsxProviderService {
for (PhysicalNetworkVO physicalNetwork : physicalNetworks) {
List<NetworkVO> networkList = networkDao.listByPhysicalNetwork(physicalNetwork.getId());
if (!CollectionUtils.isNullOrEmpty(networkList)) {
// Networks with broadcast type vcs are ours
for (NetworkVO network : networkList) {
if (network.getBroadcastDomainType() == Networks.BroadcastDomainType.NSX) {
if ((network.getState() != Network.State.Shutdown) && (network.getState() != Network.State.Destroy)) {
throw new CloudRuntimeException("This NSX Controller cannot be deleted as there are one or more logical networks provisioned by CloudStack on it.");
}
}
}
validateNetworkState(networkList);
}
}
nsxProviderDao.remove(nsxControllerId);
@ -192,4 +195,14 @@ public class NsxProviderServiceImpl implements NsxProviderService {
cmdList.add(DeleteNsxControllerCmd.class);
return cmdList;
}
private void validateNetworkState(List<NetworkVO> networkList) {
for (NetworkVO network : networkList) {
if (network.getBroadcastDomainType() == Networks.BroadcastDomainType.NSX) {
if ((network.getState() != Network.State.Shutdown) && (network.getState() != Network.State.Destroy)) {
throw new CloudRuntimeException("This NSX Controller cannot be deleted as there are one or more logical networks provisioned by CloudStack on it.");
}
}
}
}
}

View File

@ -47,6 +47,8 @@ import javax.naming.ConfigurationException;
import com.cloud.hypervisor.HypervisorGuru;
import com.cloud.network.dao.NsxProviderDao;
import com.cloud.network.element.NsxProviderVO;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.AffinityGroupService;
@ -451,6 +453,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
Ipv6GuestPrefixSubnetNetworkMapDao ipv6GuestPrefixSubnetNetworkMapDao;
@Inject
Ipv6Service ipv6Service;
@Inject
NsxProviderDao nsxProviderDao;
// FIXME - why don't we have interface for DataCenterLinkLocalIpAddressDao?
@Inject
@ -2527,6 +2531,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
}
// we should actually find the mapping and remove if it exists
// but we don't know about vmware/plugin/hypervisors at this point
NsxProviderVO nsxProvider = nsxProviderDao.findByZoneId(zoneId);
nsxProviderDao.remove(nsxProvider.getId());
final boolean success = _zoneDao.remove(zoneId);

View File

@ -368,8 +368,8 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
}
// configure default vpc offering with NSX as network service provider
if (_vpcOffDao.findByUniqueName(VpcOffering.defaultVPCNSXSOfferingName) == null) {
s_logger.debug("Creating default VPC offering with NSX as network service provider" + VpcOffering.defaultVPCNSXSOfferingName);
if (_vpcOffDao.findByUniqueName(VpcOffering.DEFAULT_VPC_NSX_OFFERING_NAME) == null) {
s_logger.debug("Creating default VPC offering with NSX as network service provider" + VpcOffering.DEFAULT_VPC_NSX_OFFERING_NAME);
final Map<Service, Set<Provider>> svcProviderMap = new HashMap<Service, Set<Provider>>();
final Set<Provider> defaultProviders = Set.of(Provider.Nsx);
for (final Service svc : getSupportedServices()) {
@ -380,7 +380,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
svcProviderMap.put(svc, defaultProviders);
}
}
createVpcOffering(VpcOffering.defaultVPCNSXSOfferingName, VpcOffering.defaultVPCNSXSOfferingName, svcProviderMap, false, State.Enabled, null, false, false, false);
createVpcOffering(VpcOffering.DEFAULT_VPC_NSX_OFFERING_NAME, VpcOffering.DEFAULT_VPC_NSX_OFFERING_NAME, svcProviderMap, false, State.Enabled, null, false, false, false);
}
}