diff --git a/pkg/services/rendering/http_mode.go b/pkg/services/rendering/http_mode.go index 2dca4749b04..5511b300387 100644 --- a/pkg/services/rendering/http_mode.go +++ b/pkg/services/rendering/http_mode.go @@ -28,6 +28,11 @@ var netClient = &http.Client{ Transport: netTransport, } +var ( + remoteVersionFetchInterval time.Duration = time.Second * 15 + remoteVersionFetchRetries uint = 4 +) + func (rs *RenderingService) renderViaHTTP(ctx context.Context, renderKey string, opts Opts) (*RenderResult, error) { filePath, err := rs.getNewFilePath(RenderPNG) if err != nil { @@ -194,6 +199,24 @@ func (rs *RenderingService) readFileResponse(ctx context.Context, resp *http.Res return nil } +func (rs *RenderingService) getRemotePluginVersionWithRetry(callback func(string, error)) { + go func() { + var err error + for try := uint(0); try < remoteVersionFetchRetries; try++ { + version, err := rs.getRemotePluginVersion() + if err == nil { + callback(version, err) + return + } + rs.log.Info("Couldn't get remote renderer version, retrying", "err", err, "try", try) + + time.Sleep(remoteVersionFetchInterval) + } + + callback("", err) + }() +} + func (rs *RenderingService) getRemotePluginVersion() (string, error) { rendererURL, err := url.Parse(rs.Cfg.RendererUrl + "/version") if err != nil { @@ -212,7 +235,10 @@ func (rs *RenderingService) getRemotePluginVersion() (string, error) { } }() - if resp.StatusCode != http.StatusOK { + if resp.StatusCode == http.StatusNotFound { + // Old versions of the renderer lacked the version endpoint + return "1.0.0", nil + } else if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("remote rendering request to get version failed, status code: %d, status: %s", resp.StatusCode, resp.Status) } diff --git a/pkg/services/rendering/rendering.go b/pkg/services/rendering/rendering.go index 60cf4d93298..4d113707e34 100644 --- a/pkg/services/rendering/rendering.go +++ b/pkg/services/rendering/rendering.go @@ -10,6 +10,7 @@ import ( "path" "path/filepath" "strings" + "sync" "sync/atomic" "time" @@ -43,6 +44,7 @@ type RenderingService struct { domain string inProgressCount int32 version string + versionMutex sync.RWMutex Cfg *setting.Cfg RemoteCacheService *remotecache.RemoteCache @@ -93,13 +95,18 @@ func (rs *RenderingService) Run(ctx context.Context) error { if rs.remoteAvailable() { rs.log = rs.log.New("renderer", "http") - version, err := rs.getRemotePluginVersion() - if err != nil { - rs.log.Info("Couldn't get remote renderer version", "err", err) - } + rs.getRemotePluginVersionWithRetry(func(version string, err error) { + if err != nil { + rs.log.Info("Couldn't get remote renderer version", "err", err) + } - rs.log.Info("Backend rendering via external http server", "version", version) - rs.version = version + rs.log.Info("Backend rendering via external http server", "version", version) + + rs.versionMutex.Lock() + defer rs.versionMutex.Unlock() + + rs.version = version + }) rs.renderAction = rs.renderViaHTTP rs.renderCSVAction = rs.renderCSVViaHTTP <-ctx.Done() @@ -153,6 +160,9 @@ func (rs *RenderingService) IsAvailable() bool { } func (rs *RenderingService) Version() string { + rs.versionMutex.RLock() + defer rs.versionMutex.RUnlock() + return rs.version } diff --git a/pkg/services/rendering/rendering_test.go b/pkg/services/rendering/rendering_test.go index f552f3ff988..e7776d82cc7 100644 --- a/pkg/services/rendering/rendering_test.go +++ b/pkg/services/rendering/rendering_test.go @@ -3,9 +3,13 @@ package rendering import ( "context" "errors" + "net/http" + "net/http/httptest" "path/filepath" "testing" + "time" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -139,3 +143,68 @@ func TestRenderLimitImage(t *testing.T) { }) } } + +func TestRenderingServiceGetRemotePluginVersion(t *testing.T) { + cfg := setting.NewCfg() + rs := &RenderingService{ + Cfg: cfg, + log: log.New("rendering-test"), + } + + t.Run("When renderer responds with correct version should return that version", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte("{\"version\":\"2.7.1828\"}")) + require.NoError(t, err) + })) + defer server.Close() + + rs.Cfg.RendererUrl = server.URL + "/render" + version, err := rs.getRemotePluginVersion() + + require.NoError(t, err) + require.Equal(t, "2.7.1828", version) + }) + + t.Run("When renderer responds with 404 should assume a valid but old version", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + + rs.Cfg.RendererUrl = server.URL + "/render" + version, err := rs.getRemotePluginVersion() + + require.NoError(t, err) + require.Equal(t, version, "1.0.0") + }) + + t.Run("When renderer responds with 500 should retry until success", func(t *testing.T) { + tries := uint(0) + ctx, cancel := context.WithCancel(context.Background()) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + tries++ + + if tries < remoteVersionFetchRetries { + w.WriteHeader(http.StatusInternalServerError) + } else { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte("{\"version\":\"3.1.4159\"}")) + require.NoError(t, err) + cancel() + } + })) + defer server.Close() + + rs.Cfg.RendererUrl = server.URL + "/render" + remoteVersionFetchInterval = time.Millisecond + remoteVersionFetchRetries = 5 + go func() { + require.NoError(t, rs.Run(ctx)) + }() + + require.Eventually(t, func() bool { return rs.Version() == "3.1.4159" }, time.Second, time.Millisecond) + }) +}