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.

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

fix(various): Small fixes before release #559

Merged
merged 1 commit into from
Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/feature_request.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Feature request
about: Suggest an idea for this project
title: "[FEATURE REQUEST]"
labels: C-feature-request
assignees: 48d90782, wolfy-j
assignees: 48d90782

---

Expand Down
2 changes: 1 addition & 1 deletion .rr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ http:
supervisor:
# watch_tick defines how often to check the state of the workers (seconds)
watch_tick: 1s
# ttl defines maximum time worker is allowed to live (seconds)
# ttl defines maximum time worker is allowed to live (seconds) (soft)
ttl: 0
# idle_ttl defines maximum duration worker can spend in idle mode after first use. Disabled when 0 (seconds)
idle_ttl: 10s
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ v2.0.0-RC.4 (20.02.2021)

- PHP tests use latest signatures (https://github.com/spiral/roadrunner/pull/550)
- Endure container update to v1.0.0-RC.2 version
- Remove unneeded mutex from the `http.Workers` method.
- Rename `checker` plugin package to `status`, remove `/v1` endpoint prefix (#557)
- Add static, headers, status, gzip plugins to the `main.go`.

v2.0.0-RC.3 (17.02.2021)
-------------------
Expand Down
8 changes: 4 additions & 4 deletions plugins/gzip/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@ import (

const PluginName = "gzip"

type Gzip struct{}
type Plugin struct{}

// needed for the Endure
func (g *Gzip) Init() error {
func (g *Plugin) Init() error {
return nil
}

func (g *Gzip) Middleware(next http.Handler) http.HandlerFunc {
func (g *Plugin) Middleware(next http.Handler) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
gziphandler.GzipHandler(next).ServeHTTP(w, r)
}
}

func (g *Gzip) Name() string {
func (g *Plugin) Name() string {
return PluginName
}
17 changes: 6 additions & 11 deletions plugins/http/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ import (
"github.com/spiral/errors"
"github.com/spiral/roadrunner/v2/pkg/pool"
"github.com/spiral/roadrunner/v2/pkg/worker"
"github.com/spiral/roadrunner/v2/plugins/checker"
"github.com/spiral/roadrunner/v2/plugins/config"
"github.com/spiral/roadrunner/v2/plugins/http/attributes"
httpConfig "github.com/spiral/roadrunner/v2/plugins/http/config"
"github.com/spiral/roadrunner/v2/plugins/logger"
"github.com/spiral/roadrunner/v2/plugins/server"
"github.com/spiral/roadrunner/v2/plugins/status"
"github.com/spiral/roadrunner/v2/utils"
"golang.org/x/net/http2"
"golang.org/x/net/http2/h2c"
Expand Down Expand Up @@ -109,12 +109,9 @@ func (s *Plugin) Init(cfg config.Configurer, log logger.Logger, server server.Se

func (s *Plugin) logCallback(event interface{}) {
if ev, ok := event.(ResponseEvent); ok {
s.log.Debug("",
s.log.Debug(fmt.Sprintf("%d %s %s", ev.Response.Status, ev.Request.Method, ev.Request.URI),
"remote", ev.Request.RemoteAddr,
"ts", ev.Elapsed().String(),
"resp.status", ev.Response.Status,
"method", ev.Request.Method,
"uri", ev.Request.URI,
"elapsed", ev.Elapsed().String(),
)
}
}
Expand Down Expand Up @@ -303,8 +300,6 @@ func (s *Plugin) ServeHTTP(w http.ResponseWriter, r *http.Request) {

// Workers returns associated pool workers
func (s *Plugin) Workers() []worker.BaseProcess {
s.Lock()
defer s.Unlock()
return s.pool.Workers()
}

Expand Down Expand Up @@ -365,17 +360,17 @@ func (s *Plugin) AddMiddleware(name endure.Named, m Middleware) {
}

// Status return status of the particular plugin
func (s *Plugin) Status() checker.Status {
func (s *Plugin) Status() status.Status {
workers := s.Workers()
for i := 0; i < len(workers); i++ {
if workers[i].State().IsActive() {
return checker.Status{
return status.Status{
Code: http.StatusOK,
}
}
}
// if there are no workers, threat this as error
return checker.Status{
return status.Status{
Code: http.StatusInternalServerError,
}
}
Expand Down
3 changes: 1 addition & 2 deletions plugins/static/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (s *Plugin) Init(cfg config.Configurer, log logger.Logger) error {

err = s.cfg.Valid()
if err != nil {
return errors.E(op, errors.Disabled, err)
return errors.E(op, err)
}

return nil
Expand Down Expand Up @@ -88,7 +88,6 @@ func (s *Plugin) handleStatic(w http.ResponseWriter, r *http.Request) bool {

f, err := s.root.Open(fPath)
if err != nil {
s.log.Error("file open error", "error", err)
if s.cfg.AlwaysServe(fPath) {
w.WriteHeader(404)
return true
Expand Down
2 changes: 1 addition & 1 deletion plugins/checker/config.go → plugins/status/config.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package checker
package status

type Config struct {
Address string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package checker
package status

// Status consists of status code from the service
type Status struct {
Expand Down
15 changes: 7 additions & 8 deletions plugins/checker/plugin.go → plugins/status/plugin.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
package checker
package status

import (
"fmt"
"net/http"
"time"

"github.com/gofiber/fiber/v2"
fiberLogger "github.com/gofiber/fiber/v2/middleware/logger"
endure "github.com/spiral/endure/pkg/container"
"github.com/spiral/errors"
"github.com/spiral/roadrunner/v2/plugins/config"
Expand Down Expand Up @@ -43,12 +42,12 @@ func (c *Plugin) Init(log logger.Logger, cfg config.Configurer) error {
func (c *Plugin) Serve() chan error {
errCh := make(chan error, 1)
c.server = fiber.New(fiber.Config{
ReadTimeout: time.Second * 5,
WriteTimeout: time.Second * 5,
IdleTimeout: time.Second * 5,
ReadTimeout: time.Second * 5,
WriteTimeout: time.Second * 5,
IdleTimeout: time.Second * 5,
DisableStartupMessage: true,
})
c.server.Group("/v1", c.healthHandler)
c.server.Use(fiberLogger.New())

c.server.Use("/health", c.healthHandler)

go func() {
Expand Down Expand Up @@ -120,7 +119,7 @@ func (c *Plugin) healthHandler(ctx *fiber.Ctx) error {

if len(plugins.Plugins) == 0 {
ctx.Status(http.StatusOK)
_, _ = ctx.WriteString("No plugins provided in query. Query should be in form of: /v1/health?plugin=plugin1&plugin=plugin2 \n")
_, _ = ctx.WriteString("No plugins provided in query. Query should be in form of: health?plugin=plugin1&plugin=plugin2 \n")
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/checker/rpc.go → plugins/status/rpc.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package checker
package status

import (
"github.com/spiral/errors"
Expand Down
3 changes: 1 addition & 2 deletions tests/plugins/checker/configs/.rr-checker-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ server:
command: "php ../../http/client.php echo pipes"
user: ""
group: ""
env:
"RR_HTTP": "true"
relay: "pipes"
relay_timeout: "20s"

status:
address: "127.0.0.1:34333"

logs:
mode: development
level: error
Expand Down
10 changes: 5 additions & 5 deletions tests/plugins/checker/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import (

endure "github.com/spiral/endure/pkg/container"
goridgeRpc "github.com/spiral/goridge/v3/pkg/rpc"
"github.com/spiral/roadrunner/v2/plugins/checker"
"github.com/spiral/roadrunner/v2/plugins/config"
httpPlugin "github.com/spiral/roadrunner/v2/plugins/http"
"github.com/spiral/roadrunner/v2/plugins/logger"
rpcPlugin "github.com/spiral/roadrunner/v2/plugins/rpc"
"github.com/spiral/roadrunner/v2/plugins/server"
"github.com/spiral/roadrunner/v2/plugins/status"
"github.com/stretchr/testify/assert"
)

Expand All @@ -37,7 +37,7 @@ func TestStatusHttp(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&checker.Plugin{},
&status.Plugin{},
)
assert.NoError(t, err)

Expand Down Expand Up @@ -95,7 +95,7 @@ const resp = `Service: http: Status: 200
Service: rpc not found`

func checkHTTPStatus(t *testing.T) {
req, err := http.NewRequest("GET", "http://127.0.0.1:34333/v1/health?plugin=http&plugin=rpc", nil)
req, err := http.NewRequest("GET", "http://127.0.0.1:34333/health?plugin=http&plugin=rpc", nil)
assert.NoError(t, err)

r, err := http.DefaultClient.Do(req)
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestStatusRPC(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&checker.Plugin{},
&status.Plugin{},
)
assert.NoError(t, err)

Expand Down Expand Up @@ -182,7 +182,7 @@ func checkRPCStatus(t *testing.T) {
assert.NoError(t, err)
client := rpc.NewClientWithCodec(goridgeRpc.NewClientCodec(conn))

st := &checker.Status{}
st := &status.Status{}

err = client.Call("status.Status", "http", &st)
assert.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions tests/plugins/gzip/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestGzipPlugin(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
)
assert.NoError(t, err)

Expand Down Expand Up @@ -126,7 +126,7 @@ func TestMiddlewareNotExist(t *testing.T) {
mockLogger,
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
)
assert.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion tests/plugins/http/http_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ logs:
mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("worker destructed", "pid", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("worker constructed", "pid", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("", "remote", gomock.Any(), "ts", gomock.Any(), "resp.status", gomock.Any(), "method", gomock.Any(), "uri", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("201 GET http://localhost:34999/?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("WORLD", "pid", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("worker event received", "event", events.EventWorkerLog, "worker state", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() // placeholder for the workerlogerror
Expand Down
71 changes: 9 additions & 62 deletions tests/plugins/static/static_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestStaticPlugin(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)
Expand Down Expand Up @@ -138,7 +138,7 @@ func serveStaticSample(t *testing.T) {
_ = r.Body.Close()
}

func TestStaticDisabled(t *testing.T) {
func TestStaticDisabled_Error(t *testing.T) {
cont, err := endure.NewContainer(nil, endure.SetLogLevel(endure.ErrorLevel))
assert.NoError(t, err)

Expand All @@ -152,66 +152,11 @@ func TestStaticDisabled(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)

err = cont.Init()
if err != nil {
t.Fatal(err)
}

ch, err := cont.Serve()
assert.NoError(t, err)

sig := make(chan os.Signal, 1)
signal.Notify(sig, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)

wg := &sync.WaitGroup{}
wg.Add(1)

stopCh := make(chan struct{}, 1)

go func() {
defer wg.Done()
for {
select {
case e := <-ch:
assert.Fail(t, "error", e.Error.Error())
err = cont.Stop()
if err != nil {
assert.FailNow(t, "error", err.Error())
}
case <-sig:
err = cont.Stop()
if err != nil {
assert.FailNow(t, "error", err.Error())
}
return
case <-stopCh:
// timeout
err = cont.Stop()
if err != nil {
assert.FailNow(t, "error", err.Error())
}
return
}
}
}()

time.Sleep(time.Second)
t.Run("StaticDisabled", staticDisabled)

stopCh <- struct{}{}
wg.Wait()
}

func staticDisabled(t *testing.T) {
_, r, err := get("http://localhost:21234/sample.txt") //nolint:bodyclose
assert.NoError(t, err)
assert.NotNil(t, r)
assert.Empty(t, r.Header.Get("X-Powered-By"))
assert.Error(t, cont.Init())
}

func TestStaticFilesDisabled(t *testing.T) {
Expand All @@ -228,7 +173,7 @@ func TestStaticFilesDisabled(t *testing.T) {
&logger.ZapLogger{},
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)
Expand Down Expand Up @@ -306,7 +251,9 @@ func TestStaticFilesForbid(t *testing.T) {

mockLogger.EXPECT().Debug("worker destructed", "pid", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("worker constructed", "pid", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("", "remote", gomock.Any(), "ts", gomock.Any(), "resp.status", gomock.Any(), "method", gomock.Any(), "uri", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Debug("201 GET http://localhost:34653/http?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("201 GET http://localhost:34653/client.XXX?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Debug("201 GET http://localhost:34653/client.php?hello=world", "remote", "127.0.0.1", "elapsed", gomock.Any()).MinTimes(1)
mockLogger.EXPECT().Error("file open error", "error", gomock.Any()).AnyTimes()
mockLogger.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() // placeholder for the workerlogerror

Expand All @@ -315,7 +262,7 @@ func TestStaticFilesForbid(t *testing.T) {
mockLogger,
&server.Plugin{},
&httpPlugin.Plugin{},
&gzip.Gzip{},
&gzip.Plugin{},
&static.Plugin{},
)
assert.NoError(t, err)
Expand Down