mirror of
https://code.forgejo.org/forgejo/runner.git
synced 2025-10-20 19:52:06 +00:00
fix: don't panic on cacheproxy startup failure (#1067)
In #1064 I theorized that a failure to start up the cache proxy server might cause it to still be registered as `ACTIONS_CACHE_URL`. It turns out to not be the case as it will just panic, since `cacheProxy` is `nil`. But regardless, it seems better to not panic and crash if the intent here is "cache will be disabled" as documented in the log message. <!--start release-notes-assistant--> <!--URL:https://code.forgejo.org/forgejo/runner--> - bug fixes - [PR](https://code.forgejo.org/forgejo/runner/pulls/1067): <!--number 1067 --><!--line 0 --><!--description Zml4OiBkb24ndCBwYW5pYyBvbiBjYWNoZXByb3h5IHN0YXJ0dXAgZmFpbHVyZQ==-->fix: don't panic on cacheproxy startup failure<!--description--> <!--end release-notes-assistant--> Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1067 Reviewed-by: earl-warren <earl-warren@noreply.code.forgejo.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
parent
a22c5a2e65
commit
0a0b25d886
2 changed files with 95 additions and 2 deletions
|
@ -150,10 +150,10 @@ func setupCache(cfg *config.Config, envs map[string]string) *cacheproxy.Handler
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Errorf("cannot init cache proxy, cache will be disabled: %v", err)
|
log.Errorf("cannot init cache proxy, cache will be disabled: %v", err)
|
||||||
|
} else {
|
||||||
|
envs["ACTIONS_CACHE_URL"] = cacheProxy.ExternalURL()
|
||||||
}
|
}
|
||||||
|
|
||||||
envs["ACTIONS_CACHE_URL"] = cacheProxy.ExternalURL()
|
|
||||||
|
|
||||||
return cacheProxy
|
return cacheProxy
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -4,6 +4,7 @@ import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"net"
|
||||||
"os"
|
"os"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
@ -421,6 +422,98 @@ jobs:
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunnerCacheStartupFailure(t *testing.T) {
|
||||||
|
if testing.Short() {
|
||||||
|
t.Skip("skipping integration test")
|
||||||
|
}
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
desc string
|
||||||
|
listen string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
desc: "disable cache server",
|
||||||
|
listen: "127.0.0.1:40715",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
desc: "disable cache proxy server",
|
||||||
|
listen: "127.0.0.1:40716",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.desc, func(t *testing.T) {
|
||||||
|
forgejoClient := &forgejoClientMock{}
|
||||||
|
|
||||||
|
forgejoClient.On("Address").Return("https://127.0.0.1:8080") // not expected to be used in this test
|
||||||
|
forgejoClient.On("UpdateLog", mock.Anything, mock.Anything).Return(nil, nil)
|
||||||
|
forgejoClient.On("UpdateTask", mock.Anything, mock.Anything).
|
||||||
|
Return(connect.NewResponse(&runnerv1.UpdateTaskResponse{}), nil)
|
||||||
|
|
||||||
|
// We'll be listening on some network port in this test that will conflict with the cache configuration...
|
||||||
|
l, err := net.Listen("tcp4", tc.listen)
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer l.Close()
|
||||||
|
|
||||||
|
runner := NewRunner(
|
||||||
|
&config.Config{
|
||||||
|
Cache: config.Cache{
|
||||||
|
Port: 40715,
|
||||||
|
ProxyPort: 40716,
|
||||||
|
Dir: t.TempDir(),
|
||||||
|
},
|
||||||
|
Host: config.Host{
|
||||||
|
WorkdirParent: t.TempDir(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
&config.Registration{
|
||||||
|
Labels: []string{"ubuntu-latest:docker://code.forgejo.org/oci/node:20-bookworm"},
|
||||||
|
},
|
||||||
|
forgejoClient)
|
||||||
|
require.NotNil(t, runner)
|
||||||
|
|
||||||
|
// Ensure that cacheProxy failed to start
|
||||||
|
assert.Nil(t, runner.cacheProxy)
|
||||||
|
|
||||||
|
runWorkflow := func(ctx context.Context, cancel context.CancelFunc, yamlContent string) {
|
||||||
|
task := &runnerv1.Task{
|
||||||
|
WorkflowPayload: []byte(yamlContent),
|
||||||
|
Context: &structpb.Struct{
|
||||||
|
Fields: map[string]*structpb.Value{
|
||||||
|
"token": structpb.NewStringValue("some token here"),
|
||||||
|
"forgejo_default_actions_url": structpb.NewStringValue("https://data.forgejo.org"),
|
||||||
|
"repository": structpb.NewStringValue("runner"),
|
||||||
|
"event_name": structpb.NewStringValue("push"),
|
||||||
|
"ref": structpb.NewStringValue("refs/heads/main"),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
reporter := report.NewReporter(ctx, cancel, forgejoClient, task, time.Second)
|
||||||
|
err := runner.run(ctx, task, reporter)
|
||||||
|
reporter.Close(nil)
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(t.Context())
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
checkCacheYaml := `
|
||||||
|
name: Verify No ACTIONS_CACHE_URL
|
||||||
|
on:
|
||||||
|
push:
|
||||||
|
jobs:
|
||||||
|
job-cache-check-1:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- run: echo $ACTIONS_CACHE_URL
|
||||||
|
- run: '[[ "$ACTIONS_CACHE_URL" = "" ]] || exit 1'
|
||||||
|
`
|
||||||
|
runWorkflow(ctx, cancel, checkCacheYaml)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestRunnerLXC(t *testing.T) {
|
func TestRunnerLXC(t *testing.T) {
|
||||||
if testing.Short() {
|
if testing.Short() {
|
||||||
t.Skip("skipping integration test")
|
t.Skip("skipping integration test")
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue