Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

METRON-2322 Add Ambari connection check to upgrade_helper script #1566

Closed
wants to merge 5 commits into from

Conversation

mmiklavc
Copy link
Contributor

@mmiklavc mmiklavc commented Nov 19, 2019

Contributor Comments

https://issues.apache.org/jira/browse/METRON-2322

Note: I also added a note about this script and the original Jira to the Upgrading.md file.

Pull Request Checklist

For all changes:

  • Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
  • Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
  • Has your PR been rebased against the latest commit within the target branch (typically master)?

For code changes:

  • Have you included steps to reproduce the behavior or problem that is being changed or addressed?

  • Have you included steps or a guide to how the change may be verified and tested manually?

  • Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:

    mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh 
    
  • n/a Have you written or updated unit tests and or integration tests to verify your changes?

  • n/a If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

  • Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via site-book/target/site/index.html:

    cd site-book
    mvn site
    
  • n/a Have you ensured that any documentation diagrams have been updated, along with their source files, using draw.io? See Metron Development Guidelines for instructions.

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.

@mmiklavc
Copy link
Contributor Author

mmiklavc commented Nov 19, 2019

Test Plan

I verified this in full dev and also ran shellcheck.

  • Follow the readme steps for performing a backup in the upgrade helper in metron-platform/metron-common. The important variation on the happy path is to test an incorrect username/password and an incorrect cluster name. This should output an error message on the console Unable to get cluster detail from Ambari. Check your username, password, and cluster name. Skipping.

done
echo Done backing up Ambari config...
echo Checking connection...
ret_status=$(curl -i -u "$username":"$password" -H "X-Requested-By: ambari" -X GET http://$ambari_address/api/v1/clusters/$cluster_name | head -n 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

@mmiklavc I get curl: (23) Failed writing body (3631 != 15905) error constantly. Looks like the piped program head is closing the read pipe before the curl completely writes .

[root@xxx-2 bin]# ./upgrade_helper.sh backup ctr-e141-1563959304486-90868-01-000002.hwx.site:8080 admin admin hcp201-solr4-build6 Backing up Ambari config... Checking connection... % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 32297 0 32297 0 0 131k 0 --:--:-- --:--:-- --:--:-- 131k curl: (23) Failed writing body (3631 != 15905) Unable to get cluster detail from Ambari. Check your username, password, and cluster name. Skipping. Skipping Metron config backup - Metron not found on this host

I ran the curl command with -s, --silent Silent mode. Don't output anything it works fine

[root@xxx-2 bin]# curl -s -i -u "admin":"admin" -H "X-Requested-By: ambari" -X GET http://ctr-e141-1563959304486-90868-01-000002.hwx.site:8080/api/v1/clusters/hcp201-solr4-build6 | head -n 1 HTTP/1.1 200 OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, good find. When I ran this locally I did not run into this problem. I will add that change.

echo Done backing up Ambari config...
echo Checking connection...
ret_status=$(curl -i -u "$username":"$password" -H "X-Requested-By: ambari" -X GET http://$ambari_address/api/v1/clusters/$cluster_name | head -n 1)
if [ "HTTP/1.1 200 OK" == "$ret_status" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

This check fails for me as the ret_status has trailing white space, an extra line of code before this worked for me
ret_status ="$(echo -e "${ret_status}" | sed -e 's/[[:space:]]*$//')"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, did not hit this locally. I will add the trim logic.

Copy link
Contributor

@MohanDV MohanDV left a comment

Choose a reason for hiding this comment

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

@mmiklavc I verified it on multinode secure cluster.

@mmiklavc
Copy link
Contributor Author

Ok @MohanDV give that a shot. I tweaked it a bit, but should still accomplish the same goal. Are you running that command on Centos 7? For Centos 6 I was not able to reproduce the issue, incidentally.

@MohanDV
Copy link
Contributor

MohanDV commented Nov 25, 2019

Ok @MohanDV give that a shot. I tweaked it a bit, but should still accomplish the same goal. Are you running that command on Centos 7? For Centos 6 I was not able to reproduce the issue, incidentally.

@mmiklavc Yes, I ran the script on centos 7.

@MohanDV
Copy link
Contributor

MohanDV commented Nov 25, 2019

+1 by verification. @mmiklavc works perfectly !! Thank you

@mmiklavc
Copy link
Contributor Author

Had to resolve a conflict with master in the upgrading.md doc. Letting Travis re-run before I merge.

@asfgit asfgit closed this in b8ea1e2 Dec 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants