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

fix zap logger rotation issues #2344

Merged
merged 2 commits into from
Nov 8, 2023
Merged

fix zap logger rotation issues #2344

merged 2 commits into from
Nov 8, 2023

Conversation

paulyufan2
Copy link
Contributor

@paulyufan2 paulyufan2 commented Nov 2, 2023

Reason for Change:

This PR is to fix log rotation issue that is crashed after windows nodes are restarted.

Issue Fixed:

Requirements:

Notes:

@paulyufan2 paulyufan2 marked this pull request as ready for review November 2, 2023 22:22
@paulyufan2 paulyufan2 requested a review from a team as a code owner November 2, 2023 22:22
@paulyufan2 paulyufan2 requested a review from behzad-mir November 2, 2023 22:22
@paulyufan2 paulyufan2 added the cni Related to CNI. label Nov 3, 2023
@paulyufan2 paulyufan2 force-pushed the zaplogissuefix branch 2 times, most recently from 8746536 to e535a9a Compare November 3, 2023 18:45
@paulyufan2 paulyufan2 enabled auto-merge (squash) November 3, 2023 19:57
@vipul-21
Copy link
Contributor

vipul-21 commented Nov 3, 2023

Can we add more to the description about what the rotation issue is ?

@@ -23,17 +22,6 @@ func main() {
var config common.PluginConfig
config.Version = version

logDirectory := "" // Sets the current location as log directory
Copy link
Member

Choose a reason for hiding this comment

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

did we validate ipam logs moved to zap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes I did with this PR: #2299
Verified azure-ipam.log

Copy link
Contributor

@behzad-mir behzad-mir left a comment

Choose a reason for hiding this comment

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

plz make ssure we don't have any "github.com/Azure/azure-container-networking/log in the CNI and network related packages

@paulyufan2 paulyufan2 force-pushed the zaplogissuefix branch 3 times, most recently from a3c2692 to ffb25af Compare November 7, 2023 22:28
@paulyufan2 paulyufan2 merged commit 6806d9a into master Nov 8, 2023
@paulyufan2 paulyufan2 deleted the zaplogissuefix branch November 8, 2023 05:01
matmerr pushed a commit that referenced this pull request Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cni Related to CNI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants