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 2021/09/07 07:09:56 UTC

[GitHub] [cloudstack] weizhouapache opened a new pull request #5411: 4.16 update vlan ip range

weizhouapache opened a new pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411


   ### Description
   
   A new endpoint that will allow the shrinking and extending of public IP range
   
   This PR extends #3997 
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- 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: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] 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)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [x] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   
   ### 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. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/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.

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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921639880


   @sureshanaparti 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] weizhouapache commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-915072339


   @blu@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] blueorangutan commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921630438


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1298


-- 
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] weizhouapache commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921574755


   @DaanHoogland @davidjumani thanks for review. 
   updated pr as per your 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] weizhouapache commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921706622


   @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] blueorangutan commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921677425


   @weizhouapache 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] davidjumani commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r709804083



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,181 @@
+// 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.acl.RoleType;
+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.16.0",
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+        authorized = {RoleType.Admin})
+public class UpdateVlanIpRangeCmd extends BaseAsyncCmd {
+
+    public static final String APINAME = "updateVlanIpRange";
+    public static final Logger s_logger = Logger.getLogger(UpdateVlanIpRangeCmd.class.getName());
+
+    /////////////////////////////////////////////////////
+    //////////////// API parameters /////////////////////
+    /////////////////////////////////////////////////////
+
+
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VlanIpRangeResponse.class, required = true,
+            description = "the UUID of the VLAN IP range")
+    private Long id;
+
+    @Parameter(name = ApiConstants.GATEWAY, type = CommandType.STRING, description = "the gateway of the VLAN IP range")
+    private String gateway;

Review comment:
       If gateway and netmask can not be changed, can the parameters be removed ?




-- 
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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r709870700



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4246,274 @@ 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(),
+                cmd.getStartIpv6(), cmd.getEndIpv6(), cmd.getIp6Gateway(), cmd.getIp6Cidr(), cmd.isForSystemVms());
+    }
+
+    @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,
+                                           String startIpv6,
+                                           String endIpv6,
+                                           String ip6Gateway,
+                                           String ip6Cidr,
+                                           Boolean forSystemVms) throws ConcurrentOperationException {
+
+        VlanVO vlanRange = _vlanDao.findById(id);
+        if (vlanRange == null) {
+            throw new InvalidParameterValueException("Please specify a valid IP range id.");
+        }
+
+        final boolean ipv4 = vlanRange.getVlanGateway() != null;
+        final boolean ipv6 = vlanRange.getIp6Gateway() != null;
+        if (!ipv4) {
+            if (startIp != null || endIp != null || gateway != null || netmask != null) {
+                throw new InvalidParameterValueException("IPv4 is not support in this IP range.");
+            }
+        }
+        if (!ipv6) {
+            if (startIpv6 != null || endIpv6 != null || ip6Gateway != null || ip6Cidr != null) {
+                throw new InvalidParameterValueException("IPv6 is not support in this IP range.");
+            }
+        }
+
+        final Boolean isRangeForSystemVM = checkIfVlanRangeIsForSystemVM(id);
+        if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
+            if (VlanType.DirectAttached.equals(vlanRange.getVlanType())) {
+                throw new InvalidParameterValueException("forSystemVms is not available for this IP range with vlan type: " + VlanType.DirectAttached);
+            }
+            // Check if range has already been dedicated
+            final List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(id);
+            if (maps != null && !maps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to an account");
+            }
+
+            List<DomainVlanMapVO> domainmaps = _domainVlanMapDao.listDomainVlanMapsByVlan(id);
+            if (domainmaps != null && !domainmaps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to a domain");
+            }
+        }
+        if (ipv4) {
+            if (gateway != null && !gateway.equals(vlanRange.getVlanGateway())) {
+                throw new InvalidParameterValueException("The input gateway " + gateway + " is not same as IP range gateway " + vlanRange.getVlanGateway());
+            }
+            if (netmask != null && !netmask.equals(vlanRange.getVlanNetmask())) {
+                throw new InvalidParameterValueException("The input netmask " + netmask + " is not same as IP range netmask " + vlanRange.getVlanNetmask());
+            }
+
+            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 String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+            if (Strings.isNullOrEmpty(cidr)) {
+                throw new InvalidParameterValueException(String.format("Invalid gateway (%s) or netmask (%s)", gateway, netmask));
+            }
+            final String cidrAddress = getCidrAddress(cidr);
+            final long cidrSize = getCidrSize(cidr);
+
+            checkIpRange(currentStartIP, currentEndIP, cidrAddress, cidrSize);
+            if (startIp != currentStartIP || endIp != currentEndIP) {
+                checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+            }
+
+            checkGatewayOverlap(startIp, endIp, gateway);
+
+            final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+            checkAllocatedIpsAreWithinVlanRange(ips, startIp, endIp, forSystemVms);
+
+            try {
+                final String newStartIP = startIp;
+                final String newEndIP = endIp;
+
+                VlanVO 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, isRangeForSystemVM, forSystemVms);
+
+            } 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);
+            }
+        }
+
+        if (ipv6) {
+            if (ip6Gateway != null && !ip6Gateway.equals(vlanRange.getIp6Gateway())) {
+                throw new InvalidParameterValueException("The input gateway " + ip6Gateway + " is not same as IP range gateway " + vlanRange.getIp6Gateway());
+            }
+            if (ip6Cidr != null && !ip6Cidr.equals(vlanRange.getIp6Cidr())) {
+                throw new InvalidParameterValueException("The input cidr " + ip6Cidr + " is not same as IP range cidr " + vlanRange.getIp6Cidr());
+            }
+            ip6Gateway = MoreObjects.firstNonNull(ip6Gateway, vlanRange.getIp6Gateway());
+            ip6Cidr = MoreObjects.firstNonNull(ip6Cidr, vlanRange.getIp6Cidr());
+
+            final String[] existingVlanIPRangeArray = vlanRange.getIp6Range().split("-");
+            final String currentStartIPv6 = existingVlanIPRangeArray[0];
+            final String currentEndIPv6 = existingVlanIPRangeArray[1];
+
+            if (startIpv6 == null) {
+                startIpv6 = currentStartIPv6;
+            }
+            if (endIpv6 == null) {
+                endIpv6 = currentEndIPv6;
+            }
+            if (startIpv6 != currentStartIPv6 || endIpv6 != currentEndIPv6) {
+                _networkModel.checkIp6Parameters(startIpv6, endIpv6, ip6Gateway, ip6Cidr);
+                final List<UserIpv6AddressVO> ips = _ipv6Dao.listByVlanId(id);
+                checkAllocatedIpv6sAreWithinVlanRange(ips, startIp, endIp);
+
+                try {
+                    VlanVO 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, startIpv6, endIpv6, currentStartIPv6, currentEndIPv6, false, isRangeForSystemVM,forSystemVms);
+
+                } 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 _vlanDao.findById(id);
+    }
+
+    private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP, final String currentStartIP, final String currentEndIP,
+                                              final boolean ipv4, final Boolean isRangeForSystemVM, final Boolean forSystemvms) {
+
+        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());
+                if (ipv4) {
+                    vlanRange.setIpRange(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                    if (!updatePublicIPRange(newStartIP, currentStartIP, newEndIP, currentEndIP, vlanRange.getDataCenterId(), vlanRange.getId(), vlanRange.getNetworkId(), vlanRange.getPhysicalNetworkId(), isRangeForSystemVM, forSystemvms)) {
+                        throw new CloudRuntimeException("Failed to update IPv4 range. Please contact Cloud Support.");
+                    }
+                } else {
+                    vlanRange.setIp6Range(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);

Review comment:
       @weizhouapache update public IP range required here same as ipv4 above?




-- 
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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-922086627


   <b>Trillian test result (tid-2111)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34901 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5411-t2111-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have errors
   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.

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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921610166


   @weizhouapache 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] blueorangutan commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921701835


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1302


-- 
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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-915308413


   @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] blueorangutan commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-915682559


   <b>Trillian test result (tid-1994)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33500 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5411-t1994-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have errors
   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.

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 merged pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
sureshanaparti merged pull request #5411:
URL: https://github.com/apache/cloudstack/pull/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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-915337619


   @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] weizhouapache commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921609517


   @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] DaanHoogland commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r710989455



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4245,260 @@ public VlanVO doInTransaction(final TransactionStatus status) {
 
     }
 
+    @Override
+    public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,

Review comment:
       I think I mentioned that before, but shouldn't this be in the `IpAddressManager`?




-- 
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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921707854


   @weizhouapache 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] sureshanaparti commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-922645988


   Merging based on approvals, manual and smoke test results.


-- 
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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-915338948


   @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] weizhouapache commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r709847061



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,181 @@
+// 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.acl.RoleType;
+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.16.0",
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+        authorized = {RoleType.Admin})
+public class UpdateVlanIpRangeCmd extends BaseAsyncCmd {
+
+    public static final String APINAME = "updateVlanIpRange";
+    public static final Logger s_logger = Logger.getLogger(UpdateVlanIpRangeCmd.class.getName());
+
+    /////////////////////////////////////////////////////
+    //////////////// API parameters /////////////////////
+    /////////////////////////////////////////////////////
+
+
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VlanIpRangeResponse.class, required = true,
+            description = "the UUID of the VLAN IP range")
+    private Long id;
+
+    @Parameter(name = ApiConstants.GATEWAY, type = CommandType.STRING, description = "the gateway of the VLAN IP range")
+    private String gateway;

Review comment:
       @davidjumani 
   I think cloudstack user might expect the gateway or netmak can be changed, but it will be risky if some IPs are in use.
   I will remove them for now.
   if someone want to change gateway or netmask, add them back then.




-- 
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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-915306527


   @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] blueorangutan commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-915334920


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1185


-- 
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] weizhouapache commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r711015528



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4245,260 @@ public VlanVO doInTransaction(final TransactionStatus status) {
 
     }
 
+    @Override
+    public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,

Review comment:
       @DaanHoogland 
   createVlanAndPublicIpRange and deleteVlanAndPublicIpRange are in this 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921676899


   @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] sureshanaparti commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r710896907



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4569,6 +4835,59 @@ protected boolean savePublicIPRange(final String startIP, final String endIP, fi
         return problemIps != null && problemIps.size() == 0;
     }
 
+    @DB
+    protected boolean updatePublicIPRange(final String newStartIP, final String currentStartIP, final String newEndIP, final String currentEndIP, final long zoneId, final long vlanDbId, final long sourceNetworkid, final long physicalNetworkId, final boolean isRangeForSystemVM, final Boolean forSystemVms) {
+        long newStartIPLong = NetUtils.ip2Long(newStartIP);
+        long newEndIPLong = NetUtils.ip2Long(newEndIP);
+        long currentStartIPLong = NetUtils.ip2Long(currentStartIP);
+        long currentEndIPLong = NetUtils.ip2Long(currentEndIP);
+
+        List<Long> currentIPRange = new ArrayList<>();
+        List<Long> newIPRange = new ArrayList<>();
+        while (newStartIPLong <= newEndIPLong) {
+            newIPRange.add(newStartIPLong);
+            newStartIPLong++;
+        }
+        while (currentStartIPLong <= currentEndIPLong) {
+            currentIPRange.add(currentStartIPLong);
+            currentStartIPLong++;
+        }
+
+        final List<String> problemIps = Transaction.execute(new TransactionCallback<List<String>>() {
+
+            @Override
+            public List<String> doInTransaction(final TransactionStatus status) {
+                final IPRangeConfig config = new IPRangeConfig();
+                Vector<String> configResult = new Vector<>();
+                List<Long> ipAddressesToAdd = new ArrayList(newIPRange);
+                ipAddressesToAdd.removeAll(currentIPRange);
+                if (ipAddressesToAdd.size() > 0) {
+                    for (Long startIP : ipAddressesToAdd) {
+                        configResult.addAll(config.savePublicIPRange(TransactionLegacy.currentTxn(), startIP, startIP, zoneId, vlanDbId, sourceNetworkid, physicalNetworkId, forSystemVms != null ? forSystemVms : isRangeForSystemVM));
+                    }
+                }
+                List<Long> ipAddressesToDelete = new ArrayList(currentIPRange);
+                ipAddressesToDelete.removeAll(newIPRange);
+                if (ipAddressesToDelete.size() > 0) {
+                    for (Long startIP : ipAddressesToDelete) {
+                        configResult.addAll(config.deletePublicIPRange(TransactionLegacy.currentTxn(), startIP, startIP, vlanDbId));
+                    }
+                }
+                if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
+                    List<Long> ipAddressesToUpdate = new ArrayList(currentIPRange);
+                    ipAddressesToUpdate.removeAll(ipAddressesToDelete);
+                    if (ipAddressesToUpdate.size() > 0) {
+                        for (Long startIP : ipAddressesToUpdate) {
+                            configResult.addAll(config.updatePublicIPRange(TransactionLegacy.currentTxn(), startIP, startIP, vlanDbId, forSystemVms));
+                        }
+                    }
+                }
+                return configResult;
+            }
+        });
+        return problemIps != null && problemIps.size() == 0;

Review comment:
       ```suggestion
           return CollectionUtils.isEmpty(problemIps);
   ```




-- 
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] DaanHoogland commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r708942078



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4569,6 +4847,59 @@ protected boolean savePublicIPRange(final String startIP, final String endIP, fi
         return problemIps != null && problemIps.size() == 0;
     }
 
+    @DB
+    protected boolean updatePublicIPRange(final String newStartIP, final String currentStartIP, final String newEndIP, final String currentEndIP, final long zoneId, final long vlanDbId, final long sourceNetworkid, final long physicalNetworkId, final boolean isRangeForSystemVM, final Boolean forSystemVms) {

Review comment:
       can you here make a javadoc with what a `true` or `false` return value mean and what the side effects are in terms of DB contents?

##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4246,274 @@ 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(),
+                cmd.getStartIpv6(), cmd.getEndIpv6(), cmd.getIp6Gateway(), cmd.getIp6Cidr(), cmd.isForSystemVms());
+    }
+
+    @DB
+    @ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_UPDATE, eventDescription = "update vlan ip Range", async
+            = false)
+    public Vlan updateVlanAndPublicIpRange(final long id, String startIp,

Review comment:
       this is a 160 lines method. can you modularise it a bit more, please?

##########
File path: api/src/main/java/com/cloud/configuration/ConfigurationService.java
##########
@@ -272,6 +273,16 @@
     Vlan createVlanAndPublicIpRange(CreateVlanIpRangeCmd cmd) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException,
         ResourceAllocationException;
 
+    /**
+     * Updates the IP address Range for the VLAN on the database
+     * @param cmd
+     * @return The Updated Vlan Object
+     * @throws ConcurrentOperationException
+     * @throws ResourceUnavailableException
+     * @throws ResourceAllocationException

Review comment:
       except for the `@return` these annotation don;t provide extra info and we might as well delete those.

##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4246,274 @@ 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(),
+                cmd.getStartIpv6(), cmd.getEndIpv6(), cmd.getIp6Gateway(), cmd.getIp6Cidr(), cmd.isForSystemVms());
+    }
+
+    @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,
+                                           String startIpv6,
+                                           String endIpv6,
+                                           String ip6Gateway,
+                                           String ip6Cidr,
+                                           Boolean forSystemVms) throws ConcurrentOperationException {
+
+        VlanVO vlanRange = _vlanDao.findById(id);
+        if (vlanRange == null) {
+            throw new InvalidParameterValueException("Please specify a valid IP range id.");
+        }
+
+        final boolean ipv4 = vlanRange.getVlanGateway() != null;
+        final boolean ipv6 = vlanRange.getIp6Gateway() != null;
+        if (!ipv4) {
+            if (startIp != null || endIp != null || gateway != null || netmask != null) {
+                throw new InvalidParameterValueException("IPv4 is not support in this IP range.");
+            }
+        }
+        if (!ipv6) {
+            if (startIpv6 != null || endIpv6 != null || ip6Gateway != null || ip6Cidr != null) {
+                throw new InvalidParameterValueException("IPv6 is not support in this IP range.");
+            }
+        }
+
+        final Boolean isRangeForSystemVM = checkIfVlanRangeIsForSystemVM(id);
+        if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
+            if (VlanType.DirectAttached.equals(vlanRange.getVlanType())) {
+                throw new InvalidParameterValueException("forSystemVms is not available for this IP range with vlan type: " + VlanType.DirectAttached);
+            }
+            // Check if range has already been dedicated
+            final List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(id);
+            if (maps != null && !maps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to an account");
+            }
+
+            List<DomainVlanMapVO> domainmaps = _domainVlanMapDao.listDomainVlanMapsByVlan(id);
+            if (domainmaps != null && !domainmaps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to a domain");
+            }
+        }
+        if (ipv4) {
+            if (gateway != null && !gateway.equals(vlanRange.getVlanGateway())) {
+                throw new InvalidParameterValueException("The input gateway " + gateway + " is not same as IP range gateway " + vlanRange.getVlanGateway());
+            }
+            if (netmask != null && !netmask.equals(vlanRange.getVlanNetmask())) {
+                throw new InvalidParameterValueException("The input netmask " + netmask + " is not same as IP range netmask " + vlanRange.getVlanNetmask());
+            }
+
+            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 String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+            if (Strings.isNullOrEmpty(cidr)) {
+                throw new InvalidParameterValueException(String.format("Invalid gateway (%s) or netmask (%s)", gateway, netmask));
+            }
+            final String cidrAddress = getCidrAddress(cidr);
+            final long cidrSize = getCidrSize(cidr);
+
+            checkIpRange(currentStartIP, currentEndIP, cidrAddress, cidrSize);
+            if (startIp != currentStartIP || endIp != currentEndIP) {
+                checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+            }
+
+            checkGatewayOverlap(startIp, endIp, gateway);
+
+            final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+            checkAllocatedIpsAreWithinVlanRange(ips, startIp, endIp, forSystemVms);
+
+            try {
+                final String newStartIP = startIp;
+                final String newEndIP = endIp;
+
+                VlanVO 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, isRangeForSystemVM, forSystemVms);
+
+            } 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);
+            }
+        }
+
+        if (ipv6) {
+            if (ip6Gateway != null && !ip6Gateway.equals(vlanRange.getIp6Gateway())) {
+                throw new InvalidParameterValueException("The input gateway " + ip6Gateway + " is not same as IP range gateway " + vlanRange.getIp6Gateway());
+            }
+            if (ip6Cidr != null && !ip6Cidr.equals(vlanRange.getIp6Cidr())) {
+                throw new InvalidParameterValueException("The input cidr " + ip6Cidr + " is not same as IP range cidr " + vlanRange.getIp6Cidr());
+            }
+            ip6Gateway = MoreObjects.firstNonNull(ip6Gateway, vlanRange.getIp6Gateway());
+            ip6Cidr = MoreObjects.firstNonNull(ip6Cidr, vlanRange.getIp6Cidr());
+
+            final String[] existingVlanIPRangeArray = vlanRange.getIp6Range().split("-");
+            final String currentStartIPv6 = existingVlanIPRangeArray[0];
+            final String currentEndIPv6 = existingVlanIPRangeArray[1];
+
+            if (startIpv6 == null) {
+                startIpv6 = currentStartIPv6;
+            }
+            if (endIpv6 == null) {
+                endIpv6 = currentEndIPv6;
+            }
+            if (startIpv6 != currentStartIPv6 || endIpv6 != currentEndIPv6) {
+                _networkModel.checkIp6Parameters(startIpv6, endIpv6, ip6Gateway, ip6Cidr);
+                final List<UserIpv6AddressVO> ips = _ipv6Dao.listByVlanId(id);
+                checkAllocatedIpv6sAreWithinVlanRange(ips, startIp, endIp);
+
+                try {
+                    VlanVO 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, startIpv6, endIpv6, currentStartIPv6, currentEndIPv6, false, isRangeForSystemVM,forSystemVms);
+
+                } 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 _vlanDao.findById(id);
+    }
+
+    private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP, final String currentStartIP, final String currentEndIP,
+                                              final boolean ipv4, final Boolean isRangeForSystemVM, final Boolean forSystemvms) {
+
+        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());
+                if (ipv4) {
+                    vlanRange.setIpRange(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                    if (!updatePublicIPRange(newStartIP, currentStartIP, newEndIP, currentEndIP, vlanRange.getDataCenterId(), vlanRange.getId(), vlanRange.getNetworkId(), vlanRange.getPhysicalNetworkId(), isRangeForSystemVM, forSystemvms)) {
+                        throw new CloudRuntimeException("Failed to update IPv4 range. Please contact Cloud Support.");
+                    }
+                } else {
+                    vlanRange.setIp6Range(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                }
+                return vlanRange;
+            }
+        });
+    }
+
+    private boolean checkIfVlanRangeIsForSystemVM(final long vlanId) {
+        List<IPAddressVO> existingPublicIPs = _publicIpAddressDao.listByVlanId(vlanId);
+        boolean initialIsSystemVmValue = existingPublicIPs.get(0).isForSystemVms();
+        for (IPAddressVO existingIPs : existingPublicIPs) {
+            if (initialIsSystemVmValue != existingIPs.isForSystemVms()) {
+                throw new CloudRuntimeException("Your \"For System VM\" value seems to be inconsistent with the rest of the records. Please contact Cloud Support");
+            }
+        }
+        return initialIsSystemVmValue;
+    }
+
+    private void checkAllocatedIpsAreWithinVlanRange(List<IPAddressVO> ips, String startIp, String endIp, Boolean forSystemVms) {
+
+        List<IPAddressVO> listAllocatedIPs = new ArrayList<>();
+        for (final IPAddressVO ip : ips) {
+            if (ip.getState() == IpAddress.State.Allocated) {
+                listAllocatedIPs.add(ip);
+            }
+        }
+        Collections.sort(listAllocatedIPs, Comparator.comparing(IPAddressVO::getAddress));
+        for (IPAddressVO allocatedIP : listAllocatedIPs) {
+            if (!Strings.isNullOrEmpty(startIp) && NetUtils.ip2Long(startIp) > NetUtils.ip2Long(allocatedIP.getAddress().addr())) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + allocatedIP.getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " + listAllocatedIPs.get(listAllocatedIPs.size() - 1).getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());
+            }
+            if (!Strings.isNullOrEmpty(endIp) && NetUtils.ip2Long(endIp) < NetUtils.ip2Long(allocatedIP.getAddress().addr())) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + listAllocatedIPs.get(0).getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " +  allocatedIP.getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());
+            }
+            if (forSystemVms != null && allocatedIP.isForSystemVms() != forSystemVms) {
+                throw new InvalidParameterValueException(String.format("IP %s is in use, cannot change forSystemVms of the IP range", allocatedIP.getAddress().addr()));
+            }
+        }
+    }
+
+    private void checkAllocatedIpv6sAreWithinVlanRange(List<UserIpv6AddressVO> ips, String startIpv6, String endIpv6) {
+
+        List<UserIpv6AddressVO> listAllocatedIPs = new ArrayList<>();
+        for (final UserIpv6AddressVO ip : ips) {
+            if (ip.getState() == UserIpv6Address.State.Allocated) {
+                listAllocatedIPs.add(ip);
+            }
+        }
+        Collections.sort(listAllocatedIPs, Comparator.comparing(UserIpv6AddressVO::getAddress));
+        for (UserIpv6AddressVO allocatedIP : listAllocatedIPs) {
+            if (!Strings.isNullOrEmpty(startIpv6)
+                    && IPv6Address.fromString(startIpv6).toBigInteger().compareTo(IPv6Address.fromString(allocatedIP.getAddress()).toBigInteger()) > 0) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + allocatedIP.getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " + listAllocatedIPs.get(listAllocatedIPs.size() - 1).getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());
+            }
+            if (!Strings.isNullOrEmpty(endIpv6)
+                    && IPv6Address.fromString(endIpv6).toBigInteger().compareTo(IPv6Address.fromString(allocatedIP.getAddress()).toBigInteger()) < 0) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + listAllocatedIPs.get(0).getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " +  allocatedIP.getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());

Review comment:
       and this

##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4246,274 @@ 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(),
+                cmd.getStartIpv6(), cmd.getEndIpv6(), cmd.getIp6Gateway(), cmd.getIp6Cidr(), cmd.isForSystemVms());
+    }
+
+    @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,
+                                           String startIpv6,
+                                           String endIpv6,
+                                           String ip6Gateway,
+                                           String ip6Cidr,
+                                           Boolean forSystemVms) throws ConcurrentOperationException {
+
+        VlanVO vlanRange = _vlanDao.findById(id);
+        if (vlanRange == null) {
+            throw new InvalidParameterValueException("Please specify a valid IP range id.");
+        }
+
+        final boolean ipv4 = vlanRange.getVlanGateway() != null;
+        final boolean ipv6 = vlanRange.getIp6Gateway() != null;
+        if (!ipv4) {
+            if (startIp != null || endIp != null || gateway != null || netmask != null) {
+                throw new InvalidParameterValueException("IPv4 is not support in this IP range.");
+            }
+        }
+        if (!ipv6) {
+            if (startIpv6 != null || endIpv6 != null || ip6Gateway != null || ip6Cidr != null) {
+                throw new InvalidParameterValueException("IPv6 is not support in this IP range.");
+            }
+        }
+
+        final Boolean isRangeForSystemVM = checkIfVlanRangeIsForSystemVM(id);
+        if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
+            if (VlanType.DirectAttached.equals(vlanRange.getVlanType())) {
+                throw new InvalidParameterValueException("forSystemVms is not available for this IP range with vlan type: " + VlanType.DirectAttached);
+            }
+            // Check if range has already been dedicated
+            final List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(id);
+            if (maps != null && !maps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to an account");
+            }
+
+            List<DomainVlanMapVO> domainmaps = _domainVlanMapDao.listDomainVlanMapsByVlan(id);
+            if (domainmaps != null && !domainmaps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to a domain");
+            }
+        }
+        if (ipv4) {
+            if (gateway != null && !gateway.equals(vlanRange.getVlanGateway())) {
+                throw new InvalidParameterValueException("The input gateway " + gateway + " is not same as IP range gateway " + vlanRange.getVlanGateway());
+            }
+            if (netmask != null && !netmask.equals(vlanRange.getVlanNetmask())) {
+                throw new InvalidParameterValueException("The input netmask " + netmask + " is not same as IP range netmask " + vlanRange.getVlanNetmask());
+            }
+
+            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 String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+            if (Strings.isNullOrEmpty(cidr)) {
+                throw new InvalidParameterValueException(String.format("Invalid gateway (%s) or netmask (%s)", gateway, netmask));
+            }
+            final String cidrAddress = getCidrAddress(cidr);
+            final long cidrSize = getCidrSize(cidr);
+
+            checkIpRange(currentStartIP, currentEndIP, cidrAddress, cidrSize);
+            if (startIp != currentStartIP || endIp != currentEndIP) {
+                checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+            }
+
+            checkGatewayOverlap(startIp, endIp, gateway);
+
+            final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+            checkAllocatedIpsAreWithinVlanRange(ips, startIp, endIp, forSystemVms);
+
+            try {
+                final String newStartIP = startIp;
+                final String newEndIP = endIp;
+
+                VlanVO 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, isRangeForSystemVM, forSystemVms);
+
+            } 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);
+            }
+        }
+
+        if (ipv6) {
+            if (ip6Gateway != null && !ip6Gateway.equals(vlanRange.getIp6Gateway())) {
+                throw new InvalidParameterValueException("The input gateway " + ip6Gateway + " is not same as IP range gateway " + vlanRange.getIp6Gateway());
+            }
+            if (ip6Cidr != null && !ip6Cidr.equals(vlanRange.getIp6Cidr())) {
+                throw new InvalidParameterValueException("The input cidr " + ip6Cidr + " is not same as IP range cidr " + vlanRange.getIp6Cidr());
+            }
+            ip6Gateway = MoreObjects.firstNonNull(ip6Gateway, vlanRange.getIp6Gateway());
+            ip6Cidr = MoreObjects.firstNonNull(ip6Cidr, vlanRange.getIp6Cidr());
+
+            final String[] existingVlanIPRangeArray = vlanRange.getIp6Range().split("-");
+            final String currentStartIPv6 = existingVlanIPRangeArray[0];
+            final String currentEndIPv6 = existingVlanIPRangeArray[1];
+
+            if (startIpv6 == null) {
+                startIpv6 = currentStartIPv6;
+            }
+            if (endIpv6 == null) {
+                endIpv6 = currentEndIPv6;
+            }
+            if (startIpv6 != currentStartIPv6 || endIpv6 != currentEndIPv6) {
+                _networkModel.checkIp6Parameters(startIpv6, endIpv6, ip6Gateway, ip6Cidr);
+                final List<UserIpv6AddressVO> ips = _ipv6Dao.listByVlanId(id);
+                checkAllocatedIpv6sAreWithinVlanRange(ips, startIp, endIp);
+
+                try {
+                    VlanVO 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, startIpv6, endIpv6, currentStartIPv6, currentEndIPv6, false, isRangeForSystemVM,forSystemVms);
+
+                } 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 _vlanDao.findById(id);
+    }
+
+    private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP, final String currentStartIP, final String currentEndIP,
+                                              final boolean ipv4, final Boolean isRangeForSystemVM, final Boolean forSystemvms) {
+
+        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());
+                if (ipv4) {
+                    vlanRange.setIpRange(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                    if (!updatePublicIPRange(newStartIP, currentStartIP, newEndIP, currentEndIP, vlanRange.getDataCenterId(), vlanRange.getId(), vlanRange.getNetworkId(), vlanRange.getPhysicalNetworkId(), isRangeForSystemVM, forSystemvms)) {
+                        throw new CloudRuntimeException("Failed to update IPv4 range. Please contact Cloud Support.");
+                    }
+                } else {
+                    vlanRange.setIp6Range(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                }
+                return vlanRange;
+            }
+        });
+    }
+
+    private boolean checkIfVlanRangeIsForSystemVM(final long vlanId) {
+        List<IPAddressVO> existingPublicIPs = _publicIpAddressDao.listByVlanId(vlanId);
+        boolean initialIsSystemVmValue = existingPublicIPs.get(0).isForSystemVms();
+        for (IPAddressVO existingIPs : existingPublicIPs) {
+            if (initialIsSystemVmValue != existingIPs.isForSystemVms()) {
+                throw new CloudRuntimeException("Your \"For System VM\" value seems to be inconsistent with the rest of the records. Please contact Cloud Support");
+            }
+        }
+        return initialIsSystemVmValue;
+    }
+
+    private void checkAllocatedIpsAreWithinVlanRange(List<IPAddressVO> ips, String startIp, String endIp, Boolean forSystemVms) {
+
+        List<IPAddressVO> listAllocatedIPs = new ArrayList<>();
+        for (final IPAddressVO ip : ips) {
+            if (ip.getState() == IpAddress.State.Allocated) {
+                listAllocatedIPs.add(ip);
+            }
+        }
+        Collections.sort(listAllocatedIPs, Comparator.comparing(IPAddressVO::getAddress));
+        for (IPAddressVO allocatedIP : listAllocatedIPs) {
+            if (!Strings.isNullOrEmpty(startIp) && NetUtils.ip2Long(startIp) > NetUtils.ip2Long(allocatedIP.getAddress().addr())) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + allocatedIP.getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " + listAllocatedIPs.get(listAllocatedIPs.size() - 1).getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());
+            }
+            if (!Strings.isNullOrEmpty(endIp) && NetUtils.ip2Long(endIp) < NetUtils.ip2Long(allocatedIP.getAddress().addr())) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + listAllocatedIPs.get(0).getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " +  allocatedIP.getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());

Review comment:
       here to, can you use `String.format()`?

##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4246,274 @@ 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(),
+                cmd.getStartIpv6(), cmd.getEndIpv6(), cmd.getIp6Gateway(), cmd.getIp6Cidr(), cmd.isForSystemVms());
+    }
+
+    @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,
+                                           String startIpv6,
+                                           String endIpv6,
+                                           String ip6Gateway,
+                                           String ip6Cidr,
+                                           Boolean forSystemVms) throws ConcurrentOperationException {
+
+        VlanVO vlanRange = _vlanDao.findById(id);
+        if (vlanRange == null) {
+            throw new InvalidParameterValueException("Please specify a valid IP range id.");
+        }
+
+        final boolean ipv4 = vlanRange.getVlanGateway() != null;
+        final boolean ipv6 = vlanRange.getIp6Gateway() != null;
+        if (!ipv4) {
+            if (startIp != null || endIp != null || gateway != null || netmask != null) {
+                throw new InvalidParameterValueException("IPv4 is not support in this IP range.");
+            }
+        }
+        if (!ipv6) {
+            if (startIpv6 != null || endIpv6 != null || ip6Gateway != null || ip6Cidr != null) {
+                throw new InvalidParameterValueException("IPv6 is not support in this IP range.");
+            }
+        }
+
+        final Boolean isRangeForSystemVM = checkIfVlanRangeIsForSystemVM(id);
+        if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
+            if (VlanType.DirectAttached.equals(vlanRange.getVlanType())) {
+                throw new InvalidParameterValueException("forSystemVms is not available for this IP range with vlan type: " + VlanType.DirectAttached);
+            }
+            // Check if range has already been dedicated
+            final List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(id);
+            if (maps != null && !maps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to an account");
+            }
+
+            List<DomainVlanMapVO> domainmaps = _domainVlanMapDao.listDomainVlanMapsByVlan(id);
+            if (domainmaps != null && !domainmaps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to a domain");
+            }
+        }
+        if (ipv4) {
+            if (gateway != null && !gateway.equals(vlanRange.getVlanGateway())) {
+                throw new InvalidParameterValueException("The input gateway " + gateway + " is not same as IP range gateway " + vlanRange.getVlanGateway());
+            }
+            if (netmask != null && !netmask.equals(vlanRange.getVlanNetmask())) {
+                throw new InvalidParameterValueException("The input netmask " + netmask + " is not same as IP range netmask " + vlanRange.getVlanNetmask());
+            }
+
+            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 String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+            if (Strings.isNullOrEmpty(cidr)) {
+                throw new InvalidParameterValueException(String.format("Invalid gateway (%s) or netmask (%s)", gateway, netmask));
+            }
+            final String cidrAddress = getCidrAddress(cidr);
+            final long cidrSize = getCidrSize(cidr);
+
+            checkIpRange(currentStartIP, currentEndIP, cidrAddress, cidrSize);
+            if (startIp != currentStartIP || endIp != currentEndIP) {
+                checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+            }
+
+            checkGatewayOverlap(startIp, endIp, gateway);
+
+            final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+            checkAllocatedIpsAreWithinVlanRange(ips, startIp, endIp, forSystemVms);
+
+            try {
+                final String newStartIP = startIp;
+                final String newEndIP = endIp;
+
+                VlanVO 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, isRangeForSystemVM, forSystemVms);
+
+            } 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);
+            }
+        }
+
+        if (ipv6) {
+            if (ip6Gateway != null && !ip6Gateway.equals(vlanRange.getIp6Gateway())) {
+                throw new InvalidParameterValueException("The input gateway " + ip6Gateway + " is not same as IP range gateway " + vlanRange.getIp6Gateway());
+            }
+            if (ip6Cidr != null && !ip6Cidr.equals(vlanRange.getIp6Cidr())) {
+                throw new InvalidParameterValueException("The input cidr " + ip6Cidr + " is not same as IP range cidr " + vlanRange.getIp6Cidr());
+            }
+            ip6Gateway = MoreObjects.firstNonNull(ip6Gateway, vlanRange.getIp6Gateway());
+            ip6Cidr = MoreObjects.firstNonNull(ip6Cidr, vlanRange.getIp6Cidr());
+
+            final String[] existingVlanIPRangeArray = vlanRange.getIp6Range().split("-");
+            final String currentStartIPv6 = existingVlanIPRangeArray[0];
+            final String currentEndIPv6 = existingVlanIPRangeArray[1];
+
+            if (startIpv6 == null) {
+                startIpv6 = currentStartIPv6;
+            }
+            if (endIpv6 == null) {
+                endIpv6 = currentEndIPv6;
+            }
+            if (startIpv6 != currentStartIPv6 || endIpv6 != currentEndIPv6) {
+                _networkModel.checkIp6Parameters(startIpv6, endIpv6, ip6Gateway, ip6Cidr);
+                final List<UserIpv6AddressVO> ips = _ipv6Dao.listByVlanId(id);
+                checkAllocatedIpv6sAreWithinVlanRange(ips, startIp, endIp);
+
+                try {
+                    VlanVO 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, startIpv6, endIpv6, currentStartIPv6, currentEndIPv6, false, isRangeForSystemVM,forSystemVms);
+
+                } 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 _vlanDao.findById(id);
+    }
+
+    private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP, final String currentStartIP, final String currentEndIP,
+                                              final boolean ipv4, final Boolean isRangeForSystemVM, final Boolean forSystemvms) {
+
+        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());
+                if (ipv4) {
+                    vlanRange.setIpRange(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                    if (!updatePublicIPRange(newStartIP, currentStartIP, newEndIP, currentEndIP, vlanRange.getDataCenterId(), vlanRange.getId(), vlanRange.getNetworkId(), vlanRange.getPhysicalNetworkId(), isRangeForSystemVM, forSystemvms)) {
+                        throw new CloudRuntimeException("Failed to update IPv4 range. Please contact Cloud Support.");
+                    }
+                } else {
+                    vlanRange.setIp6Range(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                }
+                return vlanRange;
+            }
+        });
+    }
+
+    private boolean checkIfVlanRangeIsForSystemVM(final long vlanId) {
+        List<IPAddressVO> existingPublicIPs = _publicIpAddressDao.listByVlanId(vlanId);
+        boolean initialIsSystemVmValue = existingPublicIPs.get(0).isForSystemVms();
+        for (IPAddressVO existingIPs : existingPublicIPs) {
+            if (initialIsSystemVmValue != existingIPs.isForSystemVms()) {
+                throw new CloudRuntimeException("Your \"For System VM\" value seems to be inconsistent with the rest of the records. Please contact Cloud Support");
+            }
+        }
+        return initialIsSystemVmValue;
+    }
+
+    private void checkAllocatedIpsAreWithinVlanRange(List<IPAddressVO> ips, String startIp, String endIp, Boolean forSystemVms) {
+
+        List<IPAddressVO> listAllocatedIPs = new ArrayList<>();
+        for (final IPAddressVO ip : ips) {
+            if (ip.getState() == IpAddress.State.Allocated) {
+                listAllocatedIPs.add(ip);
+            }
+        }
+        Collections.sort(listAllocatedIPs, Comparator.comparing(IPAddressVO::getAddress));
+        for (IPAddressVO allocatedIP : listAllocatedIPs) {
+            if (!Strings.isNullOrEmpty(startIp) && NetUtils.ip2Long(startIp) > NetUtils.ip2Long(allocatedIP.getAddress().addr())) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + allocatedIP.getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " + listAllocatedIPs.get(listAllocatedIPs.size() - 1).getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());

Review comment:
       `String.format(..)` ?

##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4246,274 @@ 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(),
+                cmd.getStartIpv6(), cmd.getEndIpv6(), cmd.getIp6Gateway(), cmd.getIp6Cidr(), cmd.isForSystemVms());
+    }
+
+    @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,
+                                           String startIpv6,
+                                           String endIpv6,
+                                           String ip6Gateway,
+                                           String ip6Cidr,
+                                           Boolean forSystemVms) throws ConcurrentOperationException {
+
+        VlanVO vlanRange = _vlanDao.findById(id);
+        if (vlanRange == null) {
+            throw new InvalidParameterValueException("Please specify a valid IP range id.");
+        }
+
+        final boolean ipv4 = vlanRange.getVlanGateway() != null;
+        final boolean ipv6 = vlanRange.getIp6Gateway() != null;
+        if (!ipv4) {
+            if (startIp != null || endIp != null || gateway != null || netmask != null) {
+                throw new InvalidParameterValueException("IPv4 is not support in this IP range.");
+            }
+        }
+        if (!ipv6) {
+            if (startIpv6 != null || endIpv6 != null || ip6Gateway != null || ip6Cidr != null) {
+                throw new InvalidParameterValueException("IPv6 is not support in this IP range.");
+            }
+        }
+
+        final Boolean isRangeForSystemVM = checkIfVlanRangeIsForSystemVM(id);
+        if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
+            if (VlanType.DirectAttached.equals(vlanRange.getVlanType())) {
+                throw new InvalidParameterValueException("forSystemVms is not available for this IP range with vlan type: " + VlanType.DirectAttached);
+            }
+            // Check if range has already been dedicated
+            final List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(id);
+            if (maps != null && !maps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to an account");
+            }
+
+            List<DomainVlanMapVO> domainmaps = _domainVlanMapDao.listDomainVlanMapsByVlan(id);
+            if (domainmaps != null && !domainmaps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to a domain");
+            }
+        }
+        if (ipv4) {
+            if (gateway != null && !gateway.equals(vlanRange.getVlanGateway())) {
+                throw new InvalidParameterValueException("The input gateway " + gateway + " is not same as IP range gateway " + vlanRange.getVlanGateway());
+            }
+            if (netmask != null && !netmask.equals(vlanRange.getVlanNetmask())) {
+                throw new InvalidParameterValueException("The input netmask " + netmask + " is not same as IP range netmask " + vlanRange.getVlanNetmask());
+            }
+
+            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 String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+            if (Strings.isNullOrEmpty(cidr)) {
+                throw new InvalidParameterValueException(String.format("Invalid gateway (%s) or netmask (%s)", gateway, netmask));
+            }
+            final String cidrAddress = getCidrAddress(cidr);
+            final long cidrSize = getCidrSize(cidr);
+
+            checkIpRange(currentStartIP, currentEndIP, cidrAddress, cidrSize);
+            if (startIp != currentStartIP || endIp != currentEndIP) {
+                checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+            }
+
+            checkGatewayOverlap(startIp, endIp, gateway);
+
+            final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+            checkAllocatedIpsAreWithinVlanRange(ips, startIp, endIp, forSystemVms);
+
+            try {
+                final String newStartIP = startIp;
+                final String newEndIP = endIp;
+
+                VlanVO 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, isRangeForSystemVM, forSystemVms);
+
+            } 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);
+            }
+        }
+
+        if (ipv6) {
+            if (ip6Gateway != null && !ip6Gateway.equals(vlanRange.getIp6Gateway())) {
+                throw new InvalidParameterValueException("The input gateway " + ip6Gateway + " is not same as IP range gateway " + vlanRange.getIp6Gateway());
+            }
+            if (ip6Cidr != null && !ip6Cidr.equals(vlanRange.getIp6Cidr())) {
+                throw new InvalidParameterValueException("The input cidr " + ip6Cidr + " is not same as IP range cidr " + vlanRange.getIp6Cidr());
+            }
+            ip6Gateway = MoreObjects.firstNonNull(ip6Gateway, vlanRange.getIp6Gateway());
+            ip6Cidr = MoreObjects.firstNonNull(ip6Cidr, vlanRange.getIp6Cidr());
+
+            final String[] existingVlanIPRangeArray = vlanRange.getIp6Range().split("-");
+            final String currentStartIPv6 = existingVlanIPRangeArray[0];
+            final String currentEndIPv6 = existingVlanIPRangeArray[1];
+
+            if (startIpv6 == null) {
+                startIpv6 = currentStartIPv6;
+            }
+            if (endIpv6 == null) {
+                endIpv6 = currentEndIPv6;
+            }
+            if (startIpv6 != currentStartIPv6 || endIpv6 != currentEndIPv6) {
+                _networkModel.checkIp6Parameters(startIpv6, endIpv6, ip6Gateway, ip6Cidr);
+                final List<UserIpv6AddressVO> ips = _ipv6Dao.listByVlanId(id);
+                checkAllocatedIpv6sAreWithinVlanRange(ips, startIp, endIp);
+
+                try {
+                    VlanVO 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, startIpv6, endIpv6, currentStartIPv6, currentEndIPv6, false, isRangeForSystemVM,forSystemVms);
+
+                } 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 _vlanDao.findById(id);
+    }
+
+    private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP, final String currentStartIP, final String currentEndIP,
+                                              final boolean ipv4, final Boolean isRangeForSystemVM, final Boolean forSystemvms) {
+
+        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());
+                if (ipv4) {
+                    vlanRange.setIpRange(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                    if (!updatePublicIPRange(newStartIP, currentStartIP, newEndIP, currentEndIP, vlanRange.getDataCenterId(), vlanRange.getId(), vlanRange.getNetworkId(), vlanRange.getPhysicalNetworkId(), isRangeForSystemVM, forSystemvms)) {
+                        throw new CloudRuntimeException("Failed to update IPv4 range. Please contact Cloud Support.");
+                    }
+                } else {
+                    vlanRange.setIp6Range(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                }
+                return vlanRange;
+            }
+        });
+    }
+
+    private boolean checkIfVlanRangeIsForSystemVM(final long vlanId) {
+        List<IPAddressVO> existingPublicIPs = _publicIpAddressDao.listByVlanId(vlanId);
+        boolean initialIsSystemVmValue = existingPublicIPs.get(0).isForSystemVms();
+        for (IPAddressVO existingIPs : existingPublicIPs) {
+            if (initialIsSystemVmValue != existingIPs.isForSystemVms()) {
+                throw new CloudRuntimeException("Your \"For System VM\" value seems to be inconsistent with the rest of the records. Please contact Cloud Support");
+            }
+        }
+        return initialIsSystemVmValue;
+    }
+
+    private void checkAllocatedIpsAreWithinVlanRange(List<IPAddressVO> ips, String startIp, String endIp, Boolean forSystemVms) {
+
+        List<IPAddressVO> listAllocatedIPs = new ArrayList<>();
+        for (final IPAddressVO ip : ips) {
+            if (ip.getState() == IpAddress.State.Allocated) {
+                listAllocatedIPs.add(ip);
+            }
+        }
+        Collections.sort(listAllocatedIPs, Comparator.comparing(IPAddressVO::getAddress));
+        for (IPAddressVO allocatedIP : listAllocatedIPs) {
+            if (!Strings.isNullOrEmpty(startIp) && NetUtils.ip2Long(startIp) > NetUtils.ip2Long(allocatedIP.getAddress().addr())) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + allocatedIP.getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " + listAllocatedIPs.get(listAllocatedIPs.size() - 1).getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());
+            }
+            if (!Strings.isNullOrEmpty(endIp) && NetUtils.ip2Long(endIp) < NetUtils.ip2Long(allocatedIP.getAddress().addr())) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + listAllocatedIPs.get(0).getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " +  allocatedIP.getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());
+            }
+            if (forSystemVms != null && allocatedIP.isForSystemVms() != forSystemVms) {
+                throw new InvalidParameterValueException(String.format("IP %s is in use, cannot change forSystemVms of the IP range", allocatedIP.getAddress().addr()));
+            }
+        }
+    }
+
+    private void checkAllocatedIpv6sAreWithinVlanRange(List<UserIpv6AddressVO> ips, String startIpv6, String endIpv6) {
+
+        List<UserIpv6AddressVO> listAllocatedIPs = new ArrayList<>();
+        for (final UserIpv6AddressVO ip : ips) {
+            if (ip.getState() == UserIpv6Address.State.Allocated) {
+                listAllocatedIPs.add(ip);
+            }
+        }
+        Collections.sort(listAllocatedIPs, Comparator.comparing(UserIpv6AddressVO::getAddress));
+        for (UserIpv6AddressVO allocatedIP : listAllocatedIPs) {
+            if (!Strings.isNullOrEmpty(startIpv6)
+                    && IPv6Address.fromString(startIpv6).toBigInteger().compareTo(IPv6Address.fromString(allocatedIP.getAddress()).toBigInteger()) > 0) {
+                throw new InvalidParameterValueException("The start IP address must have a lower IP address value "
+                        + "than " + allocatedIP.getAddress() + " which is already in use. The end IP must have a "
+                        + "higher IP address than " + listAllocatedIPs.get(listAllocatedIPs.size() - 1).getAddress() + " .IPs "
+                        + "already allocated in this range: " + listAllocatedIPs.size());
+            }

Review comment:
       and this




-- 
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 pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-921639639


   @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] DaanHoogland commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r711123345



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4245,260 @@ public VlanVO doInTransaction(final TransactionStatus status) {
 
     }
 
+    @Override
+    public Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,

Review comment:
       all of them should go to IpAddressManager :( (not in this PR probably)




-- 
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] weizhouapache commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r710823426



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,181 @@
+// 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.acl.RoleType;
+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.16.0",
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+        authorized = {RoleType.Admin})
+public class UpdateVlanIpRangeCmd extends BaseAsyncCmd {
+
+    public static final String APINAME = "updateVlanIpRange";
+    public static final Logger s_logger = Logger.getLogger(UpdateVlanIpRangeCmd.class.getName());
+
+    /////////////////////////////////////////////////////
+    //////////////// API parameters /////////////////////
+    /////////////////////////////////////////////////////
+
+
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VlanIpRangeResponse.class, required = true,
+            description = "the UUID of the VLAN IP range")
+    private Long id;
+
+    @Parameter(name = ApiConstants.GATEWAY, type = CommandType.STRING, description = "the gateway of the VLAN IP range")
+    private String gateway;

Review comment:
       @davidjumani 
   add support on gateway/netmask change

##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/UpdateVlanIpRangeCmd.java
##########
@@ -0,0 +1,181 @@
+// 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.acl.RoleType;
+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.16.0",
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+        authorized = {RoleType.Admin})
+public class UpdateVlanIpRangeCmd extends BaseAsyncCmd {
+
+    public static final String APINAME = "updateVlanIpRange";
+    public static final Logger s_logger = Logger.getLogger(UpdateVlanIpRangeCmd.class.getName());
+
+    /////////////////////////////////////////////////////
+    //////////////// API parameters /////////////////////
+    /////////////////////////////////////////////////////
+
+
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VlanIpRangeResponse.class, required = true,
+            description = "the UUID of the VLAN IP range")
+    private Long id;
+
+    @Parameter(name = ApiConstants.GATEWAY, type = CommandType.STRING, description = "the gateway of the VLAN IP range")
+    private String gateway;

Review comment:
       @davidjumani 
   added support on gateway/netmask change




-- 
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 #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#issuecomment-922041681


   <b>Trillian test result (tid-2110)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36051 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5411-t2110-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have errors
   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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5411: Add New API endpoint: UpdateVlanIpRange

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5411:
URL: https://github.com/apache/cloudstack/pull/5411#discussion_r710053269



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -4237,6 +4246,274 @@ 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(),
+                cmd.getStartIpv6(), cmd.getEndIpv6(), cmd.getIp6Gateway(), cmd.getIp6Cidr(), cmd.isForSystemVms());
+    }
+
+    @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,
+                                           String startIpv6,
+                                           String endIpv6,
+                                           String ip6Gateway,
+                                           String ip6Cidr,
+                                           Boolean forSystemVms) throws ConcurrentOperationException {
+
+        VlanVO vlanRange = _vlanDao.findById(id);
+        if (vlanRange == null) {
+            throw new InvalidParameterValueException("Please specify a valid IP range id.");
+        }
+
+        final boolean ipv4 = vlanRange.getVlanGateway() != null;
+        final boolean ipv6 = vlanRange.getIp6Gateway() != null;
+        if (!ipv4) {
+            if (startIp != null || endIp != null || gateway != null || netmask != null) {
+                throw new InvalidParameterValueException("IPv4 is not support in this IP range.");
+            }
+        }
+        if (!ipv6) {
+            if (startIpv6 != null || endIpv6 != null || ip6Gateway != null || ip6Cidr != null) {
+                throw new InvalidParameterValueException("IPv6 is not support in this IP range.");
+            }
+        }
+
+        final Boolean isRangeForSystemVM = checkIfVlanRangeIsForSystemVM(id);
+        if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
+            if (VlanType.DirectAttached.equals(vlanRange.getVlanType())) {
+                throw new InvalidParameterValueException("forSystemVms is not available for this IP range with vlan type: " + VlanType.DirectAttached);
+            }
+            // Check if range has already been dedicated
+            final List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(id);
+            if (maps != null && !maps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to an account");
+            }
+
+            List<DomainVlanMapVO> domainmaps = _domainVlanMapDao.listDomainVlanMapsByVlan(id);
+            if (domainmaps != null && !domainmaps.isEmpty()) {
+                throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to a domain");
+            }
+        }
+        if (ipv4) {
+            if (gateway != null && !gateway.equals(vlanRange.getVlanGateway())) {
+                throw new InvalidParameterValueException("The input gateway " + gateway + " is not same as IP range gateway " + vlanRange.getVlanGateway());
+            }
+            if (netmask != null && !netmask.equals(vlanRange.getVlanNetmask())) {
+                throw new InvalidParameterValueException("The input netmask " + netmask + " is not same as IP range netmask " + vlanRange.getVlanNetmask());
+            }
+
+            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 String cidr = NetUtils.ipAndNetMaskToCidr(gateway, netmask);
+            if (Strings.isNullOrEmpty(cidr)) {
+                throw new InvalidParameterValueException(String.format("Invalid gateway (%s) or netmask (%s)", gateway, netmask));
+            }
+            final String cidrAddress = getCidrAddress(cidr);
+            final long cidrSize = getCidrSize(cidr);
+
+            checkIpRange(currentStartIP, currentEndIP, cidrAddress, cidrSize);
+            if (startIp != currentStartIP || endIp != currentEndIP) {
+                checkIpRange(startIp, endIp, cidrAddress, cidrSize);
+            }
+
+            checkGatewayOverlap(startIp, endIp, gateway);
+
+            final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(id);
+            checkAllocatedIpsAreWithinVlanRange(ips, startIp, endIp, forSystemVms);
+
+            try {
+                final String newStartIP = startIp;
+                final String newEndIP = endIp;
+
+                VlanVO 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, isRangeForSystemVM, forSystemVms);
+
+            } 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);
+            }
+        }
+
+        if (ipv6) {
+            if (ip6Gateway != null && !ip6Gateway.equals(vlanRange.getIp6Gateway())) {
+                throw new InvalidParameterValueException("The input gateway " + ip6Gateway + " is not same as IP range gateway " + vlanRange.getIp6Gateway());
+            }
+            if (ip6Cidr != null && !ip6Cidr.equals(vlanRange.getIp6Cidr())) {
+                throw new InvalidParameterValueException("The input cidr " + ip6Cidr + " is not same as IP range cidr " + vlanRange.getIp6Cidr());
+            }
+            ip6Gateway = MoreObjects.firstNonNull(ip6Gateway, vlanRange.getIp6Gateway());
+            ip6Cidr = MoreObjects.firstNonNull(ip6Cidr, vlanRange.getIp6Cidr());
+
+            final String[] existingVlanIPRangeArray = vlanRange.getIp6Range().split("-");
+            final String currentStartIPv6 = existingVlanIPRangeArray[0];
+            final String currentEndIPv6 = existingVlanIPRangeArray[1];
+
+            if (startIpv6 == null) {
+                startIpv6 = currentStartIPv6;
+            }
+            if (endIpv6 == null) {
+                endIpv6 = currentEndIPv6;
+            }
+            if (startIpv6 != currentStartIPv6 || endIpv6 != currentEndIPv6) {
+                _networkModel.checkIp6Parameters(startIpv6, endIpv6, ip6Gateway, ip6Cidr);
+                final List<UserIpv6AddressVO> ips = _ipv6Dao.listByVlanId(id);
+                checkAllocatedIpv6sAreWithinVlanRange(ips, startIp, endIp);
+
+                try {
+                    VlanVO 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, startIpv6, endIpv6, currentStartIPv6, currentEndIPv6, false, isRangeForSystemVM,forSystemVms);
+
+                } 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 _vlanDao.findById(id);
+    }
+
+    private VlanVO commitUpdateVlanAndIpRange(final Long id, final String newStartIP, final String newEndIP, final String currentStartIP, final String currentEndIP,
+                                              final boolean ipv4, final Boolean isRangeForSystemVM, final Boolean forSystemvms) {
+
+        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());
+                if (ipv4) {
+                    vlanRange.setIpRange(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);
+                    if (!updatePublicIPRange(newStartIP, currentStartIP, newEndIP, currentEndIP, vlanRange.getDataCenterId(), vlanRange.getId(), vlanRange.getNetworkId(), vlanRange.getPhysicalNetworkId(), isRangeForSystemVM, forSystemvms)) {
+                        throw new CloudRuntimeException("Failed to update IPv4 range. Please contact Cloud Support.");
+                    }
+                } else {
+                    vlanRange.setIp6Range(newStartIP + "-" + newEndIP);
+                    _vlanDao.update(vlanRange.getId(), vlanRange);

Review comment:
       @sureshanaparti 
   it is not needed. 
   there is a record in user_ip_address table for each ipv4 address. however, user_ipv6_address table only contains the ipv6 address which are in use (Allocated state).




-- 
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