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.

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

Fluentd for Audit Logging #2853

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Fluentd for Audit Logging #2853

merged 1 commit into from
Nov 5, 2021

Conversation

qiyueyao
Copy link
Contributor

@qiyueyao qiyueyao commented Sep 28, 2021

This is to complete this issue request Add documentation support for Fluentd integration with Audit Logging

  • Added a cookbook for using Fluentd with Antrea
  • Changed go get to go install in file hack/update-toc.sh and hack/update-spelling.sh
  • Changed set jvm guide link to 7.8 version to avoid 404 status

After investigating with top, the CPU consumption percentage does not have significant change after deploying Fluentd.

@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2021

Codecov Report

Merging #2853 (d5152b0) into main (0feed23) will increase coverage by 19.45%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2853       +/-   ##
===========================================
+ Coverage   40.23%   59.69%   +19.45%     
===========================================
  Files         166      289      +123     
  Lines       20693    24540     +3847     
===========================================
+ Hits         8326    14648     +6322     
+ Misses      11558     8322     -3236     
- Partials      809     1570      +761     
Flag Coverage Δ
kind-e2e-tests 46.25% <ø> (?)
unit-tests 40.20% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/apiserver/storage/ram/store.go 78.35% <0.00%> (ø)
pkg/apiserver/handlers/webhook/convert_crd.go 2.56% <0.00%> (ø)
pkg/apiserver/handlers/webhook/mutation_labels.go 24.71% <0.00%> (ø)
pkg/ipfix/ipfix_collector.go 81.81% <0.00%> (ø)
...agent/apiserver/handlers/appliedtogroup/handler.go 0.00% <0.00%> (ø)
pkg/legacyapis/controlplane/register.go 88.23% <0.00%> (ø)
...lientset/versioned/typed/security/v1alpha1/tier.go 27.39% <0.00%> (ø)
pkg/legacyapis/stats/v1alpha1/register.go 90.90% <0.00%> (ø)
pkg/signals/signals.go 100.00% <0.00%> (ø)
...versions/security/v1alpha1/clusternetworkpolicy.go 64.28% <0.00%> (ø)
... and 227 more

@abhiraut abhiraut self-requested a review September 28, 2021 22:57
@abhiraut abhiraut added the area/network-policy Issues or PRs related to network policies. label Sep 28, 2021
docs/antrea-network-policy.md Outdated Show resolved Hide resolved
docs/antrea-network-policy.md Outdated Show resolved Hide resolved
docs/antrea-network-policy.md Outdated Show resolved Hide resolved
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

I think https://github.com/antrea-io/antrea/tree/main/docs/cookbooks/ may be a better location for this stuff (with a reference in docs/antrea-network-policy.md). @abhiraut?

@abhiraut
Copy link
Contributor

abhiraut commented Oct 1, 2021

I think https://github.com/antrea-io/antrea/tree/main/docs/cookbooks/ may be a better location for this stuff (with a reference in docs/antrea-network-policy.md). @abhiraut?

that's a good idea.. probably cookbook/fluentd-integration or something like that?

@qiyueyao
Copy link
Contributor Author

qiyueyao commented Oct 1, 2021

I think https://github.com/antrea-io/antrea/tree/main/docs/cookbooks/ may be a better location for this stuff (with a reference in docs/antrea-network-policy.md). @abhiraut?

that's a good idea.. probably cookbook/fluentd-integration or something like that?

Sounds good, shall I move the YAML files from build/YAMLs/audit-logging to this folder as well?

@antoninbas
Copy link
Contributor

maybe cookbooks/netpol-logs-fluentd?

yes, please move the YAML manifests there as well

@abhiraut
Copy link
Contributor

abhiraut commented Oct 1, 2021

I think https://github.com/antrea-io/antrea/tree/main/docs/cookbooks/ may be a better location for this stuff (with a reference in docs/antrea-network-policy.md). @abhiraut?

that's a good idea.. probably cookbook/fluentd-integration or something like that?

Sounds good, shall I move the YAML files from build/YAMLs/audit-logging to this folder as well?

you can take a look at the cookbooks/multus section to see how it is organized

@abhiraut
Copy link
Contributor

abhiraut commented Oct 1, 2021

maybe cookbooks/netpol-logs-fluentd?

yes, please move the YAML manifests there as well

do we think we could extend fluentd integration beyond netpol in future?

@qiyueyao
Copy link
Contributor Author

qiyueyao commented Oct 1, 2021

maybe cookbooks/netpol-logs-fluentd?
yes, please move the YAML manifests there as well

do we think we could extend fluentd integration beyond netpol in future?

Yes, it could tail any file as long as we specify the correct parse in kubernetes.conf.

@qiyueyao qiyueyao force-pushed the fluentd branch 2 times, most recently from 3d55847 to 0b06e8c Compare October 5, 2021 00:30
@qiyueyao qiyueyao marked this pull request as ready for review October 5, 2021 00:38
docs/antrea-network-policy.md Outdated Show resolved Hide resolved
docs/antrea-network-policy.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
abhiraut
abhiraut previously approved these changes Oct 14, 2021
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

Could you use more consistent line wrapping (~80 chars) in the README?

docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/build/fluentd.yml Outdated Show resolved Hide resolved
@github-actions github-actions bot requested a review from GraysonWu October 20, 2021 20:34
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
@qiyueyao
Copy link
Contributor Author

Screen Shot 2021-10-20 at 5 55 50 PM
@zyiou This link currently shows 404 that causes markdown test to fail, shall I change it to current heap size setting or version 7.8 heap size setting? Thanks!

@zyiou
Copy link
Contributor

zyiou commented Oct 21, 2021

Screen Shot 2021-10-20 at 5 55 50 PM @zyiou This link currently shows 404 that causes markdown test to fail, shall I change it to current heap size setting or version 7.8 heap size setting? Thanks!

Please change it to latter one https://www.elastic.co/guide/en/elasticsearch/reference/7.8/heap-size.html. Thanks for updating this link.

@antoninbas
Copy link
Contributor

@zyiou @qiyueyao Quan already changed the link in #2919 to avoid CI failures. Feel free to update the link again in this PR if you think Quan didn't pick the most appropriate one.

@zyiou
Copy link
Contributor

zyiou commented Oct 21, 2021

@zyiou @qiyueyao Quan already changed the link in #2919 to avoid CI failures. Feel free to update the link again in this PR if you think Quan didn't pick the most appropriate one.

I think we should keep 7.8.0 version of the link since it is consistent with the images we are using. We can keep this change in the PR. Thanks!

docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
hack/update-toc.sh Outdated Show resolved Hide resolved
@qiyueyao qiyueyao force-pushed the fluentd branch 2 times, most recently from dbafb1a to 4b1507e Compare October 23, 2021 00:26
abhiraut
abhiraut previously approved these changes Oct 28, 2021
@abhiraut abhiraut added this to the Antrea v1.4 release milestone Oct 29, 2021
@tnqn
Copy link
Member

tnqn commented Nov 3, 2021

@qiyueyao @antoninbas @abhiraut do we plan to include it in v1.4?

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

I found some small issues

@tnqn this can be postponed to 1.5

docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/cookbooks/fluentd/README.md Outdated Show resolved Hide resolved
docs/antrea-network-policy.md Outdated Show resolved Hide resolved
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

There is still a lot of DeamonSet typos (instead of DaemonSet)

antoninbas
antoninbas previously approved these changes Nov 4, 2021
@antoninbas
Copy link
Contributor

/skip-all

@antoninbas
Copy link
Contributor

@qiyueyao the DCO check is failing. It may be a good opportunity to squash the commits since you need to sign them anyway.

@qiyueyao
Copy link
Contributor Author

qiyueyao commented Nov 4, 2021

@qiyueyao the DCO check is failing. It may be a good opportunity to squash the commits since you need to sign them anyway.

Thanks for the suggestion! Cleaned up the commits and checked sign offs.

Moved to cookbooks, added email alerting documentation.
Change elastic link in flow visibility doc, updated shell scripts to go install.

Signed-off-by: Qiyue Yao <[email protected]>
@antoninbas
Copy link
Contributor

/skip-all

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

@qiyueyao next time please squash as one commit only and edit the resulting commit message as needed

@antoninbas antoninbas merged commit ae67d55 into antrea-io:main Nov 5, 2021
@qiyueyao qiyueyao deleted the fluentd branch December 17, 2021 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/network-policy Issues or PRs related to network policies. review-manager-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants