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.

Sign up for GitHub

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

Bug 1862643: UPSTREAM: 96120: kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint #383

Merged
merged 1 commit into from
Nov 11, 2020
Merged

Bug 1862643: UPSTREAM: 96120: kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint #383

merged 1 commit into from
Nov 11, 2020

Conversation

LorbusChris
Copy link
Member

@LorbusChris LorbusChris commented Oct 1, 2020 •
edited
Loading

Provide an administrator a streaming view of event logs on Windows
machines without them having to implement a client side reader.

The kubelet API for querying the Linux journal is re-used for invoking
the Get-WinEvent cmdlet in a PowerShell.
Parameters that have no functional equivalence in Get-WinEvent are
ignored when assembling the command.

Only available to cluster admins.

What type of PR is this?

/kind feature

What this PR does / why we need it:
Enable event log collection on Windows worker nodes

kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint

Upstream patch reference: kubernetes#96120

@openshift-ci-robot openshift-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. labels Oct 1, 2020
@openshift-ci-robot
Copy link

@LorbusChris: This pull request references Bugzilla bug 1862643, which is invalid:

  • expected the bug to target the "4.6.0" release, but it targets "4.7.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

WIP: Bug 1862643: UPSTREAM: : kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Oct 1, 2020
@LorbusChris LorbusChris force-pushed the windows-event-logs branch 8 times, most recently from f5d7db5 to 84d29db Compare October 6, 2020 13:57
@LorbusChris
Copy link
Member Author

LorbusChris commented Oct 6, 2020 •
edited
Loading

Putting // +build !windows in pkg/kubelet/kubelet_server_journal_linux.go doesn't seem to work properly (https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_kubernetes/383/pull-ci-openshift-kubernetes-master-verify/1313465989928587264). I've create a stub file and function for darwin now.

@LorbusChris LorbusChris changed the title WIP: Bug 1862643: UPSTREAM: <carry>: kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint UPSTREAM: <carry>: kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint Oct 8, 2020
@openshift-ci-robot openshift-ci-robot removed bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Oct 8, 2020
@openshift-ci-robot
Copy link

@LorbusChris: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

UPSTREAM: : kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@LorbusChris
Copy link
Member Author

/retest

@LorbusChris
Copy link
Member Author

LorbusChris commented Oct 8, 2020 •
edited
Loading

I've verified this works:

$ oc adm node-logs -l kubernetes.io/os=windows --path=journal -u docker -u Microsoft-Windows-LoadPerf --since "2020-10-08 15:00:07" --until "2020-10-08 15:06:00" --tail 6


   ProviderName: docker

TimeCreated          Id LevelDisplayName Message                             
-----------          -- ---------------- -------                             
10/8/2020 3:00:13 PM  1 Information      Daemon has completed initialization 
10/8/2020 3:00:13 PM  1 Information      API listen on //./pipe/docker_engine


   ProviderName: Microsoft-Windows-LoadPerf

TimeCreated            Id LevelDisplayName Message                                                                     
-----------            -- ---------------- -------                                                                     
10/8/2020 3:04:24 PM 1001 Information      Performance counters for the WmiApRpl (WmiApRpl) service were removed       
                                           successfully. The Record Data contains the new values of the system Last    
                                           Counter and Last Help registry entries.                                     
10/8/2020 3:04:24 PM 1000 Information      Performance counters for the WmiApRpl (WmiApRpl) service were loaded        
                                           successfully. The Record Data in the data section contains the new index    
                                           values assigned to this service.                                            
10/8/2020 3:05:07 PM 1001 Information      Performance counters for the WmiApRpl (WmiApRpl) service were removed       
                                           successfully. The Record Data contains the new values of the system Last    
                                           Counter and Last Help registry entries.                                     
10/8/2020 3:05:07 PM 1000 Information      Performance counters for the WmiApRpl (WmiApRpl) service were loaded        
                                           successfully. The Record Data in the data section contains the new index    
                                           values assigned to this service.

There are a few gotchas:

  • --since and --until only support absolute times, not relative ones.
  • the filtering done with --grep happens after getting the specified max number of events from the log with --tail, i.e. one may end up with a shorter tail than expected in that case
  • --format is not supported
  • --case-sensitive is not supported
  • only --boot 0 is supported really, but if boot != 0, the command will return true and not return any logs. This is due to the fact that the command is invoked twice if the boot flag is unspecified, once for boot=0 and once for boot=-1. I wanted to make it succeed and not error in that case.

PTAL @aravindhp

@openshift-ci-robot
Copy link

@LorbusChris: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/k8s-e2e-gcp 84d29dbcd1e2bd86546a6839d52a9590f0e069d6 link /test k8s-e2e-gcp

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@sjenning
Copy link

We aren't going to carry something like this. This needs to go upstream. Unless there is some reason of which I am not aware.

@sjenning
Copy link

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 21, 2020
@LorbusChris
Copy link
Member Author

@sjenning this PR only extends an already downstream-only feature/API to make it usable on Windows nodes. pkg/kubelet/kubelet_server_journal.go does not exist upstream at all.

I guess upstreaming it would be good, but then 1f71fb7 also has to be upstreamed.

@LorbusChris
UPSTREAM: 96120: kubelet: Expose a simple Get-WinEvent shim on the ku…
1954664 
…belet logs endpoint

Provide an administrator a streaming view of event logs on Windows
machines without them having to implement a client side reader.

The kubelet API for querying the Linux journal is re-used for invoking
the Get-WinEvent cmdlet in a PowerShell.
Parameters that have no functional equivalence in Get-WinEvent are
ignored when assembling the command.

Only available to cluster admins.
@openshift-ci-robot
Copy link

@LorbusChris: This pull request references Bugzilla bug 1862643, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1862643: UPSTREAM: 96120: kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@derekwaynecarr
Copy link
Member

derekwaynecarr commented Nov 10, 2020 •
edited
Loading

@aravindhp why is this required for windows? a change like this upstream really requires a kep.

@sttts
Copy link

sttts commented Nov 10, 2020

The upstream PR won't make it into 1.21, and probably needs a KEP according to @derekwaynecarr.

Approving as a temporary carry.

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 10, 2020
@sjenning
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LorbusChris, sjenning, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@LorbusChris
Copy link
Member Author

@derekwaynecarr we need a way to get the Container Runtime logs (Docker at this time) without SSH'into the Windows machine. Docker on Windows always logs to the WinEvent log - it's not configurable.

Please note that the client side to make this feature usable should also be upstreamed to kubectl:
openshift/origin@d9dc689

@LorbusChris
Copy link
Member Author

/cherry-pick release-4.6

@openshift-cherrypick-robot
Copy link

@LorbusChris: once the present PR merges, I will cherry-pick it on top of release-4.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@LorbusChris
Copy link
Member Author

/retest

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

8 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 5fc3f4d into openshift:master Nov 11, 2020
@openshift-ci-robot
Copy link

@LorbusChris: All pull requests linked via external trackers have merged:

Bugzilla bug 1862643 has been moved to the MODIFIED state.

In response to this:

Bug 1862643: UPSTREAM: 96120: kubelet: Expose a simple Get-WinEvent shim on the kubelet logs endpoint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot
Copy link

@LorbusChris: new pull request created: #446

In response to this:

/cherry-pick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 11, 2020
Merged
LorbusChris added a commit to LorbusChris/must-gather that referenced this pull request Nov 30, 2020
@LorbusChris
Collect docker logs from WinEvent log on Windows machines
6a7ab47 
Use the Get-WinEvent shim on kubelet logs endpoint introduced in
openshift/kubernetes#383 to collect docker
runtime logs from Windows machines.
@LorbusChris LorbusChris mentioned this pull request Nov 30, 2020
Merged
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/must-gather that referenced this pull request Dec 4, 2020
@LorbusChris
Collect docker logs from WinEvent log on Windows machines
91761ad 
Use the Get-WinEvent shim on kubelet logs endpoint introduced in
openshift/kubernetes#383 to collect docker
runtime logs from Windows machines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@sjenning sjenning Awaiting requested review from sjenning

Assignees

@sjenning sjenning

@sttts sttts

@smarterclayton smarterclayton

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.