Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLOUDSTACK-9403 : Support for shared networks in Nuage VSP plugin #1579

Merged
merged 1 commit into from Dec 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/src/com/cloud/network/Network.java
Expand Up @@ -60,7 +60,8 @@ public static class Service {
public static final Service PortForwarding = new Service("PortForwarding");
public static final Service SecurityGroup = new Service("SecurityGroup");
public static final Service NetworkACL = new Service("NetworkACL", Capability.SupportedProtocols);
public static final Service Connectivity = new Service("Connectivity", Capability.DistributedRouter, Capability.RegionLevelVpc, Capability.StretchedL2Subnet);
public static final Service Connectivity = new Service("Connectivity", Capability.DistributedRouter, Capability.RegionLevelVpc, Capability.StretchedL2Subnet,
Capability.NoVlan, Capability.PublicAccess);

private final String name;
private final Capability[] caps;
Expand Down Expand Up @@ -215,6 +216,8 @@ public static class Capability {
public static final Capability DistributedRouter = new Capability("DistributedRouter");
public static final Capability StretchedL2Subnet = new Capability("StretchedL2Subnet");
public static final Capability RegionLevelVpc = new Capability("RegionLevelVpc");
public static final Capability NoVlan = new Capability("NoVlan");
public static final Capability PublicAccess = new Capability("PublicAccess");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if an untagged capability can be used, and default egress policy can be used; instead of introducing these two capabilities?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NoVlan is a Capability we're currently using when adding a new IP range to an existing shared network. Without this, the Broadcast URI would become vlan://untagged, while the shared network has a Broadcast URI of vsp://network-uuid. When the CIDR of the new ip range is the same as one of the already existing ip ranges, it would still not be allowed as the vlan id is different.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining @fmaximus


private final String name;

Expand Down
2 changes: 2 additions & 0 deletions api/src/com/cloud/network/NetworkModel.java
Expand Up @@ -154,6 +154,8 @@ public interface NetworkModel {

boolean checkIpForService(IpAddress ip, Service service, Long networkId);

boolean providerSupportsCapability(Set<Provider> providers, Service service, Capability cap);

void checkCapabilityForProvider(Set<Provider> providers, Service service, Capability cap, String capValue);

Provider getDefaultUniqueProviderForService(String serviceName);
Expand Down
2 changes: 2 additions & 0 deletions api/src/com/cloud/offering/NetworkOffering.java
Expand Up @@ -131,4 +131,6 @@ public enum Detail {
boolean isKeepAliveEnabled();

boolean getSupportsStrechedL2();

boolean getSupportsPublicAccess();
}
1 change: 1 addition & 0 deletions api/src/org/apache/cloudstack/api/ApiConstants.java
Expand Up @@ -639,6 +639,7 @@ public class ApiConstants {
public static final String READ_ONLY = "readonly";
public static final String SUPPORTS_REGION_LEVEL_VPC = "supportsregionLevelvpc";
public static final String SUPPORTS_STRECHED_L2_SUBNET = "supportsstrechedl2subnet";
public static final String SUPPORTS_PUBLIC_ACCESS = "supportspublicaccess";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guest VM's with public network access is possible by checking default egress rule. Without adding this new notion of a 'public access', networks can reuse the Egress Default Policy.

public static final String REGION_LEVEL_VPC = "regionlevelvpc";
public static final String STRECHED_L2_SUBNET = "strechedl2subnet";
public static final String NETWORK_SPANNED_ZONES = "zonesnetworkspans";
Expand Down
Expand Up @@ -120,6 +120,10 @@ public class NetworkOfferingResponse extends BaseResponse {
@Param(description = "true if network offering supports network that span multiple zones", since = "4.4")
private Boolean supportsStrechedL2Subnet;

@SerializedName(ApiConstants.SUPPORTS_PUBLIC_ACCESS)
@Param(description = "true if network offering supports public access for guest networks", since = "4.10.0")
private Boolean supportsPublicAccess;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New networks can reuse the Egress Default Policy?


public void setId(String id) {
this.id = id;
}
Expand Down Expand Up @@ -207,4 +211,8 @@ public void setConcurrentConnections(Integer concurrentConnections) {
public void setSupportsStrechedL2Subnet(Boolean supportsStrechedL2Subnet) {
this.supportsStrechedL2Subnet = supportsStrechedL2Subnet;
}

public void setSupportsPublicAccess(Boolean supportsPublicAccess) {
this.supportsPublicAccess = supportsPublicAccess;
}
}
20 changes: 5 additions & 15 deletions client/pom.xml
Expand Up @@ -166,6 +166,11 @@
<artifactId>cloud-plugin-network-vcs</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.apache.cloudstack</groupId>
<artifactId>cloud-plugin-network-vsp</artifactId>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are nuage specific dependencies Apache 2.0 license compliant and available on maven? Only then, I would be in a favour of moving the plugin to the default build profile.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming @fmaximus

<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.apache.cloudstack</groupId>
<artifactId>cloud-plugin-hypervisor-xenserver</artifactId>
Expand Down Expand Up @@ -990,21 +995,6 @@
</dependency>
</dependencies>
</profile>
<profile>
<id>nuagevsp</id>
<activation>
<property>
<name>noredist</name>
</property>
</activation>
<dependencies>
<dependency>
<groupId>org.apache.cloudstack</groupId>
<artifactId>cloud-plugin-network-vsp</artifactId>
<version>${project.version}</version>
</dependency>
</dependencies>
</profile>
<profile>
<id>srx</id>
<activation>
Expand Down
2 changes: 2 additions & 0 deletions engine/schema/src/com/cloud/dc/dao/VlanDao.java
Expand Up @@ -49,6 +49,8 @@ public interface VlanDao extends GenericDao<VlanVO, Long> {

List<VlanVO> listVlansByNetworkId(long networkId);

List<VlanVO> listVlansByNetworkIdIncludingRemoved(long networkId);

List<VlanVO> listVlansByPhysicalNetworkId(long physicalNetworkId);

List<VlanVO> listZoneWideNonDedicatedVlans(long zoneId);
Expand Down
12 changes: 9 additions & 3 deletions engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java
Expand Up @@ -105,7 +105,7 @@ public VlanDaoImpl() {
ZoneTypeSearch.done();

NetworkVlanSearch = createSearchBuilder();
NetworkVlanSearch.and("networkOfferingId", NetworkVlanSearch.entity().getNetworkId(), SearchCriteria.Op.EQ);
NetworkVlanSearch.and("networkId", NetworkVlanSearch.entity().getNetworkId(), SearchCriteria.Op.EQ);
NetworkVlanSearch.done();

PhysicalNetworkVlanSearch = createSearchBuilder();
Expand Down Expand Up @@ -330,12 +330,18 @@ public List<VlanVO> searchForZoneWideVlans(long dcId, String vlanType, String vl
}

@Override
public List<VlanVO> listVlansByNetworkId(long networkOfferingId) {
public List<VlanVO> listVlansByNetworkId(long networkId) {
SearchCriteria<VlanVO> sc = NetworkVlanSearch.create();
sc.setParameters("networkOfferingId", networkOfferingId);
sc.setParameters("networkId", networkId);
return listBy(sc);
}

@Override public List<VlanVO> listVlansByNetworkIdIncludingRemoved(long networkId) {
SearchCriteria<VlanVO> sc = NetworkVlanSearch.create();
sc.setParameters("networkId", networkId);
return listIncludingRemovedBy(sc);
}

@Override
public List<VlanVO> listVlansByNetworkIdAndGateway(long networkid, String gateway) {
SearchCriteria<VlanVO> sc = VlanGatewaysearch.create();
Expand Down
11 changes: 10 additions & 1 deletion engine/schema/src/com/cloud/offerings/NetworkOfferingVO.java
Expand Up @@ -142,6 +142,9 @@ public class NetworkOfferingVO implements NetworkOffering {
@Column(name="supports_streched_l2")
boolean supportsStrechedL2 = false;

@Column(name="supports_public_access")
boolean supportsPublicAccess = false;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use egress_default_policy? Justify introduction of a new column?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our SDN solution, shared networks are not always reachable.
If a shared network is routable, in the same way a public network is,
this flag will let the Nuage VSP know that it should distribute routes for that shared network.

@Override
public String getDisplayText() {
return displayText;
Expand Down Expand Up @@ -334,7 +337,7 @@ public NetworkOfferingVO(String name, String displayText, TrafficType trafficTyp
public NetworkOfferingVO(String name, String displayText, TrafficType trafficType, boolean systemOnly, boolean specifyVlan, Integer rateMbps,
Integer multicastRateMbps, boolean isDefault, Availability availability, String tags, Network.GuestType guestType, boolean conserveMode, boolean dedicatedLb,
boolean sharedSourceNat, boolean redundantRouter, boolean elasticIp, boolean elasticLb, boolean specifyIpRanges, boolean inline, boolean isPersistent,
boolean associatePublicIP, boolean publicLb, boolean internalLb, boolean egressdefaultpolicy, boolean supportsStrechedL2) {
boolean associatePublicIP, boolean publicLb, boolean internalLb, boolean egressdefaultpolicy, boolean supportsStrechedL2, boolean supportsPublicAccess) {
this(name,
displayText,
trafficType,
Expand All @@ -360,6 +363,7 @@ public NetworkOfferingVO(String name, String displayText, TrafficType trafficTyp
this.eipAssociatePublicIp = associatePublicIP;
this.egressdefaultpolicy = egressdefaultpolicy;
this.supportsStrechedL2 = supportsStrechedL2;
this.supportsPublicAccess = supportsPublicAccess;
}

public NetworkOfferingVO() {
Expand Down Expand Up @@ -495,4 +499,9 @@ public void setPublicLb(boolean publicLb) {
public boolean getSupportsStrechedL2() {
return supportsStrechedL2;
}

@Override
public boolean getSupportsPublicAccess() {
return supportsPublicAccess;
}
}
4 changes: 2 additions & 2 deletions plugins/network-elements/nuage-vsp/pom.xml
Expand Up @@ -36,9 +36,9 @@
</repositories>
<dependencies>
<dependency>
<groupId>net.nuage.vsp</groupId>
<groupId>net.nuagenetworks.vsp</groupId>
<artifactId>nuage-vsp-acs-client</artifactId>
<version>3.2.8.1</version>
<version>1.0.0</version>
</dependency>
</dependencies>
<build>
Expand Down
@@ -0,0 +1,72 @@
//
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//

package com.cloud.agent.api.manager;

import com.cloud.agent.api.Command;

import com.google.common.base.Preconditions;
import net.nuage.vsp.acs.client.api.model.VspDomainCleanUp;
import org.apache.commons.lang.builder.HashCodeBuilder;

import java.util.Objects;

public class CleanUpDomainCommand extends Command {

private final VspDomainCleanUp _domainCleanUp;

public CleanUpDomainCommand(VspDomainCleanUp domainCleanUp) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it acceptable for domainCleanup to be null? If not, please consider adding a Preconditions.checkArgument check to verify that the domainCleanup is not null. You may also be to remove null checks from the equals and hashCode methods if this precondition is added.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added a null check (Preconditions.checkNotNull(domainCleanUp);).

super();
Preconditions.checkNotNull(domainCleanUp);
this._domainCleanUp = domainCleanUp;
}

public VspDomainCleanUp getDomainCleanUp() {
return _domainCleanUp;
}

@Override
public boolean executeInSequence() {
return false;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}

if (!(o instanceof CleanUpDomainCommand)) {
return false;
}

CleanUpDomainCommand that = (CleanUpDomainCommand) o;

return super.equals(that)
&& Objects.equals(_domainCleanUp, that._domainCleanUp);
}

@Override
public int hashCode() {
return new HashCodeBuilder()
.appendSuper(super.hashCode())
.append(_domainCleanUp)
.toHashCode();
}
}
Expand Up @@ -19,6 +19,10 @@

package com.cloud.agent.api.manager;

import org.apache.commons.lang.builder.HashCodeBuilder;

import java.util.Objects;

import com.cloud.agent.api.Command;

public class EntityExistsCommand<T> extends Command {
Expand Down Expand Up @@ -55,23 +59,27 @@ public boolean executeInSequence() {

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof EntityExistsCommand)) return false;
if (!super.equals(o)) return false;
if (this == o) {
return true;
}

EntityExistsCommand that = (EntityExistsCommand) o;
if (!(o instanceof EntityExistsCommand)) {
return false;
}

if (getType() != null ? !getType().equals(that.getType()) : that.getType() != null) return false;
if (_uuid != null ? !_uuid.equals(that._uuid) : that._uuid != null) return false;
EntityExistsCommand that = (EntityExistsCommand) o;

return true;
return super.equals(that)
&& Objects.equals(getType(), that.getType())
&& Objects.equals(_uuid, that._uuid);
}

@Override
public int hashCode() {
int result = super.hashCode();
result = 31 * result + (getType() != null ? getType().hashCode() : 0);
result = 31 * result + (_uuid != null ? _uuid.hashCode() : 0);
return result;
return new HashCodeBuilder()
.appendSuper(super.hashCode())
.append(getType())
.append(_uuid)
.toHashCode();
}
}
@@ -0,0 +1,70 @@
//
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//

package com.cloud.agent.api.manager;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class appears to be Nuage specific. Why is is being added to core CloudStack and not the Nuage plugin?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is part of the plugin not core CS.


import org.apache.commons.lang.builder.HashCodeBuilder;

import java.util.Objects;

import com.cloud.agent.api.Command;
import com.cloud.network.resource.NuageVspResourceConfiguration;

public class UpdateNuageVspDeviceCommand extends Command {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add equals, hashCode, and toString implementations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added it.


private final NuageVspResourceConfiguration configuration;

public UpdateNuageVspDeviceCommand(NuageVspResourceConfiguration configuration) {
super();
this.configuration = configuration;
}

public NuageVspResourceConfiguration getConfiguration() {
return configuration;
}

@Override
public boolean executeInSequence() {
return false;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}

if (!(o instanceof UpdateNuageVspDeviceCommand)) {
return false;
}

UpdateNuageVspDeviceCommand that = (UpdateNuageVspDeviceCommand) o;

return super.equals(that)
&& Objects.equals(configuration, that.configuration);
}

@Override
public int hashCode() {
return new HashCodeBuilder()
.appendSuper(super.hashCode())
.append(configuration)
.toHashCode();
}
}