[WINC-1971] [ote] Fix OCP-68320 test missing SpecContext parameter

[rrasouli]

Problem

Test OCP-68320 uses g.SpecTimeout(45*time.Minute) decorator but the function signature was missing the required context parameter.

This causes Ginkgo to fail during test discovery with:

Invalid NodeTimeout SpecTimeout, or GracePeriod
/build/windows-machine-config-operator/ote/test/e2e/proxy.go:185
  [It] was passed NodeTimeout, SpecTimeout, or GracePeriod but does not have a
  callback that accepts a SpecContext or context.Context

This prevents both aws-e2e-ote and vsphere-proxy-e2e-ote jobs from discovering any proxy tests, causing them to fail with "no tests to run".

Solution

Changed function signature from:

func() {

to:

func(ctx g.SpecContext) {

Testing

CI will verify that openshift-tests run windows-machine-config-operator/proxy successfully discovers and runs proxy tests.

Related

• Introduced in: WINC-1971: [ote] Migrate proxy tests from OTP to OTE #4459
• Blocks: [WINC-1971] [wmco] Optimize OTE job triggers and remove 0-test workaround release#84352

Summary by CodeRabbit

• Tests
  • Updated an end-to-end test to support context-aware execution.
  • No user-visible behavior changed.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[coderabbitai]

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

Run ID: a53a2c4b-3a60-45ea-9fe4-71e01443633f

📥 Commits

Reviewing files that changed from the base of the PR and between 2f35102 and d956ef9.

📒 Files selected for processing (1)

• ote/test/e2e/proxy.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The Windows custom CA certificate import test now declares its closure with a g.SpecContext parameter. This enables context-aware cancellation and timeout handling during test execution. No other test behavior changed.

Merge Risk: ⚪ Minimal · up to d956e

The Windows custom CA import test can now be discovered and run with its configured timeout. No remaining merge-readiness risk is identified.

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	The signature change makes the SpecTimeout-decorated It block discoverable and executable. The activated test still contains two direct assertions without failure messages at `ote/test/e2e/proxy.g…	Add meaningful messages to both file-read assertions. For example: o.Expect(err).NotTo(o.HaveOccurred(), "failed to read generated certificate %s", crtPath) and `o.Expect(err).NotTo(o.HaveOccurred(), "failed to read rotated certificate %s…

✅ Passed checks (14 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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. The pull request changes only the OTE Ginkgo callback signature from func() to func(ctx g.SpecContext) in ote/test/e2e/proxy.go. The change adds no ignored errors, error handling, panic, p…
Security: Secrets, Ssh & Csr	✅ Passed	PASS: The pull request changes only the OCP-68320 Ginkgo callback signature from func() to func(ctx g.SpecContext). The parent-to-HEAD diff contains no changes to credentials, private keys, secret…
Kubernetes Controller Patterns	✅ Passed	PASS: The pull request changes only the Ginkgo callback signature in ote/test/e2e/proxy.go from func() to func(ctx g.SpecContext) while retaining the existing g.SpecTimeout. The base-to-head d…
Windows Service Management	✅ Passed	PASS: The pull request changes only the callback signature in ote/test/e2e/proxy.go from func() to func(ctx g.SpecContext). The parent and current versions have identical service-related code, i…
Platform-Specific Requirements	✅ Passed	The pull request changes only the Ginkgo callback signature in ote/test/e2e/proxy.go from func() to func(ctx g.SpecContext). The diff introduces no vSphere machine naming, AWS EC2LaunchV2, Azure…
Stable And Deterministic Test Names	✅ Passed	PASS — The pull request changes only the callback signature in ote/test/e2e/proxy.go. The g.It title remains the literal, static string `Smokerun-Author:rrasouli-Critical-68320-[node-proxy]-Import…
Microshift Test Compatibility	✅ Passed	PASS: The pull request changes only the existing callback signature from func() to func(ctx g.SpecContext). It adds no Ginkgo test or new MicroShift API usage. The existing test is also inside a `…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request does not add a Ginkgo test or change test behavior. The exact diff changes only the existing OCP-68320 callback from func() to func(ctx g.SpecContext), which fixes `g.SpecTi…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: The pull request changes only the Ginkgo callback signature in ote/test/e2e/proxy.go. The exact patch is one line changed from func() to func(ctx g.SpecContext). It does not add or modify …
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the affected OTE proxy test and the missing SpecContext parameter. It matches the primary change and explains the test-discovery fix.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: Go Best Practices & Build Tags

Explanation

PASS. The pull request changes only the OTE Ginkgo callback signature from func() to func(ctx g.SpecContext) in ote/test/e2e/proxy.go. The change adds no ignored errors, error handling, panic, pointer dereference, or build-tag code. The file is an OTE e2e test, not daemon or controller code, and the existing SpecTimeout callbacks in the same file already use g.SpecContext. The required build tags for daemon/controller packages remain present and are unrelated to this patch.

Full details: Security: Secrets, Ssh & Csr

Explanation

PASS: The pull request changes only the OCP-68320 Ginkgo callback signature from func() to func(ctx g.SpecContext). The parent-to-HEAD diff contains no changes to credentials, private keys, secrets, SSH/SFTP sessions, certificate storage, or CSR approval logic. The existing certificate test body remains unchanged, so this pull request introduces no failure under the security check.

Full details: Kubernetes Controller Patterns

Explanation

PASS: The pull request changes only the Ginkgo callback signature in ote/test/e2e/proxy.go from func() to func(ctx g.SpecContext) while retaining the existing g.SpecTimeout. The base-to-head diff contains one file and one line change. It does not modify controller reconciliation, requeue behavior, status conditions, watch predicates, finalizers, or owner references. The Kubernetes Controller Patterns check is therefore not applicable.

Full details: Windows Service Management

Explanation

PASS: The pull request changes only the callback signature in ote/test/e2e/proxy.go from func() to func(ctx g.SpecContext). The parent and current versions have identical service-related code, including WICD ConfigMap checks and WMCO restart checks. The change does not alter service priority, dependencies, descriptions, cleanup, reboot requirements, or Service Control Manager interactions.

Full details: Platform-Specific Requirements

Explanation

The pull request changes only the Ginkgo callback signature in ote/test/e2e/proxy.go from func() to func(ctx g.SpecContext). The diff introduces no vSphere machine naming, AWS EC2LaunchV2, Azure cloud-node-manager, GCP hostname, or MachineSet behavior. It introduces no new platform-specific limitation that requires documentation.

Full details: Stable And Deterministic Test Names

Explanation

PASS — The pull request changes only the callback signature in ote/test/e2e/proxy.go. The g.It title remains the literal, static string Smokerun-Author:rrasouli-Critical-68320-[node-proxy]-Import custom CA certificates into Windows node system store [Serial][Disruptive]. It contains no runtime-generated names, timestamps, UUIDs, node names, namespaces, IP addresses, or dynamic title construction.

Full details: Test Structure And Quality

Explanation

The signature change makes the SpecTimeout-decorated It block discoverable and executable. The activated test still contains two direct assertions without failure messages at ote/test/e2e/proxy.go:221 and :234: o.Expect(err).NotTo(o.HaveOccurred()). This matches the custom check's explicit assertion-message failure condition. The test otherwise uses a 45-minute spec timeout, bounded certificate polling, and cleanup for the temporary files and ConfigMap state.

Resolution

Add meaningful messages to both file-read assertions. For example: o.Expect(err).NotTo(o.HaveOccurred(), "failed to read generated certificate %s", crtPath) and o.Expect(err).NotTo(o.HaveOccurred(), "failed to read rotated certificate %s", crtPath).

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request changes only the existing callback signature from func() to func(ctx g.SpecContext). It adds no Ginkgo test or new MicroShift API usage. The existing test is also inside a Describe tagged [apigroup:config.openshift.io], which the check explicitly treats as protected from MicroShift execution.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request does not add a Ginkgo test or change test behavior. The exact diff changes only the existing OCP-68320 callback from func() to func(ctx g.SpecContext), which fixes g.SpecTimeout discovery compatibility. The existing test iterates over Windows nodes but does not require multiple nodes or any other listed HA condition.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only the Ginkgo callback signature in ote/test/e2e/proxy.go. The exact patch is one line changed from func() to func(ctx g.SpecContext). It does not add or modify deployment manifests, operator code, controllers, replicas, affinity, tolerations, selectors, topology spread constraints, or PDBs. The topology-aware scheduling check is therefore not applicable.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[jrvaldes]

/lgtm

[openshift-merge-bot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test aws-e2e-ote

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jrvaldes, 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 [jrvaldes,rrasouli]

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

[rrasouli]

@coderabbitai how did you miss this critical bug in PR #4459 ?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="openshift/windows-machine-config-operator"

echo "== PR metadata =="
gh pr view 4459 --repo "$repo" --json number,title,state,mergedAt,baseRefName,headRefName,author,body,files,comments,reviews \
  --jq '{number,title,state,mergedAt,baseRefName,headRefName,author:.author.login,body,files:[.files[]|{path,additions,deletions}],comments:[.comments[]|{author:.author.login,body,createdAt,url}],reviews:[.reviews[]|{author:.author.login,state,body,submittedAt}]}'

echo
echo "== PR diff: proxy test context/timeout declarations =="
gh pr diff 4459 --repo "$repo" -- ote/test/e2e/proxy.go || true

echo
echo "== Current corrected hunk =="
ast-grep outline ote/test/e2e/proxy.go --items all --type function
rg -n -C 8 'SpecTimeout|Import custom CA certificates|func\(ctx g\.SpecContext\)' ote/test/e2e/proxy.go

Length of output: 80043

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="openshift/windows-machine-config-operator"

gh api "repos/${repo}/pulls/4459/files?per_page=100" \
  --jq '.[] | select(.filename == "ote/test/e2e/proxy.go") | .patch' \
  > /tmp/pr-4459-proxy.patch

echo "== OCP-68320 hunk from PR `#4459` =="
rg -n -C 10 '68320|SpecTimeout|SpecContext|func\(\)' /tmp/pr-4459-proxy.patch

echo
echo "== CodeRabbit review findings that mention Ginkgo timeout/context/signature =="
gh pr view 4459 --repo "$repo" --json reviews --jq '
  .reviews[]
  | select(.author.login == "coderabbitai")
  | .body
' \
  | rg -in -C 2 'SpecTimeout|SpecContext|callback|Ginkgo.*signature|signature.*Ginkgo' || true

Length of output: 10117

---

@rrasouli I missed it.

In #4459, OCP-68320 used:

g.SpecTimeout(45*time.Minute),
func() {

g.SpecTimeout requires an interruptible Ginkgo callback, such as:

func(ctx g.SpecContext) {

The #4459 review found other issues in proxy.go, but it did not validate the Ginkgo v2 contract between g.SpecTimeout and the callback signature. That omission allowed a test-discovery failure into the merged PR.

#4566 corrects the defect. The review should have treated Ginkgo node decorators and callback signatures as a required compatibility check, especially for newly migrated OTE tests.

You are interacting with an AI system.