Skip to content

Forward logs to different external system(s)#24029

Merged
mburke5678 merged 1 commit intoopenshift:masterfrom
mburke5678:logging-log-forward-ga
Sep 29, 2020
Merged

Forward logs to different external system(s)#24029
mburke5678 merged 1 commit intoopenshift:masterfrom
mburke5678:logging-log-forward-ga

Conversation

@mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented Jul 21, 2020

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 21, 2020
@mburke5678 mburke5678 added this to the Future Release milestone Jul 21, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 23, 2020
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this sentence needs finishing?

@mburke5678 mburke5678 force-pushed the logging-log-forward-ga branch from dc067c1 to 30eb7df Compare July 30, 2020 15:37
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 5, 2020
@mburke5678 mburke5678 force-pushed the logging-log-forward-ga branch from bb3c1c2 to 7ca6a28 Compare August 8, 2020 01:01
@mburke5678
Copy link
Contributor Author

Comment on lines 27 to 31

Choose a reason for hiding this comment

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

@mburke5678 could you please remove this output? In clusterlogforwarder, we have a default output named as default, if user want to set forward logs to default ES, just need to add - default to the outputRefs, no need to define an output. You can find the details in https://issues.redhat.com/browse/LOG-706.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this file.

Choose a reason for hiding this comment

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

Start from 4.5, we use ES v6.x, so I think maybe we should say you can optionally forward logs to an external Elasticsearch v6.x instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sichvoge What did we decide on this?

Choose a reason for hiding this comment

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

apiVersion: logging.openshift.io/v1

Choose a reason for hiding this comment

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

@mburke5678 could you please update this part from line 57 to 88?
With the recent changes syslog supports TLS and both RFC 3164 & 5424.
For ref: https://github.com/openshift/cluster-logging-operator/blob/master/manifests/4.6/logging.openshift.io_clusterlogforwarders_crd.yaml#L152

Comment on lines 75 to 76

Choose a reason for hiding this comment

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

apiVersion: logging.openshift.io/v1
kind: ClusterLogForwarder

Choose a reason for hiding this comment

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

url: http://elasticsearch.example.com:9200

Choose a reason for hiding this comment

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

container-logs is not accurate, I was thinking we could use another name.

Comment on lines 17 to 18

Choose a reason for hiding this comment

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

apiVersion: logging.openshift.io/v1
kind: ClusterLogForwarder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Choose a reason for hiding this comment

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

url: http://elasticsearch.example.com:9200

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Comment on lines 50 to 51

Choose a reason for hiding this comment

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

apiVersion: logging.openshift.io/v1
kind: ClusterLogForwarder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Choose a reason for hiding this comment

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

url: http://elasticsearch.example.com:9200

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Choose a reason for hiding this comment

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

url: 'tcp://fluentdserver.example.com:24224'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Copy link

@QiaolingTang QiaolingTang Aug 21, 2020

Choose a reason for hiding this comment

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

url: http://elasticsearch.insecure.example.com.svc:9200

@mburke5678 out of curiosity, why do you give so many outputs here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Choose a reason for hiding this comment

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

Per my understanding, no matter what kind of authentication the forward use, if user want to enable TLS, then they must create a secret and specify it in the cluster log forwarder CR. @alanconway could you please help to confirm this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Comment on lines 328 to 329

Choose a reason for hiding this comment

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

apiVersion: logging.openshift.io/v1
kind: ClusterLogForwarder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this topic.

@mburke5678 mburke5678 force-pushed the logging-log-forward-ga branch from 2356ac4 to c35b5e6 Compare September 2, 2020 18:26
Copy link

@alanconway alanconway left a comment

Choose a reason for hiding this comment

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

Yep!

Copy link

@anpingli anpingli left a comment

Choose a reason for hiding this comment

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

/lgtm

[id="cluster-logging-collector-legacy_{context}"]
= Forwarding logs using the legacy configuration method

You can configure cluster logging to send logs to destinations outside of your {product-title} cluster instead of the default Elasticsearch log store using Fluentd or syslog by creating a configuration file and ConfigMap.
Copy link

Choose a reason for hiding this comment

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

@mburke5678 Yes, unmanaged is not required

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2020
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 14, 2020
@mburke5678 mburke5678 force-pushed the logging-log-forward-ga branch from 3a7357f to 1044e8c Compare September 24, 2020 20:16
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2020
@mburke5678 mburke5678 force-pushed the logging-log-forward-ga branch 4 times, most recently from b9cd829 to 366befc Compare September 25, 2020 15:49
@mburke5678
Copy link
Contributor Author

@mburke5678 mburke5678 force-pushed the logging-log-forward-ga branch from 366befc to 92f1113 Compare September 26, 2020 01:06
Copy link
Contributor

@sheriff-rh sheriff-rh left a comment

Choose a reason for hiding this comment

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

LGTM! Great work! 🎉 🌮 🥇

@sheriff-rh sheriff-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Sep 29, 2020
@mburke5678
Copy link
Contributor Author

I am merging this PR as #25229 depends on it. Any further comments can be tracked in a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.6 peer-review-done Signifies that the peer review team has reviewed this PR size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants