WINC-1953: Add OCP-89616 log rotation verification test to OTE

[rrasouli]

Summary

Port OCP-89616 log rotation test from OTP PR #30034 to OTE, replacing SSH with oc debug node per WINC-1931.

What the test does (OCP-89616):

1. Patches the WMCO OLM subscription to set log rotation env vars (SERVICES_LOG_FILE_SIZE=1M, SERVICES_LOG_FILE_AGE=168h, SERVICES_LOG_FLUSH_INTERVAL=5s)
2. Waits for WMCO to reconcile and Windows nodes to become Ready
3. On each Windows node, verifies:
   • kubelet service PathName contains kube-log-runner with -log-file and -log-file-size flags
   • kube-proxy service PathName contains kube-log-runner with -log-file flag
   • kubelet.log file exists at C:\var\log\kubelet\kubelet.log
   • Checks for rotated log files (kubelet-*.log)
4. Cleanup: removes env vars from subscription, waits for nodes to recover

Other changes:

• Add reusable helpers for future OTE tests (runDebugNodePS, waitWindowsNodesReady, getNodeNameFromIP, windowsDebugImage)
• Fix missing k8s.io/externaljwt replace directive in ote/go.mod

Related: WINC-1635, WINC-1953, OTP PR openshift/openshift-tests-private#30034

Test plan

• go build passes (GOWORK=off)
• gofmt clean
• Test count: 6 (5 existing + 1 new)
• aws-e2e-ote CI validates on real cluster

Summary by CodeRabbit

• New Features

  • Added support for configuring Windows service log rotation, including file size, retention age, and flush interval settings.
  • Added verification that kubelet and kube-proxy services use the configured log rotation settings and that kubelet logs are written to the expected location.

• Tests

  • Added end-to-end coverage for Windows node readiness, debug command execution, and log rotation behavior.

[openshift-ci-robot]

@rrasouli: This pull request references WINC-1953 which is a valid jira issue.

Details

In response to this:

Summary

• Port OCP-89616 log rotation test from OTP PR #30034 to OTE
• Replace all SSH access with oc debug node per WINC-1931
• Add reusable helpers for future OTE tests (runDebugNodePS, waitWindowsNodesReady, getNodeNameFromIP)
• Fix missing k8s.io/externaljwt replace directive in ote/go.mod

Test plan

• go build passes (GOWORK=off)
• gofmt clean
• Test count: 6 (5 existing + 1 new)
• aws-e2e-ote CI validates on real cluster

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rrasouli

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• ote/OWNERS [rrasouli]

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

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 69a95bb0-59d5-4e30-80b9-b00701530622

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a pinned k8s.io/externaljwt replacement. Extends Windows e2e utilities with node lookup, readiness polling, and PowerShell debug execution. Adds a Ginkgo test that enables WMCO log rotation settings, verifies kubelet and kube-proxy service configuration, checks the kubelet log file, reports rotated logs, and restores the subscription settings.

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 6 warnings)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	New e2e logging prints node names/IP-derived hostnames in By/e2e.Logf and error messages, exposing internal hostnames.	Avoid logging nodeName/InternalIP/hostnames; use opaque IDs or counts only, and prevent raw command failures from echoing node identifiers.
Go Best Practices & Build Tags	⚠️ Warning	New helper uses fmt.Errorf(...%v) instead of wrapping, and the cleanup patch in defer discards its error; both violate the error-handling rule.	Use %w in getKubeletVersionWithRetry, and capture/expect the cleanup patch error in the defer so subscription restore failures fail the test.
Windows Service Management	⚠️ Warning	PR only checks Win32_Service.PathName/logs; it does not validate priority, dependencies, descriptions, cleanup, reboot docs, or full SCM behavior.	Add assertions/docs for service order, dependencies, descriptions, cleanup, reboot requirements, and SCM interactions—or narrow the check to what the PR actually covers.
Test Structure And Quality	⚠️ Warning	The new It never hard-fails on missing rotated logs, uses a public mcr.microsoft.com debug image, and ignores subscription cleanup errors.	Make rotated-log presence an assertion, use a mirrored/release debug image, and check the cleanup patch error before restoring nodes.
Microshift Test Compatibility	⚠️ Warning	The new Ginkgo test patches the WMCO Subscription (OLM) and the file has no MicroShift skip/tag or IsMicroShift guard.	Add a [Skipped:MicroShift] or [apigroup:...] label, or guard the test with exutil.IsMicroShiftCluster()+g.Skip() before using Subscription.
Single Node Openshift (Sno) Test Compatibility	⚠️ Warning	The new log-rotation It has no SNO guard and counts Windows nodes; on SNO it would wait for a non-existent Windows node set.	Add a [Skipped:SingleReplicaTopology] label or runtime single-node skip before using Windows-node/WMCO logic.
Ipv6 And Disconnected Network Test Compatibility	⚠️ Warning	The new e2e helper hardcodes mcr.microsoft.com/powershell:lts-nanoserver-ltsc2022 for oc debug, creating a public-registry dependency in disconnected CI.	Use a release-payload or cluster-mirrored debug image for oc debug, or skip the test in disconnected environments if no mirrored image exists.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Security: Secrets, Ssh & Csr	✅ Passed	No secret/private-key content is logged, no SSH/CSR approval code was introduced, and the new node access path uses oc debug with sanitized output.
Kubernetes Controller Patterns	✅ Passed	PR only changes e2e helpers/tests and go.mod; no reconciler, predicates, finalizers, status, or owner-ref code to review.
Platform-Specific Requirements	✅ Passed	Not applicable: the PR only adds Windows log-rotation helpers/tests and a go.mod replace; it doesn’t change vSphere/AWS/Azure/GCP provisioning or hostname logic.
Stable And Deterministic Test Names	✅ Passed	The added Ginkgo title is a fixed literal; no runtime values, dates, node/namespace names, UUIDs, or formatted/interpolated titles were introduced.
Topology-Aware Scheduling Compatibility	✅ Passed	Only e2e test/helpers and go.mod changed; no manifests/controllers with node selectors, affinity, spread, replicas, or PDBs were added.
Ote Binary Stdout Contract	✅ Passed	No process-level stdout writes found; touched files only add helpers/tests, and main.go has no fmt.Print/log.SetOutput/klog stdout hooks.
No-Weak-Crypto	✅ Passed	No weak-crypto APIs or custom crypto were added; the diff only adds Windows e2e helpers and log-rotation test logic.
Container-Privileges	✅ Passed	PR only updates Go test code and go.mod; no container/K8s manifests or securityContext/privilege fields were added in the diff.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding an OCP-89616 log rotation verification test to OTE.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ote/test/e2e/utils.go`:
- Line 28: Replace the public-registry value of windowsDebugImage with the
project’s release-payload or cluster-mirrored debug image reference, keeping the
existing helper’s expected Windows debug-image behavior intact.

In `@ote/test/e2e/winc.go`:
- Line 226: Update the smokerun test beginning with the “Verify log rotation for
kubelet and kube-proxy services” It block to explicitly skip or tag MicroShift
before it accesses OLM-backed resources such as Subscription, using the
repository’s established MicroShift skip mechanism.
- Around line 233-245: Update the subscription patch and deferred cleanup in the
surrounding test to preserve the existing spec.config.env list: capture the
original environment configuration before patching, modify only
SERVICES_LOG_FILE_SIZE, SERVICES_LOG_FILE_AGE, and SERVICES_LOG_FLUSH_INTERVAL,
then restore the exact original list during cleanup instead of replacing it with
an empty array.
- Around line 227-230: Validate that winNodeCount is non-empty after trimming
the command output and before splitting it in the Windows-node counting flow.
Fail immediately when no Windows nodes are returned, while preserving the
existing split and expectedNodes calculation for non-empty output.
- Around line 248-249: Update the reconciliation wait following the Subscription
patch in the test flow around waitWindowsNodesReady so it cannot succeed solely
because all Windows nodes are already Ready. Poll a post-patch convergence
signal, such as the updated WMCO generation/status or the expected service
command line containing the new flags, and proceed only after that signal
reflects the Subscription change.
- Around line 282-290: The rotation check around runDebugNodePS currently only
logs whether files exist and never triggers or waits for rotation. Update this
test flow to generate deterministic kubelet and kube-proxy activity exceeding
the 1 MiB threshold, then poll until rotated files appear and assert their
presence; apply the same validation to both log types instead of accepting an
empty result.
- Around line 242-245: Handle the error returned by Execute() in the cleanup
patch within the defer path, and fail the test immediately when reverting the
subscription fails. Update the surrounding cleanup flow near
waitWindowsNodesReady so the readiness check only runs after a successful
revert.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: ec881869-2596-4451-b6e5-e79627dee3c9

📥 Commits

Reviewing files that changed from the base of the PR and between e9f57a5 and 7618f26.

📒 Files selected for processing (3)

• ote/go.mod
• ote/test/e2e/utils.go
• ote/test/e2e/winc.go

[openshift-ci]

@rrasouli: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/aws-e2e-ote	92f5a90	link	false	/test aws-e2e-ote

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.