-
Notifications
You must be signed in to change notification settings - Fork 1.7k
AMBARI-26056: Add ranger kms support #3775
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
Conversation
|
@virajjasani Could you help review this pr? |
virajjasani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, skimmed through the changes. Unless there is any objection, can merge in 24 hr
|
@virajjasani CI/CD failures for all Ambari PRs, attributed to a recent merge that removed PhantomJS in PR #3760. In order to ensure the smooth execution of CI for future PRs, could you help check and considering a revert of this PR. Here is the link to the problematic PR: PR #3760 The detailed discussion on the failure reasons can be found here: PR #3751 - Comment |
|
@arshadmohammad are you fine with reverting #3760? |
|
The commit has been reverted 35fbf98 |
|
Let's re-trigger the build? |
|
@virajjasani |
|
@brahmareddybattula @arshadmohammad are you aware of this by any chance? |
|
@JiaLiangC I do see this in the configs |
|
@virajjasani thanks for your effort
For modifying the Jenkins script, there are two suggested approaches: Approach 1: Limit the Search ScopeSince stage('Parallel Unit Tests') {
parallel {
stage('Ambari WebUI Tests') {
steps {
sh 'lsb_release -a'
sh 'ls /usr/bin'
// Directly check possible installation locations
sh 'if [ -f /usr/bin/chromium-browser ]; then export CHROME_BIN=/usr/bin/chromium-browser; fi'
sh 'if [ -f /snap/bin/chromium ]; then export CHROME_BIN=/snap/bin/chromium; fi'
sh 'mvn -X -T 2C -am test -pl ambari-web,ambari-admin -Dmaven.artifact.threads=10 -Drat.skip'
}
}
// Other test stages...
}
}Approach 2: Using sudoIf you need to search the entire filesystem for sh 'sudo find / -name "chromium-browser" 2>/dev/null'Here, Security Tip: In production environments, avoid using |
|
Sure we can try these options. I wonder if we strictly need the jenkins build to pass for this PR though. Perhaps we can fix the build separately. For Ranger support, is there any additional testing you would like to do? |
conf versioning cant enable in bigtop stack
|
@virajjasani agree, the CI/CD can be fixed separately. This PR itself has passed all the unit tests, and the failure of the CI/CD is solely due to the chromium-browser. As for the support for Ranger KMS services, there are no additional tests needed, and it can be merged. |
|
Thanks @JiaLiangC |
|
@virajjasani Thanks for your patient review. |
|
@virajjasani |
|
Thanks @JiaLiangC, let me see if i can make changes |
What changes were proposed in this pull request?
Add ranger kms support to ambari
(Please fill in changes proposed in this fix)
How was this patch tested?
Manual test
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
Please review Ambari Contributing Guide before opening a pull request.