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

e2e: add NodeLogQuery tests for Windows nodes #122594

Merged
merged 1 commit into from Feb 6, 2024
Merged

e2e: add NodeLogQuery tests for Windows nodes #122594

merged 1 commit into from Feb 6, 2024

Conversation

aravindhp
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add e2e tests for the NodeLogQuery feature against Windows nodes

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 4, 2024
@k8s-ci-robot
Copy link
Contributor

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

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/test sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 4, 2024
@aravindhp
Copy link
Contributor Author

/sig windows

@k8s-ci-robot k8s-ci-robot added the sig/windows Categorizes an issue or PR as relevant to SIG Windows. label Jan 4, 2024
@aravindhp
Copy link
Contributor Author

/cc @marosset @jsturtevant

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 6, 2024
aravindhp
aravindhp commented Jan 6, 2024
View reviewed changes
Copy link
Contributor Author

@aravindhp aravindhp left a comment

Choose a reason for hiding this comment

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

@jsturtevant @marosset any ideas on how to normalize the newlines across Windows and Linux for comparison purposes?

test/e2e/node/kubelet.go Show resolved Hide resolved
test/e2e/node/kubelet.go Outdated
}

func getWinEventCommandOnNode(nodeName string, providerName, args string) string {
output := commandOnNode(nodeName, "Get-WinEvent -FilterHashtable @{LogName='Application'; ProviderName='"+providerName+"'}"+args+" | Format-Table -AutoSize -Wrap")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This shows up as:

  Jan  5 16:15:51.164: INFO: ssh capi@capz-conf-2c5l6:22: stdout:    "\r\n\r\n   ProviderName: Microsoft-Windows-Security-SPP\r\n\r\nTimeCreated             Id LevelDisplayName Message                                                \r\n-----------             -- ---------------- -------                                                \r\n1/5/2024 11:36:45 PM 16384 Information      Successfully scheduled Software Protection service for \r\n                                            re-start at 2024-01-12T22:32:45Z. Reason: RulesEngine. \r\n1/5/2024 11:36:15 PM 16394 Information      Offline downlevel migration succeeded.                 \r\n1/5/2024 11:30:44 PM 16384 Information      Successfully scheduled Software Protection service for \r\n                                            re-start at 2024-01-12T22:32:44Z. Reason: RulesEngine. \r\n\r\n\r\n"

test/e2e/node/kubelet.go Outdated Show resolved Hide resolved
@bart0sh bart0sh added this to WIP in SIG Node PR Triage Jan 7, 2024
@kannon92
Copy link
Contributor

PR looks to be related to kubernetes/enhancements#2258.

Its unclear to me at what stage this feature is by the issue.

/triage accepted
/priority important-soon

We should get these e2e tests in this release as this feature is currently alpha (I think!).

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jan 15, 2024
@aravindhp aravindhp force-pushed the nlq-windows-e2e branch 2 times, most recently from 1d3eda2 to fa7e7f1 Compare January 15, 2024 22:49
@kannon92
Copy link
Contributor

Thank you @aravindhp. I was commented on it as I was traging the feature and decided the priority.

@aravindhp aravindhp mentioned this pull request Jan 15, 2024
Open
9 tasks
@aravindhp aravindhp marked this pull request as ready for review January 16, 2024 00:12
@aravindhp
Copy link
Contributor Author

/retest-required

1 similar comment
@aravindhp
Copy link
Contributor Author

/retest-required

@aravindhp
Copy link
Contributor Author

/retitle e2e: add NodeLogQuery tests for Windows nodes

@k8s-ci-robot k8s-ci-robot changed the title WIP: e2e: add NodeLogQuery tests for Windows nodes e2e: add NodeLogQuery tests for Windows nodes Jan 16, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2024
@aravindhp
Copy link
Contributor Author

@marosset @jsturtevant please review

@aravindhp
Copy link
Contributor Author

/test pull-kubernetes-verify-lint pull-kubernetes-linter-hints

@jsturtevant
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 17, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 4d8ee212e0b9166e787a2c7613ee6de870981e7b

@tzneal tzneal moved this from Triage to PRs - Needs Reviewer in SIG Node CI/Test Board Jan 17, 2024
@bart0sh bart0sh moved this from WIP to Needs Approver in SIG Node PR Triage Jan 25, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aravindhp, mrunalp

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 6, 2024
@k8s-ci-robot k8s-ci-robot merged commit 2ec3ef6 into kubernetes:master Feb 6, 2024
14 of 15 checks passed
SIG Node CI/Test Board automation moved this from PRs - Needs Reviewer to Done Feb 6, 2024
SIG Node PR Triage automation moved this from Needs Approver to Done Feb 6, 2024
@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Feb 6, 2024
@rphillips rphillips mentioned this pull request Feb 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrunalp mrunalp mrunalp approved these changes

@haircommander haircommander Awaiting requested review from haircommander

@odinuge odinuge Awaiting requested review from odinuge

@jsturtevant jsturtevant Awaiting requested review from jsturtevant

@marosset marosset Awaiting requested review from marosset

@bart0sh bart0sh Awaiting requested review from bart0sh

@feiskyer feiskyer Awaiting requested review from feiskyer

Assignees

@jsturtevant jsturtevant

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node CI/Test Board
Done
SIG Node PR Triage
Done
[sig-windows] PR tracking
Status: Done
Milestone
v1.30
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@aravindhp @k8s-ci-robot @kannon92 @jsturtevant @mrunalp