Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix noisy runs-on error logging #2102

Merged
merged 3 commits into from
Dec 16, 2023
Merged

Fix noisy runs-on error logging #2102

merged 3 commits into from
Dec 16, 2023

Conversation

jenseng
Copy link
Contributor

@jenseng jenseng commented Nov 23, 2023 •
edited
Loading

Move the logging back up a level to fix a minor logging issue introduced in #2088

The RunContext for a composite action has a dummy/blank Job with no runs-on, meaning its calls to withGithubEnv would result in an inaccurate log message complaining that 'runs-on' key not defined in ...

@jenseng
Fix noisy runs-on error logging
1cd1a0e 
Move the logging back up a level to fix a minor logging issue introduced in nektos#2088

`RunContext`s for composite actions have dummy/blank `Job`s with no `runs-on`,
meaning their calls to `withGithubEnv` would result in an inaccurate log message
complaining that `'runs-on' key not defined in ...`
@jenseng jenseng force-pushed the fix-runs-on-error-logging branch from 96cd6cb to 1cd1a0e Compare November 23, 2023 07:05
@codecov Codecov
Copy link

codecov bot commented Nov 23, 2023 •
edited
Loading

Codecov Report

Attention: 872 lines in your changes are missing coverage. Please review.

Comparison is base (4989f44) 61.22% compared to head (676db9a) 61.64%.
Report is 285 commits behind head on master.

Files Patch % Lines
pkg/artifactcache/handler.go 65.46% 102 Missing and 42 partials ⚠️
pkg/runner/run_context.go 73.37% 75 Missing and 19 partials ⚠️
pkg/runner/expression.go 55.17% 66 Missing and 12 partials ⚠️
pkg/runner/action_cache.go 50.74% 49 Missing and 17 partials ⚠️
pkg/container/docker_network.go 0.00% 56 Missing ⚠️
pkg/container/docker_run.go 2.00% 48 Missing and 1 partial ⚠️
pkg/model/workflow.go 43.37% 40 Missing and 7 partials ⚠️
pkg/common/outbound_ip.go 0.00% 44 Missing ⚠️
pkg/container/host_environment.go 0.00% 43 Missing ⚠️
pkg/artifactcache/storage.go 60.22% 24 Missing and 11 partials ⚠️
... and 25 more
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2102      +/-   ##
==========================================
+ Coverage   61.22%   61.64%   +0.42%     
==========================================
  Files          46       53       +7     
  Lines        7141     8808    +1667     
==========================================
+ Hits         4372     5430    +1058     
- Misses       2462     2948     +486     
- Partials      307      430     +123

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jenseng jenseng marked this pull request as ready for review November 23, 2023 07:25
@jenseng jenseng requested a review from a team as a code owner November 23, 2023 07:25
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 13, 2023

@jenseng this pull request has failed checks 🛠

@mergify mergify bot added the needs-work Extra attention is needed label Dec 13, 2023
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 16, 2023

@jenseng this pull request has failed checks 🛠

@ChristopherHX
Copy link
Contributor

Hmm,

fatal error: concurrent map iteration and map write
  
  goroutine 26095 [running]:
  github.com/nektos/act/pkg/runner.valueMasker.func1(0xc0001d6540)
  	/home/runner/work/act/act/pkg/runner/logger.go:154 +0xa9
  github.com/nektos/act/pkg/runner.(*maskedFormatter).Format(0xc0004888d0, 0xc000232f58?)
  	/home/runner/work/act/act/pkg/runner/logger.go:176 +0x64
  github.com/sirupsen/logrus.(*Entry).write(0xc0001d6540)
  	/home/runner/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:289 +0xa5
  github.com/sirupsen/logrus.(*Entry).log(0xc0001da3f0, 0x5, {0xc0009c2a20, 0x23})
  	/home/runner/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:252 +0x474
  github.com/sirupsen/logrus.(*Entry).Log(0xc0001da3f0, 0x5, {0xc0002330f0?, 0x0?, 0x0?})
  	/home/runner/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:304 +0x4f
  github.com/sirupsen/logrus.(*Entry).Logf(0xc0001da3f0, 0x5, {0xd016d2?, 0xcd0fa0?}, {0x0?, 0xa?, 0x40b18d?})
  	/home/runner/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:349 +0x85
  github.com/sirupsen/logrus.(*Entry).Debugf(0xe2e410?, {0xd016d2?, 0xbcca2f?}, {0x0?, 0xc0002331b0?, 0xc0002331b0?})
  	/home/runner/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:358 +0x37
  github.com/nektos/act/pkg/common/git.FindGitRef({0xe2e410, 0xc00032ae40}, {0xc000276c30, 0x2d})
  	/home/runner/work/act/act/pkg/common/git/git.go:90 +0x75
  github.com/nektos/act/pkg/model.(*GithubContext).SetRef(0xc000540960, {0xe2e410, 0xc00032ae40}, {0x0, 0x0}, {0xc000276c30, 0x2d})
  	/home/runner/work/act/act/pkg/model/github_context.go:120 +0x735
  github.com/nektos/act/pkg/runner.(*RunContext).getGithubContext(0xc000a65320, {0xe2e410, 0xc00032ae40})
  	/home/runner/work/act/act/pkg/runner/run_context.go:834 +0xd30
  github.com/nektos/act/pkg/runner.(*stepRun).getGithubContext(0xe2e410?, {0xe2e410?, 0xc00032ae40?})
  	/home/runner/work/act/act/pkg/runner/step_run.go:57 +0xa5
  github.com/nektos/act/pkg/runner.(*RunContext).NewStepExpressionEvaluator(0xc000a65320, {0xe2e410, 0xc00032ae40}, {0xe31270?, 0xc0005bfcc0})
  	/home/runner/work/act/act/pkg/runner/expression.go:134 +0x642
  github.com/nektos/act/pkg/runner.isStepEnabled({0xe2e410, 0xc00032ae40}, {0x0, 0x0}, {0xe31270?, 0xc0005bfcc0?}, 0x1)
  	/home/runner/work/act/act/pkg/runner/step.go:251 +0x125
  github.com/nektos/act/pkg/runner.runStepExecutor.func1({0xe2e410, 0xc00032ae40})
  	/home/runner/work/act/act/pkg/runner/step.go:84 +0x279
  github.com/nektos/act/pkg/runner.newJobExecutor.func4({0xe2e410, 0xc00032ae40})
  	/home/runner/work/act/act/pkg/runner/job_executor.go:78 +0x6a
  github.com/nektos/act/pkg/runner.useStepLogger.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/runner/job_executor.go:197 +0x443
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:148 +0x144
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Finally.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:182 +0x34
  github.com/nektos/act/pkg/common.Executor.Finally.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:182 +0x34
  github.com/nektos/act/pkg/common.Executor.Finally.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:182 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:148 +0x144
  github.com/nektos/act/pkg/runner.(*RunContext).Executor.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/runner/run_context.go:620 +0x8b
  github.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1.1({0xe2e410, 0xc00052f9b0})
  	/home/runner/work/act/act/pkg/runner/runner.go:192 +0x1e3
  github.com/nektos/act/pkg/common.NewParallelExecutor.func1.1(0x803a26?, 0xc000a9a000?)
  	/home/runner/work/act/act/pkg/common/executor.go:107 +0x5c
  created by github.com/nektos/act/pkg/common.NewParallelExecutor.func1
  	/home/runner/work/act/act/pkg/common/executor.go:105 +0x10b

weird, never saw this crash in the tests.

@mergify mergify bot removed the needs-work Extra attention is needed label Dec 16, 2023
@mergify mergify bot merged commit 00fbfa7 into nektos:master Dec 16, 2023
10 checks passed
jmikedupont2 pushed a commit to meta-introspector/act that referenced this pull request Mar 10, 2024
@jenseng @wolfogre @mergify
Fix noisy runs-on error logging (nektos#2102)
7000822 
Move the logging back up a level to fix a minor logging issue introduced in nektos#2088

`RunContext`s for composite actions have dummy/blank `Job`s with no `runs-on`,
meaning their calls to `withGithubEnv` would result in an inaccurate log message
complaining that `'runs-on' key not defined in ...`

Co-authored-by: Jason Song <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristopherHX ChristopherHX ChristopherHX approved these changes

@wolfogre wolfogre wolfogre approved these changes

Assignees
No one assigned
Labels
size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jenseng @ChristopherHX @wolfogre