You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/07/14 00:28:35 UTC
[GitHub] [cloudstack] kioie opened a new pull request #3997: New API endpoint: UpdateVlanIpRange
kioie opened a new pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997
## Description
<!--- Describe your changes in detail -->
Following #3111
A new endpoint that will allow the shrinking and extending of public IP range
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Run tests for shrinking IP range to not include IPs that have already been used.
Run tests for invalid IPs(invalid format, invalid length etc)
Run tests for gateway conflicts
Checked user_ip_address table after edits
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-870114558
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r510876233
##########
File path: api/src/main/java/com/cloud/configuration/ConfigurationService.java
##########
@@ -274,18 +274,12 @@ Vlan createVlanAndPublicIpRange(CreateVlanIpRangeCmd cmd) throws InsufficientCap
ResourceAllocationException;
/**
- * Updates the IP address Range for the VLAN on the database, a
- *
+ * Updates the IP address Range for the VLAN on the database
* @param cmd
- * @param vlanId
- * @param gateway
- * @param startIP
- * @param endIP
- * @param netmask
- * @throws com.cloud.exception.ConcurrentOperationException
- * @throws com.cloud.exception.ResourceUnavailableException
- * @throws com.cloud.exception.ResourceAllocationException
- * @return The updated Vlan object
+ * @return
+ * @throws ConcurrentOperationException
+ * @throws ResourceUnavailableException
+ * @throws ResourceAllocationException
Review comment:
rectified
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r463187096
##########
File path: engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
##########
@@ -220,6 +220,11 @@ Vlan createVlanAndPublicIpRange(long zoneId, long networkId, long physicalNetwor
String vlanGateway, String vlanNetmask, String vlanId, boolean bypassVlanOverlapCheck, Domain domain, Account vlanOwner, String startIPv6, String endIPv6, String vlanIp6Gateway, String vlanIp6Cidr)
throws InsufficientCapacityException, ConcurrentOperationException, InvalidParameterValueException;
+ /* (non-Javadoc)
+ * @see com.cloud.configuration.ConfigurationManager#updateVlanAndPublicIpRange(long,String,String,String,String)
Review comment:
non javadoc but with a javadoc annotation?
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.vlan;
+
+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.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VlanIpRangeResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.dc.Vlan;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.utils.net.NetUtils;
+
+@APICommand(name = UpdateVlanIpRangeCmd.APINAME, description = "Updates a VLAN IP range.", responseObject =
+ VlanIpRangeResponse.class, since = "4.13.0",
Review comment:
better update to 4.15.0
##########
File path: api/src/main/java/com/cloud/configuration/ConfigurationService.java
##########
@@ -272,6 +273,22 @@
Vlan createVlanAndPublicIpRange(CreateVlanIpRangeCmd cmd) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException,
ResourceAllocationException;
+ /**
+ * Updates the IP address Range for the VLAN on the database, a
+ *
+ * @param cmd
+ * @param vlanId
+ * @param gateway
+ * @param startIP
+ * @param endIP
+ * @param netmask
+ * @throws com.cloud.exception.ConcurrentOperationException
+ * @throws com.cloud.exception.ResourceUnavailableException
+ * @throws com.cloud.exception.ResourceAllocationException
Review comment:
let's not leave unused javadoc annotations
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-714378621
> @ravening is this lgtm-to-you?
@DaanHoogland i will review it today
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-665510566
@kioie is this ready for review/ testing again?
And do you intent to move this forward before 4.15?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r510026508
##########
File path: api/src/main/java/com/cloud/configuration/ConfigurationService.java
##########
@@ -274,18 +274,12 @@ Vlan createVlanAndPublicIpRange(CreateVlanIpRangeCmd cmd) throws InsufficientCap
ResourceAllocationException;
/**
- * Updates the IP address Range for the VLAN on the database, a
- *
+ * Updates the IP address Range for the VLAN on the database
* @param cmd
- * @param vlanId
- * @param gateway
- * @param startIP
- * @param endIP
- * @param netmask
- * @throws com.cloud.exception.ConcurrentOperationException
- * @throws com.cloud.exception.ResourceUnavailableException
- * @throws com.cloud.exception.ResourceAllocationException
- * @return The updated Vlan object
+ * @return
+ * @throws ConcurrentOperationException
+ * @throws ResourceUnavailableException
+ * @throws ResourceAllocationException
Review comment:
why leave these empty items?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-721594480
@PaulAngus this is API and DB only and has beenn around for a long time. Can we consider putting it in?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-906182281
ping @kioie , can you address the outstanding comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r450074666
##########
File path: api/src/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.vlan;
+
+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.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VlanIpRangeResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.dc.Vlan;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.utils.net.NetUtils;
+
+@APICommand(name = UpdateVlanIpRangeCmd.APINAME, description = "Updates a VLAN IP range.", responseObject =
+ VlanIpRangeResponse.class, since = "4.13.0",
Review comment:
I can move it to 4.15.0 if recommended
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r510879078
##########
File path: server/src/main/java/com/cloud/test/IPRangeConfig.java
##########
@@ -313,35 +313,35 @@ public static long getCidrSize(String pod, String zone) {
return problemIPs;
}
- private Vector<String> deletePublicIPRange(TransactionLegacy txn, long startIP, long endIP, long vlanDbId) {
- String deleteSql = "DELETE FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_id = ?";
- String isPublicIPAllocatedSelectSql = "SELECT * FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_id = ?";
+ public Vector<String> deletePublicIPRange(TransactionLegacy txn, long startIP, long endIP, long vlanDbId) {
+ String deleteSql = "DELETE FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_db_id = ?";
+ String isPublicIPAllocatedSelectSql = "SELECT * FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_db_id = ?";
Vector<String> problemIPs = new Vector<String>();
Connection conn = null;
try {
conn = txn.getConnection();
}
catch (SQLException e) {
- System.out.println("deletePublicIPRange. Exception: " +e.getMessage());
+ System.out.println("deletePublicIPRange. Exception: " + e.getMessage());
return null;
}
- try(PreparedStatement stmt = conn.prepareStatement(deleteSql);
+ try (PreparedStatement stmt = conn.prepareStatement(deleteSql);
PreparedStatement isAllocatedStmt = conn.prepareStatement(isPublicIPAllocatedSelectSql);) {
while (startIP <= endIP) {
- if (!isPublicIPAllocated(startIP, vlanDbId, isAllocatedStmt)) {
+ if (!isPublicIPAllocated(NetUtils.long2Ip(startIP), vlanDbId, isAllocatedStmt)) {
Review comment:
Good point! Allow me to explore. Will this be a blocker?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-725583311
<b>Trillian test result (tid-3146)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35081 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3997-t3146-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-712788890
@kioie any updates (about to go to code freeze)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-870131509
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r510876036
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.vlan;
+
+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.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VlanIpRangeResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.dc.Vlan;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.utils.net.NetUtils;
+
+@APICommand(name = UpdateVlanIpRangeCmd.APINAME, description = "Updates a VLAN IP range.", responseObject =
+ VlanIpRangeResponse.class, since = "4.13.0",
Review comment:
updated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r450097735
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4080,149 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException{
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException{
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP= existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null){
+ startIp= currentStartIP;
+ }
+
+ if(endIp == null){
+ endIp= currentEndIP;
+ }
+
+ final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+ checkAllocatedIpsAreWithinVlanRange(ips,startIp,endIp);
+
+ final String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+ final String cidrAddress = getCidrAddress(cidr);
+ final long cidrSize = getCidrSize(cidr);
+
+ checkIpRange(currentStartIP,currentEndIP,cidrAddress,cidrSize);
+
+ checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+
+ checkGatewayOverlap(startIp,endIp,gateway);
+
+ VlanVO range;
+ try {
+ final String newStartIP= startIp;
+ final String newEndIP= endIp;
+
+ range = _vlanDao.acquireInLockTable(id, 30);
+ if (range == null) {
+ throw new CloudRuntimeException("Unable to acquire vlan configuration: " + id);
+ }
+
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("lock vlan " + id + " is acquired");
+ }
+
+ commitUpdateVlanAndIpRange(id, newStartIP, newEndIP, currentStartIP, currentEndIP,true);
+
+ } catch (final Exception e) {
+ s_logger.error("Unable to edit VlanRange due to " + e.getMessage(), e);
+ throw new CloudRuntimeException("Failed to edit VlanRange. Please contact Cloud Support.");
+ }finally {
+ _vlanDao.releaseFromLockTable(id);
+ }
+
+ return vlanRange;
+ }
+ private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP,final String currentStartIP, final String currentEndIP, final boolean ipv4) {
+ return Transaction.execute(new TransactionCallback<VlanVO>() {
+ @Override
+ public VlanVO doInTransaction(final TransactionStatus status) {
+ VlanVO vlanRange = _vlanDao.findById(id);
+ s_logger.debug("Updating vlan range " + vlanRange.getId());
+ vlanRange.setIpRange(vlanRange.getIpRange().replace(currentStartIP+"-", newStartIP+"-").replace(currentEndIP,
Review comment:
Because from what I gathered, sometimes the IPrange is not always stored in this format only, sometimes, it can include multiple ranges for example `172.xxx.xxx.2-172.xxx.xxx.20,172.xxx.xxx.200-172.xxx.xxx.201`, when you set IP range this way, you loose the second range
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-870131939
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r521407293
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4079,152 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException {
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException {
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP = existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null) {
+ startIp = currentStartIP;
+ }
+
+ if(endIp == null) {
+ endIp = currentEndIP;
+ }
+
+ final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+ checkAllocatedIpsAreWithinVlanRange(ips,startIp,endIp);
+
+ final String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+ final String cidrAddress = getCidrAddress(cidr);
+ final long cidrSize = getCidrSize(cidr);
+
+ checkIpRange(currentStartIP,currentEndIP,cidrAddress,cidrSize);
+
+ checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+
+ checkGatewayOverlap(startIp,endIp,gateway);
+
+ VlanVO range;
+ try {
+ final String newStartIP = startIp;
+ final String newEndIP = endIp;
+
+ range = _vlanDao.acquireInLockTable(id, 30);
+ if (range == null) {
+ throw new CloudRuntimeException("Unable to acquire vlan configuration: " + id);
+ }
+
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("lock vlan " + id + " is acquired");
+ }
+
+ commitUpdateVlanAndIpRange(id, newStartIP, newEndIP, currentStartIP, currentEndIP,true);
Review comment:
are you using the `ipv4` parameter to check if its ipv4 or ipv6 ip range or some other reason?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-714486126
@kioie Have few questions.
1. Are you considering the domain/account resource limit when increasing/decreasing ip range?
2. What if ip is already allocated when trying to shrink the range?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-870128177
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 401
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r447253279
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4080,149 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException{
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException{
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP= existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null){
+ startIp= currentStartIP;
+ }
+
+ if(endIp == null){
+ endIp= currentEndIP;
+ }
+
+ final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+ checkAllocatedIpsAreWithinVlanRange(ips,startIp,endIp);
+
+ final String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+ final String cidrAddress = getCidrAddress(cidr);
+ final long cidrSize = getCidrSize(cidr);
+
+ checkIpRange(currentStartIP,currentEndIP,cidrAddress,cidrSize);
+
+ checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+
+ checkGatewayOverlap(startIp,endIp,gateway);
+
+ VlanVO range;
+ try {
+ final String newStartIP= startIp;
+ final String newEndIP= endIp;
+
+ range = _vlanDao.acquireInLockTable(id, 30);
+ if (range == null) {
+ throw new CloudRuntimeException("Unable to acquire vlan configuration: " + id);
+ }
+
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("lock vlan " + id + " is acquired");
+ }
+
+ commitUpdateVlanAndIpRange(id, newStartIP, newEndIP, currentStartIP, currentEndIP,true);
+
+ } catch (final Exception e) {
+ s_logger.error("Unable to edit VlanRange due to " + e.getMessage(), e);
+ throw new CloudRuntimeException("Failed to edit VlanRange. Please contact Cloud Support.");
+ }finally {
+ _vlanDao.releaseFromLockTable(id);
+ }
+
+ return vlanRange;
+ }
+ private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP,final String currentStartIP, final String currentEndIP, final boolean ipv4) {
+ return Transaction.execute(new TransactionCallback<VlanVO>() {
+ @Override
+ public VlanVO doInTransaction(final TransactionStatus status) {
+ VlanVO vlanRange = _vlanDao.findById(id);
+ s_logger.debug("Updating vlan range " + vlanRange.getId());
+ vlanRange.setIpRange(vlanRange.getIpRange().replace(currentStartIP+"-", newStartIP+"-").replace(currentEndIP,
Review comment:
How about `vlanRange.setIpRange(newStartIP + "-" + newEndIP)` ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r517174628
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4079,151 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException {
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException {
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP = existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null) {
+ startIp = currentStartIP;
+ }
+
+ if(endIp == null) {
+ endIp = currentEndIP;
+ }
+
+ final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+ checkAllocatedIpsAreWithinVlanRange(ips,startIp,endIp);
+
+ final String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+ final String cidrAddress = getCidrAddress(cidr);
+ final long cidrSize = getCidrSize(cidr);
+
+ checkIpRange(currentStartIP,currentEndIP,cidrAddress,cidrSize);
+
+ checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+
+ checkGatewayOverlap(startIp,endIp,gateway);
+
+ VlanVO range;
+ try {
+ final String newStartIP = startIp;
+ final String newEndIP = endIp;
+
+ range = _vlanDao.acquireInLockTable(id, 30);
+ if (range == null) {
+ throw new CloudRuntimeException("Unable to acquire vlan configuration: " + id);
+ }
+
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("lock vlan " + id + " is acquired");
+ }
+
+ commitUpdateVlanAndIpRange(id, newStartIP, newEndIP, currentStartIP, currentEndIP,true);
+
+ } catch (final Exception e) {
+ s_logger.error("Unable to edit VlanRange due to " + e.getMessage(), e);
+ throw new CloudRuntimeException("Failed to edit VlanRange. Please contact Cloud Support.");
+ } finally {
+ _vlanDao.releaseFromLockTable(id);
+ }
+
+ return vlanRange;
+ }
+ private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP,final String currentStartIP, final String currentEndIP, final boolean ipv4) {
Review comment:
```suggestion
}
private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP,final String currentStartIP, final String currentEndIP, final boolean ipv4) {
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-914074091
@nvazquez @DaanHoogland
I have created #5411 which contains all commits in this pr.
we can close this pr if nobody objects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-870114750
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r660545196
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.vlan;
+
+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.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VlanIpRangeResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.dc.Vlan;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.utils.net.NetUtils;
+
+@APICommand(name = UpdateVlanIpRangeCmd.APINAME, description = "Updates a VLAN IP range.", responseObject =
+ VlanIpRangeResponse.class, since = "4.15.0",
Review comment:
@kioie can you change since param to 4.16.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r447248656
##########
File path: api/src/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.vlan;
+
+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.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VlanIpRangeResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.dc.Vlan;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.utils.net.NetUtils;
+
+@APICommand(name = UpdateVlanIpRangeCmd.APINAME, description = "Updates a VLAN IP range.", responseObject =
+ VlanIpRangeResponse.class, since = "4.13.0",
Review comment:
will it be still 4.13.0?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r450097735
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4080,149 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException{
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException{
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP= existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null){
+ startIp= currentStartIP;
+ }
+
+ if(endIp == null){
+ endIp= currentEndIP;
+ }
+
+ final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+ checkAllocatedIpsAreWithinVlanRange(ips,startIp,endIp);
+
+ final String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+ final String cidrAddress = getCidrAddress(cidr);
+ final long cidrSize = getCidrSize(cidr);
+
+ checkIpRange(currentStartIP,currentEndIP,cidrAddress,cidrSize);
+
+ checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+
+ checkGatewayOverlap(startIp,endIp,gateway);
+
+ VlanVO range;
+ try {
+ final String newStartIP= startIp;
+ final String newEndIP= endIp;
+
+ range = _vlanDao.acquireInLockTable(id, 30);
+ if (range == null) {
+ throw new CloudRuntimeException("Unable to acquire vlan configuration: " + id);
+ }
+
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("lock vlan " + id + " is acquired");
+ }
+
+ commitUpdateVlanAndIpRange(id, newStartIP, newEndIP, currentStartIP, currentEndIP,true);
+
+ } catch (final Exception e) {
+ s_logger.error("Unable to edit VlanRange due to " + e.getMessage(), e);
+ throw new CloudRuntimeException("Failed to edit VlanRange. Please contact Cloud Support.");
+ }finally {
+ _vlanDao.releaseFromLockTable(id);
+ }
+
+ return vlanRange;
+ }
+ private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP,final String currentStartIP, final String currentEndIP, final boolean ipv4) {
+ return Transaction.execute(new TransactionCallback<VlanVO>() {
+ @Override
+ public VlanVO doInTransaction(final TransactionStatus status) {
+ VlanVO vlanRange = _vlanDao.findById(id);
+ s_logger.debug("Updating vlan range " + vlanRange.getId());
+ vlanRange.setIpRange(vlanRange.getIpRange().replace(currentStartIP+"-", newStartIP+"-").replace(currentEndIP,
Review comment:
Because from what I gathered, sometimes the IPrange is not always stored in this format only, sometimes, it can include multiple ranges for example `172.xxx.xxx.2-172.xxx.xxx.20,172.xxx.xxx.200-172.xxx.xxx.201`, when you set IP range this way, you run the risk of loosing the second range
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r660545196
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.vlan;
+
+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.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VlanIpRangeResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.dc.Vlan;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.utils.net.NetUtils;
+
+@APICommand(name = UpdateVlanIpRangeCmd.APINAME, description = "Updates a VLAN IP range.", responseObject =
+ VlanIpRangeResponse.class, since = "4.15.0",
Review comment:
@kioie can you change since param to 4.16.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-724980993
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2358
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-666169519
> @kioie is this ready for review/ testing again?
> And do you intent to move this forward before 4.15?
Yes, its open for review now.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-869192839
ping @kioie can you please address the open comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-914354109
Thanks @weizhouapache - closing it and continuing on #5411
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-870114558
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-870114750
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez closed pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
nvazquez closed pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] kioie closed pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie closed pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r661166678
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.vlan;
+
+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.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VlanIpRangeResponse;
+import org.apache.log4j.Logger;
+
+import com.cloud.dc.Vlan;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.utils.net.NetUtils;
+
+@APICommand(name = UpdateVlanIpRangeCmd.APINAME, description = "Updates a VLAN IP range.", responseObject =
+ VlanIpRangeResponse.class, since = "4.15.0",
Review comment:
```suggestion
VlanIpRangeResponse.class, since = "4.16.0",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r521409868
##########
File path: server/src/main/java/com/cloud/test/IPRangeConfig.java
##########
@@ -313,35 +313,35 @@ public static long getCidrSize(String pod, String zone) {
return problemIPs;
}
- private Vector<String> deletePublicIPRange(TransactionLegacy txn, long startIP, long endIP, long vlanDbId) {
- String deleteSql = "DELETE FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_id = ?";
- String isPublicIPAllocatedSelectSql = "SELECT * FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_id = ?";
+ public Vector<String> deletePublicIPRange(TransactionLegacy txn, long startIP, long endIP, long vlanDbId) {
+ String deleteSql = "DELETE FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_db_id = ?";
+ String isPublicIPAllocatedSelectSql = "SELECT * FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_db_id = ?";
Vector<String> problemIPs = new Vector<String>();
Connection conn = null;
try {
conn = txn.getConnection();
}
catch (SQLException e) {
- System.out.println("deletePublicIPRange. Exception: " +e.getMessage());
+ System.out.println("deletePublicIPRange. Exception: " + e.getMessage());
return null;
}
- try(PreparedStatement stmt = conn.prepareStatement(deleteSql);
+ try (PreparedStatement stmt = conn.prepareStatement(deleteSql);
PreparedStatement isAllocatedStmt = conn.prepareStatement(isPublicIPAllocatedSelectSql);) {
while (startIP <= endIP) {
- if (!isPublicIPAllocated(startIP, vlanDbId, isAllocatedStmt)) {
+ if (!isPublicIPAllocated(NetUtils.long2Ip(startIP), vlanDbId, isAllocatedStmt)) {
Review comment:
> Good point! Allow me to explore. Will this be a blocker?
Not a blocker but performance degradation
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-724948117
> @ravening are all your concerns met?
>
> @blueorangutan package
Will review it again soon
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] kioie closed pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie closed pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-724948186
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-907040968
@sureshanaparti , @kioie has not been active on github since februari. I think we'll need to adopt this if we want it in. (cc @nvazquez )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r450099761
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4080,149 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException{
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException{
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP= existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null){
Review comment:
This would more or less happen during the IP range checks down the line.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-724947450
@ravening are all your concerns met?
@blueorangutan package
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r510163155
##########
File path: server/src/main/java/com/cloud/test/IPRangeConfig.java
##########
@@ -313,35 +313,35 @@ public static long getCidrSize(String pod, String zone) {
return problemIPs;
}
- private Vector<String> deletePublicIPRange(TransactionLegacy txn, long startIP, long endIP, long vlanDbId) {
- String deleteSql = "DELETE FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_id = ?";
- String isPublicIPAllocatedSelectSql = "SELECT * FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_id = ?";
+ public Vector<String> deletePublicIPRange(TransactionLegacy txn, long startIP, long endIP, long vlanDbId) {
+ String deleteSql = "DELETE FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_db_id = ?";
+ String isPublicIPAllocatedSelectSql = "SELECT * FROM `cloud`.`user_ip_address` WHERE public_ip_address = ? AND vlan_db_id = ?";
Vector<String> problemIPs = new Vector<String>();
Connection conn = null;
try {
conn = txn.getConnection();
}
catch (SQLException e) {
- System.out.println("deletePublicIPRange. Exception: " +e.getMessage());
+ System.out.println("deletePublicIPRange. Exception: " + e.getMessage());
return null;
}
- try(PreparedStatement stmt = conn.prepareStatement(deleteSql);
+ try (PreparedStatement stmt = conn.prepareStatement(deleteSql);
PreparedStatement isAllocatedStmt = conn.prepareStatement(isPublicIPAllocatedSelectSql);) {
while (startIP <= endIP) {
- if (!isPublicIPAllocated(startIP, vlanDbId, isAllocatedStmt)) {
+ if (!isPublicIPAllocated(NetUtils.long2Ip(startIP), vlanDbId, isAllocatedStmt)) {
Review comment:
If start ip 10.10.10.1 and endip is 10.10.10.250 then you are running sql query 250 times right which is bad?
Why dont you fetch all ip's of particular vldn id and check the state of ip address? so it will be just one query per ip range
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-725270633
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r521402651
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4079,152 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException {
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException {
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP = existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null) {
+ startIp = currentStartIP;
+ }
+
+ if(endIp == null) {
+ endIp = currentEndIP;
+ }
+
+ final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+ checkAllocatedIpsAreWithinVlanRange(ips,startIp,endIp);
+
+ final String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+ final String cidrAddress = getCidrAddress(cidr);
+ final long cidrSize = getCidrSize(cidr);
+
+ checkIpRange(currentStartIP,currentEndIP,cidrAddress,cidrSize);
Review comment:
You dont need to call this function twice if `startIp = currentStartIP`. same withe endip 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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-876951630
Hi @kioie can you please resolve the outstanding comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r510875852
##########
File path: api/src/main/java/com/cloud/configuration/ConfigurationService.java
##########
@@ -272,6 +273,22 @@
Vlan createVlanAndPublicIpRange(CreateVlanIpRangeCmd cmd) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException,
ResourceAllocationException;
+ /**
+ * Updates the IP address Range for the VLAN on the database, a
+ *
+ * @param cmd
+ * @param vlanId
+ * @param gateway
+ * @param startIP
+ * @param endIP
+ * @param netmask
+ * @throws com.cloud.exception.ConcurrentOperationException
+ * @throws com.cloud.exception.ResourceUnavailableException
+ * @throws com.cloud.exception.ResourceAllocationException
Review comment:
added unit tests.
##########
File path: engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
##########
@@ -220,6 +220,11 @@ Vlan createVlanAndPublicIpRange(long zoneId, long networkId, long physicalNetwor
String vlanGateway, String vlanNetmask, String vlanId, boolean bypassVlanOverlapCheck, Domain domain, Account vlanOwner, String startIPv6, String endIPv6, String vlanIp6Gateway, String vlanIp6Cidr)
throws InsufficientCapacityException, ConcurrentOperationException, InvalidParameterValueException;
+ /* (non-Javadoc)
+ * @see com.cloud.configuration.ConfigurationManager#updateVlanAndPublicIpRange(long,String,String,String,String)
Review comment:
resolved
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] kioie commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
kioie commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-715335198
> @kioie Have few questions.
>
> 1. Are you considering the domain/account resource limit when increasing/decreasing ip range?
>
> 2. What if ip is already allocated when trying to shrink the range?
If an IP is already allocated, it will fail.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-725270322
@blueorangutan test
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#discussion_r447251379
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4075,6 +4080,149 @@ public VlanVO doInTransaction(final TransactionStatus status) {
}
+ @Override
+ public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
+ ResourceUnavailableException,ResourceAllocationException{
+
+ return updateVlanAndPublicIpRange(cmd.getId(), cmd.getStartIp(),cmd.getEndIp(),
+ cmd.getGateway(),cmd.getNetmask());
+ }
+
+ @DB
+ @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+ = false)
+ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
+ String endIp,
+ String gateway,
+ String netmask) throws ConcurrentOperationException{
+
+ VlanVO vlanRange = _vlanDao.findById(id);
+ if (vlanRange == null) {
+ throw new InvalidParameterValueException("Please specify a valid IP range id.");
+ }
+
+ if (gateway == null) {
+ gateway = vlanRange.getVlanGateway();
+ }
+
+ if (netmask == null) {
+ netmask = vlanRange.getVlanNetmask();
+ }
+
+ final String[] existingVlanIPRangeArray = vlanRange.getIpRange().split("-");
+ final String currentStartIP= existingVlanIPRangeArray[0];
+ final String currentEndIP = existingVlanIPRangeArray [1];
+
+ if(startIp == null){
Review comment:
What if I dont pass both start and end ip? Why dont you just return back?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3997:
URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-714377900
@ravening is this lgtm-to-you?
----------------------------------------------------------------
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:
users@infra.apache.org