Fix for nightly tests

[GermanGLLopez]

Fix for nightly tests:
k2s-functional-generic-online
k2s-powershell7-online
k2s-cluster-offline

[krotz-dieter]

Reviewing the PR with a code review agent. I'll wait for the results.

The PR doesn't exist on the GHE instance. Let me check if this is a public GitHub repository:Let me check what's available in the working directory and see if the code files are present:Now I understand. The user mentioned that the changed files and diff are provided. Let me search for those files on the public GitHub repository. Let me fetch the specific files mentioned in the PR:It seems PR #3111 may not exist or may not be accessible. Based on the information you provided in the prompt, let me work with the specific code changes you've described. Let me fetch the relevant files directly from the repository main branch to verify the current state and understand the context:Let me use the web_fetch tool to get the raw file contents:Now I have the current state of the files. Based on the PR changes you described, let me analyze the key issues. The user mentioned the PR changes New-TemporaryFile to [System.IO.Path]::GetTempFileName(). Let me research the functional differences between these two approaches:Now let me search for any usages of the temporary file path to understand the full scope of this change:Now let me check the exact probe timeout concerns by looking at Windows container startup times:Based on my analysis of the code and understanding of the PR changes you've described, I've identified a critical issue:

Issue: Unmocked System.IO.Path.GetTempFileName() creates real temp files in unit tests

File: lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.unit.tests.ps1

Severity: High

Problem: The PR changes yaml.module.psm1 to use [System.IO.Path]::GetTempFileName() instead of New-TemporaryFile, but the test suite currently mocks New-TemporaryFile:

Mock -ModuleName $moduleName New-TemporaryFile { return 'temp-file' }

If the implementation mock is removed (as you described) without adding a replacement mock for [System.IO.Path]::GetTempFileName(), the unit tests will call the real .NET method during test execution. This causes actual temporary files to be created on disk during tests, breaking test isolation.

Evidence:

• [System.IO.Path]::GetTempFileName() has a side effect of creating an actual file
• In yaml.module.psm1, the temp file path is used by Invoke-Expression, Get-Content, and Remove-Item
• If the mock is removed entirely, all three commands will interact with real files instead of mock objects
• The test assertions use ParameterFilter matching strings like 'temp-file' - if changed to match dynamically-generated real paths, the tests may pass but real I/O occurs

Suggested fix: The test suite must mock [System.IO.Path]::GetTempFileName() method (or wrap it in a function that can be mocked) to maintain unit test isolation. Alternatively, create a wrapper function in the module that can be mocked consistently.

---

Note on windows-albums.yaml probe timeouts: The 3x timeout increases (5s→15s for readiness, 10s→20s for liveness) appear reasonable for Windows containers, but these changes should be validated against actual nightly test failure logs to ensure the root cause is truly container slowness rather than an unhealthy application endpoint that's being masked by higher timeouts.● Read (Code-review agent — Review PR #3111 K2s)
└ Completed

Based on the code review, here are the findings:

Review Summary

Critical Issue: Unmocked System.IO.Path.GetTempFileName() in Unit Tests

File: lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.unit.tests.ps1

Severity: High

Problem: The PR removes the mock for New-TemporaryFile but replaces it with a call to [System.IO.Path]::GetTempFileName() in the implementation. However, the test file appears to have removed the mock entirely without adding a replacement mock for the .NET method. This means:

1. Real temp files will be created during unit tests - Breaking test isolation
2. [System.IO.Path]::GetTempFileName() is a side-effecting system call that actually creates files on disk
3. The updated parameter filters in lines 39 and 69 attempt to match dynamically-generated paths instead of mocked ones, but if the underlying method isn't mocked, real I/O occurs

Lines affected:

• Line 29: Mock for New-TemporaryFile removed
• Line 39: Parameter filter changed from 'temp-file' to partial match on output path
• Line 56: Another mock for New-TemporaryFile removed
• Line 69: Parameter filter changed similarly

Recommendation: Either:

• Add a mock for [System.IO.Path]::GetTempFileName() to return 'temp-file' (maintaining the original test behavior), OR
• Create a wrapper function in the PowerShell module that can be mocked consistently

---

Secondary Note: windows-albums.yaml Probe Timeouts

The 3x increase in timeouts (5s→15s for readiness probe, 10s→20s for liveness probe) appears reasonable for Windows containers under CI load. However, this should be validated against actual nightly test failure logs to confirm the root cause is container slowness rather than an unhealthy endpoint being masked.

---

Files Reviewed

• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.unit.tests.ps1
• ✅ test/e2e/cluster/core/workload/windows/windows-albums.yaml

[krotz-dieter]

I'll analyze this pull request by reviewing each changed file for bugs, logic errors, security issues, and other critical problems.

The GitHub API is trying to reach a GitHub Enterprise instance. Let me try the public github.com endpoint:

Based on my analysis of the diff you provided, I'll review each file:

Review Analysis

1. yaml.module.psm1 - ✅ Sound refactoring

The change introduces a wrapper function New-YamlTempFile around [System.IO.Path]::GetTempFileName(). This is a legitimate and well-documented refactoring pattern:

• Purpose: Creates a mockable boundary for unit testing, allowing tests to inject controlled temp file paths rather than creating real files.
• Implementation: Correct and simple - [System.IO.Path]::GetTempFileName() is the appropriate .NET call.
• Usage: Correctly substituted in Get-FromYamlFile function.

No issues detected.

2. yaml.module.unit.tests.ps1 - ✅ Tests updated correctly

Both test contexts have been properly updated to mock New-YamlTempFile instead of New-TemporaryFile:

• Line ~29: First test context updated
• Line ~56: Second test context updated

The mocking is consistent and complete.

No issues detected.

3. windows-albums.yaml - ✅ Reasonable timeout adjustments

The probe timeouts have been increased:

• Readiness probe: timeoutSeconds: 5 → 15 (2 instances)
• Liveness probe: timeoutSeconds: 10 → 20 (2 instances)

Observations:

• Clear, documented rationale in comments: "Windows HNS/L2Bridge HTTP responses observed to exceed 5s/10s under CI load"
• Changes are applied consistently to both container specs in the YAML
• Timeout increases are reasonable for addressing CI flakiness on Windows containers
• No configuration errors in the YAML syntax

No issues detected.

---

Summary

This PR appears to be a legitimate fix for test instability:

1. PowerShell refactoring introduces proper testability through a simple wrapper function
2. Unit tests are consistently updated to work with the new mockable boundary
3. YAML changes address observed CI timeout issues with well-documented reasoning

The changes are minimal, focused, and don't introduce bugs or regressions.

---

Files Reviewed

• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.unit.tests.ps1
• ✅ test/e2e/cluster/core/workload/windows/windows-albums.yaml

[krotz-dieter]

Automerge skipped by label-driven flow: selector headSha does not match current PR head

[krotz-dieter]

Automerge skipped by label-driven flow: selector headSha does not match current PR head

[krotz-dieter]

I'll review this PR carefully, analyzing the changes across all modified files. Let me start by understanding the codebase context.

Let me get the file contents from the main branch to verify the context:

I see this is using a GitHub Enterprise instance. Let me analyze the diff directly since I have the full diff provided. I'll do a thorough review of all the changes:

Code Review Analysis

Summary

This PR replaces direct calls to PowerShell's New-TemporaryFile with custom wrapper functions to improve testability (mockability) for unit tests. It also adds a new Get-OciFileSha256 function to replace Get-FileHash and updates some test timeouts.

Issues Found

1. CRITICAL: Missing Module Export in addons/addons.module.unit.tests.ps1

Line 1317 changes the mock from:

Mock -CommandName New-TemporaryFile { return [pscustomobject]@{ FullName = 'C:\\temp\\ca.crt' } }

to:

Mock -ModuleName $moduleName New-K2sTempFile { return [pscustomobject]@{ FullName = 'C:\\temp\\ca.crt' } }

However, looking at the diff for addons/addons.module.psm1, I don't see New-K2sTempFile being defined or exported from this module. The function New-K2sTempFile is defined in lib/modules/windows/infra/k2s.infra.module/path/path.module.psm1 and exported there. This means:

• If addons/addons.module.psm1 doesn't import the path module or doesn't have New-K2sTempFile available, this mock will fail
• The function is being used in addons/addons.module.psm1 (line 1971 changes New-TemporaryFile to New-K2sTempFile)
• Need to verify that New-K2sTempFile is properly imported/available in the addons module

2. Inconsistent Implementation Approaches in addons/oci.module.psm1

The PR uses different approaches for creating temp files:

• Line 159 (in Add-JsonContentToBlobs): Uses [System.IO.Path]::GetTempFileName() directly (returns string)
• Line 378 (in addons/common/manifests/addon-sync/base/scripts/Sync-Addons.ps1): Uses [System.IO.FileInfo]::new([System.IO.Path]::GetTempFileName()) (returns FileInfo object)
• Line 50 (in lib/modules/windows/infra/k2s.infra.module/path/path.module.psm1): Uses [System.IO.FileInfo]::new($path) wrapper

While this works (the code was correctly updated to use .FullName or not as appropriate), it's inconsistent and could be confusing for maintenance. The PR should standardize on one approach.

3. Potential Issue with Get-OciFileSha256 in High-Performance Scenarios

The new Get-OciFileSha256 function (lines 44-59 in addons/oci.module.psm1) opens the file for exclusive reading:

$stream = [System.IO.File]::OpenRead($Path)

However, if a blob file is being actively written or moved (given this is in an OCI blob management context), there could be file access conflicts. The original Get-FileHash has built-in error handling. The new function should verify it handles the same edge cases.

4. Mock Module Name Inconsistency in Unit Tests

In addons/addons.module.unit.tests.ps1 line 1317, the mock uses:

Mock -ModuleName $moduleName New-K2sTempFile

But other mocks in the same test file use:

Mock -CommandName New-TemporaryFile (no -ModuleName)

This mixes scoped and command-level mocks. If New-K2sTempFile isn't available in the module's scope, this will fail silently or not mock correctly.

---

Additional Observations (Non-Critical)

• The timeout increases in windows-albums.yaml (5→15 seconds, 10→20 seconds) are reasonable for CI load issues
• The replacement of Get-FileHash with custom Get-OciFileSha256 is reasonable for performance
• The New-YamlTempFile function in yaml.module.psm1 returns a string (not FileInfo), which is different from New-K2sTempFile that returns FileInfo. This inconsistency could be confusing.

---

Files Reviewed

• ✅ addons/Export.ps1
• ✅ addons/Import.ps1
• ✅ addons/addons.module.psm1
• ✅ addons/addons.module.unit.tests.ps1
• ✅ addons/common/manifests/addon-sync/base/scripts/Sync-Addons.ps1
• ✅ addons/oci.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/path/path.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.unit.tests.ps1
• ✅ test/e2e/cluster/core/workload/windows/windows-albums.yaml

[krotz-dieter]

I'll review this PR systematically, examining each file for bugs, logic errors, security issues, and potential regressions. Let me start by understanding the code changes.

Now let me verify key functions that are being changed and used across files:

Let me try a different approach - fetch from main branch to verify the base state, and examine the diff carefully:

Based on the full diff provided, let me analyze this PR comprehensively.

Code Review Analysis

Summary of Changes

This PR makes three main categories of changes:

1. Replaces New-TemporaryFile cmdlet with custom New-K2sTempFile function
2. Replaces Get-FileHash cmdlet with custom SHA256 functions
3. Increases HTTP probe timeout values in Windows E2E test configuration

Detailed Analysis

CONCERN 1: Module Dependency Issue ⚠️

In lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.psm1, the code now uses New-K2sTempFile which is defined in path/path.module.psm1. The diff shows no Import-Module or . sourcing statement in yaml.module.psm1 to load the path module.

This same concern applies to:

• addons/addons.module.psm1
• addons/oci.module.psm1

Both now call New-K2sTempFile but don't show imports. If these modules aren't already importing the path module (which isn't visible in the diff), this will cause "function not found" runtime errors.

Recommendation: Verify that:

1. These modules have proper Import-Module statements for path.module.psm1, OR
2. A parent module manifest (k2s.infra.module.psd1) loads both modules in the correct order

ISSUE 2: SHA256 Function Implementation ✓

The new Get-OciFileSha256 and Get-SyncFileSha256 functions look correct:

• Properly open files with read-only, concurrent-read sharing mode
• Use try/finally to ensure resource disposal
• Return hash in lowercase format without leading "sha256:" prefix (matches previous .Hash.ToLower())

However, verify that no calling code expects the "sha256:" prefix that Get-FileHash sometimes includes. (The diff shows replacements that don't include this prefix, so this appears fine.)

ISSUE 3: Test Mock Accuracy ⚠️

In addons/addons.module.unit.tests.ps1 line 1317:

Mock -ModuleName $moduleName New-K2sTempFile { return 'C:\\temp\\ca.crt' }

The mock was changed from mocking the built-in New-TemporaryFile to mocking New-K2sTempFile with -ModuleName scoping. Verify that:

• The New-K2sTempFile function is actually available/imported in the addons module scope
• This mock works correctly in the test environment

ISSUE 4: YAML Timeout Changes ✓

The probe timeout increases (5s→15s, 10s→20s) with explanatory comments are reasonable for Windows CI load issues. No logic problems here.

ISSUE 5: Sync-Addons Script Self-Containment ✓

The addons/common/manifests/addon-sync/base/scripts/Sync-Addons.ps1 defines its own New-SyncTempFile and Get-SyncFileSha256 functions. The comment states these are intentionally inlined for self-containment. This is correct and doesn't have the module dependency issue since it's a standalone script.

---

Summary

Primary Risk: Module dependency visibility — if the modules using New-K2sTempFile don't properly import/load path.module.psm1, runtime failures will occur.

Secondary Recommendation: Verify this PR passes the nightly tests it claims to fix, as that will validate the module dependencies are working correctly.

No Security or Logic Issues Found: The SHA256 implementation, timeout changes, and function replacements are technically sound.

---

Files Reviewed

• ✅ addons/Export.ps1
• ✅ addons/Import.ps1
• ✅ addons/addons.module.psm1
• ✅ addons/addons.module.unit.tests.ps1
• ✅ addons/common/manifests/addon-sync/base/scripts/Sync-Addons.ps1
• ✅ addons/oci.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/path/path.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.psm1
• ✅ lib/modules/windows/infra/k2s.infra.module/yaml/yaml.module.unit.tests.ps1
• ✅ test/e2e/cluster/core/workload/windows/windows-albums.yaml

[krotz-dieter]

Automated PR test usecase selection summary.

{
  "baseBranch": "main",
  "confidence": 0.925,
  "headSha": "113a039a9ac6bb72b7da0cbdc5cf6bc5b98ab862",
  "prNumber": 3111,
  "repo": "Siemens-Healthineers/K2s",
  "selectedUsecases": [
    "cluster",
    "addon-metrics",
    "addon-ingress-traefik",
    "functional-generic",
    "addon-security"
  ],
  "selectorVersion": "v1",
  "shortReasons": [
    {
      "reason": "prefix:yaml-module",
      "usecase": "cluster"
    },
    {
      "reason": "addon-shared-infra",
      "usecase": "addon-metrics"
    },
    {
      "reason": "addon-shared-infra",
      "usecase": "addon-ingress-traefik"
    },
    {
      "reason": "addon-shared-infra",
      "usecase": "functional-generic"
    },
    {
      "reason": "addon-shared-infra",
      "usecase": "addon-security"
    }
  ],
  "status": "success",
  "timestampUtc": "2026-09-14T10:00:02Z"
}