Skip to content

Fix LogURIGenerator on Windows#7351

Merged
mxpv merged 1 commit intocontainerd:mainfrom
kzys:log-windows
Sep 22, 2022
Merged

Fix LogURIGenerator on Windows#7351
mxpv merged 1 commit intocontainerd:mainfrom
kzys:log-windows

Conversation

@kzys
Copy link
Member

@kzys kzys commented Aug 31, 2022

Checking / is not the right way to distinguish an absolute path in
Windows.

Fixes #5786.

Signed-off-by: Kazuyoshi Kato [email protected]

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kzys kzys force-pushed the log-windows branch 5 times, most recently from 780b111 to f17cdbc Compare August 31, 2022 21:09
@kzys kzys force-pushed the log-windows branch 2 times, most recently from 35ae525 to f73ad60 Compare September 1, 2022 21:51
@dcantah
Copy link
Member

dcantah commented Sep 2, 2022

Hmm, I'd thought binary logging worked on windows.. @anmaxvl any thoughts?

@anmaxvl
Copy link
Contributor

anmaxvl commented Sep 2, 2022

Hmm, I'd thought binary logging worked on windows.. @anmaxvl any thoughts?

I added a windows driver at some point (#4759), but I probably tested it with ctr which accepts log-uri directly.

@kzys kzys force-pushed the log-windows branch 2 times, most recently from 9a3356c to 1b4b6c3 Compare September 2, 2022 17:39
@kzys kzys force-pushed the log-windows branch 3 times, most recently from 19d3630 to 9e2e878 Compare September 16, 2022 20:33
@kzys kzys marked this pull request as ready for review September 16, 2022 20:34
@kzys kzys force-pushed the log-windows branch 3 times, most recently from be7c7b2 to 450343e Compare September 17, 2022 00:14
Checking / is not the right way to distinguish an absolute path in
Windows.

Fixes containerd#5786.

Signed-off-by: Kazuyoshi Kato <[email protected]>
@kzys
Copy link
Member Author

kzys commented Sep 20, 2022

@jterry75 @dcantah @anmaxvl Can you take a look that again?

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.

Cannot find binary for binary container logging.

6 participants