From 3965c61c6407c8304e59bc127ba3a4cb1190fce8 Mon Sep 17 00:00:00 2001 From: Shreeja Datta Date: Tue, 31 Mar 2020 18:29:54 +0000 Subject: [PATCH 1/6] Changed probe path in autoscaler and networking in compliance with activator --- pkg/network/status/status.go | 6 +++++- pkg/reconciler/autoscaling/kpa/scaler.go | 12 +++++++++++- test/config/security/policy.yaml | 2 ++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/pkg/network/status/status.go b/pkg/network/status/status.go index 1a7ba0431a32..45fa86525de0 100644 --- a/pkg/network/status/status.go +++ b/pkg/network/status/status.go @@ -45,6 +45,7 @@ const ( probeConcurrency = 15 //probeTimeout defines the maximum amount of time a request will wait probeTimeout = 1 * time.Second + probePath = "/_internal/knative/networking/probe" ) var dialContext = (&net.Dialer{Timeout: probeTimeout}).DialContext @@ -356,10 +357,13 @@ func (m *Prober) processWorkItem() bool { return dialContext(ctx, network, net.JoinHostPort(item.podIP, item.podPort)) }} + probeUrl := *item.url + probeUrl.Path = probePath + ok, err := prober.Do( item.context, transport, - item.url.String(), + probeUrl.String(), prober.WithHeader(network.UserAgentKey, network.IngressReadinessUserAgent), prober.WithHeader(network.ProbeHeaderName, network.ProbeHeaderValue), m.probeVerifier(item)) diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go index c684ddd2469f..13486372b34a 100644 --- a/pkg/reconciler/autoscaling/kpa/scaler.go +++ b/pkg/reconciler/autoscaling/kpa/scaler.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net/http" + "net/url" "time" "knative.dev/pkg/apis/duck" @@ -47,6 +48,8 @@ const ( scaleUnknown = -1 probePeriod = 1 * time.Second probeTimeout = 45 * time.Second + probePath = "/_internal/knative/autoscaler/probe" + // The time after which the PA will be re-enqueued. // This number is small, since `handleScaleToZero` below will // re-enqueue for the configured grace period. @@ -116,7 +119,14 @@ func newScaler(ctx context.Context, psInformerFactory duck.InformerFactory, enqu func paToProbeTarget(pa *pav1alpha1.PodAutoscaler) string { svc := pkgnet.GetServiceHostname(pa.Status.ServiceName, pa.Namespace) port := networking.ServicePort(pa.Spec.ProtocolType) - return fmt.Sprintf("http://%s:%d/", svc, port) + + httpDest := url.URL{ + Scheme: "http", + Host: fmt.Sprintf("%s:%d", svc, port), + Path: probePath, + } + + return httpDest.String() } // activatorProbe returns true if via probe it determines that the diff --git a/test/config/security/policy.yaml b/test/config/security/policy.yaml index 533399c82d6c..b921f4a428e2 100644 --- a/test/config/security/policy.yaml +++ b/test/config/security/policy.yaml @@ -46,4 +46,6 @@ spec: - excludedPaths: - prefix: /metrics - prefix: /_internal/knative/activator/probe + - prefix: /_internal/knative/autoscaler/probe + - prefix: /_internal/knative/networking/probe principalBinding: USE_ORIGIN From a623b87fd5c451b1928e39e92490bfcc1a2a1986 Mon Sep 17 00:00:00 2001 From: Shreeja Datta Date: Tue, 31 Mar 2020 20:06:50 +0000 Subject: [PATCH 2/6] Format Go code --- pkg/network/status/status.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/network/status/status.go b/pkg/network/status/status.go index 45fa86525de0..dc60b7f4ded8 100644 --- a/pkg/network/status/status.go +++ b/pkg/network/status/status.go @@ -44,8 +44,8 @@ const ( // probeConcurrency defines how many probing calls can be issued simultaneously probeConcurrency = 15 //probeTimeout defines the maximum amount of time a request will wait - probeTimeout = 1 * time.Second - probePath = "/_internal/knative/networking/probe" + probeTimeout = 1 * time.Second + probePath = "/_internal/knative/networking/probe" ) var dialContext = (&net.Dialer{Timeout: probeTimeout}).DialContext @@ -357,7 +357,7 @@ func (m *Prober) processWorkItem() bool { return dialContext(ctx, network, net.JoinHostPort(item.podIP, item.podPort)) }} - probeUrl := *item.url + probeUrl, _ := url.Parse(item.url.String()) probeUrl.Path = probePath ok, err := prober.Do( From bedc563cb3395288820c286cd96809bddbaaf731 Mon Sep 17 00:00:00 2001 From: Shreeja Datta Date: Wed, 1 Apr 2020 12:02:28 +0000 Subject: [PATCH 3/6] Changed probe path to /healthz --- pkg/activator/net/revision_backends.go | 2 +- pkg/network/status/status.go | 17 ++++++++++++----- pkg/reconciler/autoscaling/kpa/scaler.go | 11 +++++------ test/config/security/policy.yaml | 4 +--- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/pkg/activator/net/revision_backends.go b/pkg/activator/net/revision_backends.go index 72ebcdf83308..8dcf3bc897f8 100644 --- a/pkg/activator/net/revision_backends.go +++ b/pkg/activator/net/revision_backends.go @@ -69,7 +69,7 @@ type dests struct { const ( probeTimeout time.Duration = 300 * time.Millisecond defaultProbeFrequency time.Duration = 200 * time.Millisecond - probePath = "/_internal/knative/activator/probe" + probePath = "/healthz" ) // revisionWatcher watches the podIPs and ClusterIP of the service for a revision. It implements the logic diff --git a/pkg/network/status/status.go b/pkg/network/status/status.go index dc60b7f4ded8..c85e6e09e27f 100644 --- a/pkg/network/status/status.go +++ b/pkg/network/status/status.go @@ -23,6 +23,7 @@ import ( "net" "net/http" "net/url" + "path" "reflect" "sync" "sync/atomic" @@ -43,9 +44,9 @@ import ( const ( // probeConcurrency defines how many probing calls can be issued simultaneously probeConcurrency = 15 - //probeTimeout defines the maximum amount of time a request will wait - probeTimeout = 1 * time.Second - probePath = "/_internal/knative/networking/probe" + // probeTimeout defines the maximum amount of time a request will wait + probeTimeout = 1 * time.Second + probePath = "/healthz" ) var dialContext = (&net.Dialer{Timeout: probeTimeout}).DialContext @@ -357,8 +358,8 @@ func (m *Prober) processWorkItem() bool { return dialContext(ctx, network, net.JoinHostPort(item.podIP, item.podPort)) }} - probeUrl, _ := url.Parse(item.url.String()) - probeUrl.Path = probePath + probeUrl := deepCopy(item.url) + probeUrl.Path = path.Join(probeUrl.Path, probePath) ok, err := prober.Do( item.context, @@ -431,3 +432,9 @@ func (m *Prober) probeVerifier(item *workItem) prober.Verifier { } } } + +func deepCopy(in *url.URL) *url.URL { + // Safe to ignore the error since this is a deep copy + newUrl, _ := url.Parse(in.String()) + return newUrl +} diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go index 13486372b34a..bcbd4dc44487 100644 --- a/pkg/reconciler/autoscaling/kpa/scaler.go +++ b/pkg/reconciler/autoscaling/kpa/scaler.go @@ -21,6 +21,7 @@ import ( "fmt" "net/http" "net/url" + "path" "time" "knative.dev/pkg/apis/duck" @@ -48,7 +49,7 @@ const ( scaleUnknown = -1 probePeriod = 1 * time.Second probeTimeout = 45 * time.Second - probePath = "/_internal/knative/autoscaler/probe" + probePath = "/healthz" // The time after which the PA will be re-enqueued. // This number is small, since `handleScaleToZero` below will @@ -120,11 +121,9 @@ func paToProbeTarget(pa *pav1alpha1.PodAutoscaler) string { svc := pkgnet.GetServiceHostname(pa.Status.ServiceName, pa.Namespace) port := networking.ServicePort(pa.Spec.ProtocolType) - httpDest := url.URL{ - Scheme: "http", - Host: fmt.Sprintf("%s:%d", svc, port), - Path: probePath, - } + // Safe to ignore error since we know the string is a valid URL + httpDest, _ := url.Parse(fmt.Sprintf("http://%s:%d/", svc, port)) + httpDest.Path = path.Join(httpDest.Path, probePath) return httpDest.String() } diff --git a/test/config/security/policy.yaml b/test/config/security/policy.yaml index b921f4a428e2..36a7b60ffea0 100644 --- a/test/config/security/policy.yaml +++ b/test/config/security/policy.yaml @@ -45,7 +45,5 @@ spec: triggerRules: - excludedPaths: - prefix: /metrics - - prefix: /_internal/knative/activator/probe - - prefix: /_internal/knative/autoscaler/probe - - prefix: /_internal/knative/networking/probe + - prefix: /healthz principalBinding: USE_ORIGIN From 94ef72c7f79220f19c1c73267283433164d2ff08 Mon Sep 17 00:00:00 2001 From: shreejad <59456116+shreejad@users.noreply.github.com> Date: Wed, 1 Apr 2020 05:13:08 -0700 Subject: [PATCH 4/6] Update pkg/reconciler/autoscaling/kpa/scaler.go Format Go code Co-Authored-By: Matt Moore --- pkg/reconciler/autoscaling/kpa/scaler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go index bcbd4dc44487..2931d5c1d61a 100644 --- a/pkg/reconciler/autoscaling/kpa/scaler.go +++ b/pkg/reconciler/autoscaling/kpa/scaler.go @@ -123,7 +123,7 @@ func paToProbeTarget(pa *pav1alpha1.PodAutoscaler) string { // Safe to ignore error since we know the string is a valid URL httpDest, _ := url.Parse(fmt.Sprintf("http://%s:%d/", svc, port)) - httpDest.Path = path.Join(httpDest.Path, probePath) + httpDest.Path = path.Join(httpDest.Path, probePath) return httpDest.String() } From 00f85baa2228f0720ba1769cc7868aff8f129ce4 Mon Sep 17 00:00:00 2001 From: Shreeja Datta Date: Thu, 2 Apr 2020 00:22:23 +0000 Subject: [PATCH 5/6] Few formatting changes --- pkg/network/status/status.go | 11 ++++++----- pkg/reconciler/autoscaling/kpa/scaler.go | 11 ++--------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/pkg/network/status/status.go b/pkg/network/status/status.go index c85e6e09e27f..1198297eade9 100644 --- a/pkg/network/status/status.go +++ b/pkg/network/status/status.go @@ -358,13 +358,13 @@ func (m *Prober) processWorkItem() bool { return dialContext(ctx, network, net.JoinHostPort(item.podIP, item.podPort)) }} - probeUrl := deepCopy(item.url) - probeUrl.Path = path.Join(probeUrl.Path, probePath) + probeURL := deepCopy(item.url) + probeURL.Path = path.Join(probeURL.Path, probePath) ok, err := prober.Do( item.context, transport, - probeUrl.String(), + probeURL.String(), prober.WithHeader(network.UserAgentKey, network.IngressReadinessUserAgent), prober.WithHeader(network.ProbeHeaderName, network.ProbeHeaderValue), m.probeVerifier(item)) @@ -433,8 +433,9 @@ func (m *Prober) probeVerifier(item *workItem) prober.Verifier { } } +// Deep copies a URL into a new one func deepCopy(in *url.URL) *url.URL { // Safe to ignore the error since this is a deep copy - newUrl, _ := url.Parse(in.String()) - return newUrl + newURL, _ := url.Parse(in.String()) + return newURL } diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go index 2931d5c1d61a..97f8d89bdcfe 100644 --- a/pkg/reconciler/autoscaling/kpa/scaler.go +++ b/pkg/reconciler/autoscaling/kpa/scaler.go @@ -20,8 +20,6 @@ import ( "context" "fmt" "net/http" - "net/url" - "path" "time" "knative.dev/pkg/apis/duck" @@ -49,7 +47,6 @@ const ( scaleUnknown = -1 probePeriod = 1 * time.Second probeTimeout = 45 * time.Second - probePath = "/healthz" // The time after which the PA will be re-enqueued. // This number is small, since `handleScaleToZero` below will @@ -116,16 +113,12 @@ func newScaler(ctx context.Context, psInformerFactory duck.InformerFactory, enqu return ks } -// Resolves the pa to hostname:port. +// Resolves the pa to the probing endpoint Eg. http://hostname:port/healthz func paToProbeTarget(pa *pav1alpha1.PodAutoscaler) string { svc := pkgnet.GetServiceHostname(pa.Status.ServiceName, pa.Namespace) port := networking.ServicePort(pa.Spec.ProtocolType) - // Safe to ignore error since we know the string is a valid URL - httpDest, _ := url.Parse(fmt.Sprintf("http://%s:%d/", svc, port)) - httpDest.Path = path.Join(httpDest.Path, probePath) - - return httpDest.String() + return fmt.Sprintf("http://%s:%d/healthz", svc, port) } // activatorProbe returns true if via probe it determines that the From 07de7c83bfe3c9d3c6956594d0a9c7cb9bfddf34 Mon Sep 17 00:00:00 2001 From: shreejad <59456116+shreejad@users.noreply.github.com> Date: Wed, 1 Apr 2020 19:19:37 -0700 Subject: [PATCH 6/6] Update pkg/network/status/status.go - Change comment Co-Authored-By: Victor Agababov --- pkg/network/status/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/network/status/status.go b/pkg/network/status/status.go index 1198297eade9..739294eae4e2 100644 --- a/pkg/network/status/status.go +++ b/pkg/network/status/status.go @@ -433,7 +433,7 @@ func (m *Prober) probeVerifier(item *workItem) prober.Verifier { } } -// Deep copies a URL into a new one +// deepCopy copies a URL into a new one func deepCopy(in *url.URL) *url.URL { // Safe to ignore the error since this is a deep copy newURL, _ := url.Parse(in.String())