Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
JRBANCEL committed Jun 26, 2019
1 parent ef2a110 commit ac095b9
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 37 deletions.
14 changes: 6 additions & 8 deletions pkg/activator/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,12 @@ func New(l *zap.SugaredLogger, r activator.StatsReporter, t *activator.Throttler
}
}

func withOrigProto(or *http.Request) prober.Option {
return prober.Option{
Prepare: func(r *http.Request) *http.Request {
r.Proto = or.Proto
r.ProtoMajor = or.ProtoMajor
r.ProtoMinor = or.ProtoMinor
return r
},
func withOrigProto(or *http.Request) prober.Preparer {
return func(r *http.Request) *http.Request {
r.Proto = or.Proto
r.ProtoMajor = or.ProtoMajor
r.ProtoMinor = or.ProtoMinor
return r
}
}

Expand Down
43 changes: 19 additions & 24 deletions pkg/network/prober/prober.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,42 +31,37 @@ import (
// TransportFactory is a function which returns an HTTP transport.
type TransportFactory func() http.RoundTripper

// Option provides a way for the caller to mutate the HTTP request before it goes out,
// and to validate the HTTP response after it comes back.
type Option struct {
Prepare func(r *http.Request) *http.Request
Validate func(r *http.Response, b []byte) (bool, error)
}
// Preparer is a way for the caller to modify the HTTP request before it goes out.
type Preparer func(r *http.Request) *http.Request

// Verifier is a way for the caller to validate the HTTP response after it comes back.
type Verifier func(r *http.Response, b []byte) (bool, error)

// WithHeader sets a header in the probe request.
func WithHeader(name, value string) Option {
return Option{
Prepare: func(r *http.Request) *http.Request {
r.Header.Set(name, value)
return r
},
func WithHeader(name, value string) Preparer {
return func(r *http.Request) *http.Request {
r.Header.Set(name, value)
return r
}
}

// ExpectsBody validates that the body of the probe response matches the provided string.
func ExpectsBody(body string) Option {
return Option{
Validate: func(r *http.Response, b []byte) (bool, error) {
return string(b) == body, nil
},
func ExpectsBody(body string) Verifier {
return func(r *http.Response, b []byte) (bool, error) {
return string(b) == body, nil
}
}

// Do sends a single probe to given target, e.g. `http://revision.default.svc.cluster.local:81`.
// Do returns whether the probe was successful or not, or there was an error probing.
func Do(ctx context.Context, transport http.RoundTripper, target string, ops ...Option) (bool, error) {
func Do(ctx context.Context, transport http.RoundTripper, target string, ops ...interface{}) (bool, error) {
req, err := http.NewRequest(http.MethodGet, target, nil)
if err != nil {
return false, errors.Wrapf(err, "%s is not a valid URL", target)
}
for _, op := range ops {
if op.Prepare != nil {
req = op.Prepare(req)
if po, ok := op.(Preparer); ok {
req = po(req)
}
}

Expand All @@ -82,8 +77,8 @@ func Do(ctx context.Context, transport http.RoundTripper, target string, ops ...
}

for _, op := range ops {
if op.Validate != nil {
ok, err := op.Validate(resp, body)
if vo, ok := op.(Verifier); ok {
ok, err := vo(resp, body)
if err != nil || !ok {
return false, err
}
Expand Down Expand Up @@ -131,7 +126,7 @@ func New(cb Done, transportFactory TransportFactory) *Manager {
// Otherwise Offer starts a goroutine that periodically executes
// `Do`, until timeout is reached, the probe succeeds, or fails with an error.
// In the end the callback is invoked with the provided `arg` and probing results.
func (m *Manager) Offer(ctx context.Context, target string, arg interface{}, period, timeout time.Duration, ops ...Option) bool {
func (m *Manager) Offer(ctx context.Context, target string, arg interface{}, period, timeout time.Duration, ops ...interface{}) bool {
m.mu.Lock()
defer m.mu.Unlock()
if m.keys.Has(target) {
Expand All @@ -143,7 +138,7 @@ func (m *Manager) Offer(ctx context.Context, target string, arg interface{}, per
}

// doAsync starts a go routine that probes the target with given period.
func (m *Manager) doAsync(ctx context.Context, transportFactory TransportFactory, target string, arg interface{}, period, timeout time.Duration, ops ...Option) {
func (m *Manager) doAsync(ctx context.Context, transportFactory TransportFactory, target string, arg interface{}, period, timeout time.Duration, ops ...interface{}) {
go func() {
defer func() {
m.mu.Lock()
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/autoscaling/kpa/scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ const (
reenqeuePeriod = 1 * time.Second
)

var probeOptions = []prober.Option {
var probeOptions = []interface{} {
prober.WithHeader(network.ProbeHeaderName, activator.Name),
prober.ExpectsBody(activator.Name),
}

// for mocking in tests
type asyncProber interface {
Offer(context.Context, string, interface{}, time.Duration, time.Duration, ...prober.Option) bool
Offer(context.Context, string, interface{}, time.Duration, time.Duration, ...interface{}) bool
}

// scaler scales the target of a kpa-class PA up or down including scaling to zero.
Expand Down
5 changes: 2 additions & 3 deletions pkg/reconciler/autoscaling/kpa/scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,11 @@ import (
"github.com/knative/serving/pkg/apis/serving/v1alpha1"
clientset "github.com/knative/serving/pkg/client/clientset/versioned"
"github.com/knative/serving/pkg/network"
"github.com/knative/serving/pkg/network/prober"
"github.com/knative/serving/pkg/reconciler/autoscaling/config"
revisionresources "github.com/knative/serving/pkg/reconciler/revision/resources"
"github.com/knative/serving/pkg/reconciler/revision/resources/names"
presources "github.com/knative/serving/pkg/resources"
v1 "k8s.io/api/apps/v1"
"k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -550,7 +549,7 @@ type countingProber struct {
count int
}

func (c *countingProber) Offer(ctx context.Context, target string, arg interface{}, period, timeout time.Duration, ops ...prober.Option) bool {
func (c *countingProber) Offer(ctx context.Context, target string, arg interface{}, period, timeout time.Duration, ops ...interface{}) bool {
c.count++
return true
}

0 comments on commit ac095b9

Please sign in to comment.