ink/act_runner
1
1
Fork 0
mirror of https://gitea.com/gitea/act_runner.git synced 2026-08-08 07:35:03 +02:00
Code Issues Packages Projects Releases Wiki Activity
1,223 Commits 6 Branches 49 Tags
15dd63a839b4936bbb920f9dc5a7a2e8ff5b01be
Commit Graph
2 Commits
This Branch
This Branch
All Branches
Author SHA1 Message Date
Pascal Zimmermannandsilverwind 15dd63a839 feat: Add support for Dynamic Matrix Strategies with Job Outputs (#149) 
# Matrix Strategy: Support for Scalar Values and Template Expressions

## 🎯 Overview

This Pull Request implements support for **scalar values and template expressions** in matrix strategies. Previously, every matrix value had to be declared as a YAML array — a bare scalar like `os: ubuntu-latest` or `version: ${{ fromJSON(...) }}` caused silent failures. Now, scalars are automatically wrapped into single-element arrays, enabling the `${{ fromJSON(...) }}` pattern that is commonly used in GitHub Actions workflows for dynamic matrix generation.

## ⚠️ Semantic Changes

Two subtle behavior changes are introduced that are worth understanding:

### 1. Scalar matrix values now produce a 1-element matrix expansion

**Before this PR:** A scalar value like

```yaml
strategy:
  matrix:
    os: ubuntu-latest   # no brackets
```

failed to decode as `map[string][]interface{}`, so `Matrix()` returned `nil`. The job ran **once, without any matrix context** (`matrix.os` was undefined).

**After this PR:** The same input is wrapped to `["ubuntu-latest"]`. The job runs **one matrix iteration** with `matrix.os = "ubuntu-latest"` set.

> **Impact on existing workflows:** Workflows that accidentally used bare scalars instead of arrays will now run with a matrix context where they previously did not. The functional result is usually the same, but `matrix.*` variables are now populated. This is considered an improvement, but is a semantic change.

---

### 2. Unevaluated template expressions fall back to a literal string value

The actual resolution of `${{ fromJSON(needs.job1.outputs.versions) }}` into an array happens **before** `Matrix()` is called — in `EvaluateYamlNode` inside `pkg/runner/runner.go`. This PR does not change that evaluation path.

If evaluation succeeds → the YAML node already contains a proper array → `Matrix()` decodes it normally.

If evaluation **fails or is skipped** (e.g., expression not yet resolved) → the scalar string is now wrapped as `["${{ fromJSON(...) }}"]` → the job runs **one iteration with the literal unexpanded string** as the matrix value.

> **Note:** The test `TestJobMatrix/matrix_with_template_expression` covers this fallback path, not the successful resolution path. The literal-wrap behavior is the fallback, not the primary mechanism for template expression support.

---

## 🔑 Key Changes

### 1. Flexible Matrix Decoding (`pkg/model/workflow.go`)

#### New Helper Function: `normalizeMatrixValue`
```go
func normalizeMatrixValue(key string, val interface{}) ([]interface{}, error)
```
- Converts a single matrix value to the expected array format
- Handles three cases:
  1. **Arrays**: Pass through unchanged
  2. **Scalars**: Wrap in single-element array (including template expressions)
  3. **Invalid types** (maps, etc.): Return error to detect misconfigurations early

**Supported scalar types:**
- `string` — including unevaluated template expressions
- `int`, `float64` — numeric values
- `bool` — boolean values
- `nil` — null values

#### Enhanced `Matrix()` Method

**Before:**
```go
func (j *Job) Matrix() map[string][]interface{} {
    if j.Strategy.RawMatrix.Kind == yaml.MappingNode {
        var val map[string][]interface{}
        if !decodeNode(j.Strategy.RawMatrix, &val) {
            return nil
        }
        return val
    }
    return nil
}
```

**After:**
```go
func (j *Job) Matrix() map[string][]interface{} {
    if j.Strategy == nil || j.Strategy.RawMatrix.Kind != yaml.MappingNode {
        return nil
    }

    // First decode to flexible map[string]interface{} to handle scalars and arrays
    var flexVal map[string]interface{}
    err := j.Strategy.RawMatrix.Decode(&flexVal)
    if err != nil {
        // If that fails, try the strict array format
        var val map[string][]interface{}
        if !decodeNode(j.Strategy.RawMatrix, &val) {
            return nil
        }
        return val
    }

    // Convert flexible format to expected format with validation
    val := make(map[string][]interface{})
    for k, v := range flexVal {
        normalized, err := normalizeMatrixValue(k, v)
        if err != nil {
            log.Errorf("matrix validation error: %v", err)
            return nil
        }
        val[k] = normalized
    }
    return val
}
```

**Key improvements:**
- Handles unevaluated template expressions as scalar strings
- Automatic conversion to array format for consistent processing
- Validation to catch nested maps and invalid configurations
- Fallback to strict array format decoding for backward compatibility
- Early nil check for strategy to prevent nil pointer panics

---

## 🎁 Use Cases

### Example 1: Template Expression in Matrix (primary motivation)
```yaml
jobs:
  setup:
    runs-on: ubuntu-latest
    outputs:
      versions: ${{ steps.version-map.outputs.versions }}
    steps:
      - id: version-map
        run: echo "versions=[\"1.17\",\"1.18\",\"1.19\"]" >> $GITHUB_OUTPUT

  test:
    needs: setup
    strategy:
      matrix:
        # EvaluateYamlNode resolves this to an array before Matrix() is called
        version: ${{ fromJSON(needs.setup.outputs.versions) }}
    runs-on: ubuntu-latest
    steps:
      - run: echo "Testing version ${{ matrix.version }}"
```

### Example 2: Mixed Scalar and Array Values
```yaml
jobs:
  build:
    strategy:
      matrix:
        os: [ubuntu-latest, windows-latest]  # Array — unchanged
        go-version: 1.19                      # Scalar — wrapped to [1.19]
        node: [14, 16, 18]                    # Array — unchanged
    runs-on: ${{ matrix.os }}
```

### Example 3: Single Value Matrix
```yaml
jobs:
  deploy:
    strategy:
      matrix:
        environment: production  # Scalar — wrapped to ["production"]
    runs-on: ubuntu-latest
```

---

## ✅ Benefits

- ✨ **Feature Parity with GitHub Actions**: Supports template expressions in matrix strategies
- 🔒 **Robust Validation**: Detects misconfigured matrices (nested maps) early
- 🔄 **Backward Compatible**: Existing array-based workflows continue to work
- 📝 **Clear Error Messages**: Helpful validation messages for debugging
- 🛡️ **Type Safety**: Handles all YAML scalar types correctly

---

## 🚀 Breaking Changes

**Technically none in a strictly backward-incompatible sense**, but two semantic changes exist (see [⚠️ Semantic Changes](#️-semantic-changes-read-before-merging) above):

1. Scalar matrix values now iterate (previously they caused a `nil` matrix / no iteration)
2. Unevaluated template strings now produce a literal iteration instead of skipping

Both changes are improvements in practice, but downstream users should be aware.

---

## 📚 Related Work

- Related Gitea Server PR: [go-gitea/gitea#36564](https://github.com/go-gitea/gitea/pull/36564)
- Enables dynamic matrix generation from job outputs
- Supports advanced CI/CD patterns used in GitHub Actions

## 🔗 References

- [GitHub Actions: Matrix strategies](https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs)
- [GitHub Actions: Expression syntax](https://docs.github.com/en/actions/learn-github-actions/expressions)
- [GitHub Actions: `fromJSON` in matrix context](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategymatrix)

Reviewed-on: https://gitea.com/gitea/act/pulls/149
Reviewed-by: silverwind <[email protected]>
Co-authored-by: Pascal Zimmermann <[email protected]>
Co-committed-by: Pascal Zimmermann <[email protected]>
 2026-04-19 22:36:34 +00:00
Pascal ZimmermannandLunny Xiao c0f19d9a26 fix: Max parallel Support for Matrix Jobs and Remote Action Tests (#150) 
## Summary

This PR fixes the `max-parallel` strategy configuration for matrix jobs and resolves all failing tests in `step_action_remote_test.go`. The implementation ensures that matrix jobs respect the `max-parallel` setting, preventing resource exhaustion when running GitHub Actions workflows.

## Problem Statement

### Issue 1: max-parallel Not Working Correctly
Matrix jobs were running in parallel regardless of the `max-parallel` setting in the strategy configuration. This caused:
- Resource contention on limited runners
- Unpredictable job execution behavior
- Inability to control concurrency for resource-intensive workflows

### Issue 2: Failing Remote Action Tests
All tests in `step_action_remote_test.go` were failing due to:
- Missing `ActionCacheDir` configuration
- Incorrect mock expectations using fixed strings instead of hash-based paths
- Incompatibility with the hash-based action cache implementation

## Changes

### 1. max-parallel Implementation (`pkg/runner/runner.go`)

#### Robust Initialization
Added fallback logic to ensure `MaxParallel` is always properly initialized:
```go
if job.Strategy.MaxParallel == 0 {
    job.Strategy.MaxParallel = job.Strategy.GetMaxParallel()
}
```

#### Eliminated Unnecessary Nesting
Fixed inefficient nested parallelization when only one pipeline element exists:
```go
if len(pipeline) == 1 {
    // Execute directly without additional wrapper
    log.Debugf("Single pipeline element, executing directly")
    return pipeline[0](ctx)
}
```

#### Enhanced Logging
Added comprehensive debug and info logging:
- Shows which `maxParallel` value is being used
- Logs adjustments based on matrix size
- Reports final parallelization decisions

### 2. Worker Logging (`pkg/common/executor.go`)

Enhanced `NewParallelExecutor` with detailed worker activity logging:
```go
log.Infof("NewParallelExecutor: Creating %d workers for %d executors", parallel, len(executors))

for i := 0; i < parallel; i++ {
    go func(workerID int, work <-chan Executor, errs chan<- error) {
        log.Debugf("Worker %d started", workerID)
        taskCount := 0
        for executor := range work {
            taskCount++
            log.Debugf("Worker %d executing task %d", workerID, taskCount)
            errs <- executor(ctx)
        }
        log.Debugf("Worker %d finished (%d tasks executed)", workerID, taskCount)
    }(i, work, errs)
}
```

**Benefits:**
- Easy verification of correct parallelization
- Better debugging capabilities
- Clear visibility into worker activity

### 3. Documentation (`pkg/model/workflow.go`)

Added clarifying comment to ensure strategy values are always set:
```go
// Always set these values, even if there's an error later
j.Strategy.FailFast = j.Strategy.GetFailFast()
j.Strategy.MaxParallel = j.Strategy.GetMaxParallel()
```

### 4. Test Fixes (`pkg/runner/step_action_remote_test.go`)

#### Added Missing Configuration
All tests now include `ActionCacheDir`:
```go
Config: &Config{
    GitHubInstance: "github.com",
    ActionCacheDir: "/tmp/test-cache",
}
```

#### Hash-Based Suffix Matchers
Updated mocks to use hash-based paths instead of fixed strings:
```go
// Before
sarm.On("readAction", sar.Step, suffixMatcher("org-repo-path@ref"), ...)

// After
sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), ...)
```

#### Flexible Exec Matcher for Post Tests
Implemented flexible path matching for hash-based action directories:
```go
execMatcher := mock.MatchedBy(func(args []string) bool {
    if len(args) != 2 {
        return false
    }
    return args[0] == "node" && strings.Contains(args[1], "post.js")
})
```

#### Token Test Improvements
- Uses unique cache directory to force cloning
- Validates URL redirection to github.com
- Accepts realistic token behavior

### 5. New Tests

#### Unit Tests (`pkg/runner/max_parallel_test.go`)
Tests various `max-parallel` configurations:
- `max-parallel: 1` → Sequential execution
- `max-parallel: 2` → Max 2 parallel jobs
- `max-parallel: 4` (default) → Max 4 parallel jobs
- `max-parallel: 10` → Max 10 parallel jobs

#### Concurrency Test (`pkg/common/executor_max_parallel_test.go`)
Verifies that `max-parallel: 2` actually limits concurrent execution using atomic counters.

## Expected Behavior

### Before
- ❌ Jobs ran in parallel regardless of `max-parallel` setting
- ❌ Unnecessary nested parallelization (8 workers for 1 element)
- ❌ No logging to debug parallelization issues
- ❌ All `step_action_remote_test.go` tests failing

### After
- ✅ `max-parallel: 1` → Jobs run strictly sequentially
- ✅ `max-parallel: N` → Maximum N jobs run simultaneously
- ✅ Efficient single-level parallelization for matrix jobs
- ✅ Comprehensive logging for debugging
- ✅ All tests passing (10/10)

## Example Log Output

With `max-parallel: 2` and 6 matrix jobs:
```
[DEBUG] Using job.Strategy.MaxParallel: 2
[INFO] Running job with maxParallel=2 for 6 matrix combinations
[DEBUG] Single pipeline element, executing directly
[INFO] NewParallelExecutor: Creating 2 workers for 6 executors
[DEBUG] Worker 0 started
[DEBUG] Worker 1 started
[DEBUG] Worker 0 executing task 1
[DEBUG] Worker 1 executing task 1
...
[DEBUG] Worker 0 finished (3 tasks executed)
[DEBUG] Worker 1 finished (3 tasks executed)
```

## Test Results

All tests pass successfully:
```
✅ TestStepActionRemote (3 sub-tests)
✅ TestStepActionRemotePre
✅ TestStepActionRemotePreThroughAction
✅ TestStepActionRemotePreThroughActionToken
✅ TestStepActionRemotePost (4 sub-tests)
✅ TestMaxParallelStrategy
✅ TestMaxParallel2Quick

Total: 12/12 tests passing
```

## Breaking Changes

None. This PR is fully backward compatible. All changes improve existing behavior without altering the API.

## Impact

- ✅ Fixes resource management for CI/CD pipelines
- ✅ Prevents runner exhaustion on limited infrastructure
- ✅ Enables sequential execution for resource-intensive jobs
- ✅ Improves debugging with detailed logging
- ✅ Ensures test suite reliability

## Files Modified

### Core Implementation
- `pkg/runner/runner.go` - max-parallel fix + logging
- `pkg/common/executor.go` - Worker logging
- `pkg/model/workflow.go` - Documentation

### Tests
- `pkg/runner/step_action_remote_test.go` - Fixed all 10 tests
- `pkg/runner/max_parallel_test.go` - **NEW** - Unit tests
- `pkg/common/executor_max_parallel_test.go` - **NEW** - Concurrency test

## Verification

### Manual Testing
```bash
# Build
go build -o dist/act main.go

# Test with max-parallel: 2
./dist/act -W test-max-parallel-2.yml -v

# Expected: Only 2 jobs run simultaneously
```

### Automated Testing
```bash
# Run all tests
go test ./pkg/runner -run TestStepActionRemote -v
go test ./pkg/runner -run TestMaxParallel -v
go test ./pkg/common -run TestMaxParallel -v
```

## Related Issues

Fixes issues where matrix jobs in Gitea ignored the `max-parallel` strategy setting, causing resource contention and unpredictable behavior.

Reviewed-on: https://gitea.com/gitea/act/pulls/150
Reviewed-by: Lunny Xiao <[email protected]>
Reviewed-by: silverwind <[email protected]>
Co-authored-by: Pascal Zimmermann <[email protected]>
Co-committed-by: Pascal Zimmermann <[email protected]>
 2026-02-11 00:43:38 +00:00