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

Log rules #1827

Merged
merged 2 commits into from
Jan 10, 2025
Merged

Log rules #1827

merged 2 commits into from
Jan 10, 2025

Conversation

ctauchen
Copy link
Collaborator

@ctauchen ctauchen commented Dec 31, 2024

  • This PR adds documentation for Calico Log action policies.
  • Draft changes to log-rules.mdx

Continues work originally submitted in #1777 .

Product Version(s):

Issue:

Link to docs preview:

https://deploy-preview-1827--calico-docs-preview-next.netlify.app/calico/next/network-policy/policy-rules/log-rules

SME review:

  • An SME has approved this change.

DOCS review:

  • A member of the docs team has approved this change.

Additional information:

Merge checklist:

  • Deploy preview inspected wherever changes were made
  • Build completed successfully
  • Test have passed

@ctauchen ctauchen requested a review from a team as a code owner December 31, 2024 16:44
Copy link

netlify bot commented Dec 31, 2024

Deploy Preview for calico-docs-preview-next ready!

Name Link
🔨 Latest commit 84571a7
🔍 Latest deploy log https://app.netlify.com/sites/calico-docs-preview-next/deploys/67813fbac65ba20008d672d0
😎 Deploy Preview https://deploy-preview-1827--calico-docs-preview-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 31, 2024

Deploy Preview succeeded!

Built without sensitive environment variables

Name Link
🔨 Latest commit 84571a7
🔍 Latest deploy log https://app.netlify.com/sites/tigera/deploys/67813fba7f5c16000850f977
😎 Deploy Preview https://deploy-preview-1827--tigera.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 27 (🟢 up 1 from production)
Accessibility: 90 (no change from production)
Best Practices: 83 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator Author

@ctauchen ctauchen left a comment

Choose a reason for hiding this comment

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

@frozenprocess To get this moving more quickly, I've gone ahead and moved your changes, with edits, into my own PR.

Have a look at my change, which mostly reflect things I suggested in my review of your PR. Add whatever comments you like to this PR.

kubectl patch kubecontrollersconfiguration default --type=merge --patch='{"spec": {"controllers": {"node": {"hostEndpoint": {"autoCreate": "Enabled"}}}}}'
```

1. Create a log rule for testing that includes host endpoints.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@frozenprocess From your PR, it wasn't clear to me whether merely creating the host endpoints was sufficient. I would expect that the next step is to create the policy with the log rule and ensure that it's got the right selector. It's better to be explicit about how, and in what order, this process should go.

Can you provide steps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Creating hostendpoints without log rules doesn't allow you to see the traffic. That is why in the PR hostendpoint is the last step after log rule creation.

  1. create a global log rule
  2. enable host endpoint.

1. Create a log rule for testing that includes host endpoints.

1. When you have completed testing, remove the log policies.
Leaving them in place can significantly affect cluster performance.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This proviso was attached to this section on host endpoints. But is this general workflow something that should apply to all log rules?

Are there cases where we want to have log rules set continuously? Or are they only to be used as part of a workflow that goes: 1) create log rule 2) see what happens, make fixes 3) remove log rule when you're done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Log rules have a performance penalty, which is why it is recommended to add them based on specific scenarios or for troubleshooting particular problems. Once the issue is resolved, it is advisable to remove these rules. That being said, there are cases where users may need to use these rules continuously. For such cases, we recommend utilizing flowlogs.

---
description: Debug your policies with Log rules.
---
# Use Log action to debug policies
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll change this doc after we settle on the other instance.

@frozenprocess
Copy link
Contributor

Lgtm

@ctauchen
Copy link
Collaborator Author

ctauchen commented Jan 6, 2025

Lgtm

@frozenprocess Thanks. Before I make final edits and merge this, can you take a quick look at my other comments?

frozenprocess and others added 2 commits January 10, 2025 15:14
- Remove the not supported `log` from eBPF pages
- Add an explanation how and when `log` action should be used
- bpf log format svg
@ctauchen ctauchen changed the title [WIP] Log rules Log rules Jan 10, 2025
@ctauchen ctauchen merged commit 91f4334 into tigera:main Jan 10, 2025
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants