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.

Sign up for GitHub

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

Jump to bottom

add back mux_client config that was removed #4554

Merged
merged 1 commit into from Jul 5, 2017
Merged

add back mux_client config that was removed #4554

merged 1 commit into from Jul 5, 2017

Conversation

richm
Copy link
Contributor

@richm richm commented Jun 23, 2017

not sure how this got removed, during the rebase of the logging
roles perhaps?
@ewolinetz @jcantrill PTAL

@richm
Copy link
Contributor Author

richm commented Jun 23, 2017

This is a test blocker - we need to merge this ASAP and build a new openshift-ansible RPM package.

portante
portante approved these changes Jun 23, 2017
View reviewed changes
roles/openshift_logging_fluentd/templates/fluentd.j2 Outdated
@@ -101,6 +106,8 @@ spec:
value: "{{ openshift_logging_fluentd_cpu_limit }}"
- name: "FLUENTD_MEMORY_LIMIT"
value: "{{ openshift_logging_fluentd_memory_limit }}"
- name: "USE_MUX_CLIENT"
value: "{{openshift_logging_use_mux_client| default('false')}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor whitespace missing before |.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@richm richm force-pushed the add-back-mux-client-config branch from ee5478d to f152ffe Compare June 23, 2017 16:45
@ewolinetz
Copy link
Contributor

@richm is this actually required in this role? Doesn't this logic belong in the openshift_logging_mux role?

@richm
Copy link
Contributor Author

richm commented Jun 23, 2017

is this actually required in this role? Doesn't this logic belong in the openshift_logging_mux role?

It is required in the fluentd role in order to configure fluentd to be a client of mux.

@richm
Copy link
Contributor Author

richm commented Jun 23, 2017

When fluentd is in mux_client mode, it means fluentd will read from the files/journal, and send the logs directly to mux using secure_forward, skipping any further filtering or processing (most notably, k8s meta filtering).

@ewolinetz
Copy link
Contributor

aos-ci-test

@ewolinetz
Copy link
Contributor

@stevekuznetsov do any of our CI tests test logging -- I cant remember.
cc @sdodson

@richm
Copy link
Contributor Author

richm commented Jun 23, 2017

[test_logging]

@richm
Copy link
Contributor Author

richm commented Jun 23, 2017

not sure if I have permission to use test_logging

@stevekuznetsov
Copy link
Contributor

[test_logging] was just for when we didn't make the job blocking for merges -- it's part of the suite now when you hit [test] for everyone

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for f152ffe (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for f152ffe (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for f152ffe (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for f152ffe (logs)

@sdodson
Copy link
Member

sdodson commented Jun 25, 2017

[merge][severity:blocker]

@richm richm force-pushed the add-back-mux-client-config branch from f152ffe to 1bdc116 Compare June 26, 2017 21:19
@sdodson
Copy link
Member

sdodson commented Jun 27, 2017

[merge]

@openshift-bot
Copy link

Evaluated for openshift ansible merge up to 1bdc116

@openshift-bot
Copy link

continuous-integration/openshift-jenkins/merge FAILURE (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/644/) (Base Commit: df511d5) (PR Branch Commit: 1bdc116) (Extended Tests: blocker)

@richm richm force-pushed the add-back-mux-client-config branch from 1bdc116 to 3b94e4e Compare June 27, 2017 14:25
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2017
@richm
add back mux_client config that was removed
e732ba8 
not sure how this got removed, during the rebase of the logging
roles perhaps?
Also ensure that the value used in the fluentd daemonset env var is
lower cased to work with the fluentd/run.sh script.
@richm richm force-pushed the add-back-mux-client-config branch from 3b94e4e to e732ba8 Compare July 5, 2017 13:50
@openshift-bot
Copy link

Evaluated for openshift ansible test up to e732ba8

@openshift-bot
Copy link

OpenShift Ansible Action Required: Please contact #openshift-ansible to have this pull request manually reviewed and tested

@ashcrow
Copy link
Member

ashcrow commented Jul 5, 2017

aos-ci-test

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 5, 2017
@openshift-bot
Copy link

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible/319/) (Base Commit: b7104cf) (PR Branch Commit: e732ba8)

@sdodson
Copy link
Member

sdodson commented Jul 5, 2017

If the aos-ci-jenkins jobs come back green ping me and I'll manually merge.

@ashcrow
Copy link
Member

ashcrow commented Jul 5, 2017 

INFO: Found status 'continuous-integration/travis-ci/pr' with state 'success'.
[ERROR] Status 'aos-ci-jenkins/OS_3.6_NOT_containerized' is required but was not found on this pull request.
[ERROR] Status 'aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests' is required but was not found on this pull request.
[ERROR] Status 'aos-ci-jenkins/OS_3.6_containerized' is required but was not found on this pull request.
[ERROR] Status 'aos-ci-jenkins/OS_3.6_containerized_e2e_tests' is required but was not found on this pull request.
[ERROR] Status 'aos-ci-jenkins/OS_3.5_NOT_containerized' is required but was not found on this pull request.
[ERROR] Status 'aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests' is required but was not found on this pull request.
[ERROR] Status 'aos-ci-jenkins/OS_3.5_containerized' is required but was not found on this pull request.
[ERROR] Status 'aos-ci-jenkins/OS_3.5_containerized_e2e_tests' is required but was not found on this pull request.

@ashcrow
Copy link
Member

ashcrow commented Jul 5, 2017

bot, retest this please

@ashcrow
Copy link
Member

ashcrow commented Jul 5, 2017

I think the openshift bot is confused ...

@openshift openshift deleted a comment from ashcrow Jul 5, 2017
@sdodson
Copy link
Member

sdodson commented Jul 5, 2017

It's still in the queue, things are just getting backed up.

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for e732ba8 (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for e732ba8 (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for e732ba8 (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for e732ba8 (logs)

@sdodson sdodson merged commit 01f91df into openshift:master Jul 5, 2017
@sdodson
Copy link
Member

sdodson commented Jul 5, 2017

All tests green, only the ordering of the jobs was off so merged it manually.

@richm richm deleted the add-back-mux-client-config branch July 5, 2017 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@portante portante portante approved these changes

@ewolinetz ewolinetz ewolinetz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@richm @ewolinetz @stevekuznetsov @openshift-bot @sdodson @ashcrow @portante