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

Jump to bottom

Move Antrea Windows log dir #3416

Merged
merged 1 commit into from Mar 30, 2022
Merged

Move Antrea Windows log dir #3416

merged 1 commit into from Mar 30, 2022

Conversation

GraysonWu
Copy link
Contributor

@GraysonWu GraysonWu commented Mar 8, 2022 •
edited

Fixes #2472

Move Antrea Windows log dir from C:\k\antrea\logs\ to
C:\var\log\antrea\

Signed-off-by: wgrayson wgrayson@vmware.com

@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2022 •
edited

Codecov Report

Merging #3416 (f6efa73) into main (b147eb2) will decrease coverage by 6.38%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3416      +/-   ##
==========================================
- Coverage   60.89%   54.51%   -6.39%     
==========================================
  Files         268      383     +115     
  Lines       26753    41787   +15034     
==========================================
+ Hits        16292    22781    +6489     
- Misses       8655    16667    +8012     
- Partials     1806     2339     +533
Flag Coverage Δ
integration-tests 35.84% <ø> (?)
kind-e2e-tests 54.07% <ø> (+6.39%) ⬆️
unit-tests 43.10% <ø> (+0.93%) ⬆️

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

Impacted Files Coverage Δ
...g/agent/apiserver/handlers/featuregates/handler.go 0.00% <0.00%> (-82.36%) ⬇️
pkg/agent/util/ethtool/ethtool_linux.go 0.00% <0.00%> (-70.00%) ⬇️
...kg/apiserver/registry/system/supportbundle/rest.go 20.45% <0.00%> (-54.55%) ⬇️
pkg/support/dump.go 8.19% <0.00%> (-49.19%) ⬇️
...egator/apiserver/handlers/recordmetrics/handler.go 0.00% <0.00%> (-44.45%) ⬇️
pkg/support/dump_others.go 0.00% <0.00%> (-44.00%) ⬇️
...g/agent/apiserver/handlers/addressgroup/handler.go 0.00% <0.00%> (-40.00%) ⬇️
...agent/apiserver/handlers/appliedtogroup/handler.go 0.00% <0.00%> (-40.00%) ⬇️
...gregator/apiserver/handlers/flowrecords/handler.go 0.00% <0.00%> (-40.00%) ⬇️
pkg/apiserver/handlers/loglevel/handler.go 0.00% <0.00%> (-38.47%) ⬇️
... and 222 more

@GraysonWu GraysonWu force-pushed the move-win-log-dir branch 2 times, most recently from b59c5fb to 64eefee Compare March 10, 2022 00:43
@GraysonWu GraysonWu changed the title [WIP]Move antrea log dir on Windows Node Move Antrea Windows log dir Mar 10, 2022
antoninbas
antoninbas previously approved these changes Mar 10, 2022
View reviewed changes
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.

LGTM

@antoninbas antoninbas added action/release-note Indicates a PR that should be included in release notes. area/OS/windows Issues or PRs related to the Windows operating system. labels Mar 10, 2022
@antoninbas antoninbas added this to the Antrea v1.6 release milestone Mar 10, 2022
@XinShuYang
Copy link
Contributor

/test-windows-all

wenyingd
wenyingd previously approved these changes Mar 10, 2022
View reviewed changes
Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

LGTM

@wenyingd
Copy link
Contributor

/test-windows-networkpolicy

@GraysonWu GraysonWu dismissed stale reviews from wenyingd and antoninbas via 7906b22 March 10, 2022 20:07
@wenyingd wenyingd removed this from the Antrea v1.6 release milestone Mar 23, 2022
@GraysonWu GraysonWu force-pushed the move-win-log-dir branch 2 times, most recently from 64d1f69 to bb3d556 Compare March 24, 2022 07:09
@GraysonWu
Move Win antrea log dir
f6efa73 
Move Antrea Windows log dir from `C:\k\antrea\logs\` to
`C:\var\log\antrea\`

Signed-off-by: wgrayson <wgrayson@vmware.com>
@GraysonWu
Copy link
Contributor Author

Change the log dir inside container from /host/var/log/antrea/ to /var/log/antrea.
Add mount /var/run/secrets/ and /var/log/antrea/ and remove mount the entire host.

@GraysonWu
Copy link
Contributor Author

/test-windows-all

@GraysonWu
Copy link
Contributor Author

/test-windows-networkpolicy

@GraysonWu
Copy link
Contributor Author

/test-windows-proxyall-e2e

@GraysonWu GraysonWu requested a review from wenyingd March 24, 2022 22:45
antoninbas
antoninbas reviewed Mar 29, 2022
View reviewed changes
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.

LGTM, but I will let @wenyingd give the final approval for this

build/yamls/antrea-windows.yml
Comment on lines +261 to +264
- hostPath:
path: /var/log/antrea/
type: DirectoryOrCreate
name: var-log-antrea
Copy link
Contributor

Choose a reason for hiding this comment

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

so this is just to ensure the directory is created, we don't actually use the mount?

Copy link
Contributor

@wenyingd wenyingd Mar 30, 2022 •
edited

Choose a reason for hiding this comment

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

For running Antrea Agent on Windows daemonset case, we don't access the log path inside the management containers. But we should ensure it exists on the host, and pass it to wins to run agent process on the host.

But if we want to access log files inside the management container manually, it should be accessible. I think that is why you also mount it in antrea-agent container, correct? @GraysonWu

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.

wenyingd
wenyingd approved these changes Mar 30, 2022
View reviewed changes
Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas
Copy link
Contributor

/skip-networkpolicy
/skip-e2e
/skip-conformance

@antoninbas antoninbas merged commit 12c4742 into antrea-io:main Mar 30, 2022
@antoninbas antoninbas deleted the move-win-log-dir branch March 30, 2022 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoninbas antoninbas antoninbas left review comments

@wenyingd wenyingd wenyingd approved these changes

@XinShuYang XinShuYang Awaiting requested review from XinShuYang

@tnqn tnqn Awaiting requested review from tnqn

Assignees
No one assigned
Labels
action/release-note Indicates a PR that should be included in release notes. area/OS/windows Issues or PRs related to the Windows operating system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Windows] Move agent log to C:\var\log\antrea
6 participants
@GraysonWu @codecov-commenter @XinShuYang @wenyingd @antoninbas @tnqn