mirror of
https://github.com/actions/runner.git
synced 2025-12-10 20:36:49 +00:00
Compare commits
1 Commits
copilot/fi
...
dependabot
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4b08500445 |
2
.github/workflows/build.yml
vendored
2
.github/workflows/build.yml
vendored
@@ -50,7 +50,7 @@ jobs:
|
||||
|
||||
runs-on: ${{ matrix.os }}
|
||||
steps:
|
||||
- uses: actions/checkout@v5
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
# Build runner layout
|
||||
- name: Build & Layout Release
|
||||
|
||||
2
.github/workflows/codeql.yml
vendored
2
.github/workflows/codeql.yml
vendored
@@ -23,7 +23,7 @@ jobs:
|
||||
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v5
|
||||
uses: actions/checkout@v4
|
||||
|
||||
# Initializes the CodeQL tools for scanning.
|
||||
- name: Initialize CodeQL
|
||||
|
||||
4
.github/workflows/docker-buildx-upgrade.yml
vendored
4
.github/workflows/docker-buildx-upgrade.yml
vendored
@@ -17,7 +17,7 @@ jobs:
|
||||
BUILDX_CURRENT_VERSION: ${{ steps.check_buildx_version.outputs.CURRENT_VERSION }}
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v5
|
||||
uses: actions/checkout@v4
|
||||
|
||||
- name: Check Docker version
|
||||
id: check_docker_version
|
||||
@@ -89,7 +89,7 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v5
|
||||
uses: actions/checkout@v4
|
||||
|
||||
- name: Update Docker version
|
||||
shell: bash
|
||||
|
||||
4
.github/workflows/dotnet-upgrade.yml
vendored
4
.github/workflows/dotnet-upgrade.yml
vendored
@@ -15,7 +15,7 @@ jobs:
|
||||
DOTNET_CURRENT_MAJOR_MINOR_VERSION: ${{ steps.fetch_current_version.outputs.DOTNET_CURRENT_MAJOR_MINOR_VERSION }}
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v5
|
||||
uses: actions/checkout@v4
|
||||
- name: Get current major minor version
|
||||
id: fetch_current_version
|
||||
shell: bash
|
||||
@@ -89,7 +89,7 @@ jobs:
|
||||
if: ${{ needs.dotnet-update.outputs.SHOULD_UPDATE == 1 && needs.dotnet-update.outputs.BRANCH_EXISTS == 0 }}
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v5
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
ref: feature/dotnetsdk-upgrade/${{ needs.dotnet-update.outputs.DOTNET_LATEST_MAJOR_MINOR_PATCH_VERSION }}
|
||||
- name: Create Pull Request
|
||||
|
||||
22
.github/workflows/release.yml
vendored
22
.github/workflows/release.yml
vendored
@@ -11,7 +11,7 @@ jobs:
|
||||
if: startsWith(github.ref, 'refs/heads/releases/') || github.ref == 'refs/heads/main'
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v5
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
# Make sure ./releaseVersion match ./src/runnerversion
|
||||
# Query GitHub release ensure version is not used
|
||||
@@ -86,7 +86,7 @@ jobs:
|
||||
|
||||
runs-on: ${{ matrix.os }}
|
||||
steps:
|
||||
- uses: actions/checkout@v5
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
# Build runner layout
|
||||
- name: Build & Layout Release
|
||||
@@ -129,41 +129,41 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
|
||||
- uses: actions/checkout@v5
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
# Download runner package tar.gz/zip produced by 'build' job
|
||||
- name: Download Artifact (win-x64)
|
||||
uses: actions/download-artifact@v5
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: runner-packages-win-x64
|
||||
path: ./
|
||||
- name: Download Artifact (win-arm64)
|
||||
uses: actions/download-artifact@v5
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: runner-packages-win-arm64
|
||||
path: ./
|
||||
- name: Download Artifact (osx-x64)
|
||||
uses: actions/download-artifact@v5
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: runner-packages-osx-x64
|
||||
path: ./
|
||||
- name: Download Artifact (osx-arm64)
|
||||
uses: actions/download-artifact@v5
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: runner-packages-osx-arm64
|
||||
path: ./
|
||||
- name: Download Artifact (linux-x64)
|
||||
uses: actions/download-artifact@v5
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: runner-packages-linux-x64
|
||||
path: ./
|
||||
- name: Download Artifact (linux-arm)
|
||||
uses: actions/download-artifact@v5
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: runner-packages-linux-arm
|
||||
path: ./
|
||||
- name: Download Artifact (linux-arm64)
|
||||
uses: actions/download-artifact@v5
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: runner-packages-linux-arm64
|
||||
path: ./
|
||||
@@ -296,7 +296,7 @@ jobs:
|
||||
IMAGE_NAME: ${{ github.repository_owner }}/actions-runner
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v5
|
||||
uses: actions/checkout@v4
|
||||
|
||||
- name: Compute image version
|
||||
id: image
|
||||
|
||||
2
.gitignore
vendored
2
.gitignore
vendored
@@ -27,4 +27,4 @@ TestResults
|
||||
TestLogs
|
||||
.DS_Store
|
||||
.mono
|
||||
**/*.DotSettings.user/tmp/
|
||||
**/*.DotSettings.user
|
||||
@@ -1,73 +0,0 @@
|
||||
# KillMode Change Implementation Summary
|
||||
|
||||
## Problem Addressed
|
||||
|
||||
The question "is this a good idea?" regarding "killmode changing?" has been thoroughly analyzed and addressed through a minimal but impactful change to the GitHub Actions Runner systemd service configuration.
|
||||
|
||||
## Solution Implemented
|
||||
|
||||
**Changed**: `KillMode=process` → `KillMode=mixed` in `src/Misc/layoutbin/actions.runner.service.template`
|
||||
|
||||
## Why This Change Makes Sense
|
||||
|
||||
### Evidence from Codebase Analysis
|
||||
1. **Orphan Process Concerns**: The codebase contains extensive orphan process cleanup mechanisms in:
|
||||
- `JobExtension.cs`: Tracks and cleans up orphan processes using `RUNNER_TRACKING_ID`
|
||||
- `JobDispatcher.cs`: Prevents orphan worker processes
|
||||
- `ProcessInvoker.cs`: Implements process tree termination
|
||||
|
||||
2. **Current Signal Flow**:
|
||||
- systemd → runsvc.sh (SIGTERM) → Node.js process (SIGINT)
|
||||
- Relies on runsvc.sh successfully forwarding signals
|
||||
|
||||
### Benefits of KillMode=mixed
|
||||
|
||||
1. **Maintains Graceful Shutdown**: Main process (runsvc.sh) still receives SIGTERM first
|
||||
2. **Adds Safety Net**: systemd ensures cleanup if signal forwarding fails
|
||||
3. **Better Process Tree Cleanup**: More robust handling of complex job hierarchies
|
||||
4. **Reduced Orphan Risk**: Addresses concerns evident throughout the codebase
|
||||
5. **Container Compatibility**: Better termination of containerized workloads
|
||||
|
||||
## Implementation Details
|
||||
|
||||
### Files Changed
|
||||
- `src/Misc/layoutbin/actions.runner.service.template`: Single line change
|
||||
- Added comprehensive test coverage in `src/Test/L0/Misc/SystemdServiceTemplateL0.cs`
|
||||
- Created analysis documentation and testing tools
|
||||
|
||||
### Testing
|
||||
- ✅ Build succeeds with no errors
|
||||
- ✅ New tests validate the change
|
||||
- ✅ Existing functionality unchanged
|
||||
- ✅ Layout generation includes the change
|
||||
|
||||
## Impact Assessment
|
||||
|
||||
### Risk Level: **LOW**
|
||||
- Only affects service shutdown behavior
|
||||
- No changes to startup or normal operation
|
||||
- Backward compatible with existing signal handling
|
||||
- Testable with standard systemd tools
|
||||
|
||||
### Compatibility
|
||||
- Maintains existing runsvc.sh signal forwarding behavior
|
||||
- Compatible with all existing process handling code
|
||||
- No breaking changes to APIs or interfaces
|
||||
|
||||
## Testing Tools Provided
|
||||
|
||||
Created `/tmp/killmode-test.sh` script that allows administrators to:
|
||||
- Test different KillMode configurations
|
||||
- Compare process cleanup behavior
|
||||
- Validate signal handling works correctly
|
||||
|
||||
## Conclusion
|
||||
|
||||
This change represents a **good idea** because it:
|
||||
1. Addresses real orphan process concerns evident in the codebase
|
||||
2. Provides better reliability with minimal risk
|
||||
3. Maintains existing graceful shutdown behavior
|
||||
4. Adds systemd's robust process cleanup as a safety net
|
||||
5. Requires only a single line change with comprehensive testing
|
||||
|
||||
The implementation follows the principle of making the smallest possible change while addressing the underlying concern about process cleanup reliability.
|
||||
@@ -1,120 +0,0 @@
|
||||
# GitHub Actions Runner KillMode Analysis
|
||||
|
||||
## Problem Statement
|
||||
The question "is this a good idea?" regarding "killmode changing?" asks us to evaluate whether the current systemd `KillMode=process` setting should be changed to a different option.
|
||||
|
||||
## Current Implementation
|
||||
|
||||
### Systemd Service Configuration
|
||||
- **KillMode**: `process` (only main process gets signal)
|
||||
- **KillSignal**: `SIGTERM`
|
||||
- **TimeoutStopSec**: `5min`
|
||||
|
||||
### Signal Handling Flow
|
||||
1. systemd sends SIGTERM to `runsvc.sh` (main process)
|
||||
2. `runsvc.sh` has trap: `trap 'kill -INT $PID' TERM INT`
|
||||
3. Converts SIGTERM → SIGINT and sends to Node.js runner process
|
||||
4. Node.js process handles graceful shutdown
|
||||
|
||||
## Analysis of Current Approach
|
||||
|
||||
### Strengths
|
||||
1. **Graceful Shutdown Control**: Manual signal conversion allows proper Node.js shutdown handling
|
||||
2. **Predictable Behavior**: Only main process receives systemd signals
|
||||
3. **Custom Logic**: Allows for runner-specific shutdown procedures
|
||||
4. **Signal Compatibility**: SIGINT is more commonly handled by Node.js applications
|
||||
|
||||
### Potential Issues
|
||||
1. **Single Point of Failure**: If `runsvc.sh` fails to forward signals, child processes orphaned
|
||||
2. **Complex Chain**: More components in signal propagation path
|
||||
3. **Process Tree Cleanup**: May not handle deep process hierarchies as robustly
|
||||
|
||||
## Orphan Process Context
|
||||
|
||||
The codebase reveals significant effort to handle orphan processes:
|
||||
|
||||
### Evidence from Code Analysis
|
||||
1. **JobExtension.cs**: Dedicated orphan process cleanup mechanism
|
||||
- Tracks processes before/after job execution
|
||||
- Uses `RUNNER_TRACKING_ID` environment variable
|
||||
- Terminates orphan processes at job completion
|
||||
|
||||
2. **JobDispatcher.cs**: Worker process orphan prevention
|
||||
- Explicit waits to prevent orphan worker processes
|
||||
- Handles "zombie worker" scenarios
|
||||
|
||||
3. **ProcessInvoker.cs**: Process tree termination
|
||||
- Implements both Windows and Unix process tree killing
|
||||
- Signal escalation: SIGINT → SIGTERM → SIGKILL
|
||||
|
||||
## Alternative KillMode Options
|
||||
|
||||
### KillMode=control-group
|
||||
**Behavior**: All processes in service's cgroup get SIGTERM, then SIGKILL after timeout
|
||||
|
||||
**Pros**:
|
||||
- Robust cleanup of entire process tree
|
||||
- Built-in systemd guarantees
|
||||
- Simpler signal flow
|
||||
- No dependency on runsvc.sh signal forwarding
|
||||
|
||||
**Cons**:
|
||||
- Less control over shutdown sequence
|
||||
- All processes get SIGTERM simultaneously
|
||||
- May interrupt graceful shutdown of worker processes
|
||||
|
||||
### KillMode=mixed
|
||||
**Behavior**: Main process gets SIGTERM, remaining processes get SIGKILL after timeout
|
||||
|
||||
**Pros**:
|
||||
- Combines benefits of both approaches
|
||||
- Main process can handle graceful shutdown
|
||||
- Systemd ensures process tree cleanup
|
||||
- Fallback protection against orphan processes
|
||||
|
||||
**Cons**:
|
||||
- More complex behavior
|
||||
- Still depends on main process signal handling
|
||||
|
||||
## Security and Reliability Considerations
|
||||
|
||||
### Current Risks
|
||||
1. If `runsvc.sh` crashes before forwarding signals, Node.js process continues running
|
||||
2. Deep process trees from job execution may not be properly cleaned up
|
||||
3. Container processes might not receive proper termination signals
|
||||
|
||||
### Reliability Improvements with control-group/mixed
|
||||
1. systemd guarantees process cleanup regardless of main process behavior
|
||||
2. Reduces risk of orphan processes surviving service shutdown
|
||||
3. More predictable behavior for administrators
|
||||
|
||||
## Recommendation
|
||||
|
||||
### Recommended Change: KillMode=mixed
|
||||
|
||||
**Rationale**:
|
||||
1. **Maintains Graceful Shutdown**: Main process (runsvc.sh) still receives SIGTERM first
|
||||
2. **Adds Safety Net**: systemd ensures cleanup if main process fails to handle signals
|
||||
3. **Reduces Orphan Risk**: Addresses the orphan process concerns evident in the codebase
|
||||
4. **Better Process Tree Handling**: More robust for complex job process hierarchies
|
||||
5. **Container Compatibility**: Better handling of containerized workloads
|
||||
|
||||
### Implementation Impact
|
||||
- **Low Risk**: Change only affects service shutdown behavior
|
||||
- **Backward Compatible**: No changes to startup or normal operation
|
||||
- **Testable**: Can be validated with process monitoring during service stops
|
||||
|
||||
### Alternative Considerations
|
||||
- **KillMode=control-group** could be considered if graceful shutdown proves problematic
|
||||
- Current **KillMode=process** could remain if the signal forwarding is deemed reliable enough
|
||||
|
||||
## Testing Recommendations
|
||||
|
||||
1. Test service shutdown with various job types running
|
||||
2. Verify process cleanup with nested process trees
|
||||
3. Test container job termination scenarios
|
||||
4. Monitor for any regressions in graceful shutdown behavior
|
||||
|
||||
## Conclusion
|
||||
|
||||
Changing to `KillMode=mixed` would provide a good balance between maintaining the current graceful shutdown behavior while adding systemd's robust process cleanup guarantees. This addresses the orphan process concerns evident throughout the codebase while maintaining compatibility.
|
||||
@@ -1,20 +1,13 @@
|
||||
## What's Changed
|
||||
* Update Docker to v28.3.2 and Buildx to v0.26.1 by @github-actions[bot] in https://github.com/actions/runner/pull/3953
|
||||
* Fix if statement structure in update script and variable reference by @salmanmkc in https://github.com/actions/runner/pull/3956
|
||||
* Add V2 flow for runner deletion by @Samirat in https://github.com/actions/runner/pull/3954
|
||||
* Node 20 -> Node 24 migration feature flagging, opt-in and opt-out environment variables by @salmanmkc in https://github.com/actions/runner/pull/3948
|
||||
* Update Node20 and Node24 to latest by @djs-intel in https://github.com/actions/runner/pull/3972
|
||||
* Redirect supported OS doc section to current public Docs location by @corycalahan in https://github.com/actions/runner/pull/3979
|
||||
* Bump Microsoft.NET.Test.Sdk from 17.13.0 to 17.14.1 by @dependabot[bot] in https://github.com/actions/runner/pull/3975
|
||||
* Bump Azure.Storage.Blobs from 12.24.0 to 12.25.0 by @dependabot[bot] in https://github.com/actions/runner/pull/3974
|
||||
* Bump actions/download-artifact from 4 to 5 by @dependabot[bot] in https://github.com/actions/runner/pull/3973
|
||||
* Bump actions/checkout from 4 to 5 by @dependabot[bot] in https://github.com/actions/runner/pull/3982
|
||||
* Try add orchestrationid into user-agent using token claim. by @TingluoHuang in https://github.com/actions/runner/pull/3945
|
||||
* Fix null reference exception in user agent handling by @salmanmkc in https://github.com/actions/runner/pull/3946
|
||||
* Runner Support for executing Node24 Actions by @salmanmkc in https://github.com/actions/runner/pull/3940
|
||||
* Update dotnet sdk to latest version @8.0.412 by @github-actions[bot] in https://github.com/actions/runner/pull/3941
|
||||
|
||||
## New Contributors
|
||||
* @Samirat made their first contribution in https://github.com/actions/runner/pull/3954
|
||||
* @djs-intel made their first contribution in https://github.com/actions/runner/pull/3972
|
||||
* @salmanmkc made their first contribution in https://github.com/actions/runner/pull/3946
|
||||
|
||||
**Full Changelog**: https://github.com/actions/runner/compare/v2.327.1...v2.328.0
|
||||
**Full Changelog**: https://github.com/actions/runner/compare/v2.326.0...v2.327.0
|
||||
|
||||
_Note: Actions Runner follows a progressive release policy, so the latest release might not be available to your enterprise, organization, or repository yet.
|
||||
To confirm which version of the Actions Runner you should expect, please view the download instructions for your enterprise, organization, or repository.
|
||||
|
||||
@@ -6,7 +6,7 @@ After=network.target
|
||||
ExecStart={{RunnerRoot}}/runsvc.sh
|
||||
User={{User}}
|
||||
WorkingDirectory={{RunnerRoot}}
|
||||
KillMode=mixed
|
||||
KillMode=process
|
||||
KillSignal=SIGTERM
|
||||
TimeoutStopSec=5min
|
||||
|
||||
|
||||
@@ -14,19 +14,19 @@
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="Azure.Storage.Blobs" Version="12.25.0" />
|
||||
<PackageReference Include="Azure.Storage.Blobs" Version="12.24.0" />
|
||||
<PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
|
||||
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
|
||||
<PackageReference Include="Microsoft.AspNet.WebApi.Client" Version="6.0.0" />
|
||||
<PackageReference Include="System.Security.Cryptography.Cng" Version="5.0.0" />
|
||||
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="8.0.0" />
|
||||
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="9.0.8" />
|
||||
<PackageReference Include="System.Security.Cryptography.ProtectedData" Version="8.0.0" />
|
||||
<PackageReference Include="Minimatch" Version="2.0.0" />
|
||||
<PackageReference Include="YamlDotNet.Signed" Version="5.3.0" />
|
||||
<PackageReference Include="System.Net.Http" Version="4.3.4" />
|
||||
<PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" />
|
||||
<PackageReference Include="System.Private.Uri" Version="4.3.2" />
|
||||
<PackageReference Include="System.Formats.Asn1" Version="8.0.1" />
|
||||
<PackageReference Include="System.Formats.Asn1" Version="9.0.8" />
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
|
||||
@@ -1,48 +0,0 @@
|
||||
using System;
|
||||
using System.IO;
|
||||
using Xunit;
|
||||
|
||||
namespace GitHub.Runner.Common.Tests.Misc
|
||||
{
|
||||
public sealed class SystemdServiceTemplateL0
|
||||
{
|
||||
[Fact]
|
||||
[Trait("Level", "L0")]
|
||||
[Trait("Category", "Common")]
|
||||
public void ServiceTemplate_ContainsExpectedKillMode()
|
||||
{
|
||||
// Arrange
|
||||
var templatePath = Path.Combine(TestUtil.GetSrcPath(), "Misc", "layoutbin", "actions.runner.service.template");
|
||||
|
||||
// Act
|
||||
var templateContent = File.ReadAllText(templatePath);
|
||||
|
||||
// Assert
|
||||
Assert.Contains("KillMode=mixed", templateContent);
|
||||
Assert.Contains("KillSignal=SIGTERM", templateContent);
|
||||
Assert.Contains("TimeoutStopSec=5min", templateContent);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
[Trait("Level", "L0")]
|
||||
[Trait("Category", "Common")]
|
||||
public void ServiceTemplate_HasValidStructure()
|
||||
{
|
||||
// Arrange
|
||||
var templatePath = Path.Combine(TestUtil.GetSrcPath(), "Misc", "layoutbin", "actions.runner.service.template");
|
||||
|
||||
// Act
|
||||
var templateContent = File.ReadAllText(templatePath);
|
||||
var lines = templateContent.Split('\n', StringSplitOptions.RemoveEmptyEntries);
|
||||
|
||||
// Assert
|
||||
Assert.Contains("[Unit]", lines);
|
||||
Assert.Contains("[Service]", lines);
|
||||
Assert.Contains("[Install]", lines);
|
||||
Assert.Contains("Description={{Description}}", lines);
|
||||
Assert.Contains("ExecStart={{RunnerRoot}}/runsvc.sh", lines);
|
||||
Assert.Contains("User={{User}}", lines);
|
||||
Assert.Contains("WorkingDirectory={{RunnerRoot}}", lines);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1 +1 @@
|
||||
2.328.0
|
||||
2.327.0
|
||||
|
||||
Reference in New Issue
Block a user