You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nitin-maharana <gi...@git.apache.org> on 2016/12/23 09:25:18 UTC
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
GitHub user nitin-maharana opened a pull request:
https://github.com/apache/cloudstack/pull/1859
CLOUDSTACK - 8672 : NCC Integration with CloudStack
Feature Specification Link :
====================
https://cwiki.apache.org/confluence/display/CLOUDSTACK/NCC+Integration+With+Auto+Provision+VPX+in+CloudStack
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-8672
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1859.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1859
----
commit 73ece624c2595f8c9cd7ba75fd8fa978e80c0ecf
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-23T08:44:07Z
Schema changes for NCC support. 1. netscaler service pacages, netscaler control center tables.Modified network offering table to hold service package
commit 8062e3dcdbf346ef3fca2d3642a6bd2113a982a2
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-23T08:45:24Z
Added Support for Adding/Registering Netscaler Control Center Service Packages to CloudStack
commit ee0587994862178069ee33290f888e5c0f26cd4c
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-23T08:49:45Z
Added Support for the Registering Netscaler Control Center, Added associated VO's Dao's DaoImpl's
commit 85fd68ebd7304665d7c45a18902f3c7b1becbb1b
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-23T08:52:48Z
Modified Network offering to include the Netscaler Service pacakge
commit 57db30f948c982a51ea3b5fb32f21b70767eccdb
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-23T08:54:39Z
Modified Netscaler Plugin to support Netscaler Control Center Changes
commit 9bfb98aa2fb4b29edd3fed102b4e631687d01b0a
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-23T08:57:08Z
Added NetscalerControlCenter Resource supporte and Change State Support
commit 1ebb6eda7e8bce9208ffd88ef380e0b4623901f6
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-23T08:58:20Z
Added json libaray mvn dependency in the Netscaler Plugin
commit ef06739713766027c05c004fcd9e421743808ad0
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-07-31T08:44:33Z
CLOUDSTACK-8673: Created NCC CommandTimeout param which is configurable to poll the AsyncJobs send to NCC
commit 053ec527e0ef585a8df6fcf277fa1a781cfe67c1
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-08-06T16:13:31Z
CLOUDSTACK-8672: Implemented, implement the network with NCC as Service Provider.
NCC will allocate the device to network, configures guest vlan and SNIP in the NS Device
commit 06a943cb4ed998fb51e69e05277879bf0e9baa93
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-08-06T16:16:56Z
CLOUDSTACK-8672: Implemented NetscalerControlResource to manage and delegate the calls to NCC Device
commit db202b7a682b0580a61373dcd542457eeff38c28
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-08-06T16:18:31Z
CLOUDSTACK-8672 Modified LoadBalancerTO to hold network id and public ip Vlan
commit 4f081bd5d9eb2743b7c74845099ac4083f1161c0
Author: Priyank Parihar <pr...@citrix.com>
Date: 2015-08-07T11:10:00Z
CLOUDSTACK-8672: Added ListRegisteredServicePackage Command.
commit 1b72dbed1d033dd51891a78fc3398b5bb25d4f5b
Author: Priyank Parihar <pr...@citrix.com>
Date: 2015-08-07T11:29:54Z
CLOUDSTACK-8672: Added DeleteServicePackageOffering Command.
commit 9a600ce63bfc5311ceb5974870529cad94306795
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-07T11:40:13Z
CLOUDSTACK-8672: Added ListNetscalerControlCenter command.
commit d68ee0ebf27da2cd50ae0534ab147ed05cf33fd7
Author: Priyank Parihar <pr...@citrix.com>
Date: 2015-08-07T11:42:26Z
CLOUDSTACK-8672: Added DeleteNetscalerControlCenter Command.
commit 7be6fab98c146a1f548a90307c8c08b70f53a900
Author: Nitin Kumar Maharana <ni...@accelerite.com>
Date: 2016-12-18T18:13:23Z
LOUDSTACK-8672: Modified NCC Controlcenter Table
commit 6c583c20424f8d240010f8f126d98e4cad5fdcfa
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-24T11:15:49Z
CLOUDSTACK-8672 Added UI Support for the SSL Termination
1. UI Support for account to upload SSL Certificate.
2. UI Support for assigning SSL Certificate to a LB Rule.
3. Modified LB Rule creation UI to show LB Protocol and to show SSL based on LB Service provider capability.
commit 9c32d7ca4b30b1d47a57c5badda25258daeb819f
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-24T11:31:46Z
CLOUDSTACK-8672 New Dialog to assign a SSL certificate to LB Rule.
commit e0735fdc79d4abb3d8ac572f089bfa636d489f29
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-25T10:55:30Z
CLOUDSTACK-8672 Added UI Support for selecting Netscaler service packages when creating network offering with LB provier as Netscaler.
commit b5a868b60a27a24d270a1a1806152e75413f9702
Author: Priyank Parihar <pr...@citrix.com>
Date: 2015-08-26T11:18:14Z
CLOUDSTACK-8672: Added Acquire Pod Ip Command.
commit f76efa274c8ae36bf67e10bcd1e2ff5853771d78
Author: Priyank Parihar <pr...@citrix.com>
Date: 2015-08-26T11:20:31Z
CLOUDSTACK-8672: Added Release Pod Ip Command.
commit ac12b4f4c5570365697dd551fd9b87258c25f189
Author: Priyank Parihar <pr...@citrix.com>
Date: 2015-08-26T11:37:43Z
CLOUDSTACK-8672: Added Delete Netscaler Control Center Command.
commit 9ffe0733550a8426b6a0c8f979022d40807d64ce
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-27T06:38:38Z
CLOUDSTACK-8672 Added code for disabling the SSL Certificate Configuration button (After being assigned to LB) in UI.
commit 41ff6a4cdbebb231f8eaca019dc320238730e500
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-27T07:39:10Z
CLOUDSTACK-8672 Added UI support for hiding the LB Isolation box when we choose any netscaler service package.
commit 48170c5588aca941b462f1442f6a3f2fc6148f8f
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-27T11:21:28Z
CLOUDSTACK-8672 Added code for deleting SSL certificate from an account, added NAME parameter to a certificate and detail view of a certificate (UI).
commit 948309e8f3f5f08af0d63663eaa7df2689ef8e6e
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-28T11:46:34Z
CLOUDSTACK-8672 Added a button in Infrastructure page for Registering NCC (UI).
commit 7069093bffb1f3c7c2eb9cd5d635f4732ef65bb7
Author: Priyank Parihar <pr...@citrix.com>
Date: 2015-08-25T12:26:16Z
CLOUDSTACK-8672: Removal of servicePackageUUID from Create Network Offering Command,
Implementation of Acquire Pod Ip and Delete Netscaler Control Center Command.
commit a273edc059ffaece8f4150458c9e94d79e91f33a
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-08-27T08:23:24Z
CLOUDSTACK-8672 : Enchanced SSL termination feature with providing Name to the certificate and modified necessary services
commit 9510428e0d649ec4cf11b342d0b432bb4d5c207b
Author: Nitin Kumar Maharana <ni...@citrix.com>
Date: 2015-08-28T15:54:08Z
CLOUDSTACK-8672 Added code for handling the incomplete job case when registering NCC in Infrastructure page (UI).
commit 2924aaa76d995dcd0340336d904f42c653cd62dc
Author: Rajesh Battala <ra...@citrix.com>
Date: 2015-08-30T08:08:01Z
CLOUDSTACK-8672: Modified SSL Service to includ Name of the certificate while listing ssl certs.
Modified schema accordingly. Removed servivce package fk relationship on network offering table.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747795
--- Diff: server/src/com/cloud/configuration/ConfigurationManagerImpl.java ---
@@ -4781,6 +4818,12 @@ public boolean deleteNetworkOffering(final DeleteNetworkOfferingCmd cmd) {
+ "To make the network offering unavaiable, disable it");
}
+// if (offering.getServicePackage() != null) {
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1859
At least since August 2015, we've been following the guideline [1][2] for 2LGTMs and to squash changes when they are accepted. AFAIK RMs for 4.6+ have been asking PR authors to squash their changes before merging the PR once their PR is accepted, the consensus has been followed and enforced.
I've only requested the PR authors to follow what has been followed in the community/PRs for a very long time, however I encourage any disagreements to be debated on dev ML than be discussed and burried in a pull request as this affects everyone. /cc @karuturi
As an individual contributor, I prefer squashing changes for a single feature/FR into one or fewer commits. Sometimes, it may make sense to squash changes to more than one commit though 90 seems like an outrageous number. If multiple authors have worked on the PR, authors can consider squashing changes to lowest possible grouped/squashed changes. Lastly, we've also seen several feature PRs with several kloc+ changes that were accepted as a single squashed commit that may be searched through Github PR search page.
[1] http://markmail.org/thread/4cvxhm67ef356ppu
[2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61311655
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747629
--- Diff: plugins/network-elements/netscaler/src/com/cloud/network/element/NetscalerLoadBalancerElementService.java ---
@@ -71,4 +121,29 @@
* @return NetscalerLoadBalancerResponse
*/
public NetscalerLoadBalancerResponse createNetscalerLoadBalancerResponse(ExternalLoadBalancerDeviceVO lbDeviceVO);
-}
+
+ /**
+ * creates API response object for netscaler load balancers
+ * @param lbDeviceVO external load balancer VO object
+ * @return NetscalerLoadBalancerResponse
+ */
+ public NetScalerServicePackageResponse registerNetscalerServicePackage(RegisterServicePackageCmd cmd);
+
+ public NetscalerControlCenterResponse createNetscalerControlCenterResponse(NetScalerControlCenterVO lncCentersVO);
+
+ public NetScalerServicePackageResponse createRegisteredServicePackageResponse(NetScalerServicePackageVO lrsPackageVO);
+
+ public NetScalerServicePackageResponse deleteNetscalerServicePackage(RegisterServicePackageCmd cmd);
+
+ public NetScalerServicePackageResponse listNetscalerServicePackage(RegisterServicePackageCmd cmd);
+
+ public NetScalerServicePackageResponse createNetscalerServicePackageResponse(NetScalerServicePackageVO servicePackageVO);
+
+ public NetScalerControlCenterVO registerNetscalerControlCenter(RegisterNetscalerControlCenterCmd registerNetscalerControlCenterCmd);
+
+ //public VirtualMachine deployNetscalerServiceVm(DeployNetscalerVpxCmd deployNetscalerVpxCmd);
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
@rajesh-battala : Currently running, will post once completes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1859
@nitin-maharana I was looking at the PR.
Do you need to split everything thing there in a different commit?
I think that we still do not have a clear understanding when and how to separate things in a commit; however, anything like 90+ commits in a PR seems exaggerated to me.
I like the philosophy that for every commit we should be able to get the system, build it and use it. This, for instance, would not happen here. Again, as I said, I think we do not have a clear rule about that, but I would like others to check this situation as well.
@DaanHoogland, @rhtyd, @swill any thoughts here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93748252
--- Diff: ui/scripts/configuration.js ---
@@ -2640,12 +2640,30 @@
}
//show LB Isolation dropdown only when (1)LB Service is checked (2)Service Provider is Netscaler OR F5
- if ((args.$form.find('.form-item[rel=\"service.Lb.isEnabled\"]').find('input[type=checkbox]').is(':checked') == true) && (args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'Netscaler' || args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'F5BigIp')) {
+ /*if ((args.$form.find('.form-item[rel=\"service.Lb.isEnabled\"]').find('input[type=checkbox]').is(':checked') == true) && (args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'Netscaler' || args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'F5BigIp')) {
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93746542
--- Diff: engine/schema/src/com/cloud/dc/dao/DataCenterIpAddressDaoImpl.java ---
@@ -177,6 +177,22 @@ public void releaseIpAddress(long nicId, String reservationId) {
}
@Override
+ public void releasePodIpAddress(long id) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Releasing ip address for ID=" + id);
+ }
+// SearchCriteria<DataCenterIpAddressVO> sc = AllFieldsSearch.create();
+// sc.setParameters("id", id);
+
+ DataCenterIpAddressVO vo = this.findById(id);
+ vo.setTakenAt(null);
+ vo.setInstanceId(null);
+ vo.setReservationId(null);
+ persist(vo);
+ //update(vo, sc);
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the issue:
https://github.com/apache/cloudstack/pull/1859
I have accepted the patch you can push it to master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Please find the test plan executed from below link(Updated in wiki page).
https://cwiki.apache.org/confluence/display/CLOUDSTACK/NCC+Integration+with+CloudStack+Test+Plan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747990
--- Diff: server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java ---
@@ -1157,14 +1222,28 @@ protected IpDeployer getIpDeployerForInlineMode(Network network) {
return null;
}
- ExternalLoadBalancerDeviceVO lbDeviceVO = getExternalLoadBalancerForNetwork(network);
+ HostVO externalLoadBalancer = null;
+
+ if(isNccServiceProvider(network)) {
+ externalLoadBalancer = getNetScalerControlCenterForNetwork(network);
+ } else {
+ ExternalLoadBalancerDeviceVO lbDeviceVO = getExternalLoadBalancerForNetwork(network);
+ if (lbDeviceVO == null) {
+ s_logger.warn("There is no external load balancer device assigned to this network either network is not implement are already shutdown so just returning");
+ return null;
+ } else {
+ externalLoadBalancer = _hostDao.findById(lbDeviceVO.getHostId());
+ }
+ }
+
+/* ExternalLoadBalancerDeviceVO lbDeviceVO = getExternalLoadBalancerForNetwork(network);
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93746500
--- Diff: engine/schema/src/com/cloud/dc/dao/DataCenterIpAddressDaoImpl.java ---
@@ -177,6 +177,22 @@ public void releaseIpAddress(long nicId, String reservationId) {
}
@Override
+ public void releasePodIpAddress(long id) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Releasing ip address for ID=" + id);
+ }
+// SearchCriteria<DataCenterIpAddressVO> sc = AllFieldsSearch.create();
--- End diff --
Remove this unused code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747510
--- Diff: plugins/network-elements/netscaler/src/com/cloud/network/NetScalerControlCenterVO.java ---
@@ -0,0 +1,129 @@
+// 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 com.cloud.network;
+
+import java.util.UUID;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.Table;
+
+import org.apache.cloudstack.api.InternalIdentity;
+
+
+/**
+ * NetScalerPodVO contains information about a EIP deployment where on datacenter L3 router a PBR (policy
--- End diff --
Remove this description and re-write it matching to the current file
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by sowmyakrishn <gi...@git.apache.org>.
Github user sowmyakrishn commented on the issue:
https://github.com/apache/cloudstack/pull/1859
LGTM based on following tests:
Tested with Automated runs for Shared and Dedicated Isolation policies - test_ncc_integration_dedicated.py and test_ncc_integration_shared.py (also part of this PR) and both tests have passed.
Result:
====
test_01_shared_first_network (integration.component.test_ncc_integration_shared.TestNccIntegrationShared) ... === TestName: test_01_shared_first_network | Status : SUCCESS ===
ok
test_02_shared_another_network (integration.component.test_ncc_integration_shared.TestNccIntegrationShared) ... === TestName: test_02_shared_another_network | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 2 tests in 472.393s
OK
test_01_dedicated_first_network (integration.component.test_ncc_integration_dedicated.TestNccIntegrationDedicated) ... === TestName: test_01_dedicated_first_network | Status : SUCCESS ===
ok
test_02_dedicated_another_network (integration.component.test_ncc_integration_dedicated.TestNccIntegrationDedicated) ... === TestName: test_02_dedicated_another_network | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 2 tests in 234.199s
OK
Further, manual tests were also carried out successfully as per the Test Plan mentioned in https://cwiki.apache.org/confluence/display/CLOUDSTACK/NCC+Integration+with+CloudStack+Test+Plan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Hi @rajesh-battala, I have modified the code. Please review it once and let me know if any improvement required. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747579
--- Diff: plugins/network-elements/netscaler/src/com/cloud/network/dao/NetScalerServicePackageDaoImpl.java ---
@@ -0,0 +1,72 @@
+// 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 com.cloud.network.dao;
+
+import java.util.List;
+
+import javax.ejb.Local;
+
+import org.springframework.stereotype.Component;
+
+import com.cloud.network.NetScalerServicePackageVO;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+
+@Component
+@Local(value = NetScalerServicePackageDao.class)
+@DB
+public class NetScalerServicePackageDaoImpl extends GenericDaoBase<NetScalerServicePackageVO, Long> implements NetScalerServicePackageDao {
+
+ final SearchBuilder<NetScalerServicePackageVO> podIdSearch;
+ final SearchBuilder<NetScalerServicePackageVO> deviceIdSearch;
+
+ protected NetScalerServicePackageDaoImpl() {
+ super();
+
+ podIdSearch = createSearchBuilder();
+ //podIdSearch.and("pod_id", podIdSearch.entity().getPodId(), Op.EQ);
+ podIdSearch.done();
+
+ deviceIdSearch = createSearchBuilder();
+ //deviceIdSearch.and("netscalerDeviceId", deviceIdSearch.entity().getNetscalerDeviceId(), Op.EQ);
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
ping @rajesh-battala @sowmyakrishn
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
@rafaelweingartner : As there are multiple contributors to this feature, If I squash it to one commit, then others are going to lose their part of contributions. Initially, we thought of making it to one commit, but this is the main reason we pushed with multiple commits. Let's wait for others to comment on this, after that we will decide. Thanks, @rafaelweingartner for pitching in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:
https://github.com/apache/cloudstack/pull/1859
I was one of the RM for 4.6. We never made such a decision nor did we follow. There were multiple discussions on ML already about squashing vs merging and we never concluded to do squashing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93749580
--- Diff: ui/scripts/configuration.js ---
@@ -2640,12 +2640,30 @@
}
//show LB Isolation dropdown only when (1)LB Service is checked (2)Service Provider is Netscaler OR F5
- if ((args.$form.find('.form-item[rel=\"service.Lb.isEnabled\"]').find('input[type=checkbox]').is(':checked') == true) && (args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'Netscaler' || args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'F5BigIp')) {
+ /*if ((args.$form.find('.form-item[rel=\"service.Lb.isEnabled\"]').find('input[type=checkbox]').is(':checked') == true) && (args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'Netscaler' || args.$form.find('.form-item[rel=\"service.Lb.provider\"]').find('select').val() == 'F5BigIp')) {
--- End diff --
@rajesh-battala : I will make required changes. Thanks for quick reivew.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Folks, what about a middle ground here?
I was checking the commits. For instance, all of the commits "Added/implemented XXX ." could be all squashed by the same author. There are a bunch of commits in this style that are introducing a single class. Also, subsequent commits that change the introduced classes by the same author can also be squashed. Therefore, no one loses merit and the history is maintained.
After the squashing process is done, we can evaluate and discuss the situation further.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
It has already two LGTMs and contains all successful test results. This is a big change, as time passes there are more chances of conflicts appearance(Already resolved once). If anyone wants to review, please do it else we should consider merging this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK-8672 : NCC Integration with CloudS...
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r94272083
--- Diff: plugins/network-elements/netscaler/src/com/cloud/api/commands/DeployNetscalerVpxCmd.java ---
@@ -0,0 +1,149 @@
+// 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 com.cloud.api.commands;
+
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.ACL;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.NetworkResponse;
+import org.apache.cloudstack.api.response.ServiceOfferingResponse;
+import org.apache.cloudstack.api.response.SystemVmResponse;
+import org.apache.cloudstack.api.response.TemplateResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.api.response.NetscalerLoadBalancerResponse;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.element.NetscalerLoadBalancerElementService;
+import com.cloud.user.Account;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+
+@APICommand(name = "deployNetscalerVpx", responseObject = NetscalerLoadBalancerResponse.class, description = "Creates new NS Vpx",
+ requestHasSensitiveInfo = true, responseHasSensitiveInfo = false)
+public class DeployNetscalerVpxCmd extends BaseAsyncCmd {
+
+ public static final Logger s_logger = Logger.getLogger(DeployNetscalerVpxCmd.class.getName());
+ private static final String s_name = "deployNetscalerVpx";
+ @Inject
+ NetscalerLoadBalancerElementService _netsclarLbService;
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, required = true, description = "availability zone for the virtual machine")
+ private Long zoneId;
+
+ @ACL
+ @Parameter(name = ApiConstants.SERVICE_OFFERING_ID, type = CommandType.UUID, entityType = ServiceOfferingResponse.class, required = true, description = "the ID of the service offering for the virtual machine")
+ private Long serviceOfferingId;
+
+ @ACL
+ @Parameter(name = ApiConstants.TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, required = true, description = "the ID of the template for the virtual machine")
+ private Long templateId;
+
+ @Parameter(name = ApiConstants.NETWORK_ID,
+ type = CommandType.UUID,
+ entityType = NetworkResponse.class, required=false,
+ description = "The network this ip address should be associated to.")
+ private Long networkId;
+ /////////////////////////////////////////////////////
+ /////////////////// Accessors ///////////////////////
+ /////////////////////////////////////////////////////
+
+
+
+ /////////////////////////////////////////////////////
+ /////////////// API Implementation///////////////////
+ /////////////////////////////////////////////////////
+
+ @Override
+ public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
+ ResourceAllocationException {
+ try {
+ Map<String,Object> resp = _netsclarLbService.deployNetscalerServiceVm(this);
+ if (resp.size() > 0) {
+ SystemVmResponse response = _responseGenerator.createSystemVmResponse((VirtualMachine)resp.get("vm"));
+ response.setGuestVlan((String)resp.get("guestvlan"));
+ response.setPublicVlan((List<String>)resp.get("publicvlan"));
+ response.setResponseName(getCommandName());
+ setResponseObject(response);
+ } else {
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Fail to start system vm");
+ }
+
+ } catch (InvalidParameterValueException invalidParamExcp) {
+ throw new ServerApiException(ApiErrorCode.PARAM_ERROR, invalidParamExcp.getMessage());
+ } catch (CloudRuntimeException runtimeExcp) {
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, runtimeExcp.getMessage());
+ }
+ }
+
+ public Long getServiceOfferingId() {
+ return serviceOfferingId;
+ }
+
+ public Long getTemplateId() {
+ return templateId;
+ }
+
+ @Override
+ public String getEventDescription() {
+ return "Adding a netscaler load balancer device";
+ }
+
+ @Override
+ public String getEventType() {
+ return EventTypes.EVENT_NETSCALER_VM_START;
+ }
+
+ @Override
+ public String getCommandName() {
+ return s_name;
+ }
+
+ public Long getZoneId() {
+ return zoneId;
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ return CallContext.current().getCallingAccount().getId();
+ }
+ public Account getAccount(){
+ return _accountService.getActiveAccountById(getEntityOwnerId());
+ }
+ public void getReservationContext() {
+ //ReservationContext context = new ReservationContextImpl(UUID.randomUUID().toString(), null , cmd.getAccount());
--- End diff --
@rajesh-battala : Right now, There is no usage of this function. So removal of this won't have any implication. And the return type should also be ReservationContext. I am correcting this code by returning the context, so that it can be used in future.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK-8672 : NCC Integration with CloudS...
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r94271338
--- Diff: plugins/network-elements/netscaler/src/com/cloud/network/vm/NetScalerVMManager.java ---
@@ -0,0 +1,86 @@
+//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 com.cloud.network.vm;
+import java.util.Map;
+
+import com.cloud.api.commands.DeployNetscalerVpxCmd;
+import com.cloud.deploy.DeployDestination;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.router.VirtualRouter;
+import com.cloud.user.Account;
+
+public interface NetScalerVMManager {
+
+//RAM/CPU for the system offering used by Internal LB VMs
+ public static final int DEFAULT_NETSCALER_VM_RAMSIZE = 2048; // 2048 MB
+ public static final int DEFAULT_NETSCALER_VM_CPU_MHZ = 1024; // 1024 MHz
+
+/* *//**
+ * Destroys Internal LB vm instance
+ * @param vmId
+ * @param caller
+ * @param callerUserId
+ * @return
+ * @throws ResourceUnavailableException
+ * @throws ConcurrentOperationException
+ *//*
+ boolean destroyInternalLbVm(long vmId, Account caller, Long callerUserId) throws ResourceUnavailableException, ConcurrentOperationException;
+
+ *//**
+ * Deploys internal lb vm
+ * @param guestNetwork
+ * @param requestedGuestIp
+ * @param dest
+ * @param owner
+ * @param params
+ * @return
+ * @throws InsufficientCapacityException
+ * @throws ConcurrentOperationException
+ * @throws ResourceUnavailableException
+ *//*
+ List<? extends VirtualRouter> deployInternalLbVm(Network guestNetwork, Ip requestedGuestIp, DeployDestination dest, Account owner, Map<Param, Object> params)
+ throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException;
+
+ *//**
+ *
+ * @param network
+ * @param rules
+ * @param internalLbVms
+ * @return
+ * @throws ResourceUnavailableException
+ *//*
+ boolean applyLoadBalancingRules(Network network, List<LoadBalancingRule> rules, List<? extends VirtualRouter> internalLbVms) throws ResourceUnavailableException;
+
+ *//**
+ * Returns existing Internal Load Balancer elements based on guestNetworkId (required) and requestedIp (optional)
+ * @param guestNetworkId
+ * @param requestedGuestIp
+ * @return
+ *//*
+ List<? extends VirtualRouter> findInternalLbVms(long guestNetworkId, Ip requestedGuestIp);
+*/
+
+ public Map<String, Object> deployNetscalerServiceVm(DeployNetscalerVpxCmd cmd);
+
+ public VirtualRouter stopNetscalerServiceVm(Long id, boolean forced, Account callingAccount, long callingUserId) throws ConcurrentOperationException, ResourceUnavailableException;
+ public Map<String, Object> deployNsVpx(Account owner, DeployDestination dest, DeploymentPlan plan, long svcOffId, long templateId) throws InsufficientCapacityException;
+
+public VirtualRouter stopNetScalerVm(Long id, boolean forced, Account callingAccount, long callingUserId);
+}
--- End diff --
@rajesh-battala : There is no such character. It just says there is no new line at the end of the file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Ping @sowmyakrishn @jayapalu
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by cloudmonger <gi...@git.apache.org>.
Github user cloudmonger commented on the issue:
https://github.com/apache/cloudstack/pull/1859
### ACS CI BVT Run
**Sumarry:**
Build Number 439
Hypervisor xenserver
NetworkType Advanced
Passed=105
Failed=0
Skipped=7
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
**Skipped tests:**
test_01_test_vm_volume_snapshot
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_11_ss_nfs_version_on_ssvm
test_nested_virtualization_vmware
test_3d_gpu_support
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_network.py
test_router_dns.py
test_non_contigiousvlan.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_routers_network_ops.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by priyankparihar <gi...@git.apache.org>.
Github user priyankparihar commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Hi @rhtyd,
>From my experience RM-ing for 4.3, 4.5, 4.9 -- the git history is pretty messed-up and it becomes far too difficult to track changes.
I think everything has its own + and -. Multiple commits provide more insight and clarity on history. Could you please provide some more advantages of squashing with proper proof.
>From my experience
This statement seems very loose. I think it needs some improvement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
@rajesh-battala : Will update the same in wiki page.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747566
--- Diff: plugins/network-elements/netscaler/src/com/cloud/network/dao/NetScalerServicePackageDaoImpl.java ---
@@ -0,0 +1,72 @@
+// 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 com.cloud.network.dao;
+
+import java.util.List;
+
+import javax.ejb.Local;
+
+import org.springframework.stereotype.Component;
+
+import com.cloud.network.NetScalerServicePackageVO;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+
+@Component
+@Local(value = NetScalerServicePackageDao.class)
+@DB
+public class NetScalerServicePackageDaoImpl extends GenericDaoBase<NetScalerServicePackageVO, Long> implements NetScalerServicePackageDao {
+
+ final SearchBuilder<NetScalerServicePackageVO> podIdSearch;
+ final SearchBuilder<NetScalerServicePackageVO> deviceIdSearch;
+
+ protected NetScalerServicePackageDaoImpl() {
+ super();
+
+ podIdSearch = createSearchBuilder();
+ //podIdSearch.and("pod_id", podIdSearch.entity().getPodId(), Op.EQ);
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747834
--- Diff: server/src/com/cloud/hypervisor/HypervisorGuruBase.java ---
@@ -132,7 +133,12 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
NicTO[] nics = new NicTO[nicProfiles.size()];
int i = 0;
for (NicProfile nicProfile : nicProfiles) {
- nics[i++] = toNicTO(nicProfile);
+ if(vm.getType() == VirtualMachine.Type.NetScalerVm) {
+ //if(i == 0){
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Thanks.
Once its update please point the same here. So that every one knows it.
Thanks
Rajesh Battala
On Tue, Mar 7, 2017 at 9:27 AM, Nitin Kumar Maharana <
notifications@github.com> wrote:
> @rajesh-battala <https://github.com/rajesh-battala> : Will update the
> same in wiki page.
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/cloudstack/pull/1859#issuecomment-284614912>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AA-EAm15q158RoiQx4WqAuynOfrv1xQBks5rjNWkgaJpZM4LUre4>
> .
>
--
Thanks
Rajesh Battala
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747256
--- Diff: plugins/network-elements/netscaler/src/com/cloud/api/commands/DeployNetscalerVpxCmd.java ---
@@ -0,0 +1,149 @@
+// 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 com.cloud.api.commands;
+
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.ACL;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.NetworkResponse;
+import org.apache.cloudstack.api.response.ServiceOfferingResponse;
+import org.apache.cloudstack.api.response.SystemVmResponse;
+import org.apache.cloudstack.api.response.TemplateResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.api.response.NetscalerLoadBalancerResponse;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.element.NetscalerLoadBalancerElementService;
+import com.cloud.user.Account;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+
+@APICommand(name = "deployNetscalerVpx", responseObject = NetscalerLoadBalancerResponse.class, description = "Creates new NS Vpx",
+ requestHasSensitiveInfo = true, responseHasSensitiveInfo = false)
+public class DeployNetscalerVpxCmd extends BaseAsyncCmd {
+
+ public static final Logger s_logger = Logger.getLogger(DeployNetscalerVpxCmd.class.getName());
+ private static final String s_name = "deployNetscalerVpx";
+ @Inject
+ NetscalerLoadBalancerElementService _netsclarLbService;
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, required = true, description = "availability zone for the virtual machine")
+ private Long zoneId;
+
+ @ACL
+ @Parameter(name = ApiConstants.SERVICE_OFFERING_ID, type = CommandType.UUID, entityType = ServiceOfferingResponse.class, required = true, description = "the ID of the service offering for the virtual machine")
+ private Long serviceOfferingId;
+
+ @ACL
+ @Parameter(name = ApiConstants.TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, required = true, description = "the ID of the template for the virtual machine")
+ private Long templateId;
+
+ @Parameter(name = ApiConstants.NETWORK_ID,
+ type = CommandType.UUID,
+ entityType = NetworkResponse.class, required=false,
+ description = "The network this ip address should be associated to.")
+ private Long networkId;
+ /////////////////////////////////////////////////////
+ /////////////////// Accessors ///////////////////////
+ /////////////////////////////////////////////////////
+
+
+
+ /////////////////////////////////////////////////////
+ /////////////// API Implementation///////////////////
+ /////////////////////////////////////////////////////
+
+ @Override
+ public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
+ ResourceAllocationException {
+ try {
+ Map<String,Object> resp = _netsclarLbService.deployNetscalerServiceVm(this);
+ if (resp.size() > 0) {
+ SystemVmResponse response = _responseGenerator.createSystemVmResponse((VirtualMachine)resp.get("vm"));
+ response.setGuestVlan((String)resp.get("guestvlan"));
+ response.setPublicVlan((List<String>)resp.get("publicvlan"));
+ response.setResponseName(getCommandName());
+ setResponseObject(response);
+ } else {
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Fail to start system vm");
+ }
+
+ } catch (InvalidParameterValueException invalidParamExcp) {
+ throw new ServerApiException(ApiErrorCode.PARAM_ERROR, invalidParamExcp.getMessage());
+ } catch (CloudRuntimeException runtimeExcp) {
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, runtimeExcp.getMessage());
+ }
+ }
+
+ public Long getServiceOfferingId() {
+ return serviceOfferingId;
+ }
+
+ public Long getTemplateId() {
+ return templateId;
+ }
+
+ @Override
+ public String getEventDescription() {
+ return "Adding a netscaler load balancer device";
+ }
+
+ @Override
+ public String getEventType() {
+ return EventTypes.EVENT_NETSCALER_VM_START;
+ }
+
+ @Override
+ public String getCommandName() {
+ return s_name;
+ }
+
+ public Long getZoneId() {
+ return zoneId;
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ return CallContext.current().getCallingAccount().getId();
+ }
+ public Account getAccount(){
+ return _accountService.getActiveAccountById(getEntityOwnerId());
+ }
+ public void getReservationContext() {
+ //ReservationContext context = new ReservationContextImpl(UUID.randomUUID().toString(), null , cmd.getAccount());
--- End diff --
Remove this unused code .
See if there are any implications if reservation context is not returned.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Current commits are in the order of some milestones.
As @nitin-maharan told if we squash them into one commit we loose
contributions of induviduals.
On Thu, Mar 16, 2017 at 7:09 PM, Rohit Yadav <no...@github.com>
wrote:
> @nitin-maharana <https://github.com/nitin-maharana> it becomes easier to
> triage changes when changes are confined to a limited number of commits
> (ideally one per PR), please squash the commits based on the author (if not
> to a single commit) if you don't agree. Ideally, you can also group/squash
> commits based on the component/framework/architecture.
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/cloudstack/pull/1859#issuecomment-287059366>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AA-EAqDvOOOnKfG81zO6sgQ9KmIN0W92ks5rmTufgaJpZM4LUre4>
> .
>
--
Thanks
Rajesh Battala
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93746916
--- Diff: plugins/network-elements/netscaler/src/com/cloud/api/commands/DeleteServicePackageOfferingCmd.java ---
@@ -0,0 +1,95 @@
+// 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 com.cloud.api.commands;
+
+import javax.inject.Inject;
+import javax.persistence.EntityExistsException;
+
+import org.apache.log4j.Logger;
+
+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.context.CallContext;
+
+import org.apache.cloudstack.api.response.SuccessResponse;
+
+import com.cloud.api.response.NetScalerServicePackageResponse;
+import com.cloud.exception.ConcurrentOperationException;
+//import com.cloud.exception.InvalidParameterValueException;
--- End diff --
Remove this unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Amazing effort.
From the test results We are good to merge to master.
Thansk
Rajesh Battala
On Wed, Mar 8, 2017 at 12:24 PM, Nitin Kumar Maharana <
notifications@github.com> wrote:
> tag:mergeready
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/cloudstack/pull/1859#issuecomment-284962756>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AA-EAmjH3MtLU9J1G4rgERa1j7a5V3a7ks5rjlCrgaJpZM4LUre4>
> .
>
--
Thanks
Rajesh Battala
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747692
--- Diff: plugins/network-elements/netscaler/src/com/cloud/network/vm/NetScalerVMManager.java ---
@@ -0,0 +1,86 @@
+//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 com.cloud.network.vm;
+import java.util.Map;
+
+import com.cloud.api.commands.DeployNetscalerVpxCmd;
+import com.cloud.deploy.DeployDestination;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.router.VirtualRouter;
+import com.cloud.user.Account;
+
+public interface NetScalerVMManager {
+
+//RAM/CPU for the system offering used by Internal LB VMs
+ public static final int DEFAULT_NETSCALER_VM_RAMSIZE = 2048; // 2048 MB
+ public static final int DEFAULT_NETSCALER_VM_CPU_MHZ = 1024; // 1024 MHz
+
+/* *//**
+ * Destroys Internal LB vm instance
+ * @param vmId
+ * @param caller
+ * @param callerUserId
+ * @return
+ * @throws ResourceUnavailableException
+ * @throws ConcurrentOperationException
+ *//*
+ boolean destroyInternalLbVm(long vmId, Account caller, Long callerUserId) throws ResourceUnavailableException, ConcurrentOperationException;
+
+ *//**
+ * Deploys internal lb vm
+ * @param guestNetwork
+ * @param requestedGuestIp
+ * @param dest
+ * @param owner
+ * @param params
+ * @return
+ * @throws InsufficientCapacityException
+ * @throws ConcurrentOperationException
+ * @throws ResourceUnavailableException
+ *//*
+ List<? extends VirtualRouter> deployInternalLbVm(Network guestNetwork, Ip requestedGuestIp, DeployDestination dest, Account owner, Map<Param, Object> params)
+ throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException;
+
+ *//**
+ *
+ * @param network
+ * @param rules
+ * @param internalLbVms
+ * @return
+ * @throws ResourceUnavailableException
+ *//*
+ boolean applyLoadBalancingRules(Network network, List<LoadBalancingRule> rules, List<? extends VirtualRouter> internalLbVms) throws ResourceUnavailableException;
+
+ *//**
+ * Returns existing Internal Load Balancer elements based on guestNetworkId (required) and requestedIp (optional)
+ * @param guestNetworkId
+ * @param requestedGuestIp
+ * @return
+ *//*
+ List<? extends VirtualRouter> findInternalLbVms(long guestNetworkId, Ip requestedGuestIp);
+*/
+
+ public Map<String, Object> deployNetscalerServiceVm(DeployNetscalerVpxCmd cmd);
+
+ public VirtualRouter stopNetscalerServiceVm(Long id, boolean forced, Account callingAccount, long callingUserId) throws ConcurrentOperationException, ResourceUnavailableException;
+ public Map<String, Object> deployNsVpx(Account owner, DeployDestination dest, DeploymentPlan plan, long svcOffId, long templateId) throws InsufficientCapacityException;
+
+public VirtualRouter stopNetScalerVm(Long id, boolean forced, Account callingAccount, long callingUserId);
+}
--- End diff --
what is this extra character ("theta") at the end of this file?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the issue:
https://github.com/apache/cloudstack/pull/1859
@nitin-maharana @sowmyakrishn Can you please check why CI tests are failing?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
tag:mergeready
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1859
@nitin-maharana it becomes easier to triage changes when changes are confined to a limited number of commits (ideally one per PR), please squash the commits based on the author (if not to a single commit) if you don't agree. Ideally, you can also group/squash commits based on the component/framework/architecture.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1859
@rajesh-battala I won't accept this PR, sorry to share this but the number of commits are simply outrageous.
From my experience RM-ing for 4.3, 4.5, 4.9 -- the git history is pretty messed-up and it becomes far too difficult to track changes, backport/up-port commits/fixes/feature. There have been big PR/features that have been accepted and merged with very few overall squashed commits.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93746791
--- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java ---
@@ -87,7 +87,15 @@ public Answer execute(final StartCommand command, final CitrixResourceBase citri
final Host host = Host.getByUuid(conn, citrixResourceBase.getHost().getUuid());
vm = citrixResourceBase.createVmFromTemplate(conn, vmSpec, host);
-
+ //Add xenstore data for the NetscalerVM
+/* if(vmSpec.getType()== VirtualMachine.Type.NetScalerVm) {
--- End diff --
Remove this unused/commented code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93746847
--- Diff: plugins/network-elements/netscaler/src/com/cloud/api/commands/DeleteNetscalerControlCenterCmd.java ---
@@ -0,0 +1,98 @@
+// 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 com.cloud.api.commands;
+
+import javax.inject.Inject;
+import javax.persistence.EntityExistsException;
+
+import org.apache.log4j.Logger;
+
+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.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.api.response.NetscalerControlCenterResponse;
+import com.cloud.exception.ConcurrentOperationException;
+//import com.cloud.exception.InvalidParameterValueException;
--- End diff --
Remove this unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:
https://github.com/apache/cloudstack/pull/1859
When debugging, I have seen issues with humongous commits with little or no history. They are very difficult to understand.
This is a feature and it need not be backported. so, backporting to previous releases is out of question. up-porting automatically happens.
If we want to revert the feature, we revert the merge which is very easy to do.
Also, reverting (for feature stability reasons) is just temporary.
I agree that the number of commits is high. But, the amount of code is also high and from different authors who wants to keep it this way.
I am +1 with keeping it as is.
I prefer merges with proper history and small logical units.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Can you please share the test cases and Test plan executed for this feature.
It would be great if community knows the testing done on this feature and its coverage.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1859: CLOUDSTACK - 8672 : NCC Integration with Clou...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1859#discussion_r93747870
--- Diff: server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java ---
@@ -900,6 +945,17 @@ public boolean applyLoadBalancerRules(Network network, List<LoadBalancingRule> l
String algorithm = rule.getAlgorithm();
String uuid = rule.getUuid();
String srcIp = rule.getSourceIp().addr();
+ String srcIpVlan = null;
+ String srcIpGateway = null;
+ String srcIpNetmask = null;
+ //IpAddress ipAddress = _networkModel.getPublicIpAddress(rule.getSourceIp().addr(), network.getDataCenterId()).getVlanId();
--- End diff --
Remove unused code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1859: CLOUDSTACK-8672 : NCC Integration with CloudStack
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1859
Sure @rajesh-battala, will look at that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---