-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Adding pods/logs to manageiq role #4673
Conversation
cc @jcantrill @richm |
Can one of the admins verify this patch?
|
1 similar comment
Can one of the admins verify this patch?
|
aos-ci-test |
@@ -24,6 +24,12 @@ | |||
- apiGroups: | |||
- "" | |||
resources: | |||
- pods/logs |
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.
I think it's called pods/log
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.
Thanks
@jcantrill is this ready for merge? |
[merge] |
I found a problem - we didn't actually assigned that cluster-role correctly before. I guess something else enabled us to access pod/proxy anyway (maybe the cluster-reader role?) The tests are failing for:
which doesn't tell me much, I don't think it is connected to this PR. |
a9ecf43
to
7f8ef2e
Compare
@jcantrill @richm Can you please take another look? |
@enoodle best I can tell LGTM |
@jcantrill who should merge this? |
[merge][severity:blocker]
…On Tue, Jul 18, 2017 at 6:54 PM, Federico Simoncelli < ***@***.***> wrote:
@enoodle <https://github.com/enoodle> best I can tell LGTM
@jcantrill <https://github.com/jcantrill> who should merge this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4673 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEVnOCSNyetgjlcI_IwjXKbUlIn3Uadcks5sPTe9gaJpZM4ONVwx>
.
--
--
Jeff Cantrill
Senior Software Engineer, Red Hat Engineering
OpenShift Integration Services
Red Hat, Inc.
*Office*: 703-748-4420 | 866-546-8970 ext. 8162420
jcantril@redhat.com
http://www.redhat.com
|
[test] |
@jcantrill I am really not sure why this PR keeps failing, The logs failing dont seem to be connected to these changes. Can you please take a look? |
aos-ci-test |
7f8ef2e
to
3079d2b
Compare
@jcantrill I have rebased this on master, maybe this would help, can you initiate the test again? aos-ci-test |
@sdodson @jcantrill This seems to be passing tests now, can you see if it is possible to merge? |
aos-ci-test |
3079d2b
to
1d1c631
Compare
aos-ci-test |
[test] |
@jcantrill Can we try to merge again? |
aos-ci-test |
@sdodson maybe we can merge this now? I am not sure what this state means.. |
[merge] |
Evaluated for openshift ansible merge up to 1d1c631 |
OpenShift Ansible Action Required: Please contact #openshift-ansible to have this pull request manually reviewed and tested |
continuous-integration/openshift-jenkins/merge FAILURE (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/784/) (Base Commit: 1765ce2) (PR Branch Commit: 1d1c631) (Extended Tests: blocker) |
This will allow manageiq to read the logs from elasticsearch in the logging project. Also, adding role to correct user and as clusterrole
Jenkins doesn't like this PR. I will rebase again... |
1d1c631
to
497c01b
Compare
aos-ci-test |
This will allow manageiq to read the logs from elasticsearch in the logging project.
Based on the changes from fabric8io/openshift-elasticsearch-plugin#73