swill commented on a change in pull request #4205:
URL: https://github.com/apache/cloudstack/pull/4205#discussion_r642585874
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java
##########
@@ -122,6 +122,11 @@
description = "true if network offering is meant to be used for
VPC, false otherwise.")
private Boolean forVpc;
+ @Parameter(name = ApiConstants.FOR_TUNGSTEN,
+ type = CommandType.BOOLEAN,
+ description = "true if network offering is meant to be used for
TUNGSTEN, false otherwise.")
Review comment:
Consider calling out "Tungsten Fabric" instead of just TUNGSTEN as this
is end user facing and may not be obvious for users what Tungsten is otherwise.
##########
File path: ui/scripts/docs.js
##########
@@ -629,6 +629,10 @@ cloudStack.docs = {
desc: 'Specify whether this offering is for a virtual private cloud',
externalLink: ''
},
+ helpNetworkOfferingTungsten: {
+ desc: 'Specify whether this offering is for tungsten SDN',
Review comment:
Again, for end user facing docs, I would recommend using "tungsten
fabric" instead of just "tungsten"
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VRouterVifDriver.java
##########
@@ -0,0 +1,95 @@
+package com.cloud.hypervisor.kvm.resource;
Review comment:
Missing the license header on this file.
##########
File path:
api/src/main/java/org/apache/cloudstack/api/response/NetworkOfferingResponse.java
##########
@@ -99,6 +99,10 @@
@Param(description = "true if network offering can be used by VPC networks
only")
private Boolean forVpc;
+ @SerializedName(ApiConstants.FOR_TUNGSTEN)
+ @Param(description = "true if network offering can be used by tungsten
networks only")
Review comment:
Another example of "tungsten" used in end user docs vs "tungsten fabric"
##########
File path:
api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java
##########
@@ -190,6 +190,10 @@
@Param(description = "VPC the network belongs to")
private String vpcId;
+ @SerializedName(ApiConstants.TUNGSTEN_VIRTUAL_ROUTER_UUID)
+ @Param(description = "Tungsten virtual router the network belongs to")
Review comment:
Again, end user docs should have full name of "tungsten fabric"
##########
File path:
plugins/network-elements/tungsten/src/main/java/org/apache/cloudstack/network/tungsten/api/command/ConfigTungstenServiceCmd.java
##########
@@ -0,0 +1,169 @@
+// 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.network.tungsten.api.command;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.Network;
+import com.cloud.network.NetworkModel;
+import com.cloud.network.Networks;
+import com.cloud.network.PhysicalNetworkServiceProvider;
+import com.cloud.network.dao.NetworkServiceMapDao;
+import com.cloud.network.dao.NetworkServiceMapVO;
+import com.cloud.network.dao.PhysicalNetworkServiceProviderDao;
+import com.cloud.network.dao.PhysicalNetworkServiceProviderVO;
+import com.cloud.offering.NetworkOffering;
+import com.cloud.offerings.NetworkOfferingServiceMapVO;
+import com.cloud.offerings.NetworkOfferingVO;
+import com.cloud.offerings.dao.NetworkOfferingDao;
+import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
+import com.cloud.user.Account;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallbackNoReturn;
+import com.cloud.utils.db.TransactionStatus;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.PhysicalNetworkResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.log4j.Logger;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.inject.Inject;
+
+@APICommand(name = "configTungstenService", description = "config tungsten
service", responseObject =
+ SuccessResponse.class, requestHasSensitiveInfo = false,
responseHasSensitiveInfo = false)
+public class ConfigTungstenServiceCmd extends BaseCmd {
+ public static final Logger s_logger =
Logger.getLogger(ConfigTungstenServiceCmd.class.getName());
+ private static final String s_name = "configtungstenserviceresponse";
+ public final static String DefaultTungstenNetworkOffering =
"DefaultTungstenNetworkOffering";
+
+ @Inject
+ NetworkModel _networkModel;
+ @Inject
+ NetworkOfferingDao _networkOfferingDao;
+ @Inject
+ NetworkOfferingServiceMapDao _networkOfferingServiceMapDao;
+ @Inject
+ NetworkServiceMapDao _networkServiceMapDao;
+ @Inject
+ PhysicalNetworkServiceProviderDao _physicalNetworkServiceProviderDao;
+
+ @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID,
entityType = ZoneResponse.class, required = true
+ , description = "the ID of zone")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.PHYSICAL_NETWORK_ID, type =
CommandType.UUID, entityType =
+ PhysicalNetworkResponse.class, required = true, description = "the ID
of physical network")
+ private Long physicalNetworkId;
+
+ public Long getZoneId() {
+ return zoneId;
+ }
+
+ public void setZoneId(final Long zoneId) {
+ this.zoneId = zoneId;
+ }
+
+ public Long getPhysicalNetworkId() {
+ return physicalNetworkId;
+ }
+
+ public void setPhysicalNetworkId(final Long physicalNetworkId) {
+ this.physicalNetworkId = physicalNetworkId;
+ }
+
+ @Override
+ public void execute() throws ResourceUnavailableException,
InsufficientCapacityException, ServerApiException,
+ ConcurrentOperationException, ResourceAllocationException,
NetworkRuleConflictException {
+ Transaction.execute(new TransactionCallbackNoReturn() {
+ @Override
+ public void doInTransactionWithoutResult(final TransactionStatus
status) {
+ NetworkOfferingVO networkOfferingVO =
_networkOfferingDao.findByUniqueName(
+ DefaultTungstenNetworkOffering);
+ if (networkOfferingVO == null) {
+ networkOfferingVO = new
NetworkOfferingVO(DefaultTungstenNetworkOffering,
+ "Default offering for Tungsten Network",
Networks.TrafficType.Guest, false, false, null, null,
+ true, NetworkOffering.Availability.Optional, null,
Network.GuestType.Isolated, true, false,
+ false, false, true, false);
+ networkOfferingVO.setForTungsten(true);
+ networkOfferingVO.setState(NetworkOffering.State.Enabled);
+ _networkOfferingDao.persist(networkOfferingVO);
+
+ Map<Network.Service, Network.Provider>
tungstenServiceProvider = new HashMap<>();
+ tungstenServiceProvider.put(Network.Service.Dhcp,
Network.Provider.Tungsten);
+ tungstenServiceProvider.put(Network.Service.Dns,
Network.Provider.Tungsten);
+ tungstenServiceProvider.put(Network.Service.UserData,
Network.Provider.Tungsten);
+ tungstenServiceProvider.put(Network.Service.SourceNat,
Network.Provider.Tungsten);
+ tungstenServiceProvider.put(Network.Service.StaticNat,
Network.Provider.Tungsten);
+ tungstenServiceProvider.put(Network.Service.Connectivity,
Network.Provider.Tungsten);
+ tungstenServiceProvider.put(Network.Service.Firewall,
Network.Provider.Tungsten);
+ tungstenServiceProvider.put(Network.Service.Lb,
Network.Provider.Tungsten);
+
tungstenServiceProvider.put(Network.Service.PortForwarding,
Network.Provider.Tungsten);
+
+ for (Network.Service service :
tungstenServiceProvider.keySet()) {
+ NetworkOfferingServiceMapVO
networkOfferingServiceMapVO = new NetworkOfferingServiceMapVO(
+ networkOfferingVO.getId(), service,
tungstenServiceProvider.get(service));
+
_networkOfferingServiceMapDao.persist(networkOfferingServiceMapVO);
+ }
+ }
+
+ PhysicalNetworkServiceProviderVO
physicalNetworkServiceProvider =
+ _physicalNetworkServiceProviderDao.findByServiceProvider(
+ physicalNetworkId, Network.Provider.Tungsten.getName());
+
physicalNetworkServiceProvider.setState(PhysicalNetworkServiceProvider.State.Enabled);
+
_physicalNetworkServiceProviderDao.persist(physicalNetworkServiceProvider);
+
+ Network publicNetwork =
_networkModel.getSystemNetworkByZoneAndTrafficType(zoneId,
+ Networks.TrafficType.Public);
+ NetworkServiceMapVO publicNetworkServiceMapVO = new
NetworkServiceMapVO(publicNetwork.getId(),
+ Network.Service.Connectivity, Network.Provider.Tungsten);
+ _networkServiceMapDao.persist(publicNetworkServiceMapVO);
+
+ Network managementNetwork =
_networkModel.getSystemNetworkByZoneAndTrafficType(zoneId,
+ Networks.TrafficType.Management);
+ NetworkServiceMapVO managementNetworkServiceMapVO = new
NetworkServiceMapVO(managementNetwork.getId(),
+ Network.Service.Connectivity, Network.Provider.Tungsten);
+ _networkServiceMapDao.persist(managementNetworkServiceMapVO);
+ }
+ });
+
+ SuccessResponse response = new SuccessResponse(getCommandName());
+ response.setDisplayText("config tungsten service successfully");
Review comment:
spelling? "configured tungsten service successfully". not a very
important change, but figured I would mention it as I noticed it.
##########
File path:
plugins/network-elements/tungsten/src/main/java/org/apache/cloudstack/network/tungsten/api/command/CreateTungstenProviderCmd.java
##########
@@ -0,0 +1,137 @@
+// 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.network.tungsten.api.command;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.TungstenProvider;
+import com.cloud.user.Account;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import
org.apache.cloudstack.network.tungsten.api.response.TungstenProviderResponse;
+import org.apache.cloudstack.network.tungsten.service.TungstenProviderService;
+
+import javax.inject.Inject;
+
+@APICommand(name = "createTungstenProvider", description = "Create tungsten
provider in cloudstack", responseObject =
Review comment:
consider referencing "tungsten fabric" instead of "tungsten" in the end
user facing api docs. implementation is fine, it is just what we show to the
users I am talking about.
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java
##########
@@ -119,14 +117,17 @@
private Long domainId;
@Parameter(name = ApiConstants.SUBDOMAIN_ACCESS,
- type = CommandType.BOOLEAN,
- description = "Defines whether to allow"
- + " subdomains to use networks dedicated to their parent
domain(s). Should be used with aclType=Domain, defaulted to
allow.subdomain.network.access global config if not specified")
+ type = CommandType.BOOLEAN,
+ description = "Defines whether to allow"
+ + " subdomains to use networks dedicated to their parent
domain(s). Should be used with aclType=Domain, defaulted to
allow.subdomain.network.access global config if not specified")
private Boolean subdomainAccess;
@Parameter(name = ApiConstants.VPC_ID, type = CommandType.UUID, entityType
= VpcResponse.class, description = "the VPC network belongs to")
private Long vpcId;
+ @Parameter(name = ApiConstants.TUNGSTEN_VIRTUAL_ROUTER_UUID, type =
CommandType.STRING, description = "tungsten virtual router the network belongs
to")
Review comment:
Again, given that it is end user facing, the description should probably
reference "Tungsten Fabric" instead of just Tungsten.
##########
File path: pom.xml
##########
@@ -131,6 +131,7 @@
<cs.google-http-client>1.34.2</cs.google-http-client>
<cs.groovy.version>2.4.17</cs.groovy.version>
<cs.gson.version>1.7.2</cs.gson.version>
+ <cs.solidfire.gson.version>2.6.2</cs.solidfire.gson.version>
Review comment:
I am not entirely sure why there is solidfire changes in this PR? Maybe
I am missing something?
##########
File path: plugins/network-elements/tungsten/pom.xml
##########
@@ -0,0 +1,47 @@
+<!--
+ 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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+ <artifactId>cloud-plugin-network-tungsten</artifactId>
+ <name>Apache CloudStack Plugin - Tungsten Network</name>
Review comment:
Maybe consider updating the human readable name to "Apache CloudStack
Plugin - Tungsten Fabric Network".
##########
File path:
plugins/network-elements/tungsten/src/main/java/org/apache/cloudstack/network/tungsten/api/command/ListTungstenProvidersCmd.java
##########
@@ -0,0 +1,81 @@
+// 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.network.tungsten.api.command;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
+import
org.apache.cloudstack.network.tungsten.api.response.TungstenProviderResponse;
+import org.apache.cloudstack.network.tungsten.service.TungstenProviderService;
+
+import javax.inject.Inject;
+import java.util.Arrays;
+
+@APICommand(name = "listTungstenProviders", responseObject =
TungstenProviderResponse.class, description = "Lists Tungsten providers",
Review comment:
again, consider using "tungsten fabric" when referencing it in user
facing docs.
##########
File path:
plugins/network-elements/tungsten/src/main/java/org/apache/cloudstack/network/tungsten/api/command/CreateTungstenProviderCmd.java
##########
@@ -0,0 +1,137 @@
+// 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.network.tungsten.api.command;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.TungstenProvider;
+import com.cloud.user.Account;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import
org.apache.cloudstack.network.tungsten.api.response.TungstenProviderResponse;
+import org.apache.cloudstack.network.tungsten.service.TungstenProviderService;
+
+import javax.inject.Inject;
+
+@APICommand(name = "createTungstenProvider", description = "Create tungsten
provider in cloudstack", responseObject =
Review comment:
I am not going to call out all the times it happens in this file. If
you take the suggestion, please update the other user facing api doc lines in
this doc as well.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
