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

[AMBARI-22875] Replay of topology requests with mpacks #1591

Merged

Conversation

@adoroszlai
Copy link
Contributor

adoroszlai commented Jun 21, 2018

What changes were proposed in this pull request?

Ambari supports replay of cluster creation request in case the server is restarted before the deployment process begins. This feature does not yet work for mpacks. The goal of this change is to implement replay of topology requests based on mpacks.

In addition, a minor fix for blueprint export, which currently fails with NPE.

How was this patch tested?

Tested replay of deployment on live cluster:

  1. Start Ambari server, but not agent
  2. Submit blueprint, cluster creation request
  3. Restart Ambari server
  4. Start Ambari agent
21 Jun 2018 07:29:30,209  INFO [ambari-client-thread-25] BlueprintResourceProvider:550 - Creating Blueprint, name=blue
21 Jun 2018 07:29:30,386  INFO [ambari-client-thread-26] ClusterResourceProvider:524 - Creating Cluster 'TEST' based on blueprint 'blue'.
21 Jun 2018 07:29:56,396  INFO [main] AmbariServer:1006 - STARTUP_MESSAGE: Starting AmbariServer.java executable
21 Jun 2018 07:30:46,391  INFO [ambari-client-thread-24] TopologyManager:1035 - TopologyManager.replayRequests: no config with TOPOLOGY_RESOLVED found, adding cluster config request

Also tested replay of scale request:

  1. Submit scale request
  2. Restart Ambari server
  3. Start another Ambari agent
21 Jun 2018 09:31:10,781  INFO [ambari-client-thread-20] ClusterTopologyImpl:260 - ClusterTopologyImpl.addHostTopology: added host = c7402.ambari.apache.org to host group = node
21 Jun 2018 09:31:10,850  INFO [ambari-client-thread-20] TopologyManager:882 - TopologyManager.processRequest: not all required hosts have been matched, so adding LogicalRequest ID = 6 to outstanding requests
21 Jun 2018 09:32:55,763  INFO [main] AmbariServer:1006 - STARTUP_MESSAGE: Starting AmbariServer.java executable
21 Jun 2018 09:33:41,941  INFO [agent-register-processor-1] LogicalRequest:100 - LogicalRequest.offer: attempting to match a request to a request for a reserved host to hostname = c7402.ambari.apache.org
21 Jun 2018 09:33:41,943  INFO [agent-register-processor-1] TopologyManager:636 - TopologyManager.onHostRegistered: processing accepted host offer for reserved host = c7402.ambari.apache.org

Note that both test cases may fail due to AMBARI-23586, which is fixed on trunk, but hasn't been merged into branch-feature-AMBARI-14714 yet.

Tested export of cluster in blueprint format.

@adoroszlai adoroszlai self-assigned this Jun 21, 2018
@asfgit

This comment has been minimized.

Copy link

asfgit commented Jun 21, 2018

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

@adoroszlai adoroszlai requested review from rnettleton and benyoka Jun 21, 2018
@@ -389,7 +376,7 @@ void saveOrUpdateQuickLinksProfile(String quickLinksProfileJson) {
private void submitCredential(String clusterName, Credential credential) {

ResourceProvider provider =
ambariContext.getClusterController().ensureResourceProvider(Resource.Type.Credential);
AmbariContext.getClusterController().ensureResourceProvider(Resource.Type.Credential);

This comment has been minimized.

Copy link
@rnettleton

rnettleton Jun 21, 2018

Contributor

Would it be better here to use the method on AmbariContext to obtain this value from the "ambariContext" field in this class?

This comment has been minimized.

Copy link
@adoroszlai

adoroszlai Jun 27, 2018

Author Contributor

Access to static members is generally preferred via the class, not instances (eg. IDEA produces warnings for the latter).

…o AMBARI-22875_replay
@asfgit

This comment has been minimized.

Copy link

asfgit commented Jun 22, 2018

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

Copy link
Contributor

benyoka left a comment

LGTM in general, please answer/address the questions.

mpacks = ImmutableSet.<MpackInstance>builder().
addAll(blueprint.getMpacks()).
addAll(request.getMpacks()).build();

ambariContext.downloadMissingMpacks(mpacks);

This comment has been minimized.

Copy link
@benyoka

benyoka Jun 26, 2018

Contributor

I am a bit reluctant to put such side effects into constructors. I think TopologyManager.provisionCluster() was a better place for this.

This comment has been minimized.

Copy link
@adoroszlai

adoroszlai Jun 27, 2018

Author Contributor

I agree. But the download needs to be performed during replay, too, in case mpacks were deleted in the meantime. I'm not sure that's a scenario Ambari needs to support, we can defer that for now.

return SecurityConfiguration.NONE;
}
}

This comment has been minimized.

Copy link
@benyoka

benyoka Jun 26, 2018

Contributor

What is the purpose of the hard-coded overrides?

This comment has been minimized.

Copy link
@adoroszlai

adoroszlai Jun 27, 2018

Author Contributor

These are not overrides, but necessary implementations of interface methods. I'm not sure they are used at all during request replay.

Changed the hard-coded values to null.

@RunWith(PowerMockRunner.class)
@PrepareForTest({ExportBlueprintRequest.ExportedHostGroup.class})
@PrepareForTest({ InetAddress.class })
public class ExportBlueprintRequestTest {

This comment has been minimized.

Copy link
@benyoka

benyoka Jun 26, 2018

Contributor

According to powermock docs, when mocking system classes, the class that calls the system class should be prepared.

https://github.com/powermock/powermock/wiki/Mock-System

This comment has been minimized.

Copy link
@adoroszlai

adoroszlai Jun 27, 2018

Author Contributor

Good point.

@asfgit

This comment has been minimized.

Copy link

asfgit commented Jun 27, 2018

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

@adoroszlai adoroszlai merged commit 2d50429 into apache:branch-feature-AMBARI-14714 Jun 27, 2018
1 check failed
1 check failed
Jenkins Build FAILURE 5263 tests run, 113 skipped, 82 failed.
Details
@adoroszlai adoroszlai deleted the adoroszlai:AMBARI-22875_replay branch Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.