Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
dcb871b
AMBARI-22878: Update Service Group API to take list of mpack name ass…
scottduan Jan 30, 2018
e97eb29
AMBARI-22878: Update Service Group API to take list of mpack name ass…
scottduan Jan 30, 2018
da41ca0
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 1, 2018
158cd85
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 1, 2018
decbf54
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 5, 2018
68c1f04
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
a2340a6
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
04345f8
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 9, 2018
90f354f
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
a331735
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
3de0e93
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 16, 2018
f74af11
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 16, 2018
afc6a3b
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 18, 2018
fb3e005
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 18, 2018
7bcd6fe
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 22, 2018
cfe5295
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 22, 2018
d370602
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 22, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ public class ServiceGroupRequest {

private String clusterName; // REF
private String serviceGroupName; // GET/CREATE/UPDATE/DELETE
private String version; // Associated stack version info

public ServiceGroupRequest(String clusterName, String serviceGroupName) {
public ServiceGroupRequest(String clusterName, String serviceGroupName, String version) {
this.clusterName = clusterName;
Copy link
Member

Choose a reason for hiding this comment

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

I think there's still a misunderstanding here - I thought it would be cleaner to separate this out into 2 fields so that it's clear to the user what is expected: mpack name and then mpack version. You can combine this and call it a stack ID internally, that's fine - but we should take 2 different properties.

Same goes for API requests where we're currently given name-version as a string. This should be broken out into 2 fields.

Copy link
Author

Choose a reason for hiding this comment

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

I am ok for this change.

Copy link
Author

Choose a reason for hiding this comment

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

@jayush What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets keep it as "version" for now for legacy purposes. When we upgrade to ambari 3.0 assuming the existing cluster is on HDP-2.6 stack, calling this as mpack name and mpack version would be misleading.

We should handle this in a separate JIRA if we really want to change this convention for all APIs.

Copy link
Member

Choose a reason for hiding this comment

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

But "version" in legacy terms meant only the numeric value. If we don't want to break it out into 2 references, I think we'd rather use stackId, right?

this.serviceGroupName = serviceGroupName;
this.version = version;
}

/**
Expand Down Expand Up @@ -57,9 +59,25 @@ public void setServiceGroupName(String serviceGroupName) {
this.serviceGroupName = serviceGroupName;
}

/**
* @return the servicegroup version
*/
public String getVersion() {
return version;
}

/**
* @param version the servicegroup version to set
*/
public void setVersion(String version) {
this.version = version;
}

@Override
public String toString() {
return String.format("clusterName=%s, serviceGroupName=%s", clusterName, serviceGroupName);
StringBuilder sb = new StringBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

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

We've been gradually moving to ToStringBuilder here instead

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we should switch this to ToStringBuilder.

sb.append("clusterName=").append(clusterName).append(", serviceGroupName=").append(serviceGroupName).append(", version=").append(version);
return sb.toString();
}

@Override
Expand All @@ -73,12 +91,11 @@ public boolean equals(Object obj) {

ServiceGroupRequest other = (ServiceGroupRequest) obj;

return Objects.equals(clusterName, other.clusterName) &&
Objects.equals(serviceGroupName, other.serviceGroupName);
return Objects.equals(clusterName, other.clusterName) && Objects.equals(serviceGroupName, other.serviceGroupName) && Objects.equals(version, other.version);
}

@Override
public int hashCode() {
return Objects.hash(clusterName, serviceGroupName);
return Objects.hash(clusterName, serviceGroupName, version);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package org.apache.ambari.server.controller;

import java.util.Objects;

import io.swagger.annotations.ApiModelProperty;

public class ServiceGroupResponse {
Expand All @@ -26,12 +28,15 @@ public class ServiceGroupResponse {
private Long serviceGroupId;
private String clusterName;
private String serviceGroupName;
private String version;

public ServiceGroupResponse(Long clusterId, String clusterName, Long serviceGroupId, String serviceGroupName) {
public ServiceGroupResponse(Long clusterId, String clusterName, Long serviceGroupId, String serviceGroupName, String version) {
this.clusterId = clusterId;
this.serviceGroupId = serviceGroupId;
this.clusterName = clusterName;
this.serviceGroupName = serviceGroupName;
this.version = version;

}

/**
Expand Down Expand Up @@ -90,27 +95,28 @@ public void setServiceGroupName(String serviceGroupName) {
this.serviceGroupName = serviceGroupName;
}

/**
* @return the servicegroup version (stackName-stackVersion)
*/
public String getVersion() {
return version;
}

/**
* @param version the servicegroup version (stackName-stackVersion)
*/
public void setVersion(String version) {
this.version = version;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

ServiceGroupResponse that = (ServiceGroupResponse) o;

if (clusterId != null ?
!clusterId.equals(that.clusterId) : that.clusterId != null) {
return false;
}
if (clusterName != null ?
!clusterName.equals(that.clusterName) : that.clusterName != null) {
return false;
}
if (serviceGroupName != null ?
!serviceGroupName.equals(that.serviceGroupName) : that.serviceGroupName != null) {
return false;
}

return true;
ServiceGroupResponse other = (ServiceGroupResponse) o;

return Objects.equals(clusterId, other.clusterId) && Objects.equals(clusterName, other.clusterName) && Objects.equals(serviceGroupName, other.serviceGroupName) && Objects.equals(version, other.version);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public void setRequiredDeleteAuthorizations(Set<RoleAuthorization> requiredDelet
*/
@Override
public RequestStatus createResources(Request request)
throws SystemException, UnsupportedPropertyException, ResourceAlreadyExistsException, NoSuchParentResourceException {
throws SystemException, UnsupportedPropertyException, ResourceAlreadyExistsException, NoSuchParentResourceException, IllegalArgumentException {
Authentication authentication = AuthorizationHelper.getAuthentication();

if (authentication == null || !authentication.isAuthenticated()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.apache.ambari.server.state.Cluster;
import org.apache.ambari.server.state.Clusters;
import org.apache.ambari.server.state.ServiceGroup;
import org.apache.ambari.server.state.StackId;
import org.apache.ambari.server.utils.StageUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.Validate;
Expand All @@ -78,6 +79,7 @@ public class ServiceGroupResourceProvider extends AbstractControllerResourceProv
public static final String SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "cluster_name";
public static final String SERVICE_GROUP_SERVICE_GROUP_ID_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "service_group_id";
public static final String SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "service_group_name";
public static final String SERVICE_GROUP_VERSION_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "version";


private static Set<String> pkPropertyIds =
Expand All @@ -103,10 +105,12 @@ public class ServiceGroupResourceProvider extends AbstractControllerResourceProv
PROPERTY_IDS.add(SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID);
PROPERTY_IDS.add(SERVICE_GROUP_SERVICE_GROUP_ID_PROPERTY_ID);
PROPERTY_IDS.add(SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID);
PROPERTY_IDS.add(SERVICE_GROUP_VERSION_PROPERTY_ID);

// keys
KEY_PROPERTY_IDS.put(Resource.Type.Cluster, SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID);
KEY_PROPERTY_IDS.put(Resource.Type.ServiceGroup, SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID);
KEY_PROPERTY_IDS.put(Resource.Type.Stack, SERVICE_GROUP_VERSION_PROPERTY_ID);
}

private Clusters clusters;
Expand Down Expand Up @@ -136,7 +140,7 @@ protected RequestStatus createResourcesAuthorized(Request request)
throws SystemException,
UnsupportedPropertyException,
ResourceAlreadyExistsException,
NoSuchParentResourceException {
NoSuchParentResourceException, IllegalArgumentException{

final Set<ServiceGroupRequest> requests = new HashSet<>();
for (Map<String, Object> propertyMap : request.getProperties()) {
Expand All @@ -145,7 +149,7 @@ protected RequestStatus createResourcesAuthorized(Request request)
Set<ServiceGroupResponse> createServiceGroups = null;
createServiceGroups = createResources(new Command<Set<ServiceGroupResponse>>() {
@Override
public Set<ServiceGroupResponse> invoke() throws AmbariException, AuthorizationException {
public Set<ServiceGroupResponse> invoke() throws AmbariException, AuthorizationException, IllegalArgumentException {
return createServiceGroups(requests);
}
});
Expand All @@ -160,6 +164,7 @@ public Set<ServiceGroupResponse> invoke() throws AmbariException, AuthorizationE
resource.setProperty(SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID, response.getClusterName());
resource.setProperty(SERVICE_GROUP_SERVICE_GROUP_ID_PROPERTY_ID, response.getServiceGroupId());
resource.setProperty(SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID, response.getServiceGroupName());
resource.setProperty(SERVICE_GROUP_VERSION_PROPERTY_ID, response.getVersion());

associatedResources.add(resource);
}
Expand Down Expand Up @@ -199,6 +204,8 @@ public Set<ServiceGroupResponse> invoke() throws AmbariException {
response.getServiceGroupId(), requestedIds);
setResourceProperty(resource, SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID,
response.getServiceGroupName(), requestedIds);
setResourceProperty(resource, SERVICE_GROUP_VERSION_PROPERTY_ID,
response.getVersion(), requestedIds);
resources.add(resource);
}
return resources;
Expand Down Expand Up @@ -268,13 +275,14 @@ protected Set<String> getPKPropertyIds() {
private ServiceGroupRequest getRequest(Map<String, Object> properties) {
String clusterName = (String) properties.get(SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID);
String serviceGroupName = (String) properties.get(SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID);
ServiceGroupRequest svcRequest = new ServiceGroupRequest(clusterName, serviceGroupName);
String version = (String) properties.get(SERVICE_GROUP_VERSION_PROPERTY_ID);
ServiceGroupRequest svcRequest = new ServiceGroupRequest(clusterName, serviceGroupName, version);
return svcRequest;
}

// Create services from the given request.
public synchronized Set<ServiceGroupResponse> createServiceGroups(Set<ServiceGroupRequest> requests)
throws AmbariException, AuthorizationException {
throws AmbariException, AuthorizationException, IllegalArgumentException {

if (requests.isEmpty()) {
LOG.warn("Received an empty requests set");
Expand All @@ -291,7 +299,7 @@ public synchronized Set<ServiceGroupResponse> createServiceGroups(Set<ServiceGro
Cluster cluster = clusters.getCluster(request.getClusterName());

// Already checked that service group does not exist
ServiceGroup sg = cluster.addServiceGroup(request.getServiceGroupName());
ServiceGroup sg = cluster.addServiceGroup(request.getServiceGroupName(), request.getVersion());
createdSvcGrps.add(sg.convertToResponse());
}
return createdSvcGrps;
Expand Down Expand Up @@ -389,21 +397,33 @@ ResourceType.CLUSTER, getClusterResourceId(serviceGroupRequest.getClusterName())


private void validateCreateRequests(Set<ServiceGroupRequest> requests, Clusters clusters)
throws AuthorizationException, AmbariException {
throws AuthorizationException, AmbariException, IllegalArgumentException {

AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo();
Map<String, Set<String>> serviceGroupNames = new HashMap<>();
Set<String> duplicates = new HashSet<>();
for (ServiceGroupRequest request : requests) {
final String clusterName = request.getClusterName();
final String serviceGroupName = request.getServiceGroupName();
String version = request.getVersion();
//TODO: This should not happen, after UI changes the code, this check should be removed
if (StringUtils.isBlank(version)) {
try {
Cluster cluster = clusters.getCluster(clusterName);
StackId stackId = cluster.getCurrentStackVersion();
request.setVersion(stackId.getStackId());
} catch (ClusterNotFoundException e) {
throw new ParentObjectNotFoundException("Cluster " + clusterName + " does not exist: ", e);
}
}

Validate.notNull(clusterName, "Cluster name should be provided when creating a service group");
Validate.notEmpty(serviceGroupName, "Service group name should be provided when creating a service group");
Validate.notEmpty(request.getVersion(), "Stack version should be provided when creating a service group");

if (LOG.isDebugEnabled()) {
LOG.debug("Received a createServiceGroup request" +
", clusterName=" + clusterName + ", serviceGroupName=" + serviceGroupName + ", request=" + request);
", clusterName=" + clusterName + ", serviceGroupName=" + serviceGroupName + ", version=" + version + ", request=" + request);
}

if (!AuthorizationHelper.isAuthorized(ResourceType.CLUSTER,
Expand All @@ -422,12 +442,14 @@ private void validateCreateRequests(Set<ServiceGroupRequest> requests, Clusters
}
serviceGroupNames.get(clusterName).add(serviceGroupName);

// TODO: Validate stack version
Cluster cluster;
try {
cluster = clusters.getCluster(clusterName);
} catch (ClusterNotFoundException e) {
throw new ParentObjectNotFoundException("Attempted to add a service group to a cluster which doesn't exist", e);
}

try {
ServiceGroup sg = cluster.getServiceGroup(serviceGroupName);
if (sg != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.ambari.server.orm.entities;

import java.util.List;
import java.util.Objects;

import javax.persistence.Column;
import javax.persistence.Entity;
Expand All @@ -31,6 +32,7 @@
import javax.persistence.NamedQueries;
import javax.persistence.NamedQuery;
import javax.persistence.OneToMany;
import javax.persistence.OneToOne;
import javax.persistence.Table;
import javax.persistence.TableGenerator;

Expand Down Expand Up @@ -70,6 +72,13 @@ public class ServiceGroupEntity {
@Column(name = "service_group_name", nullable = false, insertable = true, updatable = true)
private String serviceGroupName;

/**
* Unidirectional one-to-one association to {@link StackEntity}
*/
@OneToOne
@JoinColumn(name = "stack_id", unique = false, nullable = false, insertable = true, updatable = true)
private StackEntity stack;

@ManyToOne
@JoinColumn(name = "cluster_id", referencedColumnName = "cluster_id", nullable = false)
private ClusterEntity clusterEntity;
Expand All @@ -96,7 +105,6 @@ public void setServiceGroupId(Long serviceGroupId) {
this.serviceGroupId = serviceGroupId;
}


public String getServiceGroupName() {
return serviceGroupName;
}
Expand All @@ -105,6 +113,14 @@ public void setServiceGroupName(String serviceGroupName) {
this.serviceGroupName = serviceGroupName;
}

public StackEntity getStack() {
return stack;
}

public void setStack(StackEntity stack) {
this.stack = stack;
}

public List<ServiceGroupDependencyEntity> getDependencies() {
return dependencies;
}
Expand All @@ -131,15 +147,15 @@ public boolean equals(Object o) {
if (clusterId != null ? !clusterId.equals(that.clusterId) : that.clusterId != null) return false;
if (serviceGroupName != null ? !serviceGroupName.equals(that.serviceGroupName) : that.serviceGroupName != null)
return false;
if (Long.valueOf(stack.getStackId()) != Long.valueOf(((ServiceGroupEntity) o).getStack().getStackId()))
return false;

return true;
}

@Override
public int hashCode() {
int result = clusterId != null ? clusterId.intValue() : 0;
result = 31 * result + (serviceGroupName != null ? serviceGroupName.hashCode() : 0);
return result;
return Objects.hash(serviceGroupId, clusterId, serviceGroupName, stack);
}

public ClusterEntity getClusterEntity() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ Service addDependencyToService(String serviceGroupName, String serviceName,
* @return
* @throws AmbariException
*/
ServiceGroup addServiceGroup(String serviceGroupName) throws AmbariException;
ServiceGroup addServiceGroup(String serviceGroupName, String version) throws AmbariException;

ServiceGroup addServiceGroup(String serviceGroupName, StackId stackId) throws AmbariException;

/**
* Add service group dependency to the service group
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public interface ServiceGroup {

Set<ServiceGroupDependencyResponse> getServiceGroupDependencyResponses();

StackId getStackId();

void debugDump(StringBuilder sb);

void refresh();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public interface ServiceGroupFactory {

ServiceGroup createNew(Cluster cluster,
@Assisted("serviceGroupName") String serviceGroupName,
@Assisted("stackId") StackId stackId,
@Assisted("serviceGroupDependencies") Set<ServiceGroupKey> serviceGroupDependencies);

ServiceGroup createExisting(Cluster cluster, ServiceGroupEntity serviceGroupEntity);
Expand Down
Loading