WINC-2046: Adds log file to windows-exporter by wgahnagl · Pull Request #4355 · openshift/windows-machine-config-operator

wgahnagl · 2026-07-20T16:18:49Z

This allows windows-exporter to use a log file, and use the globally configurable log files.

Summary by CodeRabbit

New Features
- Windows exporter services now support configurable logging levels, using detailed debug logging when debug mode is enabled and standard informational logging otherwise.
- Exporter logs are written to a dedicated log file for easier troubleshooting and monitoring.
Bug Fixes
- Windows exporter logs are now included in required system log collection, improving diagnostic coverage.

openshift-ci · 2026-07-20T16:18:58Z

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

coderabbitai · 2026-07-20T16:19:03Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Excluded labels (none allowed) (2)

do-not-merge/work-in-progress
do-not-merge/hold

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: Enterprise

Run ID: 113453da-a01d-4be3-b533-1222fcb343a5

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 58beb92e-0450-4b79-a214-56886b7b0cce

📥 Commits

Reviewing files that changed from the base of the PR and between 603768b and 9673eac.

📒 Files selected for processing (4)

pkg/services/services.go
pkg/services/services_test.go
pkg/windows/windows.go
test/e2e/logs_test.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

Walkthrough

GenerateManifest now sets the Windows exporter log level to info or debug and passes the configured log file path. Windows exporter log constants and its required directory are added. Unit tests cover default and debug configurations. End-to-end log checks include windows_exporter.log.

Suggested reviewers: jrvaldes, mansikulkarni96

🚥 Pre-merge checks | ✅ 20

✅ Passed checks (20 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.
Go Best Practices & Build Tags	✅ Passed	PASS: No ignored errors or panic were added. NewData returns nonnil data on nil error; the test uses require.NoError. Shared operator manifest code is not OS-specific, and daemon files remain windo...
Security: Secrets, Ssh & Csr	✅ Passed	The diff only configures windows_exporter log level and file paths; it adds no secret, SSH, certificate, CSR, or credential handling behavior.
Kubernetes Controller Patterns	✅ Passed	The diff changes service manifest arguments, Windows log constants/directories, and tests only; no reconciliation, requeue, status, predicate, finalizer, or owner-reference code changed.
Windows Service Management	✅ Passed	The diff only changes the exporter command and log directory. Its priority and dependencies remain unchanged, while existing SCM reconciliation, descriptions, reverse-order cleanup, and reboot logi...
Platform-Specific Requirements	✅ Passed	The PR changes only Windows exporter logging; vSphere limits, AWS EC2LaunchV2 2.0.1643+, Azure cloud-node-manager, and GCP hostname-script wiring remain documented and unchanged.
Stable And Deterministic Test Names	✅ Passed	The PR adds only static subtest names, “Default logging” and “Debug logging enabled”; it adds no Ginkgo titles, and the existing node.Name t.Run title is unchanged.
Test Structure And Quality	✅ Passed	The changed tests use Go testing.T, not Ginkgo. The unit test creates no resources, and the e2e log retrieval uses a bounded PollImmediate timeout.
Microshift Test Compatibility	✅ Passed	The PR adds a standard Go unit test, not a new Ginkgo e2e test. Changed tests contain no Ginkgo constructs or MicroShift-unavailable API references.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The commit adds a standard Go unit test and extends an existing node-log check; it adds no Ginkgo e2e test or multi-node/HA assumption.
Topology-Aware Scheduling Compatibility	✅ Passed	The parent-to-HEAD diff changes only Windows service logging, log directories, and tests; it adds no manifests, controllers, replicas, affinity, topology spread, selectors, tolerations, or PDBs.
Ote Binary Stdout Contract	✅ Passed	The diff changes service configuration, Windows constants, and e2e log collection only; it adds no stdout writes or OTE entrypoint/suite-setup changes.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The diff adds a plain Go unit test and one log path; it adds no Ginkgo test, IPv4-only logic, or external network operation.
No-Weak-Crypto	✅ Passed	The PR diff adds Windows exporter logging and log collection only. It introduces no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret/token comparisons; existing SHA-256 use is unchanged.
Container-Privileges	✅ Passed	The pull request changes Windows service logging and tests only; no container or Kubernetes manifest privilege settings are changed or introduced.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds only fixed windows_exporter log routing and info/debug selection; it passes no passwords, tokens, PII, hostnames, or customer data to logging.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding log file support to windows-exporter.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

jrvaldes

@wgahnagl thanks for working on this, PTAL at the comments

jrvaldes · 2026-08-12T17:03:08Z

@wgahnagl for the e2e test, ensure the resulting log file is captured in the artifacts, for example
https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_windows-machine-config-operator/4355/pull-ci-openshift-windows-machine-config-operator-master-aws-e2e-operator/2087271378465067008/artifacts/aws-e2e-operator/windows-e2e-operator-test/artifacts/nodes/ip-10-0-61-59.ec2.internal/

jrvaldes · 2026-08-12T17:04:28Z

after this merge, need to update the must-gather collection script to include the new file in https://github.com/openshift/must-gather/blob/main/collection-scripts/gather_windows_node_logs#L11

@wgahnagl create a follow-up task/story for this.

openshift-ci-robot · 2026-08-12T17:21:42Z

@wgahnagl: This pull request references WINC-2046 which is a valid jira issue.

Details

In response to this:

This allows windows-exporter to use a log file, and use the globally configurable log files.

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.

jrvaldes

thanks for pushing the commit with tests, PTAL at the comments

windows-exporter was not ommitting logs to a log file. This creates the log file and adds a unit test for the logs.

jrvaldes · 2026-08-18T18:25:29Z

/test unit

jrvaldes · 2026-08-18T18:25:33Z

/test lint

jrvaldes · 2026-08-18T18:26:23Z

/test gcp-e2e-operator

jrvaldes · 2026-08-18T18:28:46Z

@coderabbitai full review

jrvaldes · 2026-08-19T17:14:05Z

azure-e2e-upgrade failed with:

=== RUN   TestWMCO/create/Node_Logs/e2e-wm-rflxh/windows_exporter/windows_exporter.log
2026/08/19 16:28:14 unable to retrieve log windows_exporter/windows_exporter.log from node e2e-wm-rflxh: oc adm node-logs failed with exit code exit status 1 and output: : error: the server could not find the requested resource
  404 page not found

which is expected as the log file does not exist in prev version, and we are testing with a newer test suite. I lean towards skipping this test. @mansikulkarni96 WDYT?

jrvaldes · 2026-08-19T17:16:55Z

/override "Red Hat Konflux / windows-machine-config-operator-release-5-0-ec / windows-machine-config-operator-release-5-0"

openshift-ci · 2026-08-19T17:17:09Z

@jrvaldes: Overrode contexts on behalf of jrvaldes: Red Hat Konflux / windows-machine-config-operator-release-5-0-ec / windows-machine-config-operator-release-5-0

Details

In response to this:

/override "Red Hat Konflux / windows-machine-config-operator-release-5-0-ec / windows-machine-config-operator-release-5-0"

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.

jrvaldes · 2026-08-19T17:45:24Z

/override ci/prow/gcp-e2e-operator

failure not related, there good CI coverage in other jobs

openshift-ci · 2026-08-19T17:45:41Z

@jrvaldes: Overrode contexts on behalf of jrvaldes: ci/prow/gcp-e2e-operator

Details

In response to this:

/override ci/prow/gcp-e2e-operator

failure not related, there good CI coverage in other jobs

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.

jrvaldes · 2026-08-19T18:03:33Z

 		"containerd/containerd.log",
 		"wicd/windows-instance-config-daemon.exe.INFO",
 		"csi-proxy/csi-proxy.log",
+		"windows_exporter/windows_exporter.log",


The azure-e2e-upgrade failure is caused by version skew: the new e2e test code expects windows_exporter/windows_exporter.log, but the upgrade job initially provisions Windows nodes with the previous WMCO version, which does not create that file. the log file is correctly created on fresh installs and after full reconciliation.

@wgahnagl explore the option to skip the windows_exporter.log expectation only in upgrade tests, not the whole Node Logs test. Add a new commit to make the new log check conditional on the node being configured by the current WMCO version, so fresh-install tests continue to enforce it.

consider bringing this back ced41e5#diff-2a3ec15f2cd9038bbb6ad1be37818927fb913fd7a7ebd7cbb29ab41aeac4c3f7R25

`optionalLogs := []string{ "windows_exporter/windows_exporter.log", }

openshift-merge-bot · 2026-08-19T20:50:47Z

/retest-required

Remaining retests: 0 against base HEAD 2af30b4 and 2 for PR HEAD 9673eac in total

jrvaldes · 2026-08-20T00:54:07Z

/hold

for https://github.com/openshift/windows-machine-config-operator/pull/4355/changes#r3815720824

jrvaldes · 2026-08-20T13:36:35Z

/hold

for https://github.com/openshift/windows-machine-config-operator/pull/4355/changes#r3815720824

@wgahnagl please move the PR back to draft, and push the fix for this, then test the azure-e2e-upgrade.

wgahnagl · 2026-08-21T14:00:20Z

/retest-required

jrvaldes · 2026-08-21T19:59:20Z

/hold cancel

jrvaldes · 2026-08-21T19:59:25Z

/lgtm

jrvaldes · 2026-08-21T20:00:31Z

/override "Red Hat Konflux / windows-machine-config-operator-release-5-0-ec / windows-machine-config-operator-release-5-0"

openshift-ci · 2026-08-21T20:00:48Z

@jrvaldes: Overrode contexts on behalf of jrvaldes: Red Hat Konflux / windows-machine-config-operator-release-5-0-ec / windows-machine-config-operator-release-5-0

Details

In response to this:

/override "Red Hat Konflux / windows-machine-config-operator-release-5-0-ec / windows-machine-config-operator-release-5-0"

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.

jrvaldes · 2026-08-21T20:02:13Z

/override "ci/prow/vsphere-disconnected-e2e-operator"

openshift-ci · 2026-08-21T20:02:32Z

@jrvaldes: Overrode contexts on behalf of jrvaldes: ci/prow/vsphere-disconnected-e2e-operator

Details

In response to this:

/override "ci/prow/vsphere-disconnected-e2e-operator"

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.

openshift-ci · 2026-08-21T20:02:34Z

@wgahnagl: 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	`49d24b1`	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.

openshift-cherrypick-robot · 2026-08-21T20:05:51Z

@jrvaldes: new pull request created: #4507

Details

In response to this:

/cherry-pick release-5.0

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.

jrvaldes · 2026-08-24T14:00:32Z

/cherry-pick release-4.23

openshift-cherrypick-robot · 2026-08-24T14:01:27Z

@jrvaldes: new pull request created: #4522

Details

In response to this:

/cherry-pick release-4.23

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.

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2026

wgahnagl changed the title ~~adds log level and log file~~ [WINC-1979] Adds log file to windows-exporter Jul 20, 2026

wgahnagl closed this Jul 21, 2026

wgahnagl reopened this Aug 11, 2026

wgahnagl marked this pull request as ready for review August 11, 2026 20:13

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 11, 2026

openshift-ci Bot requested review from jrvaldes and mansikulkarni96 August 11, 2026 20:14

wgahnagl changed the title ~~[WINC-1979] Adds log file to windows-exporter~~ [WINC-2046] Adds log file to windows-exporter Aug 11, 2026

jrvaldes requested changes Aug 12, 2026

View reviewed changes

Comment thread pkg/services/services.go

Comment thread pkg/windows/windows.go

Comment thread pkg/services/services.go

wgahnagl marked this pull request as draft August 12, 2026 17:07

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 12, 2026

jrvaldes changed the title ~~[WINC-2046] Adds log file to windows-exporter~~ WINC-2046: Adds log file to windows-exporter Aug 12, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 12, 2026

jrvaldes reviewed Aug 13, 2026

View reviewed changes

Comment thread pkg/services/services.go

jrvaldes reviewed Aug 13, 2026

View reviewed changes

Comment thread pkg/services/services.go

jrvaldes requested changes Aug 17, 2026

View reviewed changes

Comment thread test/e2e/logs_test.go

Comment thread test/e2e/validation_test.go Outdated

wgahnagl force-pushed the WINC-1979 branch 2 times, most recently from 8e17d00 to 42c196d Compare August 18, 2026 18:05

[WINC 2046] Adds log level and log file to windows-exporter

9673eac

windows-exporter was not ommitting logs to a log file. This creates the log file and adds a unit test for the logs.

wgahnagl force-pushed the WINC-1979 branch from 42c196d to 9673eac Compare August 18, 2026 18:07

jrvaldes reviewed Aug 19, 2026

View reviewed changes

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 20, 2026

makes windows exporter log an optional log

49d24b1

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 20, 2026

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 21, 2026

openshift-ci Bot assigned jrvaldes Aug 21, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 21, 2026

openshift-merge-bot Bot merged commit caa0b00 into openshift:master Aug 21, 2026
19 of 21 checks passed

openshift-cherrypick-robot mentioned this pull request Aug 21, 2026

[release-5.0] WINC-2046: Adds log file to windows-exporter #4507

Merged

openshift-cherrypick-robot mentioned this pull request Aug 24, 2026

[release-4.23] WINC-2046: Adds log file to windows-exporter #4522

Closed

jrvaldes mentioned this pull request Aug 26, 2026

[docs] adds troubleshooting instructions for windows-exporter logs #4530

Merged

Conversation

wgahnagl commented Jul 20, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

openshift-ci Bot commented Jul 20, 2026

Uh oh!

coderabbitai Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Uh oh!

jrvaldes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrvaldes commented Aug 12, 2026

Uh oh!

jrvaldes commented Aug 12, 2026

Uh oh!

openshift-ci-robot commented Aug 12, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrvaldes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jrvaldes commented Aug 18, 2026

Uh oh!

jrvaldes commented Aug 18, 2026

Uh oh!

jrvaldes commented Aug 18, 2026

Uh oh!

jrvaldes commented Aug 18, 2026

Uh oh!

jrvaldes commented Aug 19, 2026

Uh oh!

jrvaldes commented Aug 19, 2026

Uh oh!

openshift-ci Bot commented Aug 19, 2026

Uh oh!

jrvaldes commented Aug 19, 2026

Uh oh!

openshift-ci Bot commented Aug 19, 2026

Uh oh!

jrvaldes Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

jrvaldes Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

wgahnagl Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

openshift-merge-bot Bot commented Aug 19, 2026

Uh oh!

jrvaldes commented Aug 20, 2026

Uh oh!

jrvaldes commented Aug 20, 2026

Uh oh!

wgahnagl commented Aug 21, 2026

Uh oh!

jrvaldes commented Aug 21, 2026

Uh oh!

jrvaldes commented Aug 21, 2026

Uh oh!

jrvaldes commented Aug 21, 2026

Uh oh!

openshift-ci Bot commented Aug 21, 2026

Uh oh!

jrvaldes commented Aug 21, 2026

Uh oh!

openshift-ci Bot commented Aug 21, 2026

Uh oh!

openshift-ci Bot commented Aug 21, 2026

wgahnagl commented Jul 20, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 20, 2026 •

edited

Loading

openshift-ci-robot commented Aug 12, 2026 •

edited by openshift-ci Bot

Loading