[AMBARI-22875] Blueprint cluster creation using manually installed mpacks

[adoroszlai]

What changes were proposed in this pull request?

Add support for creating cluster via blueprint using one or more manually installed mpacks.

Mpacks referenced in the blueprint are combined into a composite stack. Both single and composite stack implement a common interface, to minimize the change required.

For now it assumes that each service is defined in only one mpack.

How was this patch tested?

Tested cluster installation using:

• blueprint with a stack
• blueprint with 2 mpacks

Tested blueprint validation using:

• blueprint referencing non-existent stack
• blueprint referencing non-existent mpack
• blueprint referencing non-existent component in existing mpack

Related unit tests are OK, no regression in other unit tests:

[ERROR] Tests run: 5104, Failures: 39, Errors: 230, Skipped: 37
->
[ERROR] Tests run: 5117, Failures: 39, Errors: 230, Skipped: 37

Checkstyle is OK:

[INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
[INFO] Starting audit...
Audit done.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/346/
Test FAILed.
Test FAILured.

[rnettleton]

Overall, the PR looks good to me, just a few minor issues that probably should be addressed prior to merging.

Thanks!

[rnettleton]

Should we consider using a JDK 8 Stream to get the first element, rather than add a new dependency?

Maybe we should use the "findFirst()" method:

https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html#findFirst--

[adoroszlai]

Good idea, I changed it.

[rnettleton]

This method should have javadocs.

[rnettleton]

These three methods should have javadocs.

[rnettleton]

Missing javadoc.

[rnettleton]

Missing javadocs.

[rnettleton]

Is this hard-coding still required, now that the new BP parsing changes are merged into the feature branch?

[adoroszlai]

Yes, unfortunately it's still needed until hdp-select is replaced (or added to all mpacks).

[rnettleton]

Why is this being changed to a runtime exception? Is this required due to the JDK8 stream usages in the new code?

[adoroszlai]

Exactly.

[rnettleton]

I would recommend updating the javadoc for these methods, to document the Blueprint parameter.

[rnettleton]

javadoc needs to be updated for this method.

[rnettleton]

Why is this change included in this PR? It doesn't appear to be related to the other Blueprint-specific changes in this PR.

[adoroszlai]

At some point I ran into an error where smoke_user was not being created, but I think the role-command order workaround addresses it, too. So we don't need this anymore.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/354/
Test FAILed.
Test FAILured.

[benyoka]

LGTM overall.
There are two issues where I would like clarification/change:

• One incomplete if-else branch
• Use of ObjectReader / Writer in JsonUtil

[benyoka]

This CompositeStack is a great idea!
One question though: Do we plan to have a sanity check somewhere to make sure a component/service/dependency/etc. is only defined in only one stack to avoid non-deterministic behavior?

[adoroszlai]

Added check for service and component.

[benyoka]

Shouldn't there be a third else branch that throws an IllegalArgimentException? Looks like if null == repoVersion and repoVersionString is null it will fall through while in other error cases it throws an exception.

[adoroszlai]

If both are null (ie. no repo version was specified in the request), createAmbariResources tries to find any repo for the stack (findRepoForStack). If both of these attempts fail, then an IllegalArgumentException is thrown by way of calling Preconditions.checkNotNull.

This is basically a slightly reorganized version of the existing logic, applied to each stack:

ambari/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java

Lines 219 to 299 in 995b5d4

	RepositoryVersionEntity repoVersion = null;
	if (StringUtils.isEmpty(repoVersionString) && null == repoVersionId) {
	List<RepositoryVersionEntity> stackRepoVersions = repositoryVersionDAO.findByStack(stackId);
	if (stackRepoVersions.isEmpty()) {
	// !!! no repos, try to get the version for the stack
	VersionDefinitionResourceProvider vdfProvider = getVersionDefinitionResourceProvider();
	Map<String, Object> properties = new HashMap<>();
	properties.put(VersionDefinitionResourceProvider.VERSION_DEF_AVAILABLE_DEFINITION, stackId.toString());
	Request request = new RequestImpl(Collections.<String>emptySet(),
	Collections.singleton(properties), Collections.<String, String>emptyMap(), null);
	Long defaultRepoVersionId = null;
	try {
	RequestStatus requestStatus = vdfProvider.createResources(request);
	if (!requestStatus.getAssociatedResources().isEmpty()) {
	Resource resource = requestStatus.getAssociatedResources().iterator().next();
	defaultRepoVersionId = (Long) resource.getPropertyValue(VersionDefinitionResourceProvider.VERSION_DEF_ID);
	}
	} catch (Exception e) {
	throw new IllegalArgumentException(String.format(
	"Failed to create a default repository version definition for stack %s. "
	+ "This typically is a result of not loading the stack correctly or being able "
	+ "to load information about released versions. Create a repository version "
	+ " and try again.", stackId), e);
	}
	repoVersion = repositoryVersionDAO.findByPK(defaultRepoVersionId);
	// !!! better not!
	if (null == repoVersion) {
	throw new IllegalArgumentException(String.format(
	"Failed to load the default repository version definition for stack %s. "
	+ "Check for a valid repository version and try again.", stackId));
	}
	} else if (stackRepoVersions.size() > 1) {
	Function<RepositoryVersionEntity, String> function = new Function<RepositoryVersionEntity, String>() {
	@Override
	public String apply(RepositoryVersionEntity input) {
	return input.getVersion();
	}
	};
	Collection<String> versions = Collections2.transform(stackRepoVersions, function);
	throw new IllegalArgumentException(String.format("Several repositories were found for %s: %s. Specify the version"
	+ " with '%s'", stackId, StringUtils.join(versions, ", "), ProvisionClusterRequest.REPO_VERSION_PROPERTY));
	} else {
	repoVersion = stackRepoVersions.get(0);
	LOG.warn("Cluster is being provisioned using the single matching repository version {}", repoVersion.getVersion());
	}
	} else if (null != repoVersionId){
	repoVersion = repositoryVersionDAO.findByPK(repoVersionId);
	if (null == repoVersion) {
	throw new IllegalArgumentException(String.format(
	"Could not identify repository version with repository version id %s for installing services. "
	+ "Specify a valid repository version id with '%s'",
	repoVersionId, ProvisionClusterRequest.REPO_VERSION_ID_PROPERTY));
	}
	} else {
	repoVersion = repositoryVersionDAO.findByStackAndVersion(stackId, repoVersionString);
	if (null == repoVersion) {
	throw new IllegalArgumentException(String.format(
	"Could not identify repository version with stack %s and version %s for installing services. "
	+ "Specify a valid version with '%s'",
	stackId, repoVersionString, ProvisionClusterRequest.REPO_VERSION_PROPERTY));
	}
	}
	// only use a STANDARD repo when creating a new cluster
	if (repoVersion.getType() != RepositoryType.STANDARD) {
	throw new IllegalArgumentException(String.format(
	"Unable to create a cluster using the following repository since it is not a STANDARD type: %s",
	repoVersion));
	}

[benyoka]

I see, thanks.

[benyoka]

Probably we should use the immutable ObjectReader / ObjectWriter classes if we want to make this a general utility. ObjectMapper is said to be thread safe but uses internal locking that can be a perf bottleneck (according to StackOverflow).

[adoroszlai]

Done.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/369/
Test FAILed.
Test FAILured.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/380/
Test FAILed.
Test FAILured.