From 59e5c42e587d663c32f0aa89b45969e1bf4afa21 Mon Sep 17 00:00:00 2001 From: Ian Milligan Date: Wed, 6 May 2020 14:56:44 -0700 Subject: [PATCH 01/11] Register stats in NewStatsReporter instead of init (#1014) --- cmd/broker/ingress/wire_gen.go | 5 ++- pkg/broker/ingress/handler_test.go | 7 +++- pkg/broker/ingress/stats_reporter.go | 46 +++++++++++------------ pkg/broker/ingress/stats_reporter_test.go | 10 ++--- 4 files changed, 36 insertions(+), 32 deletions(-) diff --git a/cmd/broker/ingress/wire_gen.go b/cmd/broker/ingress/wire_gen.go index 1537bd8a76..427c6107f1 100644 --- a/cmd/broker/ingress/wire_gen.go +++ b/cmd/broker/ingress/wire_gen.go @@ -29,7 +29,10 @@ func InitializeHandler(ctx context.Context, port ingress.Port, projectID ingress return nil, err } multiTopicDecoupleSink := ingress.NewMultiTopicDecoupleSink(ctx, readonlyTargets, clientClient) - statsReporter := ingress.NewStatsReporter(podName, containerName2) + statsReporter, err := ingress.NewStatsReporter(podName, containerName2) + if err != nil { + return nil, err + } handler := ingress.NewHandler(ctx, httpMessageReceiver, multiTopicDecoupleSink, statsReporter) return handler, nil } diff --git a/pkg/broker/ingress/handler_test.go b/pkg/broker/ingress/handler_test.go index f100c8758a..6a5d5277a2 100644 --- a/pkg/broker/ingress/handler_test.go +++ b/pkg/broker/ingress/handler_test.go @@ -327,7 +327,12 @@ func createAndStartIngress(ctx context.Context, t *testing.T, psSrv *pstest.Serv decouple := NewMultiTopicDecoupleSink(ctx, memory.NewTargets(brokerConfig), client) receiver := &testHttpMessageReceiver{urlCh: make(chan string)} - h := NewHandler(ctx, receiver, decouple, NewStatsReporter(PodName(pod), ContainerName(container))) + statsReporter, err := NewStatsReporter(PodName(pod), ContainerName(container)) + if err != nil { + cancel() + t.Fatal(err) + } + h := NewHandler(ctx, receiver, decouple, statsReporter) errCh := make(chan error, 1) go func() { diff --git a/pkg/broker/ingress/stats_reporter.go b/pkg/broker/ingress/stats_reporter.go index 27de470d0b..4819676660 100644 --- a/pkg/broker/ingress/stats_reporter.go +++ b/pkg/broker/ingress/stats_reporter.go @@ -19,7 +19,6 @@ package ingress import ( "context" "fmt" - "log" "strconv" "time" @@ -36,14 +35,6 @@ import ( // - Removed StatsReporter interface and directly use helper methods instead. var ( - // dispatchTimeInMsecM records the time spent dispatching an event to - // a decouple queue, in milliseconds. - dispatchTimeInMsecM = stats.Float64( - "event_dispatch_latencies", - "The time spent dispatching an event to the decouple topic", - stats.UnitMilliseconds, - ) - // Create the tag keys that will be used to add tags to our measurements. // Tag keys must conform to the restrictions described in // go.opencensus.io/tag/validate.go. Currently those restrictions are: @@ -68,11 +59,7 @@ type reportArgs struct { responseCode int } -func init() { - register() -} - -func register() { +func (r *StatsReporter) register() error { tagKeys := []tag.Key{ namespaceKey, brokerKey, @@ -84,39 +71,48 @@ func register() { } // Create view to see our measurements. - err := view.Register( + return view.Register( &view.View{ Name: "event_count", Description: "Number of events received by a Broker", - Measure: dispatchTimeInMsecM, + Measure: r.dispatchTimeInMsecM, Aggregation: view.Count(), TagKeys: tagKeys, }, &view.View{ - Name: dispatchTimeInMsecM.Name(), - Description: dispatchTimeInMsecM.Description(), - Measure: dispatchTimeInMsecM, + Name: r.dispatchTimeInMsecM.Name(), + Description: r.dispatchTimeInMsecM.Description(), + Measure: r.dispatchTimeInMsecM, Aggregation: view.Distribution(metrics.Buckets125(1, 10000)...), // 1, 2, 5, 10, 20, 50, 100, 500, 1000, 5000, 10000 TagKeys: tagKeys, }, ) - if err != nil { - log.Fatalf("failed to register opencensus views, %s", err) - } } // NewStatsReporter creates a new StatsReporter. -func NewStatsReporter(podName PodName, containerName ContainerName) *StatsReporter { - return &StatsReporter{ +func NewStatsReporter(podName PodName, containerName ContainerName) (*StatsReporter, error) { + r := &StatsReporter{ podName: podName, containerName: containerName, + dispatchTimeInMsecM: stats.Float64( + "event_dispatch_latencies", + "The time spent dispatching an event to the decouple topic", + stats.UnitMilliseconds, + ), + } + if err := r.register(); err != nil { + return nil, fmt.Errorf("failed to register ingress stats: %w", err) } + return r, nil } // StatsReporter reports ingress metrics. type StatsReporter struct { podName PodName containerName ContainerName + // dispatchTimeInMsecM records the time spent dispatching an event to a decouple queue, in + // milliseconds. + dispatchTimeInMsecM *stats.Float64Measure } func (r *StatsReporter) reportEventDispatchTime(ctx context.Context, args reportArgs, d time.Duration) error { @@ -134,6 +130,6 @@ func (r *StatsReporter) reportEventDispatchTime(ctx context.Context, args report return fmt.Errorf("failed to create metrics tag: %v", err) } // convert time.Duration in nanoseconds to milliseconds. - metrics.Record(tag, dispatchTimeInMsecM.M(float64(d/time.Millisecond))) + metrics.Record(tag, r.dispatchTimeInMsecM.M(float64(d/time.Millisecond))) return nil } diff --git a/pkg/broker/ingress/stats_reporter_test.go b/pkg/broker/ingress/stats_reporter_test.go index 8ee3b31bb2..393619350d 100644 --- a/pkg/broker/ingress/stats_reporter_test.go +++ b/pkg/broker/ingress/stats_reporter_test.go @@ -44,7 +44,10 @@ func TestStatsReporter(t *testing.T) { metricskey.PodName: "testpod", } - r := NewStatsReporter(PodName("testpod"), ContainerName("testcontainer")) + r, err := NewStatsReporter(PodName("testpod"), ContainerName("testcontainer")) + if err != nil { + t.Fatal(err) + } // test ReportDispatchTime expectSuccess(t, func() error { @@ -59,10 +62,7 @@ func TestStatsReporter(t *testing.T) { func resetMetrics() { // OpenCensus metrics carry global state that need to be reset between unit tests. - metricstest.Unregister( - "event_count", - "event_dispatch_latencies") - register() + metricstest.Unregister("event_count", "event_dispatch_latencies") } func expectSuccess(t *testing.T, f func() error) { From 240dd17957eee45c359a9848d92efb7150f9604a Mon Sep 17 00:00:00 2001 From: Grace Gao <52978759+grac3gao@users.noreply.github.com> Date: Wed, 6 May 2020 15:42:44 -0700 Subject: [PATCH 02/11] Make deployment controller listen to 'Create' and 'Delete' events of Secret (#995) * update configure * Revert "update configure" This reverts commit fa4c9bb3 * deployment controller * Update pkg/reconciler/deployment/controller.go Co-authored-by: Adam Harwayne Co-authored-by: Adam Harwayne --- pkg/reconciler/deployment/controller.go | 29 ++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/pkg/reconciler/deployment/controller.go b/pkg/reconciler/deployment/controller.go index 12ba8c3b1f..de4d7caf94 100644 --- a/pkg/reconciler/deployment/controller.go +++ b/pkg/reconciler/deployment/controller.go @@ -17,9 +17,8 @@ package deployment import ( "context" + "os" - "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" - "github.com/google/knative-gcp/pkg/reconciler" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/clock" "k8s.io/client-go/tools/cache" @@ -27,6 +26,9 @@ import ( "knative.dev/pkg/client/injection/kube/informers/core/v1/secret" "knative.dev/pkg/configmap" "knative.dev/pkg/controller" + + "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" + "github.com/google/knative-gcp/pkg/reconciler" ) const ( @@ -40,6 +42,7 @@ const ( namespace = "cloud-run-events" secretName = v1alpha1.DefaultSecretName deploymentName = "controller" + envKey = "GOOGLE_APPLICATION_CREDENTIALS" ) // NewController initializes the controller and is called by the generated code @@ -63,21 +66,27 @@ func NewController( r.Logger.Info("Setting up event handlers") - sentinel := impl.EnqueueSentinel(types.NamespacedName{Name: deploymentName, Namespace: namespace}) + sentinel := impl.EnqueueSentinel(types.NamespacedName{Namespace: namespace, Name: deploymentName}) secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ FilterFunc: controller.FilterWithNameAndNamespace(namespace, secretName), - Handler: handleUpdateOnly(sentinel), + Handler: handler(sentinel), }) return impl } -func handleUpdateOnly(h func(interface{})) cache.ResourceEventHandler { +func handler(h func(interface{})) cache.ResourceEventHandler { return cache.ResourceEventHandlerFuncs{ - AddFunc: doNothing, + // For AddFunc, only enqueue deployment key when envKey is not set. + // In such case, the controller pod hasn't restarted before. + // This helps to avoid infinite loop for restarting controller pod. + AddFunc: func(obj interface{}) { + if _, ok := os.LookupEnv(envKey); !ok { + h(obj) + } + }, UpdateFunc: controller.PassNew(h), - DeleteFunc: doNothing, + // If secret is deleted, the controller pod will restart, in order to unset the envKey. + // This is needed when changing authentication configuration from k8s Secret to Workload Identity. + DeleteFunc: h, } } - -func doNothing(obj interface{}) { -} From f01b364d6959876302e86b0ec022abefeb9686ad Mon Sep 17 00:00:00 2001 From: Grace Gao <52978759+grac3gao@users.noreply.github.com> Date: Wed, 6 May 2020 16:41:44 -0700 Subject: [PATCH 03/11] Update cluster version for e2e-wi-tests. (#1011) * update configure * Revert "update configure" This reverts commit fa4c9bb3 * update-version --- go.mod | 2 +- go.sum | 4 ++-- test/e2e-wi-tests.sh | 3 ++- test/e2e/e2e_test.go | 6 ------ vendor/modules.txt | 2 +- 5 files changed, 6 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index b90215a7f2..7cee9b4a64 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( knative.dev/eventing v0.14.1-0.20200506063944-e9eb527e1295 knative.dev/pkg v0.0.0-20200506142844-5b98a558168e knative.dev/serving v0.14.1-0.20200506171253-f2c76cb8a6dd - knative.dev/test-infra v0.0.0-20200506181845-a5dff5817ab3 // indirect + knative.dev/test-infra v0.0.0-20200506193944-431dda291f8c // indirect ) replace ( diff --git a/go.sum b/go.sum index c776ccc974..9df88b0295 100644 --- a/go.sum +++ b/go.sum @@ -1278,8 +1278,8 @@ knative.dev/test-infra v0.0.0-20200505052144-5ea2f705bb55/go.mod h1:WqF1Azka+FxP knative.dev/test-infra v0.0.0-20200505192244-75864c82db21 h1:SsvqMKpvrn7cl7UqRUIT90SXDowHzpzHwHaTu+wN70s= knative.dev/test-infra v0.0.0-20200505192244-75864c82db21/go.mod h1:AqweEMgaMbb2xmYq9ZOPsH/lQ61qNx2XGr5tGltj5QU= knative.dev/test-infra v0.0.0-20200506045344-e71b1288c15c/go.mod h1:aMif0KXL4g19YCYwsy4Ocjjz5xgPlseYV+B95Oo4JGE= -knative.dev/test-infra v0.0.0-20200506181845-a5dff5817ab3 h1:p9DUu44EjGNNf2yWWSVhhgpZpqXHBqPCsz+NKCaBUQc= -knative.dev/test-infra v0.0.0-20200506181845-a5dff5817ab3/go.mod h1:aMif0KXL4g19YCYwsy4Ocjjz5xgPlseYV+B95Oo4JGE= +knative.dev/test-infra v0.0.0-20200506193944-431dda291f8c h1:1AX0dmooDhCBV/r5ooRH88hsb6eQo6CJAb2F3PicMbQ= +knative.dev/test-infra v0.0.0-20200506193944-431dda291f8c/go.mod h1:aMif0KXL4g19YCYwsy4Ocjjz5xgPlseYV+B95Oo4JGE= modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw= modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk= modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k= diff --git a/test/e2e-wi-tests.sh b/test/e2e-wi-tests.sh index 3182ef54e5..1b7fdddb76 100755 --- a/test/e2e-wi-tests.sh +++ b/test/e2e-wi-tests.sh @@ -142,7 +142,8 @@ function create_private_key_for_pubsub_service_account { } # Create a cluster with Workload Identity enabled. -initialize $@ --cluster-creation-flag "--workload-pool=\${PROJECT}.svc.id.goog" +# Specify cluster version for issue: https://github.com/google/knative-gcp/issues/1000 +initialize $@ --cluster-version "1.15.11-gke.9" --cluster-creation-flag "--workload-pool=\${PROJECT}.svc.id.goog" # Channel related e2e tests we have in Eventing is not running here. go_test_e2e -timeout=30m -parallel=6 ./test/e2e -workloadIndentity=true -pubsubServiceAccount=${PUBSUB_SERVICE_ACCOUNT_EMAIL} || fail_test diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 10a0586065..9eb2ecbc2d 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -273,9 +273,6 @@ func TestCloudSchedulerSourceBrokerWithPubSubChannel(t *testing.T) { // TestCloudStorageSource tests we can knock down a target from a CloudStorageSource. func TestCloudStorageSource(t *testing.T) { - if authConfig.WorkloadIdentity { - t.Skip("Skip this test temporally for issue: https://github.com/google/knative-gcp/issues/1000") - } cancel := logstream.Start(t) defer cancel() CloudStorageSourceWithTestImpl(t, false /*assertMetrics */, authConfig) @@ -291,9 +288,6 @@ func TestCloudStorageSourceStackDriverMetrics(t *testing.T) { // TestCloudAuditLogsSource tests we can knock down a target from an CloudAuditLogsSource. func TestCloudAuditLogsSource(t *testing.T) { - if authConfig.WorkloadIdentity { - t.Skip("Skip this test temporally for issue: https://github.com/google/knative-gcp/issues/1000") - } cancel := logstream.Start(t) defer cancel() CloudAuditLogsSourceWithTestImpl(t, authConfig) diff --git a/vendor/modules.txt b/vendor/modules.txt index ef1a2556e8..8b7f67abe9 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1179,7 +1179,7 @@ knative.dev/serving/pkg/client/listers/networking/v1alpha1 knative.dev/serving/pkg/client/listers/serving/v1 knative.dev/serving/pkg/client/listers/serving/v1alpha1 knative.dev/serving/pkg/client/listers/serving/v1beta1 -# knative.dev/test-infra v0.0.0-20200506181845-a5dff5817ab3 +# knative.dev/test-infra v0.0.0-20200506193944-431dda291f8c knative.dev/test-infra/scripts # sigs.k8s.io/yaml v1.2.0 => sigs.k8s.io/yaml v1.1.0 sigs.k8s.io/yaml From 12200f7088cd728b7e1a683fe5b6db80b7a36468 Mon Sep 17 00:00:00 2001 From: Eric Lemar Date: Thu, 7 May 2020 10:44:45 -0700 Subject: [PATCH 04/11] Add the x bit for e2e-secret-tests.sh (#1017) --- test/e2e-secret-tests.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 test/e2e-secret-tests.sh diff --git a/test/e2e-secret-tests.sh b/test/e2e-secret-tests.sh old mode 100644 new mode 100755 From eed8388e628b8669a093ac9d75db39ead266661d Mon Sep 17 00:00:00 2001 From: Ian Milligan Date: Thu, 7 May 2020 14:52:45 -0700 Subject: [PATCH 05/11] Use wire to initialize fanout and retry SyncPools (#1015) * Use wire to initialize fanout and retry SyncPools Simplifies option handling by removing client-related options from pool.Options leaving only the pool-specific options. * Merge fanout and retry pools into pkg/broker/handler/pool The SyncPool implementations are simple enough that they should live alongside the interface definition. This makes the repo easier to navigate by flattening the package structure and allows the shared pool providers to gain coverage from the fanout and retry pool tests. * Add missing doc comments * Add comments on wire codegen --- cmd/broker/fanout/main.go | 26 +++--- cmd/broker/fanout/wire.go | 35 ++++++++ cmd/broker/fanout/wire_gen.go | 50 +++++++++++ cmd/broker/retry/main.go | 25 +++--- cmd/broker/retry/wire.go | 35 ++++++++ cmd/broker/retry/wire_gen.go | 46 ++++++++++ .../pool/{fanout/pool.go => fanout.go} | 89 +++++++------------ .../{fanout/pool_test.go => fanout_test.go} | 30 +++---- pkg/broker/handler/pool/options.go | 38 -------- pkg/broker/handler/pool/options_test.go | 42 ++------- pkg/broker/handler/pool/providers.go | 75 ++++++++++++++++ .../handler/pool/{retry/pool.go => retry.go} | 64 ++++++------- .../{retry/pool_test.go => retry_test.go} | 27 +++--- pkg/broker/handler/pool/wire.go | 58 ++++++++++++ pkg/broker/handler/pool/wire_gen.go | 65 ++++++++++++++ 15 files changed, 481 insertions(+), 224 deletions(-) create mode 100644 cmd/broker/fanout/wire.go create mode 100644 cmd/broker/fanout/wire_gen.go create mode 100644 cmd/broker/retry/wire.go create mode 100644 cmd/broker/retry/wire_gen.go rename pkg/broker/handler/pool/{fanout/pool.go => fanout.go} (72%) rename pkg/broker/handler/pool/{fanout/pool_test.go => fanout_test.go} (91%) create mode 100644 pkg/broker/handler/pool/providers.go rename pkg/broker/handler/pool/{retry/pool.go => retry.go} (74%) rename pkg/broker/handler/pool/{retry/pool_test.go => retry_test.go} (91%) create mode 100644 pkg/broker/handler/pool/wire.go create mode 100644 pkg/broker/handler/pool/wire_gen.go diff --git a/cmd/broker/fanout/main.go b/cmd/broker/fanout/main.go index 9362b2275a..e3c824f257 100644 --- a/cmd/broker/fanout/main.go +++ b/cmd/broker/fanout/main.go @@ -36,8 +36,8 @@ import ( "github.com/google/knative-gcp/pkg/broker/config/volume" "github.com/google/knative-gcp/pkg/broker/handler/pool" - "github.com/google/knative-gcp/pkg/broker/handler/pool/fanout" "github.com/google/knative-gcp/pkg/observability" + "github.com/google/knative-gcp/pkg/utils" "github.com/google/knative-gcp/pkg/utils/appcredentials" ) @@ -112,17 +112,24 @@ func main() { // Give the signal channel some buffer so that reconciling handlers won't // block the targets config update? targetsUpdateCh := make(chan struct{}) - targetsConifg, err := volume.NewTargetsFromFile( - volume.WithPath(env.TargetsConfigPath), - volume.WithNotifyChan(targetsUpdateCh)) - if err != nil { - logger.Fatalw("Failed to load targets config", zap.String("path", env.TargetsConfigPath), zap.Error(err)) - } logger.Info("Starting the broker fanout") + projectID, err := utils.ProjectID(env.ProjectID) + if err != nil { + log.Fatalf("failed to get default ProjectID: %v", err) + } + syncSignal := poolSyncSignal(ctx, targetsUpdateCh) - syncPool, err := fanout.NewSyncPool(ctx, targetsConifg, buildPoolOptions(env)...) + syncPool, err := InitializeSyncPool( + ctx, + pool.ProjectID(projectID), + []volume.Option{ + volume.WithPath(env.TargetsConfigPath), + volume.WithNotifyChan(targetsUpdateCh), + }, + buildPoolOptions(env)..., + ) if err != nil { logger.Fatal("Failed to create fanout sync pool", zap.Error(err)) } @@ -168,9 +175,6 @@ func buildPoolOptions(env envConfig) []pool.Option { if env.MaxConcurrencyPerEvent > 0 { opts = append(opts, pool.WithMaxConcurrentPerEvent(env.MaxConcurrencyPerEvent)) } - if env.ProjectID != "" { - opts = append(opts, pool.WithProjectID(env.ProjectID)) - } if env.TimeoutPerEvent > 0 { opts = append(opts, pool.WithTimeoutPerEvent(env.TimeoutPerEvent)) } diff --git a/cmd/broker/fanout/wire.go b/cmd/broker/fanout/wire.go new file mode 100644 index 0000000000..0e0a330327 --- /dev/null +++ b/cmd/broker/fanout/wire.go @@ -0,0 +1,35 @@ +// +build wireinject + +/* +Copyright 2020 Google LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + + "github.com/google/knative-gcp/pkg/broker/config/volume" + "github.com/google/knative-gcp/pkg/broker/handler/pool" + "github.com/google/wire" +) + +// InitializeSyncPool initializes the fanout sync pool. Uses the given projectID to initialize the +// retry pool's pubsub client and uses targetsVolumeOpts to initialize the targets volume watcher. +func InitializeSyncPool(ctx context.Context, projectID pool.ProjectID, targetsVolumeOpts []volume.Option, opts ...pool.Option) (*pool.FanoutPool, error) { + // Implementation generated by wire. Providers for required FanoutPool dependencies should be + // added here. + panic(wire.Build(pool.ProviderSet, volume.NewTargetsFromFile)) +} diff --git a/cmd/broker/fanout/wire_gen.go b/cmd/broker/fanout/wire_gen.go new file mode 100644 index 0000000000..3278c2a9fb --- /dev/null +++ b/cmd/broker/fanout/wire_gen.go @@ -0,0 +1,50 @@ +// Code generated by Wire. DO NOT EDIT. + +//go:generate wire +//+build !wireinject + +package main + +import ( + "context" + "github.com/cloudevents/sdk-go/v2/protocol/http" + "github.com/google/knative-gcp/pkg/broker/config/volume" + "github.com/google/knative-gcp/pkg/broker/handler/pool" +) + +// Injectors from wire.go: + +func InitializeSyncPool(ctx context.Context, projectID pool.ProjectID, targetsVolumeOpts []volume.Option, opts ...pool.Option) (*pool.FanoutPool, error) { + readonlyTargets, err := volume.NewTargetsFromFile(targetsVolumeOpts...) + if err != nil { + return nil, err + } + client, err := pool.NewPubsubClient(ctx, projectID) + if err != nil { + return nil, err + } + v := _wireValue + protocol, err := http.New(v...) + if err != nil { + return nil, err + } + v2 := _wireValue2 + deliverClient, err := pool.NewDeliverClient(protocol, v2...) + if err != nil { + return nil, err + } + retryClient, err := pool.NewRetryClient(ctx, client, v2...) + if err != nil { + return nil, err + } + fanoutPool, err := pool.NewFanoutPool(readonlyTargets, client, deliverClient, retryClient, opts...) + if err != nil { + return nil, err + } + return fanoutPool, nil +} + +var ( + _wireValue = []http.Option(nil) + _wireValue2 = pool.DefaultCEClientOpts +) diff --git a/cmd/broker/retry/main.go b/cmd/broker/retry/main.go index 56ad0ab489..192dfe91ea 100644 --- a/cmd/broker/retry/main.go +++ b/cmd/broker/retry/main.go @@ -38,8 +38,8 @@ import ( "github.com/google/knative-gcp/pkg/broker/config/volume" "github.com/google/knative-gcp/pkg/broker/handler/pool" - "github.com/google/knative-gcp/pkg/broker/handler/pool/retry" "github.com/google/knative-gcp/pkg/observability" + "github.com/google/knative-gcp/pkg/utils" "github.com/google/knative-gcp/pkg/utils/appcredentials" ) @@ -112,17 +112,23 @@ func main() { // Give the signal channel some buffer so that reconciling handlers won't // block the targets config update? targetsUpdateCh := make(chan struct{}) - targetsConifg, err := volume.NewTargetsFromFile( - volume.WithPath(env.TargetsConfigPath), - volume.WithNotifyChan(targetsUpdateCh)) + logger.Info("Starting the broker retry") + + projectID, err := utils.ProjectID(env.ProjectID) if err != nil { - logger.Fatal("Failed to load targets config", zap.String("path", env.TargetsConfigPath), zap.Error(err)) + log.Fatalf("failed to get default ProjectID: %v", err) } - logger.Info("Starting the broker retry") - syncSignal := poolSyncSignal(ctx, targetsUpdateCh) - syncPool, err := retry.NewSyncPool(targetsConifg, buildPoolOptions(env)...) + syncPool, err := InitializeSyncPool( + ctx, + pool.ProjectID(projectID), + []volume.Option{ + volume.WithPath(env.TargetsConfigPath), + volume.WithNotifyChan(targetsUpdateCh), + }, + buildPoolOptions(env)..., + ) if err != nil { logger.Fatal("Failed to get retry sync pool", zap.Error(err)) } @@ -167,9 +173,6 @@ func buildPoolOptions(env envConfig) []pool.Option { opts = append(opts, pool.WithHandlerConcurrency(env.HandlerConcurrency)) rs.NumGoroutines = env.HandlerConcurrency } - if env.ProjectID != "" { - opts = append(opts, pool.WithProjectID(env.ProjectID)) - } if env.TimeoutPerEvent > 0 { opts = append(opts, pool.WithTimeoutPerEvent(env.TimeoutPerEvent)) } diff --git a/cmd/broker/retry/wire.go b/cmd/broker/retry/wire.go new file mode 100644 index 0000000000..727bf11557 --- /dev/null +++ b/cmd/broker/retry/wire.go @@ -0,0 +1,35 @@ +// +build wireinject + +/* +Copyright 2020 Google LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + + "github.com/google/knative-gcp/pkg/broker/config/volume" + "github.com/google/knative-gcp/pkg/broker/handler/pool" + "github.com/google/wire" +) + +// InitializeSyncPool initializes the retry sync pool. Uses the given projectID to initialize the +// retry pool's pubsub client and uses targetsVolumeOpts to initialize the targets volume watcher. +func InitializeSyncPool(ctx context.Context, projectID pool.ProjectID, targetsVolumeOpts []volume.Option, opts ...pool.Option) (*pool.RetryPool, error) { + // Implementation generated by wire. Providers for required RetryPool dependencies should be + // added here. + panic(wire.Build(pool.ProviderSet, volume.NewTargetsFromFile)) +} diff --git a/cmd/broker/retry/wire_gen.go b/cmd/broker/retry/wire_gen.go new file mode 100644 index 0000000000..d5763fb0cf --- /dev/null +++ b/cmd/broker/retry/wire_gen.go @@ -0,0 +1,46 @@ +// Code generated by Wire. DO NOT EDIT. + +//go:generate wire +//+build !wireinject + +package main + +import ( + "context" + "github.com/cloudevents/sdk-go/v2/protocol/http" + "github.com/google/knative-gcp/pkg/broker/config/volume" + "github.com/google/knative-gcp/pkg/broker/handler/pool" +) + +// Injectors from wire.go: + +func InitializeSyncPool(ctx context.Context, projectID pool.ProjectID, targetsVolumeOpts []volume.Option, opts ...pool.Option) (*pool.RetryPool, error) { + readonlyTargets, err := volume.NewTargetsFromFile(targetsVolumeOpts...) + if err != nil { + return nil, err + } + client, err := pool.NewPubsubClient(ctx, projectID) + if err != nil { + return nil, err + } + v := _wireValue + protocol, err := http.New(v...) + if err != nil { + return nil, err + } + v2 := _wireValue2 + deliverClient, err := pool.NewDeliverClient(protocol, v2...) + if err != nil { + return nil, err + } + retryPool, err := pool.NewRetryPool(readonlyTargets, client, deliverClient, opts...) + if err != nil { + return nil, err + } + return retryPool, nil +} + +var ( + _wireValue = []http.Option(nil) + _wireValue2 = pool.DefaultCEClientOpts +) diff --git a/pkg/broker/handler/pool/fanout/pool.go b/pkg/broker/handler/pool/fanout.go similarity index 72% rename from pkg/broker/handler/pool/fanout/pool.go rename to pkg/broker/handler/pool/fanout.go index fd0ee87c27..12a5733fca 100644 --- a/pkg/broker/handler/pool/fanout/pool.go +++ b/pkg/broker/handler/pool/fanout.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package fanout +package pool import ( "context" @@ -22,31 +22,32 @@ import ( "sync" "time" + "cloud.google.com/go/pubsub" ceclient "github.com/cloudevents/sdk-go/v2/client" - cehttp "github.com/cloudevents/sdk-go/v2/protocol/http" - "github.com/cloudevents/sdk-go/v2/protocol/pubsub" + cepubsub "github.com/cloudevents/sdk-go/v2/protocol/pubsub" "go.uber.org/zap" "knative.dev/eventing/pkg/logging" "github.com/google/knative-gcp/pkg/broker/config" "github.com/google/knative-gcp/pkg/broker/handler" handlerctx "github.com/google/knative-gcp/pkg/broker/handler/context" - "github.com/google/knative-gcp/pkg/broker/handler/pool" "github.com/google/knative-gcp/pkg/broker/handler/processors" "github.com/google/knative-gcp/pkg/broker/handler/processors/deliver" "github.com/google/knative-gcp/pkg/broker/handler/processors/fanout" "github.com/google/knative-gcp/pkg/broker/handler/processors/filter" ) -// SyncPool is the sync pool for fanout handlers. +// FanoutPool is the sync pool for fanout handlers. // For each broker in the config, it will attempt to create a handler. // It will also stop/delete the handler if the corresponding broker is deleted // in the config. -type SyncPool struct { - options *pool.Options +type FanoutPool struct { + options *Options targets config.ReadonlyTargets pool sync.Map + // Pubsub client used to pull events from decoupling topics. + pubsubClient *pubsub.Client // For sending retry events. We only need a shared client. // And we can set retry topic dynamically. deliverRetryClient ceclient.Client @@ -61,14 +62,14 @@ type SyncPool struct { deliverTimeout time.Duration } -type handlerCache struct { +type fanoutHandlerCache struct { handler.Handler b *config.Broker } // If somehow the existing handler's setting has deviated from the current broker config, // we need to renew the handler. -func (hc *handlerCache) shouldRenew(b *config.Broker) bool { +func (hc *fanoutHandlerCache) shouldRenew(b *config.Broker) bool { if !hc.IsAlive() { return true } @@ -84,27 +85,15 @@ func (hc *handlerCache) shouldRenew(b *config.Broker) bool { return false } -// NewSyncPool creates a new fanout handler pool. -func NewSyncPool(ctx context.Context, targets config.ReadonlyTargets, opts ...pool.Option) (*SyncPool, error) { - options, err := pool.NewOptions(opts...) - if err != nil { - return nil, err - } - - rps, err := defaultRetryPubsubProtocol(ctx, options) - if err != nil { - return nil, err - } - retryClient, err := ceclient.NewObserved(rps, options.CeClientOptions...) - if err != nil { - return nil, err - } - - hp, err := cehttp.New() - if err != nil { - return nil, err - } - deliverClient, err := ceclient.NewObserved(hp, options.CeClientOptions...) +// NewFanoutPool creates a new fanout handler pool. +func NewFanoutPool( + targets config.ReadonlyTargets, + pubsubClient *pubsub.Client, + deliverClient DeliverClient, + retryClient RetryClient, + opts ...Option, +) (*FanoutPool, error) { + options, err := NewOptions(opts...) if err != nil { return nil, err } @@ -113,9 +102,10 @@ func NewSyncPool(ctx context.Context, targets config.ReadonlyTargets, opts ...po return nil, fmt.Errorf("timeout per event cannot be lower than %v", 5*time.Second) } - p := &SyncPool{ + p := &FanoutPool{ targets: targets, options: options, + pubsubClient: pubsubClient, deliverClient: deliverClient, deliverRetryClient: retryClient, // Set the deliver timeout slightly less than the total timeout for each event. @@ -124,23 +114,13 @@ func NewSyncPool(ctx context.Context, targets config.ReadonlyTargets, opts ...po return p, nil } -func defaultRetryPubsubProtocol(ctx context.Context, options *pool.Options) (*pubsub.Protocol, error) { - opts := []pubsub.Option{ - pubsub.WithProjectID(options.ProjectID), - } - if options.PubsubClient != nil { - opts = append(opts, pubsub.WithClient(options.PubsubClient)) - } - return pubsub.New(ctx, opts...) -} - // SyncOnce syncs once the handler pool based on the targets config. -func (p *SyncPool) SyncOnce(ctx context.Context) error { +func (p *FanoutPool) SyncOnce(ctx context.Context) error { var errs int p.pool.Range(func(key, value interface{}) bool { if _, ok := p.targets.GetBrokerByKey(key.(string)); !ok { - value.(*handlerCache).Stop() + value.(*fanoutHandlerCache).Stop() p.pool.Delete(key) } return true @@ -149,32 +129,27 @@ func (p *SyncPool) SyncOnce(ctx context.Context) error { p.targets.RangeBrokers(func(b *config.Broker) bool { if value, ok := p.pool.Load(b.Key()); ok { // Skip if we don't need to renew the handler. - if !value.(*handlerCache).shouldRenew(b) { + if !value.(*fanoutHandlerCache).shouldRenew(b) { return true } // Stop and clean up the old handler before we start a new one. - value.(*handlerCache).Stop() + value.(*fanoutHandlerCache).Stop() p.pool.Delete(b.Key()) } - opts := []pubsub.Option{ - pubsub.WithProjectID(p.options.ProjectID), - pubsub.WithTopicID(b.DecoupleQueue.Topic), - pubsub.WithSubscriptionID(b.DecoupleQueue.Subscription), - pubsub.WithReceiveSettings(&p.options.PubsubReceiveSettings), - } - - if p.options.PubsubClient != nil { - opts = append(opts, pubsub.WithClient(p.options.PubsubClient)) - } - ps, err := pubsub.New(ctx, opts...) + ps, err := cepubsub.New(ctx, + cepubsub.WithClient(p.pubsubClient), + cepubsub.WithTopicID(b.DecoupleQueue.Topic), + cepubsub.WithSubscriptionID(b.DecoupleQueue.Subscription), + cepubsub.WithReceiveSettings(&p.options.PubsubReceiveSettings), + ) if err != nil { logging.FromContext(ctx).Error("failed to create pubsub protocol", zap.String("broker", b.Key()), zap.Error(err)) errs++ return true } - hc := &handlerCache{ + hc := &fanoutHandlerCache{ Handler: handler.Handler{ Timeout: p.options.TimeoutPerEvent, PubsubEvents: ps, diff --git a/pkg/broker/handler/pool/fanout/pool_test.go b/pkg/broker/handler/pool/fanout_test.go similarity index 91% rename from pkg/broker/handler/pool/fanout/pool_test.go rename to pkg/broker/handler/pool/fanout_test.go index e5e686e57e..65a72b7fc6 100644 --- a/pkg/broker/handler/pool/fanout/pool_test.go +++ b/pkg/broker/handler/pool/fanout_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package fanout +package pool import ( "context" @@ -26,11 +26,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/knative-gcp/pkg/broker/config" - "github.com/google/knative-gcp/pkg/broker/handler/pool" pooltesting "github.com/google/knative-gcp/pkg/broker/handler/pool/testing" ) -func TestWatchAndSync(t *testing.T) { +func TestFanoutWatchAndSync(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() testProject := "test-project" @@ -41,19 +40,17 @@ func TestWatchAndSync(t *testing.T) { defer helper.Close() signal := make(chan struct{}) - syncPool, err := NewSyncPool(ctx, helper.Targets, - pool.WithPubsubClient(helper.PubsubClient), - pool.WithProjectID(testProject)) + syncPool, err := InitializeTestFanoutPool(ctx, helper.Targets, helper.PubsubClient) if err != nil { t.Errorf("unexpected error from getting sync pool: %v", err) } t.Run("start sync pool creates no handler", func(t *testing.T) { - _, err = pool.StartSyncPool(ctx, syncPool, signal) + _, err = StartSyncPool(ctx, syncPool, signal) if err != nil { t.Errorf("unexpected error from starting sync pool: %v", err) } - assertHandlers(t, syncPool, helper.Targets) + assertFanoutHandlers(t, syncPool, helper.Targets) }) bs := make([]*config.Broker, 0, 4) @@ -66,7 +63,7 @@ func TestWatchAndSync(t *testing.T) { signal <- struct{}{} // Wait a short period for the handlers to be updated. <-time.After(time.Second) - assertHandlers(t, syncPool, helper.Targets) + assertFanoutHandlers(t, syncPool, helper.Targets) }) t.Run("adding and deleting brokers changes handlers", func(t *testing.T) { @@ -78,7 +75,7 @@ func TestWatchAndSync(t *testing.T) { signal <- struct{}{} // Wait a short period for the handlers to be updated. <-time.After(time.Second) - assertHandlers(t, syncPool, helper.Targets) + assertFanoutHandlers(t, syncPool, helper.Targets) }) t.Run("deleting all brokers deletes all handlers", func(t *testing.T) { @@ -89,7 +86,7 @@ func TestWatchAndSync(t *testing.T) { signal <- struct{}{} // Wait a short period for the handlers to be updated. <-time.After(time.Second) - assertHandlers(t, syncPool, helper.Targets) + assertFanoutHandlers(t, syncPool, helper.Targets) }) } @@ -113,16 +110,15 @@ func TestFanoutSyncPoolE2E(t *testing.T) { t3 := helper.GenerateTarget(ctx, t, b2.Key(), nil) signal := make(chan struct{}) - syncPool, err := NewSyncPool(ctx, helper.Targets, - pool.WithPubsubClient(helper.PubsubClient), - pool.WithProjectID(testProject), - pool.WithDeliveryTimeout(500*time.Millisecond), + syncPool, err := InitializeTestFanoutPool( + ctx, helper.Targets, helper.PubsubClient, + WithDeliveryTimeout(500*time.Millisecond), ) if err != nil { t.Errorf("unexpected error from getting sync pool: %v", err) } - if _, err := pool.StartSyncPool(ctx, syncPool, signal); err != nil { + if _, err := StartSyncPool(ctx, syncPool, signal); err != nil { t.Errorf("unexpected error from starting sync pool: %v", err) } @@ -258,7 +254,7 @@ func TestFanoutSyncPoolE2E(t *testing.T) { }) } -func assertHandlers(t *testing.T, p *SyncPool, targets config.Targets) { +func assertFanoutHandlers(t *testing.T, p *FanoutPool, targets config.Targets) { t.Helper() gotHandlers := make(map[string]bool) wantHandlers := make(map[string]bool) diff --git a/pkg/broker/handler/pool/options.go b/pkg/broker/handler/pool/options.go index 0c39443b15..726a8073da 100644 --- a/pkg/broker/handler/pool/options.go +++ b/pkg/broker/handler/pool/options.go @@ -17,24 +17,16 @@ limitations under the License. package pool import ( - "fmt" "runtime" "time" "cloud.google.com/go/pubsub" - ceclient "github.com/cloudevents/sdk-go/v2/client" - "github.com/google/knative-gcp/pkg/utils" ) var ( defaultHandlerConcurrency = runtime.NumCPU() defaultMaxConcurrencyPerEvent = 1 defaultTimeout = 10 * time.Minute - defaultCeClientOpts = []ceclient.Option{ - ceclient.WithUUIDs(), - ceclient.WithTimeNow(), - ceclient.WithTracePropagation(), - } // This is the pubsub default MaxExtension. // It would not make sense for handler timeout per event be greater @@ -46,8 +38,6 @@ var ( // Options holds all the options for create handler pool. type Options struct { - // ProjectID is the project for pubsub. - ProjectID string // HandlerConcurrency is the number of goroutines // will be spawned in each handler. HandlerConcurrency int @@ -58,12 +48,8 @@ type Options struct { TimeoutPerEvent time.Duration // DeliveryTimeout is the timeout for delivering an event to a consumer. DeliveryTimeout time.Duration - // PubsubClient is the pubsub client used to receive pubsub messages. - PubsubClient *pubsub.Client // PubsubReceiveSettings is the pubsub receive settings. PubsubReceiveSettings pubsub.ReceiveSettings - // CeClientOptions is the options used to create cloudevents client. - CeClientOptions []ceclient.Option } // NewOptions creates a Options. @@ -77,29 +63,12 @@ func NewOptions(opts ...Option) (*Options, error) { for _, o := range opts { o(opt) } - if opt.ProjectID == "" { - pid, err := utils.ProjectID(opt.ProjectID) - if err != nil { - return nil, fmt.Errorf("failed to get default ProjectID: %w", err) - } - opt.ProjectID = pid - } - if opt.CeClientOptions == nil { - opt.CeClientOptions = defaultCeClientOpts - } return opt, nil } // Option is for providing individual option. type Option func(*Options) -// WithProjectID sets project ID. -func WithProjectID(id string) Option { - return func(o *Options) { - o.ProjectID = id - } -} - // WithHandlerConcurrency sets HandlerConcurrency. func WithHandlerConcurrency(c int) Option { return func(o *Options) { @@ -125,13 +94,6 @@ func WithTimeoutPerEvent(t time.Duration) Option { } } -// WithPubsubClient sets the PubsubClient. -func WithPubsubClient(c *pubsub.Client) Option { - return func(o *Options) { - o.PubsubClient = c - } -} - // WithPubsubReceiveSettings sets PubsubReceiveSettings. func WithPubsubReceiveSettings(s pubsub.ReceiveSettings) Option { return func(o *Options) { diff --git a/pkg/broker/handler/pool/options_test.go b/pkg/broker/handler/pool/options_test.go index af9af1767b..dcf4213ce9 100644 --- a/pkg/broker/handler/pool/options_test.go +++ b/pkg/broker/handler/pool/options_test.go @@ -24,21 +24,10 @@ import ( "github.com/google/go-cmp/cmp" ) -func TestWithProjectID(t *testing.T) { - want := "random" - opt, err := NewOptions(WithProjectID(want)) - if err != nil { - t.Errorf("NewOptions got unexpected error: %v", err) - } - if opt.ProjectID != want { - t.Errorf("options project id got=%s, want=%s", opt.ProjectID, want) - } -} - func TestWithHandlerConcurrency(t *testing.T) { want := 10 // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err := NewOptions(WithHandlerConcurrency(want), WithProjectID("pid")) + opt, err := NewOptions(WithHandlerConcurrency(want)) if err != nil { t.Errorf("NewOptions got unexpected error: %v", err) } @@ -50,7 +39,7 @@ func TestWithHandlerConcurrency(t *testing.T) { func TestWithMaxConcurrency(t *testing.T) { want := 10 // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err := NewOptions(WithMaxConcurrentPerEvent(want), WithProjectID("pid")) + opt, err := NewOptions(WithMaxConcurrentPerEvent(want)) if err != nil { t.Errorf("NewOptions got unexpected error: %v", err) } @@ -62,7 +51,7 @@ func TestWithMaxConcurrency(t *testing.T) { func TestWithTimeout(t *testing.T) { want := 2 * time.Minute // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err := NewOptions(WithTimeoutPerEvent(want), WithProjectID("pid")) + opt, err := NewOptions(WithTimeoutPerEvent(want)) if err != nil { t.Errorf("NewOptions got unexpected error: %v", err) } @@ -72,7 +61,7 @@ func TestWithTimeout(t *testing.T) { // Set timeout greater than the max value and verify it fallbacks to the max value. // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err = NewOptions(WithTimeoutPerEvent(20*time.Minute), WithProjectID("pid")) + opt, err = NewOptions(WithTimeoutPerEvent(20 * time.Minute)) if err != nil { t.Errorf("NewOptions got unexpected error: %v", err) } @@ -87,7 +76,7 @@ func TestWithReceiveSettings(t *testing.T) { MaxExtension: time.Minute, } // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err := NewOptions(WithPubsubReceiveSettings(want), WithProjectID("pid")) + opt, err := NewOptions(WithPubsubReceiveSettings(want)) if err != nil { t.Errorf("NewOptions got unexpected error: %v", err) } @@ -96,29 +85,10 @@ func TestWithReceiveSettings(t *testing.T) { } } -func TestWithPubsubClient(t *testing.T) { - // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err := NewOptions(WithProjectID("pid")) - if err != nil { - t.Errorf("NewOptions got unexpected error: %v", err) - } - if opt.PubsubClient != nil { - t.Errorf("options PubsubClient got=%v, want=nil", opt.PubsubClient) - } - // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err = NewOptions(WithPubsubClient(&pubsub.Client{}), WithProjectID("pid")) - if err != nil { - t.Errorf("NewOptions got unexpected error: %v", err) - } - if opt.PubsubClient == nil { - t.Error("options PubsubClient got=nil, want=non-nil client") - } -} - func TestWithDeliveryTimeout(t *testing.T) { want := 10 * time.Minute // Always add project id because the default value can only be retrieved on GCE/GKE machines. - opt, err := NewOptions(WithDeliveryTimeout(want), WithProjectID("pid")) + opt, err := NewOptions(WithDeliveryTimeout(want)) if err != nil { t.Errorf("NewOptions got unexpected error: %v", err) } diff --git a/pkg/broker/handler/pool/providers.go b/pkg/broker/handler/pool/providers.go new file mode 100644 index 0000000000..9852b6e816 --- /dev/null +++ b/pkg/broker/handler/pool/providers.go @@ -0,0 +1,75 @@ +/* +Copyright 2020 Google LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pool + +import ( + "context" + + "cloud.google.com/go/pubsub" + ceclient "github.com/cloudevents/sdk-go/v2/client" + cehttp "github.com/cloudevents/sdk-go/v2/protocol/http" + cepubsub "github.com/cloudevents/sdk-go/v2/protocol/pubsub" + "github.com/google/wire" +) + +var ( + DefaultCEClientOpts = []ceclient.Option{ + ceclient.WithUUIDs(), + ceclient.WithTimeNow(), + ceclient.WithTracePropagation(), + } + + // ProviderSet provides the fanout and retry sync pools using the default client options. In + // order to inject either pool, ProjectID, []Option, and config.ReadOnlyTargets must be + // externally provided. + ProviderSet = wire.NewSet( + NewFanoutPool, + NewRetryPool, + cehttp.New, + NewDeliverClient, + NewPubsubClient, + NewRetryClient, + wire.Value([]cehttp.Option(nil)), + wire.Value(DefaultCEClientOpts), + ) +) + +type ( + ProjectID string + DeliverClient ceclient.Client + RetryClient ceclient.Client +) + +// NewDeliverClient provides a delivery CE client from an HTTP protocol and a list of CE client options. +func NewDeliverClient(hp *cehttp.Protocol, opts ...ceclient.Option) (DeliverClient, error) { + return ceclient.NewObserved(hp, opts...) +} + +// NewPubsubClient provides a pubsub client for the supplied project ID. +func NewPubsubClient(ctx context.Context, projectID ProjectID) (*pubsub.Client, error) { + return pubsub.NewClient(ctx, string(projectID)) +} + +// NewRetryClient provides a retry CE client from a PubSub client and list of CE client options. +func NewRetryClient(ctx context.Context, client *pubsub.Client, opts ...ceclient.Option) (RetryClient, error) { + rps, err := cepubsub.New(ctx, cepubsub.WithClient(client)) + if err != nil { + return nil, err + } + + return ceclient.NewObserved(rps, opts...) +} diff --git a/pkg/broker/handler/pool/retry/pool.go b/pkg/broker/handler/pool/retry.go similarity index 74% rename from pkg/broker/handler/pool/retry/pool.go rename to pkg/broker/handler/pool/retry.go index f02c68f66b..b699068c7c 100644 --- a/pkg/broker/handler/pool/retry/pool.go +++ b/pkg/broker/handler/pool/retry.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package retry +package pool import ( "context" @@ -22,42 +22,43 @@ import ( "sync" ceclient "github.com/cloudevents/sdk-go/v2/client" - cehttp "github.com/cloudevents/sdk-go/v2/protocol/http" "go.uber.org/zap" "knative.dev/eventing/pkg/logging" - "github.com/cloudevents/sdk-go/v2/protocol/pubsub" + "cloud.google.com/go/pubsub" + cepubsub "github.com/cloudevents/sdk-go/v2/protocol/pubsub" "github.com/google/knative-gcp/pkg/broker/config" "github.com/google/knative-gcp/pkg/broker/handler" handlerctx "github.com/google/knative-gcp/pkg/broker/handler/context" - "github.com/google/knative-gcp/pkg/broker/handler/pool" "github.com/google/knative-gcp/pkg/broker/handler/processors" "github.com/google/knative-gcp/pkg/broker/handler/processors/deliver" "github.com/google/knative-gcp/pkg/broker/handler/processors/filter" ) -// SyncPool is the sync pool for retry handlers. +// RetryPool is the sync pool for retry handlers. // For each trigger in the config, it will attempt to create a handler. // It will also stop/delete the handler if the corresponding trigger is deleted // in the config. -type SyncPool struct { - options *pool.Options +type RetryPool struct { + options *Options targets config.ReadonlyTargets pool sync.Map + // Pubsub client used to pull events from decoupling topics. + pubsubClient *pubsub.Client // For initial events delivery. We only need a shared client. // And we can set target address dynamically. deliverClient ceclient.Client } -type handlerCache struct { +type retryHandlerCache struct { handler.Handler t *config.Target } // If somehow the existing handler's setting has deviated from the current target config, // we need to renew the handler. -func (hc *handlerCache) shouldRenew(t *config.Target) bool { +func (hc *retryHandlerCache) shouldRenew(t *config.Target) bool { if !hc.IsAlive() { return true } @@ -73,38 +74,30 @@ func (hc *handlerCache) shouldRenew(t *config.Target) bool { return false } -// NewSyncPool creates a new retry handler pool. -func NewSyncPool(targets config.ReadonlyTargets, opts ...pool.Option) (*SyncPool, error) { - options, err := pool.NewOptions(opts...) +// NewRetryPool creates a new retry handler pool. +func NewRetryPool(targets config.ReadonlyTargets, pubsubClient *pubsub.Client, deliverClient DeliverClient, opts ...Option) (*RetryPool, error) { + options, err := NewOptions(opts...) if err != nil { return nil, err } - hp, err := cehttp.New() - if err != nil { - return nil, err - } - deliverClient, err := ceclient.NewObserved(hp, options.CeClientOptions...) - if err != nil { - return nil, err - } - - p := &SyncPool{ + p := &RetryPool{ targets: targets, options: options, + pubsubClient: pubsubClient, deliverClient: deliverClient, } return p, nil } // SyncOnce syncs once the handler pool based on the targets config. -func (p *SyncPool) SyncOnce(ctx context.Context) error { +func (p *RetryPool) SyncOnce(ctx context.Context) error { var errs int p.pool.Range(func(key, value interface{}) bool { // Each target represents a trigger. if _, ok := p.targets.GetTargetByKey(key.(string)); !ok { - value.(*handlerCache).Stop() + value.(*retryHandlerCache).Stop() p.pool.Delete(key) } return true @@ -113,32 +106,27 @@ func (p *SyncPool) SyncOnce(ctx context.Context) error { p.targets.RangeAllTargets(func(t *config.Target) bool { if value, ok := p.pool.Load(t.Key()); ok { // Skip if we don't need to renew the handler. - if !value.(*handlerCache).shouldRenew(t) { + if !value.(*retryHandlerCache).shouldRenew(t) { return true } // Stop and clean up the old handler before we start a new one. - value.(*handlerCache).Stop() + value.(*retryHandlerCache).Stop() p.pool.Delete(t.Key()) } - opts := []pubsub.Option{ - pubsub.WithProjectID(p.options.ProjectID), - pubsub.WithTopicID(t.RetryQueue.Topic), - pubsub.WithSubscriptionID(t.RetryQueue.Subscription), - pubsub.WithReceiveSettings(&p.options.PubsubReceiveSettings), - } - - if p.options.PubsubClient != nil { - opts = append(opts, pubsub.WithClient(p.options.PubsubClient)) - } - ps, err := pubsub.New(ctx, opts...) + ps, err := cepubsub.New(ctx, + cepubsub.WithClient(p.pubsubClient), + cepubsub.WithTopicID(t.RetryQueue.Topic), + cepubsub.WithSubscriptionID(t.RetryQueue.Subscription), + cepubsub.WithReceiveSettings(&p.options.PubsubReceiveSettings), + ) if err != nil { logging.FromContext(ctx).Error("failed to create pubsub protocol", zap.String("trigger", t.Key()), zap.Error(err)) errs++ return true } - hc := &handlerCache{ + hc := &retryHandlerCache{ Handler: handler.Handler{ Timeout: p.options.TimeoutPerEvent, PubsubEvents: ps, diff --git a/pkg/broker/handler/pool/retry/pool_test.go b/pkg/broker/handler/pool/retry_test.go similarity index 91% rename from pkg/broker/handler/pool/retry/pool_test.go rename to pkg/broker/handler/pool/retry_test.go index 914f3b6bd5..0db0d19999 100644 --- a/pkg/broker/handler/pool/retry/pool_test.go +++ b/pkg/broker/handler/pool/retry_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package retry +package pool import ( "context" @@ -25,11 +25,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/knative-gcp/pkg/broker/config" - "github.com/google/knative-gcp/pkg/broker/handler/pool" pooltesting "github.com/google/knative-gcp/pkg/broker/handler/pool/testing" ) -func TestWatchAndSync(t *testing.T) { +func TestRetryWatchAndSync(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() testProject := "test-project" @@ -40,19 +39,17 @@ func TestWatchAndSync(t *testing.T) { defer helper.Close() signal := make(chan struct{}) - syncPool, err := NewSyncPool(helper.Targets, - pool.WithPubsubClient(helper.PubsubClient), - pool.WithProjectID(testProject)) + syncPool, err := InitializeTestRetryPool(helper.Targets, helper.PubsubClient) if err != nil { t.Errorf("unexpected error from getting sync pool: %v", err) } t.Run("start sync pool creates no handler", func(t *testing.T) { - _, err = pool.StartSyncPool(ctx, syncPool, signal) + _, err = StartSyncPool(ctx, syncPool, signal) if err != nil { t.Errorf("unexpected error from starting sync pool: %v", err) } - assertHandlers(t, syncPool, helper.Targets) + assertRetryHandlers(t, syncPool, helper.Targets) }) bs := make([]*config.Broker, 0, 4) @@ -67,7 +64,7 @@ func TestWatchAndSync(t *testing.T) { signal <- struct{}{} // Wait a short period for the handlers to be updated. <-time.After(time.Second) - assertHandlers(t, syncPool, helper.Targets) + assertRetryHandlers(t, syncPool, helper.Targets) }) t.Run("delete and adding targets in brokers", func(t *testing.T) { @@ -80,7 +77,7 @@ func TestWatchAndSync(t *testing.T) { signal <- struct{}{} // Wait a short period for the handlers to be updated. <-time.After(time.Second) - assertHandlers(t, syncPool, helper.Targets) + assertRetryHandlers(t, syncPool, helper.Targets) }) t.Run("deleting all brokers with their targets", func(t *testing.T) { @@ -91,7 +88,7 @@ func TestWatchAndSync(t *testing.T) { signal <- struct{}{} // Wait a short period for the handlers to be updated. <-time.After(time.Second) - assertHandlers(t, syncPool, helper.Targets) + assertRetryHandlers(t, syncPool, helper.Targets) }) } @@ -115,14 +112,12 @@ func TestRetrySyncPoolE2E(t *testing.T) { t3 := helper.GenerateTarget(ctx, t, b2.Key(), nil) signal := make(chan struct{}) - syncPool, err := NewSyncPool(helper.Targets, - pool.WithPubsubClient(helper.PubsubClient), - pool.WithProjectID(testProject)) + syncPool, err := InitializeTestRetryPool(helper.Targets, helper.PubsubClient) if err != nil { t.Errorf("unexpected error from getting sync pool: %v", err) } - if _, err := pool.StartSyncPool(ctx, syncPool, signal); err != nil { + if _, err := StartSyncPool(ctx, syncPool, signal); err != nil { t.Errorf("unexpected error from starting sync pool: %v", err) } @@ -225,7 +220,7 @@ func TestRetrySyncPoolE2E(t *testing.T) { }) } -func assertHandlers(t *testing.T, p *SyncPool, targets config.Targets) { +func assertRetryHandlers(t *testing.T, p *RetryPool, targets config.Targets) { t.Helper() gotHandlers := make(map[string]bool) wantHandlers := make(map[string]bool) diff --git a/pkg/broker/handler/pool/wire.go b/pkg/broker/handler/pool/wire.go new file mode 100644 index 0000000000..f5e69b81cf --- /dev/null +++ b/pkg/broker/handler/pool/wire.go @@ -0,0 +1,58 @@ +// +build wireinject + +/* +Copyright 2020 Google LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pool + +import ( + "context" + + "cloud.google.com/go/pubsub" + cehttp "github.com/cloudevents/sdk-go/v2/protocol/http" + "github.com/google/knative-gcp/pkg/broker/config" + "github.com/google/wire" +) + +func InitializeTestFanoutPool( + ctx context.Context, + targets config.ReadonlyTargets, + pubsubClient *pubsub.Client, + opts ...Option, +) (*FanoutPool, error) { + panic(wire.Build( + NewFanoutPool, + NewDeliverClient, + NewRetryClient, + cehttp.New, + wire.Value([]cehttp.Option(nil)), + wire.Value(DefaultCEClientOpts), + )) +} + +func InitializeTestRetryPool( + targets config.ReadonlyTargets, + pubsubClient *pubsub.Client, + opts ...Option, +) (*RetryPool, error) { + panic(wire.Build( + NewRetryPool, + NewDeliverClient, + cehttp.New, + wire.Value([]cehttp.Option(nil)), + wire.Value(DefaultCEClientOpts), + )) +} diff --git a/pkg/broker/handler/pool/wire_gen.go b/pkg/broker/handler/pool/wire_gen.go new file mode 100644 index 0000000000..0056e74e1d --- /dev/null +++ b/pkg/broker/handler/pool/wire_gen.go @@ -0,0 +1,65 @@ +// Code generated by Wire. DO NOT EDIT. + +//go:generate wire +//+build !wireinject + +package pool + +import ( + "cloud.google.com/go/pubsub" + "context" + "github.com/cloudevents/sdk-go/v2/protocol/http" + "github.com/google/knative-gcp/pkg/broker/config" +) + +// Injectors from wire.go: + +func InitializeTestFanoutPool(ctx context.Context, targets config.ReadonlyTargets, pubsubClient *pubsub.Client, opts ...Option) (*FanoutPool, error) { + v := _wireValue + protocol, err := http.New(v...) + if err != nil { + return nil, err + } + v2 := _wireValue2 + deliverClient, err := NewDeliverClient(protocol, v2...) + if err != nil { + return nil, err + } + retryClient, err := NewRetryClient(ctx, pubsubClient, v2...) + if err != nil { + return nil, err + } + fanoutPool, err := NewFanoutPool(targets, pubsubClient, deliverClient, retryClient, opts...) + if err != nil { + return nil, err + } + return fanoutPool, nil +} + +var ( + _wireValue = []http.Option(nil) + _wireValue2 = DefaultCEClientOpts +) + +func InitializeTestRetryPool(targets config.ReadonlyTargets, pubsubClient *pubsub.Client, opts ...Option) (*RetryPool, error) { + v := _wireValue3 + protocol, err := http.New(v...) + if err != nil { + return nil, err + } + v2 := _wireValue4 + deliverClient, err := NewDeliverClient(protocol, v2...) + if err != nil { + return nil, err + } + retryPool, err := NewRetryPool(targets, pubsubClient, deliverClient, opts...) + if err != nil { + return nil, err + } + return retryPool, nil +} + +var ( + _wireValue3 = []http.Option(nil) + _wireValue4 = DefaultCEClientOpts +) From 02be60d1e984f42d23550774d7dfe40d07ef1945 Mon Sep 17 00:00:00 2001 From: Ian Milligan Date: Thu, 7 May 2020 15:33:45 -0700 Subject: [PATCH 06/11] Unpin k8s.io/gengo (#1009) Updates gengo to v0.0.0-20200205140755-e0e292d8aa12. --- go.mod | 1 - go.sum | 8 +- vendor/k8s.io/gengo/args/args.go | 5 +- .../import-boss/generators/import_restrict.go | 215 ++++++++++++++---- .../k8s.io/gengo/generator/default_package.go | 7 +- vendor/k8s.io/gengo/generator/execute.go | 12 +- vendor/k8s.io/gengo/generator/generator.go | 53 ++++- .../gengo/generator/transitive_closure.go | 65 ++++++ vendor/k8s.io/gengo/namer/order.go | 3 + vendor/k8s.io/gengo/namer/plural_namer.go | 2 +- vendor/k8s.io/gengo/parser/parse.go | 51 ++++- vendor/k8s.io/gengo/types/types.go | 27 +++ vendor/modules.txt | 2 +- 13 files changed, 380 insertions(+), 71 deletions(-) create mode 100644 vendor/k8s.io/gengo/generator/transitive_closure.go diff --git a/go.mod b/go.mod index 7cee9b4a64..b9e390f406 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,6 @@ replace ( k8s.io/apimachinery => k8s.io/apimachinery v0.16.5-beta.1 k8s.io/client-go => k8s.io/client-go v0.16.4 k8s.io/code-generator => k8s.io/code-generator v0.16.5-beta.1 - k8s.io/gengo => k8s.io/gengo v0.0.0-20190907103519-ebc107f98eab k8s.io/kube-openapi => k8s.io/kube-openapi v0.0.0-20190918143330-0270cf2f1c1d ) diff --git a/go.sum b/go.sum index 9df88b0295..ecc5429a65 100644 --- a/go.sum +++ b/go.sum @@ -1219,8 +1219,12 @@ k8s.io/component-base v0.17.2/go.mod h1:zMPW3g5aH7cHJpKYQ/ZsGMcgbsA/VyhEugF3QT1a k8s.io/component-base v0.17.4/go.mod h1:5BRqHMbbQPm2kKu35v3G+CpVq4K0RJKC7TRioF0I9lE= k8s.io/csi-translation-lib v0.17.0/go.mod h1:HEF7MEz7pOLJCnxabi45IPkhSsE/KmxPQksuCrHKWls= k8s.io/csi-translation-lib v0.17.4/go.mod h1:CsxmjwxEI0tTNMzffIAcgR9lX4wOh6AKHdxQrT7L0oo= -k8s.io/gengo v0.0.0-20190907103519-ebc107f98eab h1:j4L8spMe0tFfBvvW6lrc0c+Ql8+nnkcV3RYfi3eSwGY= -k8s.io/gengo v0.0.0-20190907103519-ebc107f98eab/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/gengo v0.0.0-20190306031000-7a1b7fb0289f/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/gengo v0.0.0-20190822140433-26a664648505/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/gengo v0.0.0-20191108084044-e500ee069b5c/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= +k8s.io/gengo v0.0.0-20200205140755-e0e292d8aa12 h1:pZzawYyz6VRNPVYpqGv61LWCimQv1BihyeqFrp50/G4= +k8s.io/gengo v0.0.0-20200205140755-e0e292d8aa12/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/klog v0.0.0-20181102134211-b9b56d5dfc92/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v0.3.0/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v0.3.1/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= diff --git a/vendor/k8s.io/gengo/args/args.go b/vendor/k8s.io/gengo/args/args.go index 7401098c5f..49cc76dac9 100644 --- a/vendor/k8s.io/gengo/args/args.go +++ b/vendor/k8s.io/gengo/args/args.go @@ -112,7 +112,7 @@ func (g *GeneratorArgs) LoadGoBoilerplate() ([]byte, error) { if err != nil { return nil, err } - b = bytes.Replace(b, []byte("YEAR"), []byte(strconv.Itoa(time.Now().Year())), -1) + b = bytes.Replace(b, []byte("YEAR"), []byte(strconv.Itoa(time.Now().UTC().Year())), -1) if g.GeneratedByCommentTemplate != "" { if len(b) != 0 { @@ -159,6 +159,9 @@ func (g *GeneratorArgs) InputIncludes(p *types.Package) bool { if strings.HasSuffix(d, "...") { d = strings.TrimSuffix(d, "...") } + if strings.HasPrefix(d, "./vendor/") { + d = strings.TrimPrefix(d, "./vendor/") + } if strings.HasPrefix(p.Path, d) { return true } diff --git a/vendor/k8s.io/gengo/examples/import-boss/generators/import_restrict.go b/vendor/k8s.io/gengo/examples/import-boss/generators/import_restrict.go index ea5716b6ce..308bb6b8d2 100644 --- a/vendor/k8s.io/gengo/examples/import-boss/generators/import_restrict.go +++ b/vendor/k8s.io/gengo/examples/import-boss/generators/import_restrict.go @@ -33,11 +33,13 @@ import ( "k8s.io/gengo/generator" "k8s.io/gengo/namer" "k8s.io/gengo/types" + "sigs.k8s.io/yaml" "k8s.io/klog" ) const ( + goModFile = "go.mod" importBossFileType = "import-boss" ) @@ -58,7 +60,7 @@ func DefaultNameSystem() string { func Packages(c *generator.Context, arguments *args.GeneratorArgs) generator.Packages { pkgs := generator.Packages{} c.FileTypes = map[string]generator.FileType{ - importBossFileType: importRuleFile{}, + importBossFileType: importRuleFile{c}, } for _, p := range c.Universe { @@ -70,6 +72,7 @@ func Packages(c *generator.Context, arguments *args.GeneratorArgs) generator.Pac pkgs = append(pkgs, &generator.DefaultPackage{ PackageName: p.Name, PackagePath: p.Path, + Source: p.SourcePath, // GeneratorFunc returns a list of generators. Each generator makes a // single file. GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) { @@ -96,10 +99,19 @@ type Rule struct { ForbiddenPrefixes []string } +type InverseRule struct { + Rule + // True if the rule is to be applied to transitive imports. + Transitive bool +} + type fileFormat struct { CurrentImports []string - Rules []Rule + Rules []Rule + InverseRules []InverseRule + + path string } func readFile(path string) (*fileFormat, error) { @@ -109,10 +121,11 @@ func readFile(path string) (*fileFormat, error) { } var current fileFormat - err = json.Unmarshal(currentBytes, ¤t) + err = yaml.Unmarshal(currentBytes, ¤t) if err != nil { return nil, fmt.Errorf("couldn't unmarshal %v: %v", path, err) } + current.path = path return ¤t, nil } @@ -131,10 +144,12 @@ func writeFile(path string, ff *fileFormat) error { } // This does the actual checking, since it knows the literal destination file. -type importRuleFile struct{} +type importRuleFile struct { + context *generator.Context +} -func (importRuleFile) AssembleFile(f *generator.File, path string) error { - return nil +func (irf importRuleFile) AssembleFile(f *generator.File, path string) error { + return irf.VerifyFile(f, path) } // TODO: make a flag to enable this, or expose this information in some other way. @@ -169,62 +184,99 @@ func removeLastDir(path string) (newPath, removedDir string) { return filepath.Join(filepath.Dir(dir), file), filepath.Base(dir) } -// Keep going up a directory until we find an .import-restrictions file. -func recursiveRead(path string) (*fileFormat, string, error) { +// isGoModRoot checks if a directory is the root directory for a package +// by checking for the existence of a 'go.mod' file in that directory. +func isGoModRoot(path string) bool { + _, err := os.Stat(filepath.Join(filepath.Dir(path), goModFile)) + return err == nil +} + +// recursiveRead collects all '.import-restriction' files, between the current directory, +// and the package root when Go modules are enabled, or $GOPATH/src when they are not. +func recursiveRead(path string) ([]*fileFormat, error) { + restrictionFiles := make([]*fileFormat, 0) + for { if _, err := os.Stat(path); err == nil { - ff, err := readFile(path) - return ff, path, err + rules, err := readFile(path) + if err != nil { + return nil, err + } + + restrictionFiles = append(restrictionFiles, rules) } nextPath, removedDir := removeLastDir(path) - if nextPath == path || removedDir == "src" { + if nextPath == path || isGoModRoot(path) || removedDir == "src" { break } + path = nextPath } - return nil, "", nil + + return restrictionFiles, nil } -func (importRuleFile) VerifyFile(f *generator.File, path string) error { - rules, actualPath, err := recursiveRead(path) +func (irf importRuleFile) VerifyFile(f *generator.File, path string) error { + restrictionFiles, err := recursiveRead(filepath.Join(f.PackageSourcePath, f.Name)) if err != nil { return fmt.Errorf("error finding rules file: %v", err) } - if rules == nil { - // No restrictions on this directory. - return nil + if err := irf.verifyRules(restrictionFiles, f); err != nil { + return err + } + + return irf.verifyInverseRules(restrictionFiles, f) +} + +func (irf importRuleFile) verifyRules(restrictionFiles []*fileFormat, f *generator.File) error { + selectors := make([][]*regexp.Regexp, len(restrictionFiles)) + for i, restrictionFile := range restrictionFiles { + for _, r := range restrictionFile.Rules { + re, err := regexp.Compile(r.SelectorRegexp) + if err != nil { + return fmt.Errorf("regexp `%s` in file %q doesn't compile: %v", r.SelectorRegexp, restrictionFile.path, err) + } + + selectors[i] = append(selectors[i], re) + } } forbiddenImports := map[string]string{} allowedMismatchedImports := []string{} - for _, r := range rules.Rules { - re, err := regexp.Compile(r.SelectorRegexp) - if err != nil { - return fmt.Errorf("regexp `%s` in file %q doesn't compile: %v", r.SelectorRegexp, actualPath, err) - } - for v := range f.Imports { - klog.V(4).Infof("Checking %v matches %v: %v\n", r.SelectorRegexp, v, re.MatchString(v)) - if !re.MatchString(v) { - continue - } - for _, forbidden := range r.ForbiddenPrefixes { - klog.V(4).Infof("Checking %v against %v\n", v, forbidden) - if strings.HasPrefix(v, forbidden) { - forbiddenImports[v] = forbidden + + for v := range f.Imports { + explicitlyAllowed := false + + NextRestrictionFiles: + for i, rules := range restrictionFiles { + for j, r := range rules.Rules { + matching := selectors[i][j].MatchString(v) + klog.V(5).Infof("Checking %v matches %v: %v\n", r.SelectorRegexp, v, matching) + if !matching { + continue } - } - found := false - for _, allowed := range r.AllowedPrefixes { - klog.V(4).Infof("Checking %v against %v\n", v, allowed) - if strings.HasPrefix(v, allowed) { - found = true - break + for _, forbidden := range r.ForbiddenPrefixes { + klog.V(4).Infof("Checking %v against %v\n", v, forbidden) + if strings.HasPrefix(v, forbidden) { + forbiddenImports[v] = forbidden + } + } + for _, allowed := range r.AllowedPrefixes { + klog.V(4).Infof("Checking %v against %v\n", v, allowed) + if strings.HasPrefix(v, allowed) { + explicitlyAllowed = true + break + } + } + + if !explicitlyAllowed { + allowedMismatchedImports = append(allowedMismatchedImports, v) + } else { + klog.V(2).Infof("%v importing %v allowed by %v\n", f.PackagePath, v, restrictionFiles[i].path) + break NextRestrictionFiles } - } - if !found { - allowedMismatchedImports = append(allowedMismatchedImports, v) } } } @@ -243,8 +295,85 @@ func (importRuleFile) VerifyFile(f *generator.File, path string) error { } return errors.New(errorBuilder.String()) } - if len(rules.Rules) > 0 { - klog.V(2).Infof("%v passes rules found in %v\n", path, actualPath) + + return nil +} + +// verifyInverseRules checks that all packages that import a package are allowed to import it. +func (irf importRuleFile) verifyInverseRules(restrictionFiles []*fileFormat, f *generator.File) error { + // compile all Selector regex in all restriction files + selectors := make([][]*regexp.Regexp, len(restrictionFiles)) + for i, restrictionFile := range restrictionFiles { + for _, r := range restrictionFile.InverseRules { + re, err := regexp.Compile(r.SelectorRegexp) + if err != nil { + return fmt.Errorf("regexp `%s` in file %q doesn't compile: %v", r.SelectorRegexp, restrictionFile.path, err) + } + + selectors[i] = append(selectors[i], re) + } + } + + directImport := map[string]bool{} + for _, imp := range irf.context.IncomingImports()[f.PackagePath] { + directImport[imp] = true + } + + forbiddenImports := map[string]string{} + allowedMismatchedImports := []string{} + + for _, v := range irf.context.TransitiveIncomingImports()[f.PackagePath] { + explicitlyAllowed := false + + NextRestrictionFiles: + for i, rules := range restrictionFiles { + for j, r := range rules.InverseRules { + if !r.Transitive && !directImport[v] { + continue + } + + re := selectors[i][j] + matching := re.MatchString(v) + klog.V(4).Infof("Checking %v matches %v (importing %v: %v\n", r.SelectorRegexp, v, f.PackagePath, matching) + if !matching { + continue + } + for _, forbidden := range r.ForbiddenPrefixes { + klog.V(4).Infof("Checking %v against %v\n", v, forbidden) + if strings.HasPrefix(v, forbidden) { + forbiddenImports[v] = forbidden + } + } + for _, allowed := range r.AllowedPrefixes { + klog.V(4).Infof("Checking %v against %v\n", v, allowed) + if strings.HasPrefix(v, allowed) { + explicitlyAllowed = true + break + } + } + if !explicitlyAllowed { + allowedMismatchedImports = append(allowedMismatchedImports, v) + } else { + klog.V(2).Infof("%v importing %v allowed by %v\n", v, f.PackagePath, restrictionFiles[i].path) + break NextRestrictionFiles + } + } + } + } + + if len(forbiddenImports) > 0 || len(allowedMismatchedImports) > 0 { + var errorBuilder strings.Builder + for i, f := range forbiddenImports { + fmt.Fprintf(&errorBuilder, "(inverse): import %v has forbidden prefix %v\n", i, f) + } + if len(allowedMismatchedImports) > 0 { + sort.Sort(sort.StringSlice(allowedMismatchedImports)) + fmt.Fprintf(&errorBuilder, "(inverse): the following imports did not match any allowed prefix:\n") + for _, i := range allowedMismatchedImports { + fmt.Fprintf(&errorBuilder, " %v\n", i) + } + } + return errors.New(errorBuilder.String()) } return nil diff --git a/vendor/k8s.io/gengo/generator/default_package.go b/vendor/k8s.io/gengo/generator/default_package.go index 11517fc6ae..dcf0883235 100644 --- a/vendor/k8s.io/gengo/generator/default_package.go +++ b/vendor/k8s.io/gengo/generator/default_package.go @@ -26,6 +26,8 @@ type DefaultPackage struct { PackageName string // Import path of the package, and the location on disk of the package. PackagePath string + // The location of the package on disk. + Source string // Emitted at the top of every file. HeaderText []byte @@ -43,8 +45,9 @@ type DefaultPackage struct { FilterFunc func(*Context, *types.Type) bool } -func (d *DefaultPackage) Name() string { return d.PackageName } -func (d *DefaultPackage) Path() string { return d.PackagePath } +func (d *DefaultPackage) Name() string { return d.PackageName } +func (d *DefaultPackage) Path() string { return d.PackagePath } +func (d *DefaultPackage) SourcePath() string { return d.Source } func (d *DefaultPackage) Filter(c *Context, t *types.Type) bool { if d.FilterFunc != nil { diff --git a/vendor/k8s.io/gengo/generator/execute.go b/vendor/k8s.io/gengo/generator/execute.go index b5f5aaeb44..d1b12258c7 100644 --- a/vendor/k8s.io/gengo/generator/execute.go +++ b/vendor/k8s.io/gengo/generator/execute.go @@ -233,11 +233,13 @@ func (c *Context) ExecutePackage(outDir string, p Package) error { if f == nil { // This is the first generator to reference this file, so start it. f = &File{ - Name: g.Filename(), - FileType: fileType, - PackageName: p.Name(), - Header: p.Header(g.Filename()), - Imports: map[string]struct{}{}, + Name: g.Filename(), + FileType: fileType, + PackageName: p.Name(), + PackagePath: p.Path(), + PackageSourcePath: p.SourcePath(), + Header: p.Header(g.Filename()), + Imports: map[string]struct{}{}, } files[f.Name] = f } else { diff --git a/vendor/k8s.io/gengo/generator/generator.go b/vendor/k8s.io/gengo/generator/generator.go index 05a3f65fe8..4b48f503cf 100644 --- a/vendor/k8s.io/gengo/generator/generator.go +++ b/vendor/k8s.io/gengo/generator/generator.go @@ -31,6 +31,8 @@ type Package interface { Name() string // Path returns the package import path. Path() string + // SourcePath returns the location of the package on disk. + SourcePath() string // Filter should return true if this package cares about this type. // Otherwise, this type will be omitted from the type ordering for @@ -50,14 +52,16 @@ type Package interface { } type File struct { - Name string - FileType string - PackageName string - Header []byte - Imports map[string]struct{} - Vars bytes.Buffer - Consts bytes.Buffer - Body bytes.Buffer + Name string + FileType string + PackageName string + Header []byte + PackagePath string + PackageSourcePath string + Imports map[string]struct{} + Vars bytes.Buffer + Consts bytes.Buffer + Body bytes.Buffer } type FileType interface { @@ -156,6 +160,12 @@ type Context struct { // All the types, in case you want to look up something. Universe types.Universe + // Incoming imports, i.e. packages importing the given package. + incomingImports map[string][]string + + // Incoming transitive imports, i.e. the transitive closure of IncomingImports + incomingTransitiveImports map[string][]string + // All the user-specified packages. This is after recursive expansion. Inputs []string @@ -203,11 +213,36 @@ func NewContext(b *parser.Builder, nameSystems namer.NameSystems, canonicalOrder return c, nil } +// IncomingImports returns the incoming imports for each package. The map is lazily computed. +func (ctxt *Context) IncomingImports() map[string][]string { + if ctxt.incomingImports == nil { + incoming := map[string][]string{} + for _, pkg := range ctxt.Universe { + for imp := range pkg.Imports { + incoming[imp] = append(incoming[imp], pkg.Path) + } + } + ctxt.incomingImports = incoming + } + return ctxt.incomingImports +} + +// TransitiveIncomingImports returns the transitive closure of the incoming imports for each package. +// The map is lazily computed. +func (ctxt *Context) TransitiveIncomingImports() map[string][]string { + if ctxt.incomingTransitiveImports == nil { + ctxt.incomingTransitiveImports = transitiveClosure(ctxt.IncomingImports()) + } + return ctxt.incomingTransitiveImports +} + // AddDir adds a Go package to the context. The specified path must be a single // go package import path. GOPATH, GOROOT, and the location of your go binary // (`which go`) will all be searched, in the normal Go fashion. // Deprecated. Please use AddDirectory. func (ctxt *Context) AddDir(path string) error { + ctxt.incomingImports = nil + ctxt.incomingTransitiveImports = nil return ctxt.builder.AddDirTo(path, &ctxt.Universe) } @@ -215,5 +250,7 @@ func (ctxt *Context) AddDir(path string) error { // single go package import path. GOPATH, GOROOT, and the location of your go // binary (`which go`) will all be searched, in the normal Go fashion. func (ctxt *Context) AddDirectory(path string) (*types.Package, error) { + ctxt.incomingImports = nil + ctxt.incomingTransitiveImports = nil return ctxt.builder.AddDirectoryTo(path, &ctxt.Universe) } diff --git a/vendor/k8s.io/gengo/generator/transitive_closure.go b/vendor/k8s.io/gengo/generator/transitive_closure.go new file mode 100644 index 0000000000..385a49fce3 --- /dev/null +++ b/vendor/k8s.io/gengo/generator/transitive_closure.go @@ -0,0 +1,65 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package generator + +import "sort" + +type edge struct { + from string + to string +} + +func transitiveClosure(in map[string][]string) map[string][]string { + adj := make(map[edge]bool) + imports := make(map[string]struct{}) + for from, tos := range in { + for _, to := range tos { + adj[edge{from, to}] = true + imports[to] = struct{}{} + } + } + + // Warshal's algorithm + for k := range in { + for i := range in { + if !adj[edge{i, k}] { + continue + } + for j := range imports { + if adj[edge{i, j}] { + continue + } + if adj[edge{k, j}] { + adj[edge{i, j}] = true + } + } + } + } + + out := make(map[string][]string, len(in)) + for i := range in { + for j := range imports { + if adj[edge{i, j}] { + out[i] = append(out[i], j) + } + } + + sort.Strings(out[i]) + } + + return out +} diff --git a/vendor/k8s.io/gengo/namer/order.go b/vendor/k8s.io/gengo/namer/order.go index f86282b9b1..fd89be9b08 100644 --- a/vendor/k8s.io/gengo/namer/order.go +++ b/vendor/k8s.io/gengo/namer/order.go @@ -43,6 +43,9 @@ func (o *Orderer) OrderUniverse(u types.Universe) []*types.Type { for _, v := range p.Variables { list.types = append(list.types, v) } + for _, v := range p.Constants { + list.types = append(list.types, v) + } } sort.Sort(list) return list.types diff --git a/vendor/k8s.io/gengo/namer/plural_namer.go b/vendor/k8s.io/gengo/namer/plural_namer.go index a9a198a702..0e3ebbf262 100644 --- a/vendor/k8s.io/gengo/namer/plural_namer.go +++ b/vendor/k8s.io/gengo/namer/plural_namer.go @@ -22,7 +22,7 @@ import ( "k8s.io/gengo/types" ) -var consonants = "bcdfghjklmnpqrsttvwxyz" +var consonants = "bcdfghjklmnpqrstvwxyz" type pluralNamer struct { // key is the case-sensitive type name, value is the case-insensitive diff --git a/vendor/k8s.io/gengo/parser/parse.go b/vendor/k8s.io/gengo/parser/parse.go index 6a3d53b256..f3abe57cce 100644 --- a/vendor/k8s.io/gengo/parser/parse.go +++ b/vendor/k8s.io/gengo/parser/parse.go @@ -28,6 +28,7 @@ import ( "os/exec" "path" "path/filepath" + "regexp" "sort" "strings" @@ -227,12 +228,16 @@ func (b *Builder) AddDirRecursive(dir string) error { klog.Warningf("Ignoring directory %v: %v", dir, err) } - // filepath.Walk includes the root dir, but we already did that, so we'll - // remove that prefix and rebuild a package import path. - prefix := b.buildPackages[dir].Dir + // filepath.Walk does not follow symlinks. We therefore evaluate symlinks and use that with + // filepath.Walk. + realPath, err := filepath.EvalSymlinks(b.buildPackages[dir].Dir) + if err != nil { + return err + } + fn := func(filePath string, info os.FileInfo, err error) error { if info != nil && info.IsDir() { - rel := filepath.ToSlash(strings.TrimPrefix(filePath, prefix)) + rel := filepath.ToSlash(strings.TrimPrefix(filePath, realPath)) if rel != "" { // Make a pkg path. pkg := path.Join(string(canonicalizeImportPath(b.buildPackages[dir].ImportPath)), rel) @@ -245,7 +250,7 @@ func (b *Builder) AddDirRecursive(dir string) error { } return nil } - if err := filepath.Walk(b.buildPackages[dir].Dir, fn); err != nil { + if err := filepath.Walk(realPath, fn); err != nil { return err } return nil @@ -330,6 +335,12 @@ func (b *Builder) addDir(dir string, userRequested bool) error { return nil } +var regexErrPackageNotFound = regexp.MustCompile(`^unable to import ".*?": cannot find package ".*?" in any of:`) + +func isErrPackageNotFound(err error) bool { + return regexErrPackageNotFound.MatchString(err.Error()) +} + // importPackage is a function that will be called by the type check package when it // needs to import a go package. 'path' is the import path. func (b *Builder) importPackage(dir string, userRequested bool) (*tc.Package, error) { @@ -352,6 +363,11 @@ func (b *Builder) importPackage(dir string, userRequested bool) (*tc.Package, er // Add it. if err := b.addDir(dir, userRequested); err != nil { + if isErrPackageNotFound(err) { + klog.V(6).Info(err) + return nil, nil + } + return nil, err } @@ -544,6 +560,10 @@ func (b *Builder) findTypesIn(pkgPath importPathString, u *types.Universe) error if ok && !tv.IsField() { b.addVariable(*u, nil, tv) } + tconst, ok := obj.(*tc.Const) + if ok { + b.addConstant(*u, nil, tconst) + } } importedPkgs := []string{} @@ -742,7 +762,10 @@ func (b *Builder) walkType(u types.Universe, useName *types.Name, in tc.Type) *t if out.Methods == nil { out.Methods = map[string]*types.Type{} } - out.Methods[t.Method(i).Name()] = b.walkType(u, nil, t.Method(i).Type()) + method := t.Method(i) + mt := b.walkType(u, nil, method.Type()) + mt.CommentLines = splitLines(b.priorCommentLines(method.Pos(), 1).Text()) + out.Methods[method.Name()] = mt } return out case *tc.Named: @@ -774,7 +797,10 @@ func (b *Builder) walkType(u types.Universe, useName *types.Name, in tc.Type) *t if out.Methods == nil { out.Methods = map[string]*types.Type{} } - out.Methods[t.Method(i).Name()] = b.walkType(u, nil, t.Method(i).Type()) + method := t.Method(i) + mt := b.walkType(u, nil, method.Type()) + mt.CommentLines = splitLines(b.priorCommentLines(method.Pos(), 1).Text()) + out.Methods[method.Name()] = mt } } return out @@ -811,6 +837,17 @@ func (b *Builder) addVariable(u types.Universe, useName *types.Name, in *tc.Var) return out } +func (b *Builder) addConstant(u types.Universe, useName *types.Name, in *tc.Const) *types.Type { + name := tcVarNameToName(in.String()) + if useName != nil { + name = *useName + } + out := u.Constant(name) + out.Kind = types.DeclarationOf + out.Underlying = b.walkType(u, nil, in.Type()) + return out +} + // canonicalizeImportPath takes an import path and returns the actual package. // It doesn't support nested vendoring. func canonicalizeImportPath(importPath string) importPathString { diff --git a/vendor/k8s.io/gengo/types/types.go b/vendor/k8s.io/gengo/types/types.go index ec25248e7e..78357bcce1 100644 --- a/vendor/k8s.io/gengo/types/types.go +++ b/vendor/k8s.io/gengo/types/types.go @@ -135,6 +135,10 @@ type Package struct { // package name). Variables map[string]*Type + // Global constants within this package, indexed by their name (*not* including + // package name). + Constants map[string]*Type + // Packages imported by this package, indexed by (canonicalized) // package path. Imports map[string]*Package @@ -193,6 +197,20 @@ func (p *Package) Variable(varName string) *Type { return t } +// Constant gets the given constant Type in this Package. If the constant is +// not already defined, this will add it. If a constant is added, it's the caller's +// responsibility to finish construction of the constant by setting Underlying +// to the correct type. +func (p *Package) Constant(constName string) *Type { + if t, ok := p.Constants[constName]; ok { + return t + } + t := &Type{Name: Name{Package: p.Path, Name: constName}} + t.Kind = DeclarationOf + p.Constants[constName] = t + return t +} + // HasImport returns true if p imports packageName. Package names include the // package directory. func (p *Package) HasImport(packageName string) bool { @@ -229,6 +247,14 @@ func (u Universe) Variable(n Name) *Type { return u.Package(n.Package).Variable(n.Name) } +// Constant returns the canonical constant for the given fully-qualified name. +// If a non-existing constant is requested, this will create (a marker for) it. +// If a marker is created, it's the caller's responsibility to finish +// construction of the constant by setting Underlying to the correct type. +func (u Universe) Constant(n Name) *Type { + return u.Package(n.Package).Constant(n.Name) +} + // AddImports registers import lines for packageName. May be called multiple times. // You are responsible for canonicalizing all package paths. func (u Universe) AddImports(packagePath string, importPaths ...string) { @@ -251,6 +277,7 @@ func (u Universe) Package(packagePath string) *Package { Types: map[string]*Type{}, Functions: map[string]*Type{}, Variables: map[string]*Type{}, + Constants: map[string]*Type{}, Imports: map[string]*Package{}, } u[packagePath] = p diff --git a/vendor/modules.txt b/vendor/modules.txt index 8b7f67abe9..60d231f75f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -924,7 +924,7 @@ k8s.io/code-generator/cmd/set-gen k8s.io/code-generator/pkg/namer k8s.io/code-generator/pkg/util k8s.io/code-generator/third_party/forked/golang/reflect -# k8s.io/gengo v0.0.0-20200205140755-e0e292d8aa12 => k8s.io/gengo v0.0.0-20190907103519-ebc107f98eab +# k8s.io/gengo v0.0.0-20200205140755-e0e292d8aa12 k8s.io/gengo/args k8s.io/gengo/examples/deepcopy-gen/generators k8s.io/gengo/examples/defaulter-gen/generators From c92157a750ca8f89fe9ca8f39f10fb5c0b718958 Mon Sep 17 00:00:00 2001 From: cshou Date: Thu, 7 May 2020 16:50:45 -0700 Subject: [PATCH 07/11] Add event TTL (hops) to broker (#996) * Add event TTL to broker * rename ttl to hops * nit * only count hops in delivery processor * add set hops * make attr shorter --- pkg/broker/eventutil/hops.go | 86 +++++++ pkg/broker/eventutil/hops_test.go | 138 +++++++++++ pkg/broker/handler/pool/fanout_test.go | 10 +- pkg/broker/handler/pool/retry.go | 5 +- pkg/broker/handler/pool/retry_test.go | 2 + pkg/broker/handler/pool/testing/helper.go | 25 +- .../handler/processors/deliver/processor.go | 26 +- .../processors/deliver/processor_test.go | 223 +++++++++++------- pkg/broker/ingress/handler.go | 1 - pkg/broker/ingress/handler_test.go | 2 +- 10 files changed, 425 insertions(+), 93 deletions(-) create mode 100644 pkg/broker/eventutil/hops.go create mode 100644 pkg/broker/eventutil/hops_test.go diff --git a/pkg/broker/eventutil/hops.go b/pkg/broker/eventutil/hops.go new file mode 100644 index 0000000000..06c37a6d05 --- /dev/null +++ b/pkg/broker/eventutil/hops.go @@ -0,0 +1,86 @@ +/* +Copyright 2020 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package eventutil + +import ( + "context" + + cetypes "github.com/cloudevents/sdk-go/pkg/cloudevents/types" + "github.com/cloudevents/sdk-go/v2/event" + "go.uber.org/zap" + "knative.dev/eventing/pkg/logging" +) + +const ( + // Intentionally make it short because the additional attribute + // increases Pubsub message size and could incur additional cost. + hopsAttribute = "kgcphops" +) + +// UpdateRemainingHops update an event with proper remaining hops. +// 1. If the event doesn't have an existing hops value or have an invalid hops value, +// it puts the preemptive hops in the event. +// 2. If the event has an existing valid hops value, +// it decrements it by 1. +func UpdateRemainingHops(ctx context.Context, event *event.Event, preemptiveHops int32) { + hops, ok := GetRemainingHops(ctx, event) + if ok { + // Decrement hops. + hops = hops - 1 + if hops < 0 { + hops = 0 + } + } else { + logging.FromContext(ctx).Debug("Remaining hops not found in event, defaulting to the preemptive value.", + zap.String("event.id", event.ID()), + zap.Int32(hopsAttribute, preemptiveHops), + ) + hops = preemptiveHops + } + + event.SetExtension(hopsAttribute, hops) +} + +// SetRemainingHops sets the remaining hops in the event. +// It ignores any existing hops value. +func SetRemainingHops(ctx context.Context, event *event.Event, hops int32) { + event.SetExtension(hopsAttribute, hops) +} + +// GetRemainingHops returns the remaining hops of the event if it presents. +// If there is no existing hops value or an invalid one, (0, false) will be returned. +func GetRemainingHops(ctx context.Context, event *event.Event) (int32, bool) { + hopsRaw, ok := event.Extensions()[hopsAttribute] + if !ok { + return 0, false + } + hops, err := cetypes.ToInteger(hopsRaw) + if err != nil { + logging.FromContext(ctx).Warn("Failed to convert existing hops value into integer, regarding it as there is no hops value.", + zap.String("event.id", event.ID()), + zap.Any(hopsAttribute, hopsRaw), + zap.Error(err), + ) + return 0, false + } + return hops, true +} + +// DeleteRemainingHops deletes hops from the event extensions. +func DeleteRemainingHops(_ context.Context, event *event.Event) { + event.SetExtension(hopsAttribute, nil) +} diff --git a/pkg/broker/eventutil/hops_test.go b/pkg/broker/eventutil/hops_test.go new file mode 100644 index 0000000000..406d1a089e --- /dev/null +++ b/pkg/broker/eventutil/hops_test.go @@ -0,0 +1,138 @@ +/* +Copyright 2020 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package eventutil + +import ( + "context" + "testing" + + "github.com/cloudevents/sdk-go/v2/event" +) + +func TestGetRemainingHops(t *testing.T) { + cases := []struct { + name string + val interface{} + wantHops int32 + wantOK bool + }{{ + name: "no hops", + val: nil, + }, { + name: "invalid hops", + val: "abc", + }, { + name: "valid hops", + val: 100, + wantOK: true, + wantHops: 100, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + e := event.New() + e.SetExtension(hopsAttribute, tc.val) + gotHops, gotOK := GetRemainingHops(context.Background(), &e) + if gotOK != tc.wantOK { + t.Errorf("Found Hops OK got=%v, want=%v", gotOK, tc.wantOK) + } + if gotHops != tc.wantHops { + t.Errorf("Hops got=%d, want=%d", gotHops, tc.wantHops) + } + }) + } +} + +func TestDeleteRemainingHops(t *testing.T) { + e := event.New() + e.SetExtension(hopsAttribute, 100) + DeleteRemainingHops(context.Background(), &e) + _, ok := e.Extensions()[hopsAttribute] + if ok { + t.Error("After DeleteRemainingHops Hops found got=true, want=false") + } +} + +func TestUpdateHops(t *testing.T) { + cases := []struct { + name string + preemptive int32 + existingHops interface{} + wantHops int32 + }{{ + name: "no existing hops", + preemptive: 100, + existingHops: nil, + wantHops: 100, + }, { + name: "invalid existing hops", + preemptive: 100, + existingHops: "abc", + wantHops: 100, + }, { + name: "valid existing hops", + preemptive: 100, + existingHops: 10, + wantHops: 9, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + e := event.New() + e.SetExtension(hopsAttribute, tc.existingHops) + UpdateRemainingHops(context.Background(), &e, tc.preemptive) + gotHops, ok := GetRemainingHops(context.Background(), &e) + if !ok { + t.Error("Found Hops after UpdateRemainingHops got=false, want=true") + } + if gotHops != tc.wantHops { + t.Errorf("Hops got=%d, want=%d", gotHops, tc.wantHops) + } + }) + } +} + +func TestSetRemainingHops(t *testing.T) { + cases := []struct { + name string + existingHops int32 + hops int32 + }{{ + name: "without existing hops", + existingHops: 0, + hops: 123, + }, { + name: "with existing hops", + existingHops: 99, + hops: 123, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + e := event.New() + e.SetExtension(hopsAttribute, tc.existingHops) + SetRemainingHops(context.Background(), &e, tc.hops) + gotHops, ok := GetRemainingHops(context.Background(), &e) + if !ok { + t.Error("Found Hops after SetRemainingHops got=false, want=true") + } + if gotHops != tc.hops { + t.Errorf("Hops got=%d, want=%d", gotHops, tc.hops) + } + }) + } +} diff --git a/pkg/broker/handler/pool/fanout_test.go b/pkg/broker/handler/pool/fanout_test.go index 65a72b7fc6..93ac072c7c 100644 --- a/pkg/broker/handler/pool/fanout_test.go +++ b/pkg/broker/handler/pool/fanout_test.go @@ -26,6 +26,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/knative-gcp/pkg/broker/config" + "github.com/google/knative-gcp/pkg/broker/eventutil" pooltesting "github.com/google/knative-gcp/pkg/broker/handler/pool/testing" ) @@ -122,11 +123,13 @@ func TestFanoutSyncPoolE2E(t *testing.T) { t.Errorf("unexpected error from starting sync pool: %v", err) } + var hops int32 = 123 e := event.New() e.SetSubject("foo") e.SetType("type") e.SetID("id") e.SetSource("source") + eventutil.UpdateRemainingHops(ctx, &e, hops) t.Run("broker's targets receive fanout events", func(t *testing.T) { // Set timeout context so that verification can be done before @@ -224,13 +227,18 @@ func TestFanoutSyncPoolE2E(t *testing.T) { reply.SetID("id") reply.SetSource("source") + // The reply to broker ingress should include the original hops. + wantReply := reply.Clone() + // -1 because the delivery processor should decrement remaining hops. + eventutil.UpdateRemainingHops(ctx, &wantReply, hops-1) + // Set timeout context so that verification can be done before // exiting test func. vctx, cancel := context.WithTimeout(ctx, 2*time.Second) defer cancel() go helper.VerifyAndRespondNextTargetEvent(ctx, t, t3.Key(), &e, &reply, http.StatusOK, 0) - go helper.VerifyNextBrokerIngressEvent(ctx, t, b2.Key(), &reply) + go helper.VerifyNextBrokerIngressEvent(ctx, t, b2.Key(), &wantReply) helper.SendEventToDecoupleQueue(ctx, t, b2.Key(), &e) <-vctx.Done() diff --git a/pkg/broker/handler/pool/retry.go b/pkg/broker/handler/pool/retry.go index b699068c7c..aa9489513a 100644 --- a/pkg/broker/handler/pool/retry.go +++ b/pkg/broker/handler/pool/retry.go @@ -132,7 +132,10 @@ func (p *RetryPool) SyncOnce(ctx context.Context) error { PubsubEvents: ps, Processor: processors.ChainProcessors( &filter.Processor{Targets: p.targets}, - &deliver.Processor{DeliverClient: p.deliverClient, Targets: p.targets}, + &deliver.Processor{ + DeliverClient: p.deliverClient, + Targets: p.targets, + }, ), }, t: t, diff --git a/pkg/broker/handler/pool/retry_test.go b/pkg/broker/handler/pool/retry_test.go index 0db0d19999..150e294776 100644 --- a/pkg/broker/handler/pool/retry_test.go +++ b/pkg/broker/handler/pool/retry_test.go @@ -25,6 +25,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/knative-gcp/pkg/broker/config" + "github.com/google/knative-gcp/pkg/broker/eventutil" pooltesting "github.com/google/knative-gcp/pkg/broker/handler/pool/testing" ) @@ -246,5 +247,6 @@ func genTestEvent(subject, t, id, source string) event.Event { e.SetType(t) e.SetID(id) e.SetSource(source) + eventutil.UpdateRemainingHops(context.Background(), &e, 123) return e } diff --git a/pkg/broker/handler/pool/testing/helper.go b/pkg/broker/handler/pool/testing/helper.go index 6d1c34016a..356fbb44a6 100644 --- a/pkg/broker/handler/pool/testing/helper.go +++ b/pkg/broker/handler/pool/testing/helper.go @@ -35,6 +35,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/knative-gcp/pkg/broker/config" "github.com/google/knative-gcp/pkg/broker/config/memory" + "github.com/google/knative-gcp/pkg/broker/eventutil" "github.com/google/uuid" "google.golang.org/api/option" "google.golang.org/grpc" @@ -387,6 +388,14 @@ func (h *Helper) VerifyNextTargetEventAndDelayResp(ctx context.Context, t *testi func (h *Helper) VerifyAndRespondNextTargetEvent(ctx context.Context, t *testing.T, targetKey string, wantEvent, replyEvent *event.Event, statusCode int, delay time.Duration) { t.Helper() + // Subscribers should not receive any event with hops. + var wantEventCopy *event.Event + if wantEvent != nil { + copy := wantEvent.Clone() + wantEventCopy = © + eventutil.DeleteRemainingHops(ctx, wantEventCopy) + } + consumer, ok := h.consumers[targetKey] if !ok { t.Errorf("target with key %q doesn't exist", targetKey) @@ -395,7 +404,7 @@ func (h *Helper) VerifyAndRespondNextTargetEvent(ctx context.Context, t *testing // On timeout or receiving an event, the defer function verifies the event in the end. var gotEvent *event.Event defer func() { - assertEvent(t, wantEvent, gotEvent, fmt.Sprintf("target (key=%q)", targetKey)) + assertEvent(t, wantEventCopy, gotEvent, fmt.Sprintf("target (key=%q)", targetKey)) }() msg, respFn, err := consumer.client.Respond(ctx) @@ -461,10 +470,24 @@ func (h *Helper) VerifyNextTargetRetryEvent(ctx context.Context, t *testing.T, t func assertEvent(t *testing.T, want, got *event.Event, msg string) { if got != nil && want != nil { + // Clone events so that we don't modify the original copy. + gotCopy, wantCopy := got.Clone(), want.Clone() + got, want = &gotCopy, &wantCopy + // Ignore time. got.SetTime(want.Time()) // Ignore traceparent. got.SetExtension(extensions.TraceParentExtension, nil) + + // Compare hops explicitly because + // cloudevents client sometimes treat hops value as string internally. + gotHops, _ := eventutil.GetRemainingHops(context.Background(), got) + wantHops, _ := eventutil.GetRemainingHops(context.Background(), want) + if gotHops != wantHops { + t.Errorf("%s event hops got=%d, want=%d", msg, gotHops, wantHops) + } + eventutil.DeleteRemainingHops(context.Background(), want) + eventutil.DeleteRemainingHops(context.Background(), got) } if diff := cmp.Diff(want, got); diff != "" { t.Errorf("%s received event (-want,+got): %v", msg, diff) diff --git a/pkg/broker/handler/processors/deliver/processor.go b/pkg/broker/handler/processors/deliver/processor.go index 24d1babba4..cdb5006989 100644 --- a/pkg/broker/handler/processors/deliver/processor.go +++ b/pkg/broker/handler/processors/deliver/processor.go @@ -29,10 +29,13 @@ import ( "knative.dev/pkg/logging" "github.com/google/knative-gcp/pkg/broker/config" + "github.com/google/knative-gcp/pkg/broker/eventutil" handlerctx "github.com/google/knative-gcp/pkg/broker/handler/context" "github.com/google/knative-gcp/pkg/broker/handler/processors" ) +const defaultEventHopsLimit int32 = 255 + // Processor delivers events based on the broker/target in the context. type Processor struct { processors.BaseProcessor @@ -81,6 +84,15 @@ func (p *Processor) Process(ctx context.Context, event *event.Event) error { return nil } + // Hops is a broker local counter so remove any hops value before forwarding. + // Do not modify the original event as we need to send the original + // event to retry queue on failure. + copy := event.Clone() + // This will decrement the remaining hops if there is an existing value. + eventutil.UpdateRemainingHops(ctx, ©, defaultEventHopsLimit) + hops, _ := eventutil.GetRemainingHops(ctx, ©) + eventutil.DeleteRemainingHops(ctx, ©) + dctx := ctx if p.DeliverTimeout > 0 { var cancel context.CancelFunc @@ -88,7 +100,8 @@ func (p *Processor) Process(ctx context.Context, event *event.Event) error { defer cancel() } - resp, res := p.DeliverClient.Request(cecontext.WithTarget(dctx, target.Address), *event) + // Forward the event copy that has hops removed. + resp, res := p.DeliverClient.Request(cecontext.WithTarget(dctx, target.Address), copy) if !protocol.IsACK(res) { if !p.RetryOnFailure { return fmt.Errorf("target delivery failed: %v", res.Error()) @@ -101,6 +114,17 @@ func (p *Processor) Process(ctx context.Context, event *event.Event) error { return nil } + if hops <= 0 { + logging.FromContext(ctx).Debug("dropping event based on remaining hops status", + zap.String("target", tk), + zap.Int32("hops", hops), + zap.String("event.id", resp.ID())) + return nil + } + + // Attach the previous hops for the reply. + eventutil.SetRemainingHops(ctx, resp, hops) + if res := p.DeliverClient.Send(cecontext.WithTarget(dctx, broker.Address), *resp); !protocol.IsACK(res) { if !p.RetryOnFailure { return fmt.Errorf("delivery of replied event failed: %v", res.Error()) diff --git a/pkg/broker/handler/processors/deliver/processor_test.go b/pkg/broker/handler/processors/deliver/processor_test.go index 6ff3d3f9cb..ca75ddc55a 100644 --- a/pkg/broker/handler/processors/deliver/processor_test.go +++ b/pkg/broker/handler/processors/deliver/processor_test.go @@ -18,6 +18,7 @@ package deliver import ( "context" + "io" "net/http" "net/http/httptest" "testing" @@ -37,6 +38,7 @@ import ( "github.com/google/knative-gcp/pkg/broker/config" "github.com/google/knative-gcp/pkg/broker/config/memory" + "github.com/google/knative-gcp/pkg/broker/eventutil" handlerctx "github.com/google/knative-gcp/pkg/broker/handler/context" ) @@ -56,87 +58,132 @@ func TestInvalidContext(t *testing.T) { } func TestDeliverSuccess(t *testing.T) { - targetClient, err := cehttp.New() - if err != nil { - t.Fatalf("failed to create target cloudevents client: %v", err) - } - ingressClient, err := cehttp.New() - if err != nil { - t.Fatalf("failed to create ingress cloudevents client: %v", err) - } - deliverClient, err := ceclient.NewDefault() - if err != nil { - t.Fatalf("failed to create requester cloudevents client: %v", err) - } - targetSvr := httptest.NewServer(targetClient) - defer targetSvr.Close() - ingressSvr := httptest.NewServer(ingressClient) - defer ingressSvr.Close() - - broker := &config.Broker{Namespace: "ns", Name: "broker"} - target := &config.Target{Namespace: "ns", Name: "target", Broker: "broker", Address: targetSvr.URL} - testTargets := memory.NewEmptyTargets() - testTargets.MutateBroker("ns", "broker", func(bm config.BrokerMutation) { - bm.SetAddress(ingressSvr.URL) - bm.UpsertTargets(target) - }) - ctx := handlerctx.WithBrokerKey(context.Background(), broker.Key()) - ctx = handlerctx.WithTargetKey(ctx, target.Key()) - - p := &Processor{DeliverClient: deliverClient, Targets: testTargets} - - origin := event.New() - origin.SetID("id") - origin.SetSource("source") - origin.SetSubject("subject") - origin.SetType("type") - origin.SetTime(time.Now()) - - reply := origin.Clone() - reply.SetID("reply") - - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - msg, resp, err := targetClient.Respond(ctx) - if err != nil { - t.Errorf("unexpected error from target receiving event: %v", err) - } - if err := resp(ctx, binding.ToMessage(&reply), protocol.ResultACK); err != nil { - t.Errorf("unexpected error from target responding event: %v", err) - } - defer msg.Finish(nil) - gotEvent, err := binding.ToEvent(ctx, msg) - if err != nil { - t.Errorf("target received message cannot be converted to an event: %v", err) - } - // Force the time to be the same so that we can compare easier. - gotEvent.SetTime(origin.Time()) - if diff := cmp.Diff(&origin, gotEvent); diff != "" { - t.Errorf("target received event (-want,+got): %v", diff) - } - }() - - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - msg, err := ingressClient.Receive(ctx) - if err != nil { - t.Errorf("unexpected error from ingress receiving event: %v", err) - } - defer msg.Finish(nil) - gotEvent, err := binding.ToEvent(ctx, msg) - if err != nil { - t.Errorf("ingress received message cannot be converted to an event: %v", err) - } - // Force the time to be the same so that we can compare easier. - if diff := cmp.Diff(&reply, gotEvent); diff != "" { - t.Errorf("ingress received event (-want,+got): %v", diff) - } - }() - - if err := p.Process(ctx, &origin); err != nil { - t.Errorf("unexpected error from processing: %v", err) + sampleEvent := event.New() + sampleEvent.SetID("id") + sampleEvent.SetSource("source") + sampleEvent.SetSubject("subject") + sampleEvent.SetType("type") + sampleEvent.SetTime(time.Now()) + + sampleReply := sampleEvent.Clone() + sampleReply.SetID("reply") + + cases := []struct { + name string + origin *event.Event + wantOrigin *event.Event + reply *event.Event + wantReply *event.Event + }{{ + name: "success", + origin: &sampleEvent, + wantOrigin: &sampleEvent, + reply: &sampleReply, + wantReply: func() *event.Event { + copy := sampleReply.Clone() + eventutil.UpdateRemainingHops(context.Background(), ©, defaultEventHopsLimit) + return © + }(), + }, { + name: "success with dropped reply", + origin: func() *event.Event { + copy := sampleEvent.Clone() + eventutil.UpdateRemainingHops(context.Background(), ©, 1) + return © + }(), + wantOrigin: &sampleEvent, + reply: &sampleReply, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + targetClient, err := cehttp.New() + if err != nil { + t.Fatalf("failed to create target cloudevents client: %v", err) + } + ingressClient, err := cehttp.New() + if err != nil { + t.Fatalf("failed to create ingress cloudevents client: %v", err) + } + deliverClient, err := ceclient.NewDefault() + if err != nil { + t.Fatalf("failed to create requester cloudevents client: %v", err) + } + targetSvr := httptest.NewServer(targetClient) + defer targetSvr.Close() + ingressSvr := httptest.NewServer(ingressClient) + defer ingressSvr.Close() + + broker := &config.Broker{Namespace: "ns", Name: "broker"} + target := &config.Target{Namespace: "ns", Name: "target", Broker: "broker", Address: targetSvr.URL} + testTargets := memory.NewEmptyTargets() + testTargets.MutateBroker("ns", "broker", func(bm config.BrokerMutation) { + bm.SetAddress(ingressSvr.URL) + bm.UpsertTargets(target) + }) + ctx := handlerctx.WithBrokerKey(context.Background(), broker.Key()) + ctx = handlerctx.WithTargetKey(ctx, target.Key()) + + p := &Processor{ + DeliverClient: deliverClient, + Targets: testTargets, + } + + rctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + go func() { + msg, resp, err := targetClient.Respond(rctx) + if err != nil && err != io.EOF { + t.Errorf("unexpected error from target receiving event: %v", err) + } + if err := resp(rctx, binding.ToMessage(tc.reply), protocol.ResultACK); err != nil { + t.Errorf("unexpected error from target responding event: %v", err) + } + defer msg.Finish(nil) + gotEvent, err := binding.ToEvent(rctx, msg) + if err != nil { + t.Errorf("target received message cannot be converted to an event: %v", err) + } + // Force the time to be the same so that we can compare easier. + gotEvent.SetTime(tc.wantOrigin.Time()) + if diff := cmp.Diff(tc.wantOrigin, gotEvent); diff != "" { + t.Errorf("target received event (-want,+got): %v", diff) + } + }() + + go func() { + msg, err := ingressClient.Receive(rctx) + if err != nil && err != io.EOF { + t.Errorf("unexpected error from ingress receiving event: %v", err) + } + var gotEvent *event.Event + if msg != nil { + defer msg.Finish(nil) + var err error + gotEvent, err = binding.ToEvent(rctx, msg) + if err != nil { + t.Errorf("ingress received message cannot be converted to an event: %v", err) + } + // Get and set the hops if it presents. + // HTTP transport changes the internal type of the hops from int32 to string. + if hops, ok := eventutil.GetRemainingHops(rctx, gotEvent); ok { + eventutil.DeleteRemainingHops(rctx, gotEvent) + eventutil.UpdateRemainingHops(rctx, gotEvent, hops) + } + } + // Force the time to be the same so that we can compare easier. + if diff := cmp.Diff(tc.wantReply, gotEvent); diff != "" { + t.Errorf("ingress received event (-want,+got): %v", diff) + } + }() + + if err := p.Process(ctx, tc.origin); err != nil { + t.Errorf("unexpected error from processing: %v", err) + } + + <-rctx.Done() + }) } } @@ -239,11 +286,12 @@ func TestDeliverFailure(t *testing.T) { origin.SetType("type") origin.SetTime(time.Now()) + rctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + go func() { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - msg, resp, err := targetClient.Respond(ctx) - if err != nil { + msg, resp, err := targetClient.Respond(rctx) + if err != nil && err != io.EOF { t.Errorf("unexpected error from target receiving event: %v", err) } defer msg.Finish(nil) @@ -251,7 +299,7 @@ func TestDeliverFailure(t *testing.T) { // If with delay, we reply OK so that we know the error is for sure caused by timeout. if tc.withRespDelay > 0 { time.Sleep(tc.withRespDelay) - if err := resp(ctx, nil, &cehttp.Result{StatusCode: http.StatusOK}); err != nil { + if err := resp(rctx, nil, &cehttp.Result{StatusCode: http.StatusOK}); err != nil { t.Errorf("unexpected error from target responding event: %v", err) } return @@ -259,7 +307,7 @@ func TestDeliverFailure(t *testing.T) { // Due to https://github.com/cloudevents/sdk-go/issues/433 // it's not possible to use Receive to easily return error. - if err := resp(ctx, nil, &cehttp.Result{StatusCode: http.StatusInternalServerError}); err != nil { + if err := resp(rctx, nil, &cehttp.Result{StatusCode: http.StatusInternalServerError}); err != nil { t.Errorf("unexpected error from target responding event: %v", err) } }() @@ -268,6 +316,7 @@ func TestDeliverFailure(t *testing.T) { if (err != nil) != tc.wantErr { t.Errorf("processing got error=%v, want=%v", err, tc.wantErr) } + <-rctx.Done() }) } } diff --git a/pkg/broker/ingress/handler.go b/pkg/broker/ingress/handler.go index ba309065c9..b0dd165512 100644 --- a/pkg/broker/ingress/handler.go +++ b/pkg/broker/ingress/handler.go @@ -74,7 +74,6 @@ type HttpMessageReceiver interface { } // handler receives events and persists them to storage (pubsub). -// TODO(liu-cong) support event TTL type Handler struct { // httpReceiver is an HTTP server to receive events. httpReceiver HttpMessageReceiver diff --git a/pkg/broker/ingress/handler_test.go b/pkg/broker/ingress/handler_test.go index 6a5d5277a2..766eea3adc 100644 --- a/pkg/broker/ingress/handler_test.go +++ b/pkg/broker/ingress/handler_test.go @@ -139,7 +139,7 @@ func TestHandler(t *testing.T) { eventAssertions: []eventAssertion{assertExtensionsExist(EventArrivalTime), assertTraceID(traceID)}, }, { - name: "valid event but unsupported http method", + name: "valid event but unsupported http method", method: "PUT", path: "/ns1/broker1", event: createTestEvent("test-event"), From 82ce353f9289274a8e8f94938139de87df65260e Mon Sep 17 00:00:00 2001 From: Al Zargarpur <58704856+zargarpur@users.noreply.github.com> Date: Thu, 7 May 2020 20:56:45 -0700 Subject: [PATCH 08/11] Add StatsReporter for Broker fanout metrics (#1018) * Add StatsReporter for Broker fanout metrics Follow up PR will use this StatsReporter to emit the metrics defined in here. These metrics are the same one's emitted for the Trigger metrics in OSS knative. Which are also listed in https://cloud.google.com/monitoring/api/metrics_knative (when you search for "internal/eventing") * Update stats_reporter.go * Add the x bit for e2e-secret-tests.sh (#1017) * Move StatsReporters to pkg/metrics & Add Licenses * Add StatsReporter for Broker fanout metrics Follow up PR will use this StatsReporter to emit the metrics defined in here. These metrics are the same one's emitted for the Trigger metrics in OSS knative. Which are also listed in https://cloud.google.com/monitoring/api/metrics_knative (when you search for "internal/eventing") * Move StatsReporters to pkg/metrics & Add Licenses * Move Ingress metrics back to original location * Remove Extras * Namespace instead of duplicated Container * Move again * New Lines * PR Comments * Fix Import Ordering * Include Metric Name explicitly * ian's pr feedback Co-authored-by: Eric Lemar --- cmd/broker/ingress/main.go | 5 +- cmd/broker/ingress/wire.go | 5 +- cmd/broker/ingress/wire_gen.go | 5 +- pkg/broker/ingress/handler.go | 20 ++- pkg/broker/ingress/handler_test.go | 6 +- pkg/metrics/delivery_reporter.go | 169 ++++++++++++++++++ pkg/metrics/delivery_reporter_test.go | 129 +++++++++++++ .../ingress_reporter.go} | 71 +++----- .../ingress_reporter_test.go} | 37 ++-- pkg/metrics/tags.go | 45 +++++ pkg/metrics/testing/helper.go | 24 +++ 11 files changed, 430 insertions(+), 86 deletions(-) create mode 100644 pkg/metrics/delivery_reporter.go create mode 100644 pkg/metrics/delivery_reporter_test.go rename pkg/{broker/ingress/stats_reporter.go => metrics/ingress_reporter.go} (57%) rename pkg/{broker/ingress/stats_reporter_test.go => metrics/ingress_reporter_test.go} (63%) create mode 100644 pkg/metrics/tags.go create mode 100644 pkg/metrics/testing/helper.go diff --git a/cmd/broker/ingress/main.go b/cmd/broker/ingress/main.go index 47676fdf75..30d279e004 100644 --- a/cmd/broker/ingress/main.go +++ b/cmd/broker/ingress/main.go @@ -21,6 +21,7 @@ import ( "log" "github.com/google/knative-gcp/pkg/broker/ingress" + "github.com/google/knative-gcp/pkg/metrics" "github.com/google/knative-gcp/pkg/observability" "github.com/google/knative-gcp/pkg/utils" "github.com/google/knative-gcp/pkg/utils/appcredentials" @@ -91,8 +92,8 @@ func main() { ctx, ingress.Port(env.Port), ingress.ProjectID(projectID), - ingress.PodName(env.PodName), - ingress.ContainerName(containerName), + metrics.PodName(env.PodName), + metrics.ContainerName(containerName), ) if err != nil { logger.Desugar().Fatal("Unable to create ingress handler: ", zap.Error(err)) diff --git a/cmd/broker/ingress/wire.go b/cmd/broker/ingress/wire.go index a16d7ff323..fa8e4343ab 100644 --- a/cmd/broker/ingress/wire.go +++ b/cmd/broker/ingress/wire.go @@ -23,6 +23,7 @@ import ( "github.com/google/knative-gcp/pkg/broker/config/volume" "github.com/google/knative-gcp/pkg/broker/ingress" + "github.com/google/knative-gcp/pkg/metrics" "github.com/google/wire" ) @@ -30,8 +31,8 @@ func InitializeHandler( ctx context.Context, port ingress.Port, projectID ingress.ProjectID, - podName ingress.PodName, - containerName ingress.ContainerName, + podName metrics.PodName, + containerName metrics.ContainerName, ) (*ingress.Handler, error) { panic(wire.Build( ingress.HandlerSet, diff --git a/cmd/broker/ingress/wire_gen.go b/cmd/broker/ingress/wire_gen.go index 427c6107f1..e17d4fd3f0 100644 --- a/cmd/broker/ingress/wire_gen.go +++ b/cmd/broker/ingress/wire_gen.go @@ -9,11 +9,12 @@ import ( "context" "github.com/google/knative-gcp/pkg/broker/config/volume" "github.com/google/knative-gcp/pkg/broker/ingress" + "github.com/google/knative-gcp/pkg/metrics" ) // Injectors from wire.go: -func InitializeHandler(ctx context.Context, port ingress.Port, projectID ingress.ProjectID, podName ingress.PodName, containerName2 ingress.ContainerName) (*ingress.Handler, error) { +func InitializeHandler(ctx context.Context, port ingress.Port, projectID ingress.ProjectID, podName metrics.PodName, containerName2 metrics.ContainerName) (*ingress.Handler, error) { httpMessageReceiver := ingress.NewHTTPMessageReceiver(port) v := _wireValue readonlyTargets, err := volume.NewTargetsFromFile(v...) @@ -29,7 +30,7 @@ func InitializeHandler(ctx context.Context, port ingress.Port, projectID ingress return nil, err } multiTopicDecoupleSink := ingress.NewMultiTopicDecoupleSink(ctx, readonlyTargets, clientClient) - statsReporter, err := ingress.NewStatsReporter(podName, containerName2) + statsReporter, err := metrics.NewIngressReporter(podName, containerName2) if err != nil { return nil, err } diff --git a/pkg/broker/ingress/handler.go b/pkg/broker/ingress/handler.go index b0dd165512..5ab6e190d2 100644 --- a/pkg/broker/ingress/handler.go +++ b/pkg/broker/ingress/handler.go @@ -27,10 +27,12 @@ import ( "go.uber.org/zap" cev2 "github.com/cloudevents/sdk-go/v2" + "github.com/cloudevents/sdk-go/v2/binding" "github.com/cloudevents/sdk-go/v2/binding/transformer" "github.com/cloudevents/sdk-go/v2/protocol" "github.com/cloudevents/sdk-go/v2/protocol/http" + "github.com/google/knative-gcp/pkg/metrics" "github.com/google/wire" "knative.dev/eventing/pkg/kncloudevents" "knative.dev/eventing/pkg/logging" @@ -59,7 +61,7 @@ var HandlerSet wire.ProviderSet = wire.NewSet( wire.Bind(new(DecoupleSink), new(*multiTopicDecoupleSink)), NewPubsubClient, NewPubsubDecoupleClient, - NewStatsReporter, + metrics.NewIngressReporter, ) // DecoupleSink is an interface to send events to a decoupling sink (e.g., pubsub). @@ -80,11 +82,11 @@ type Handler struct { // decouple is the client to send events to a decouple sink. decouple DecoupleSink logger *zap.Logger - reporter *StatsReporter + reporter *metrics.IngressReporter } // NewHandler creates a new ingress handler. -func NewHandler(ctx context.Context, httpReceiver HttpMessageReceiver, decouple DecoupleSink, reporter *StatsReporter) *Handler { +func NewHandler(ctx context.Context, httpReceiver HttpMessageReceiver, decouple DecoupleSink, reporter *metrics.IngressReporter) *Handler { return &Handler{ httpReceiver: httpReceiver, decouple: decouple, @@ -170,13 +172,13 @@ func (h *Handler) toEvent(request *nethttp.Request) (event *cev2.Event, msg stri } func (h *Handler) reportMetrics(ctx context.Context, ns, broker string, event *cev2.Event, statusCode int, start time.Time) { - args := reportArgs{ - namespace: ns, - broker: broker, - eventType: event.Type(), - responseCode: statusCode, + args := metrics.IngressReportArgs{ + Namespace: ns, + Broker: broker, + EventType: event.Type(), + ResponseCode: statusCode, } - if err := h.reporter.reportEventDispatchTime(ctx, args, time.Since(start)); err != nil { + if err := h.reporter.ReportEventDispatchTime(ctx, args, time.Since(start)); err != nil { h.logger.Warn("Failed to record metrics.", zap.Any("namespace", ns), zap.Any("broker", broker), zap.Error(err)) } } diff --git a/pkg/broker/ingress/handler_test.go b/pkg/broker/ingress/handler_test.go index 766eea3adc..1fd801bf29 100644 --- a/pkg/broker/ingress/handler_test.go +++ b/pkg/broker/ingress/handler_test.go @@ -36,6 +36,8 @@ import ( cepubsub "github.com/cloudevents/sdk-go/v2/protocol/pubsub" "github.com/google/knative-gcp/pkg/broker/config" "github.com/google/knative-gcp/pkg/broker/config/memory" + "github.com/google/knative-gcp/pkg/metrics" + reportertest "github.com/google/knative-gcp/pkg/metrics/testing" "google.golang.org/api/option" "google.golang.org/grpc" "knative.dev/eventing/pkg/kncloudevents" @@ -227,7 +229,7 @@ func TestHandler(t *testing.T) { defer client.CloseIdleConnections() for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - resetMetrics() + reportertest.ResetIngressMetrics() ctx := logging.WithLogger(context.Background(), logtest.TestLogger(t)) ctx, cancel := context.WithTimeout(ctx, 3*time.Second) defer cancel() @@ -327,7 +329,7 @@ func createAndStartIngress(ctx context.Context, t *testing.T, psSrv *pstest.Serv decouple := NewMultiTopicDecoupleSink(ctx, memory.NewTargets(brokerConfig), client) receiver := &testHttpMessageReceiver{urlCh: make(chan string)} - statsReporter, err := NewStatsReporter(PodName(pod), ContainerName(container)) + statsReporter, err := metrics.NewIngressReporter(metrics.PodName(pod), metrics.ContainerName(container)) if err != nil { cancel() t.Fatal(err) diff --git a/pkg/metrics/delivery_reporter.go b/pkg/metrics/delivery_reporter.go new file mode 100644 index 0000000000..2881455652 --- /dev/null +++ b/pkg/metrics/delivery_reporter.go @@ -0,0 +1,169 @@ +/* +Copyright 2020 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "context" + "fmt" + "strconv" + "time" + + "go.opencensus.io/stats" + "go.opencensus.io/stats/view" + "go.opencensus.io/tag" + "knative.dev/pkg/metrics" +) + +type DeliveryReporter struct { + podName PodName + containerName ContainerName + dispatchTimeInMsecM *stats.Float64Measure + processingTimeInMsecM *stats.Float64Measure +} + +type DeliveryReportArgs struct { + Namespace string + Broker string + Trigger string + FilterType string +} + +func (r *DeliveryReporter) register() error { + return view.Register( + &view.View{ + Name: "event_count", + Description: "Number of events delivered to a Trigger subscriber", + Measure: r.dispatchTimeInMsecM, + Aggregation: view.Count(), + TagKeys: []tag.Key{ + NamespaceNameKey, + BrokerNameKey, + TriggerNameKey, + TriggerFilterTypeKey, + ResponseCodeKey, + ResponseCodeClassKey, + PodNameKey, + ContainerNameKey, + }, + }, + &view.View{ + Name: r.dispatchTimeInMsecM.Name(), + Description: r.dispatchTimeInMsecM.Description(), + Measure: r.dispatchTimeInMsecM, + Aggregation: view.Distribution(metrics.Buckets125(1, 10000)...), // 1, 2, 5, 10, 20, 50, 100, 1000, 5000, 10000 + TagKeys: []tag.Key{ + NamespaceNameKey, + BrokerNameKey, + TriggerNameKey, + TriggerFilterTypeKey, + ResponseCodeKey, + ResponseCodeClassKey, + PodNameKey, + ContainerNameKey, + }, + }, + &view.View{ + Name: r.processingTimeInMsecM.Name(), + Description: r.processingTimeInMsecM.Description(), + Measure: r.processingTimeInMsecM, + Aggregation: view.Distribution(metrics.Buckets125(1, 10000)...), // 1, 2, 5, 10, 20, 50, 100, 1000, 5000, 10000 + TagKeys: []tag.Key{ + NamespaceNameKey, + BrokerNameKey, + TriggerNameKey, + TriggerFilterTypeKey, + PodNameKey, + ContainerNameKey, + }, + }, + ) +} + +// NewDeliveryReporter creates a new DeliveryReporter. +func NewDeliveryReporter(podName PodName, containerName ContainerName) (*DeliveryReporter, error) { + r := &DeliveryReporter{ + podName: podName, + containerName: containerName, + // dispatchTimeInMsecM records the time spent dispatching an event to + // a Trigger subscriber, in milliseconds. + dispatchTimeInMsecM: stats.Float64( + "event_dispatch_latencies", + "The time spent dispatching an event to a Trigger subscriber", + stats.UnitMilliseconds, + ), + // processingTimeInMsecM records the time spent between arrival at the Broker + // and the delivery to the Trigger subscriber. + processingTimeInMsecM: stats.Float64( + "event_processing_latencies", + "The time spent processing an event before it is dispatched to a Trigger subscriber", + stats.UnitMilliseconds, + ), + } + + if err := r.register(); err != nil { + return nil, fmt.Errorf("failed to register delivery stats: %w", err) + } + return r, nil +} + +// ReportEventDispatchTime captures dispatch times. +func (r *DeliveryReporter) ReportEventDispatchTime(ctx context.Context, args DeliveryReportArgs, d time.Duration, responseCode int) error { + tag, err := tag.New( + ctx, + tag.Insert(NamespaceNameKey, args.Namespace), + tag.Insert(BrokerNameKey, args.Broker), + tag.Insert(TriggerNameKey, args.Trigger), + tag.Insert(TriggerFilterTypeKey, filterTypeValue(args.FilterType)), + tag.Insert(PodNameKey, string(r.podName)), + tag.Insert(ContainerNameKey, string(r.containerName)), + tag.Insert(ResponseCodeKey, strconv.Itoa(responseCode)), + tag.Insert(ResponseCodeClassKey, metrics.ResponseCodeClass(responseCode)), + ) + if err != nil { + return fmt.Errorf("failed to create metrics tag: %v", err) + } + // convert time.Duration in nanoseconds to milliseconds. + metrics.Record(tag, r.dispatchTimeInMsecM.M(float64(d/time.Millisecond))) + return nil +} + +// ReportEventProcessingTime captures event processing times. +func (r *DeliveryReporter) ReportEventProcessingTime(ctx context.Context, args DeliveryReportArgs, d time.Duration) error { + tag, err := tag.New( + ctx, + tag.Insert(NamespaceNameKey, args.Namespace), + tag.Insert(BrokerNameKey, args.Broker), + tag.Insert(TriggerNameKey, args.Trigger), + tag.Insert(TriggerFilterTypeKey, filterTypeValue(args.FilterType)), + tag.Insert(PodNameKey, string(r.podName)), + tag.Insert(ContainerNameKey, string(r.containerName)), + ) + if err != nil { + return fmt.Errorf("failed to create metrics tag: %v", err) + } + // convert time.Duration in nanoseconds to milliseconds. + metrics.Record(tag, r.processingTimeInMsecM.M(float64(d/time.Millisecond))) + return nil +} + +func filterTypeValue(v string) string { + if v != "" { + return v + } + // the default value if the filter attributes are empty. + return "any" +} diff --git a/pkg/metrics/delivery_reporter_test.go b/pkg/metrics/delivery_reporter_test.go new file mode 100644 index 0000000000..724093878e --- /dev/null +++ b/pkg/metrics/delivery_reporter_test.go @@ -0,0 +1,129 @@ +/* +Copyright 2020 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "context" + "testing" + "time" + + reportertest "github.com/google/knative-gcp/pkg/metrics/testing" + "knative.dev/pkg/metrics/metricskey" + "knative.dev/pkg/metrics/metricstest" +) + +func TestReportEventDispatchTime(t *testing.T) { + reportertest.ResetDeliveryMetrics() + + args := DeliveryReportArgs{ + Namespace: "testns", + Broker: "testbroker", + Trigger: "testtrigger", + FilterType: "testeventtype", + } + + wantTags := map[string]string{ + metricskey.LabelNamespaceName: "testns", + metricskey.LabelBrokerName: "testbroker", + metricskey.LabelTriggerName: "testtrigger", + metricskey.LabelFilterType: "testeventtype", + metricskey.LabelResponseCode: "202", + metricskey.LabelResponseCodeClass: "2xx", + metricskey.PodName: "testpod", + metricskey.ContainerName: "testcontainer", + } + + r, err := NewDeliveryReporter("testpod", "testcontainer") + if err != nil { + t.Fatal(err) + } + + reportertest.ExpectMetrics(t, func() error { + return r.ReportEventDispatchTime(context.Background(), args, 1100*time.Millisecond, 202) + }) + reportertest.ExpectMetrics(t, func() error { + return r.ReportEventDispatchTime(context.Background(), args, 9100*time.Millisecond, 202) + }) + metricstest.CheckCountData(t, "event_count", wantTags, 2) + metricstest.CheckDistributionData(t, "event_dispatch_latencies", wantTags, 2, 1100.0, 9100.0) +} + +func TestReportEventProcessingTime(t *testing.T) { + reportertest.ResetDeliveryMetrics() + + args := DeliveryReportArgs{ + Namespace: "testns", + Broker: "testbroker", + Trigger: "testtrigger", + FilterType: "testeventtype", + } + + wantTags := map[string]string{ + metricskey.LabelNamespaceName: "testns", + metricskey.LabelBrokerName: "testbroker", + metricskey.LabelTriggerName: "testtrigger", + metricskey.LabelFilterType: "testeventtype", + metricskey.PodName: "testpod", + metricskey.ContainerName: "testcontainer", + } + + r, err := NewDeliveryReporter("testpod", "testcontainer") + if err != nil { + t.Fatal(err) + } + + // test ReportDispatchTime + reportertest.ExpectMetrics(t, func() error { + return r.ReportEventProcessingTime(context.Background(), args, 1100*time.Millisecond) + }) + reportertest.ExpectMetrics(t, func() error { + return r.ReportEventProcessingTime(context.Background(), args, 9100*time.Millisecond) + }) + metricstest.CheckDistributionData(t, "event_processing_latencies", wantTags, 2, 1100.0, 9100.0) +} + +func TestMetricsWithEmptySourceAndTypeFilter(t *testing.T) { + reportertest.ResetDeliveryMetrics() + + args := DeliveryReportArgs{ + Namespace: "testns", + Broker: "testbroker", + Trigger: "testtrigger", + FilterType: "", // No Filter Type + } + + wantTags := map[string]string{ + metricskey.LabelNamespaceName: "testns", + metricskey.LabelBrokerName: "testbroker", + metricskey.LabelTriggerName: "testtrigger", + metricskey.LabelFilterType: "any", // Expects this to be "any" instead of empty string + metricskey.LabelResponseCode: "202", + metricskey.LabelResponseCodeClass: "2xx", + metricskey.PodName: "testpod", + metricskey.ContainerName: "testcontainer", + } + + r, err := NewDeliveryReporter("testpod", "testcontainer") + if err != nil { + t.Fatal(err) + } + + reportertest.ExpectMetrics(t, func() error { + return r.ReportEventDispatchTime(context.Background(), args, 1100*time.Millisecond, 202) + }) + metricstest.CheckCountData(t, "event_count", wantTags, 1) +} diff --git a/pkg/broker/ingress/stats_reporter.go b/pkg/metrics/ingress_reporter.go similarity index 57% rename from pkg/broker/ingress/stats_reporter.go rename to pkg/metrics/ingress_reporter.go index 4819676660..4edca66852 100644 --- a/pkg/broker/ingress/stats_reporter.go +++ b/pkg/metrics/ingress_reporter.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ingress +package metrics import ( "context" @@ -26,7 +26,6 @@ import ( "go.opencensus.io/stats/view" "go.opencensus.io/tag" "knative.dev/pkg/metrics" - "knative.dev/pkg/metrics/metricskey" ) // stats_exporter is adapted from knative.dev/eventing/pkg/broker/ingress/stats_reporter.go @@ -34,40 +33,22 @@ import ( // - Metric descriptions are updated to match GCP broker specifics. // - Removed StatsReporter interface and directly use helper methods instead. -var ( - // Create the tag keys that will be used to add tags to our measurements. - // Tag keys must conform to the restrictions described in - // go.opencensus.io/tag/validate.go. Currently those restrictions are: - // - length between 1 and 255 inclusive - // - characters are printable US-ASCII - namespaceKey = tag.MustNewKey(metricskey.LabelNamespaceName) - brokerKey = tag.MustNewKey(metricskey.LabelBrokerName) - eventTypeKey = tag.MustNewKey(metricskey.LabelEventType) - responseCodeKey = tag.MustNewKey(metricskey.LabelResponseCode) - responseCodeClassKey = tag.MustNewKey(metricskey.LabelResponseCodeClass) - podKey = tag.MustNewKey(metricskey.PodName) - containerKey = tag.MustNewKey(metricskey.ContainerName) -) - -type PodName string -type ContainerName string - -type reportArgs struct { - namespace string - broker string - eventType string - responseCode int +type IngressReportArgs struct { + Namespace string + Broker string + EventType string + ResponseCode int } -func (r *StatsReporter) register() error { +func (r *IngressReporter) register() error { tagKeys := []tag.Key{ - namespaceKey, - brokerKey, - eventTypeKey, - responseCodeKey, - responseCodeClassKey, - podKey, - containerKey, + NamespaceNameKey, + BrokerNameKey, + EventTypeKey, + ResponseCodeKey, + ResponseCodeClassKey, + PodNameKey, + ContainerNameKey, } // Create view to see our measurements. @@ -89,9 +70,9 @@ func (r *StatsReporter) register() error { ) } -// NewStatsReporter creates a new StatsReporter. -func NewStatsReporter(podName PodName, containerName ContainerName) (*StatsReporter, error) { - r := &StatsReporter{ +// NewIngressReporter creates a new StatsReporter. +func NewIngressReporter(podName PodName, containerName ContainerName) (*IngressReporter, error) { + r := &IngressReporter{ podName: podName, containerName: containerName, dispatchTimeInMsecM: stats.Float64( @@ -107,7 +88,7 @@ func NewStatsReporter(podName PodName, containerName ContainerName) (*StatsRepor } // StatsReporter reports ingress metrics. -type StatsReporter struct { +type IngressReporter struct { podName PodName containerName ContainerName // dispatchTimeInMsecM records the time spent dispatching an event to a decouple queue, in @@ -115,16 +96,16 @@ type StatsReporter struct { dispatchTimeInMsecM *stats.Float64Measure } -func (r *StatsReporter) reportEventDispatchTime(ctx context.Context, args reportArgs, d time.Duration) error { +func (r *IngressReporter) ReportEventDispatchTime(ctx context.Context, args IngressReportArgs, d time.Duration) error { tag, err := tag.New( ctx, - tag.Insert(podKey, string(r.podName)), - tag.Insert(containerKey, string(r.containerName)), - tag.Insert(namespaceKey, args.namespace), - tag.Insert(brokerKey, args.broker), - tag.Insert(eventTypeKey, args.eventType), - tag.Insert(responseCodeKey, strconv.Itoa(args.responseCode)), - tag.Insert(responseCodeClassKey, metrics.ResponseCodeClass(args.responseCode)), + tag.Insert(PodNameKey, string(r.podName)), + tag.Insert(ContainerNameKey, string(r.containerName)), + tag.Insert(NamespaceNameKey, args.Namespace), + tag.Insert(BrokerNameKey, args.Broker), + tag.Insert(EventTypeKey, args.EventType), + tag.Insert(ResponseCodeKey, strconv.Itoa(args.ResponseCode)), + tag.Insert(ResponseCodeClassKey, metrics.ResponseCodeClass(args.ResponseCode)), ) if err != nil { return fmt.Errorf("failed to create metrics tag: %v", err) diff --git a/pkg/broker/ingress/stats_reporter_test.go b/pkg/metrics/ingress_reporter_test.go similarity index 63% rename from pkg/broker/ingress/stats_reporter_test.go rename to pkg/metrics/ingress_reporter_test.go index 393619350d..d64b3cbd7b 100644 --- a/pkg/broker/ingress/stats_reporter_test.go +++ b/pkg/metrics/ingress_reporter_test.go @@ -14,25 +14,26 @@ See the License for the specific language governing permissions and limitations under the License. */ -package ingress +package metrics import ( "context" "testing" "time" + reportertest "github.com/google/knative-gcp/pkg/metrics/testing" "knative.dev/pkg/metrics/metricskey" "knative.dev/pkg/metrics/metricstest" ) func TestStatsReporter(t *testing.T) { - resetMetrics() + reportertest.ResetIngressMetrics() - args := reportArgs{ - namespace: "testns", - broker: "testbroker", - eventType: "testeventtype", - responseCode: 202, + args := IngressReportArgs{ + Namespace: "testns", + Broker: "testbroker", + EventType: "testeventtype", + ResponseCode: 202, } wantTags := map[string]string{ metricskey.LabelNamespaceName: "testns", @@ -44,30 +45,18 @@ func TestStatsReporter(t *testing.T) { metricskey.PodName: "testpod", } - r, err := NewStatsReporter(PodName("testpod"), ContainerName("testcontainer")) + r, err := NewIngressReporter(PodName("testpod"), ContainerName("testcontainer")) if err != nil { t.Fatal(err) } // test ReportDispatchTime - expectSuccess(t, func() error { - return r.reportEventDispatchTime(context.Background(), args, 1100*time.Millisecond) + reportertest.ExpectMetrics(t, func() error { + return r.ReportEventDispatchTime(context.Background(), args, 1100*time.Millisecond) }) - expectSuccess(t, func() error { - return r.reportEventDispatchTime(context.Background(), args, 9100*time.Millisecond) + reportertest.ExpectMetrics(t, func() error { + return r.ReportEventDispatchTime(context.Background(), args, 9100*time.Millisecond) }) metricstest.CheckCountData(t, "event_count", wantTags, 2) metricstest.CheckDistributionData(t, "event_dispatch_latencies", wantTags, 2, 1100.0, 9100.0) } - -func resetMetrics() { - // OpenCensus metrics carry global state that need to be reset between unit tests. - metricstest.Unregister("event_count", "event_dispatch_latencies") -} - -func expectSuccess(t *testing.T, f func() error) { - t.Helper() - if err := f(); err != nil { - t.Errorf("Reporter expected success but got error: %v", err) - } -} diff --git a/pkg/metrics/tags.go b/pkg/metrics/tags.go new file mode 100644 index 0000000000..fca1d9727b --- /dev/null +++ b/pkg/metrics/tags.go @@ -0,0 +1,45 @@ +/* +Copyright 2020 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "go.opencensus.io/tag" + "knative.dev/pkg/metrics/metricskey" +) + +type PodName string +type ContainerName string + +var ( + // Create the tag keys that will be used to add tags to our measurements. + // Tag keys must conform to the restrictions described in + // go.opencensus.io/tag/validate.go. Currently those restrictions are: + // - length between 1 and 255 inclusive + // - characters are printable US-ASCII + NamespaceNameKey = tag.MustNewKey(metricskey.LabelNamespaceName) + + BrokerNameKey = tag.MustNewKey(metricskey.LabelBrokerName) + EventTypeKey = tag.MustNewKey(metricskey.LabelEventType) + TriggerNameKey = tag.MustNewKey(metricskey.LabelTriggerName) + TriggerFilterTypeKey = tag.MustNewKey(metricskey.LabelFilterType) + + ResponseCodeKey = tag.MustNewKey(metricskey.LabelResponseCode) + ResponseCodeClassKey = tag.MustNewKey(metricskey.LabelResponseCodeClass) + + PodNameKey = tag.MustNewKey(metricskey.PodName) + ContainerNameKey = tag.MustNewKey(metricskey.ContainerName) +) diff --git a/pkg/metrics/testing/helper.go b/pkg/metrics/testing/helper.go new file mode 100644 index 0000000000..3dea7406f6 --- /dev/null +++ b/pkg/metrics/testing/helper.go @@ -0,0 +1,24 @@ +package testing + +import ( + "testing" + + "knative.dev/pkg/metrics/metricstest" +) + +func ResetIngressMetrics() { + // OpenCensus metrics carry global state that need to be reset between unit tests. + metricstest.Unregister("event_count", "event_dispatch_latencies") +} + +func ResetDeliveryMetrics() { + // OpenCensus metrics carry global state that need to be reset between unit tests. + metricstest.Unregister("event_count", "event_dispatch_latencies", "event_processing_latencies") +} + +func ExpectMetrics(t *testing.T, f func() error) { + t.Helper() + if err := f(); err != nil { + t.Errorf("Reporter expected success but got error: %v", err) + } +} From 7c23576865d20d41cd67a004982558fc3fb0f069 Mon Sep 17 00:00:00 2001 From: Ian Milligan Date: Fri, 8 May 2020 08:13:45 -0700 Subject: [PATCH 09/11] Bump go version to 1.14 (#991) * Update knative.dev/pkg to include additional codegen fixes * Bump go version to 1.14 Enables -mod=vendor by default. --- go.mod | 4 +- go.sum | 4 +- hack/update-deps.sh | 2 +- .../broker/v1beta1/broker/reconciler.go | 8 +-- .../broker/v1beta1/trigger/reconciler.go | 8 +-- .../cloudauditlogssource/reconciler.go | 8 +-- .../v1alpha1/cloudbuildsource/reconciler.go | 8 +-- .../v1alpha1/cloudpubsubsource/reconciler.go | 8 +-- .../cloudschedulersource/reconciler.go | 8 +-- .../v1alpha1/cloudstoragesource/reconciler.go | 8 +-- .../cloudauditlogssource/reconciler.go | 8 +-- .../v1beta1/cloudpubsubsource/reconciler.go | 8 +-- .../cloudschedulersource/reconciler.go | 8 +-- .../v1beta1/cloudstoragesource/reconciler.go | 8 +-- .../v1alpha1/brokercell/reconciler.go | 8 +-- .../v1alpha1/pullsubscription/reconciler.go | 8 +-- .../intevents/v1alpha1/topic/reconciler.go | 8 +-- .../messaging/v1alpha1/channel/reconciler.go | 8 +-- .../messaging/v1beta1/channel/reconciler.go | 8 +-- .../v1alpha1/eventpolicybinding/reconciler.go | 8 +-- .../v1alpha1/httppolicybinding/reconciler.go | 8 +-- .../v1alpha1/pullsubscription/reconciler.go | 8 +-- .../pubsub/v1alpha1/topic/reconciler.go | 8 +-- .../v1beta1/pullsubscription/reconciler.go | 8 +-- .../pubsub/v1beta1/topic/reconciler.go | 8 +-- vendor/knative.dev/pkg/apis/condition_set.go | 5 ++ .../pkg/apis/duck/v1/kresource_type.go | 12 ++-- .../generators/reconciler_reconciler.go | 12 ++-- .../knative.dev/pkg/controller/controller.go | 6 +- .../pkg/injection/sharedmain/main.go | 14 ++-- vendor/knative.dev/pkg/logging/config.go | 12 ++-- .../pkg/logging/object_encoders.go | 59 ++++++++++++++++ .../pkg/reconciler/reconcile_common.go | 55 +++++++++++++++ vendor/modules.txt | 69 ++++++++++++++++++- 34 files changed, 310 insertions(+), 120 deletions(-) create mode 100644 vendor/knative.dev/pkg/logging/object_encoders.go create mode 100644 vendor/knative.dev/pkg/reconciler/reconcile_common.go diff --git a/go.mod b/go.mod index b9e390f406..f29ddedd54 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/google/knative-gcp -go 1.13 +go 1.14 require ( cloud.google.com/go v0.56.0 @@ -34,7 +34,7 @@ require ( k8s.io/apimachinery v0.18.1 k8s.io/client-go v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible knative.dev/eventing v0.14.1-0.20200506063944-e9eb527e1295 - knative.dev/pkg v0.0.0-20200506142844-5b98a558168e + knative.dev/pkg v0.0.0-20200507220045-66f1d63f1019 knative.dev/serving v0.14.1-0.20200506171253-f2c76cb8a6dd knative.dev/test-infra v0.0.0-20200506193944-431dda291f8c // indirect ) diff --git a/go.sum b/go.sum index ecc5429a65..2654de530b 100644 --- a/go.sum +++ b/go.sum @@ -1269,8 +1269,8 @@ knative.dev/pkg v0.0.0-20200504180943-4a2ba059b008/go.mod h1:1RvwKBbKqKYt5rgI4lf knative.dev/pkg v0.0.0-20200505191044-3da93ebb24c2/go.mod h1:Q6sL35DdGs8hIQZKdaCXJGgY8f90BmNBKSb8z6d/BTM= knative.dev/pkg v0.0.0-20200506001744-478962f05e2b h1:SFCuEj+NeA8dVn4Ms1ymfF4FUru8jc7D56L17Co1qe8= knative.dev/pkg v0.0.0-20200506001744-478962f05e2b/go.mod h1:9UQS6bJECqqFG0q9BPaATbcG78co0s9Q6Dzo/6mR4uI= -knative.dev/pkg v0.0.0-20200506142844-5b98a558168e h1:Sao/grhEcDE2xPtC3nDi5lxfbtF+arA1gq6xvX354Gk= -knative.dev/pkg v0.0.0-20200506142844-5b98a558168e/go.mod h1:MUQe/bW75GmlVeyHmdKag77FJWQrNH3rxt2Q9E1JZJs= +knative.dev/pkg v0.0.0-20200507220045-66f1d63f1019 h1:dKpUitJ2FGacWzOcxVMAgBTVkxIF6gUJY3a6byXhwCY= +knative.dev/pkg v0.0.0-20200507220045-66f1d63f1019/go.mod h1:MUQe/bW75GmlVeyHmdKag77FJWQrNH3rxt2Q9E1JZJs= knative.dev/serving v0.14.1-0.20200506171253-f2c76cb8a6dd h1:yfdOB/N7tSEUAwQVNIrIA8K27xxtRexywXsxQPmak3c= knative.dev/serving v0.14.1-0.20200506171253-f2c76cb8a6dd/go.mod h1:BtRGQeKUz73iNbf0Kopkt3IDCXVubIakKmYoZWcLd5A= knative.dev/test-infra v0.0.0-20200407185800-1b88cb3b45a5/go.mod h1:xcdUkMJrLlBswIZqL5zCuBFOC22WIPMQoVX1L35i0vQ= diff --git a/hack/update-deps.sh b/hack/update-deps.sh index 61b3be4bbd..e90ed55935 100755 --- a/hack/update-deps.sh +++ b/hack/update-deps.sh @@ -56,4 +56,4 @@ go mod vendor find vendor/ \( -name OWNERS -o -name OWNERS_ALIASES -o -name BUILD -o -name BUILD.bazel \) -delete -GOFLAGS="-mod=vendor" update_licenses third_party/VENDOR-LICENSE "./..." +update_licenses third_party/VENDOR-LICENSE "./..." diff --git a/pkg/client/injection/reconciler/broker/v1beta1/broker/reconciler.go b/pkg/client/injection/reconciler/broker/v1beta1/broker/reconciler.go index f26fbe5f38..253a94aab9 100644 --- a/pkg/client/injection/reconciler/broker/v1beta1/broker/reconciler.go +++ b/pkg/client/injection/reconciler/broker/v1beta1/broker/reconciler.go @@ -20,18 +20,18 @@ package broker import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/broker/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" brokerv1beta1 "github.com/google/knative-gcp/pkg/client/listers/broker/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/broker/v1beta1/trigger/reconciler.go b/pkg/client/injection/reconciler/broker/v1beta1/trigger/reconciler.go index 977ed54b3c..8abc2d37f5 100644 --- a/pkg/client/injection/reconciler/broker/v1beta1/trigger/reconciler.go +++ b/pkg/client/injection/reconciler/broker/v1beta1/trigger/reconciler.go @@ -20,18 +20,18 @@ package trigger import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/broker/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" brokerv1beta1 "github.com/google/knative-gcp/pkg/client/listers/broker/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1alpha1/cloudauditlogssource/reconciler.go b/pkg/client/injection/reconciler/events/v1alpha1/cloudauditlogssource/reconciler.go index c549da284d..208c2502ae 100644 --- a/pkg/client/injection/reconciler/events/v1alpha1/cloudauditlogssource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1alpha1/cloudauditlogssource/reconciler.go @@ -20,18 +20,18 @@ package cloudauditlogssource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/events/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/events/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1alpha1/cloudbuildsource/reconciler.go b/pkg/client/injection/reconciler/events/v1alpha1/cloudbuildsource/reconciler.go index 43ec3f8df0..20d5451069 100644 --- a/pkg/client/injection/reconciler/events/v1alpha1/cloudbuildsource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1alpha1/cloudbuildsource/reconciler.go @@ -20,18 +20,18 @@ package cloudbuildsource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/events/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/events/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1alpha1/cloudpubsubsource/reconciler.go b/pkg/client/injection/reconciler/events/v1alpha1/cloudpubsubsource/reconciler.go index e596aad55e..45d0317803 100644 --- a/pkg/client/injection/reconciler/events/v1alpha1/cloudpubsubsource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1alpha1/cloudpubsubsource/reconciler.go @@ -20,18 +20,18 @@ package cloudpubsubsource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/events/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/events/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1alpha1/cloudschedulersource/reconciler.go b/pkg/client/injection/reconciler/events/v1alpha1/cloudschedulersource/reconciler.go index 7b07f485f8..6a36bd2b02 100644 --- a/pkg/client/injection/reconciler/events/v1alpha1/cloudschedulersource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1alpha1/cloudschedulersource/reconciler.go @@ -20,18 +20,18 @@ package cloudschedulersource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/events/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/events/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1alpha1/cloudstoragesource/reconciler.go b/pkg/client/injection/reconciler/events/v1alpha1/cloudstoragesource/reconciler.go index 5585f1ce74..d82ec1e9ad 100644 --- a/pkg/client/injection/reconciler/events/v1alpha1/cloudstoragesource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1alpha1/cloudstoragesource/reconciler.go @@ -20,18 +20,18 @@ package cloudstoragesource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/events/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/events/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1beta1/cloudauditlogssource/reconciler.go b/pkg/client/injection/reconciler/events/v1beta1/cloudauditlogssource/reconciler.go index 50fe749d56..398a40f210 100644 --- a/pkg/client/injection/reconciler/events/v1beta1/cloudauditlogssource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1beta1/cloudauditlogssource/reconciler.go @@ -20,18 +20,18 @@ package cloudauditlogssource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/events/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1beta1 "github.com/google/knative-gcp/pkg/client/listers/events/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1beta1/cloudpubsubsource/reconciler.go b/pkg/client/injection/reconciler/events/v1beta1/cloudpubsubsource/reconciler.go index ffb7cc6f47..b3637edd46 100644 --- a/pkg/client/injection/reconciler/events/v1beta1/cloudpubsubsource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1beta1/cloudpubsubsource/reconciler.go @@ -20,18 +20,18 @@ package cloudpubsubsource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/events/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1beta1 "github.com/google/knative-gcp/pkg/client/listers/events/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1beta1/cloudschedulersource/reconciler.go b/pkg/client/injection/reconciler/events/v1beta1/cloudschedulersource/reconciler.go index a0ae88e355..5f6da55472 100644 --- a/pkg/client/injection/reconciler/events/v1beta1/cloudschedulersource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1beta1/cloudschedulersource/reconciler.go @@ -20,18 +20,18 @@ package cloudschedulersource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/events/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1beta1 "github.com/google/knative-gcp/pkg/client/listers/events/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/events/v1beta1/cloudstoragesource/reconciler.go b/pkg/client/injection/reconciler/events/v1beta1/cloudstoragesource/reconciler.go index 2701263c73..493c5e85bd 100644 --- a/pkg/client/injection/reconciler/events/v1beta1/cloudstoragesource/reconciler.go +++ b/pkg/client/injection/reconciler/events/v1beta1/cloudstoragesource/reconciler.go @@ -20,18 +20,18 @@ package cloudstoragesource import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/events/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" eventsv1beta1 "github.com/google/knative-gcp/pkg/client/listers/events/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/intevents/v1alpha1/brokercell/reconciler.go b/pkg/client/injection/reconciler/intevents/v1alpha1/brokercell/reconciler.go index 131d212dc5..9a56bf62cd 100644 --- a/pkg/client/injection/reconciler/intevents/v1alpha1/brokercell/reconciler.go +++ b/pkg/client/injection/reconciler/intevents/v1alpha1/brokercell/reconciler.go @@ -20,18 +20,18 @@ package brokercell import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/intevents/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" inteventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/intevents/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/intevents/v1alpha1/pullsubscription/reconciler.go b/pkg/client/injection/reconciler/intevents/v1alpha1/pullsubscription/reconciler.go index 646b38d7de..2f0ff07a9c 100644 --- a/pkg/client/injection/reconciler/intevents/v1alpha1/pullsubscription/reconciler.go +++ b/pkg/client/injection/reconciler/intevents/v1alpha1/pullsubscription/reconciler.go @@ -20,18 +20,18 @@ package pullsubscription import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/intevents/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" inteventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/intevents/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/intevents/v1alpha1/topic/reconciler.go b/pkg/client/injection/reconciler/intevents/v1alpha1/topic/reconciler.go index f7966d70d1..14ac877291 100644 --- a/pkg/client/injection/reconciler/intevents/v1alpha1/topic/reconciler.go +++ b/pkg/client/injection/reconciler/intevents/v1alpha1/topic/reconciler.go @@ -20,18 +20,18 @@ package topic import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/intevents/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" inteventsv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/intevents/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/messaging/v1alpha1/channel/reconciler.go b/pkg/client/injection/reconciler/messaging/v1alpha1/channel/reconciler.go index 032a2b5068..ff92c0853d 100644 --- a/pkg/client/injection/reconciler/messaging/v1alpha1/channel/reconciler.go +++ b/pkg/client/injection/reconciler/messaging/v1alpha1/channel/reconciler.go @@ -20,18 +20,18 @@ package channel import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/messaging/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" messagingv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/messaging/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/messaging/v1beta1/channel/reconciler.go b/pkg/client/injection/reconciler/messaging/v1beta1/channel/reconciler.go index 1c5ee27c88..ee1c38ae27 100644 --- a/pkg/client/injection/reconciler/messaging/v1beta1/channel/reconciler.go +++ b/pkg/client/injection/reconciler/messaging/v1beta1/channel/reconciler.go @@ -20,18 +20,18 @@ package channel import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/messaging/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" messagingv1beta1 "github.com/google/knative-gcp/pkg/client/listers/messaging/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/policy/v1alpha1/eventpolicybinding/reconciler.go b/pkg/client/injection/reconciler/policy/v1alpha1/eventpolicybinding/reconciler.go index a505a9c744..b45d4d058b 100644 --- a/pkg/client/injection/reconciler/policy/v1alpha1/eventpolicybinding/reconciler.go +++ b/pkg/client/injection/reconciler/policy/v1alpha1/eventpolicybinding/reconciler.go @@ -20,18 +20,18 @@ package eventpolicybinding import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/policy/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" policyv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/policy/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/policy/v1alpha1/httppolicybinding/reconciler.go b/pkg/client/injection/reconciler/policy/v1alpha1/httppolicybinding/reconciler.go index d55846642f..1a84142941 100644 --- a/pkg/client/injection/reconciler/policy/v1alpha1/httppolicybinding/reconciler.go +++ b/pkg/client/injection/reconciler/policy/v1alpha1/httppolicybinding/reconciler.go @@ -20,18 +20,18 @@ package httppolicybinding import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/policy/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" policyv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/policy/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/pubsub/v1alpha1/pullsubscription/reconciler.go b/pkg/client/injection/reconciler/pubsub/v1alpha1/pullsubscription/reconciler.go index 944812f5d7..89b6ef8870 100644 --- a/pkg/client/injection/reconciler/pubsub/v1alpha1/pullsubscription/reconciler.go +++ b/pkg/client/injection/reconciler/pubsub/v1alpha1/pullsubscription/reconciler.go @@ -20,18 +20,18 @@ package pullsubscription import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" pubsubv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/pubsub/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/pubsub/v1alpha1/topic/reconciler.go b/pkg/client/injection/reconciler/pubsub/v1alpha1/topic/reconciler.go index 6cb7cf06ae..24a9f1c9dc 100644 --- a/pkg/client/injection/reconciler/pubsub/v1alpha1/topic/reconciler.go +++ b/pkg/client/injection/reconciler/pubsub/v1alpha1/topic/reconciler.go @@ -20,18 +20,18 @@ package topic import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" pubsubv1alpha1 "github.com/google/knative-gcp/pkg/client/listers/pubsub/v1alpha1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/pubsub/v1beta1/pullsubscription/reconciler.go b/pkg/client/injection/reconciler/pubsub/v1beta1/pullsubscription/reconciler.go index 22ea917423..4fc3991873 100644 --- a/pkg/client/injection/reconciler/pubsub/v1beta1/pullsubscription/reconciler.go +++ b/pkg/client/injection/reconciler/pubsub/v1beta1/pullsubscription/reconciler.go @@ -20,18 +20,18 @@ package pullsubscription import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" pubsubv1beta1 "github.com/google/knative-gcp/pkg/client/listers/pubsub/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/pkg/client/injection/reconciler/pubsub/v1beta1/topic/reconciler.go b/pkg/client/injection/reconciler/pubsub/v1beta1/topic/reconciler.go index 44854207ff..5514d987a6 100644 --- a/pkg/client/injection/reconciler/pubsub/v1beta1/topic/reconciler.go +++ b/pkg/client/injection/reconciler/pubsub/v1beta1/topic/reconciler.go @@ -20,18 +20,18 @@ package topic import ( context "context" - "encoding/json" - "reflect" + json "encoding/json" + reflect "reflect" v1beta1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1beta1" versioned "github.com/google/knative-gcp/pkg/client/clientset/versioned" pubsubv1beta1 "github.com/google/knative-gcp/pkg/client/listers/pubsub/v1beta1" zap "go.uber.org/zap" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" + equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" record "k8s.io/client-go/tools/record" diff --git a/vendor/knative.dev/pkg/apis/condition_set.go b/vendor/knative.dev/pkg/apis/condition_set.go index 49f9d2e9b6..8d02fec485 100644 --- a/vendor/knative.dev/pkg/apis/condition_set.go +++ b/vendor/knative.dev/pkg/apis/condition_set.go @@ -142,6 +142,11 @@ type conditionsImpl struct { accessor ConditionsAccessor } +// GetTopLevelConditionType is an accessor for the top-level happy condition. +func (r ConditionSet) GetTopLevelConditionType() ConditionType { + return r.happy +} + // Manage creates a ConditionManager from an accessor object using the original // ConditionSet as a reference. Status must be a pointer to a struct. func (r ConditionSet) Manage(status ConditionsAccessor) ConditionManager { diff --git a/vendor/knative.dev/pkg/apis/duck/v1/kresource_type.go b/vendor/knative.dev/pkg/apis/duck/v1/kresource_type.go index b64977fcb5..80558428c0 100644 --- a/vendor/knative.dev/pkg/apis/duck/v1/kresource_type.go +++ b/vendor/knative.dev/pkg/apis/duck/v1/kresource_type.go @@ -26,7 +26,7 @@ import ( "knative.dev/pkg/apis" ) -// KRShaped is an interface for retrieving the duck elements of an arbitraty resource. +// KRShaped is an interface for retrieving the duck elements of an arbitrary resource. type KRShaped interface { metav1.ObjectMetaAccessor @@ -34,7 +34,7 @@ type KRShaped interface { GetStatus() *Status - GetTopLevelConditionType() apis.ConditionType + GetConditionSet() apis.ConditionSet } // Asserts KResource conformance with KRShaped @@ -91,12 +91,12 @@ func (t *KResource) GetStatus() *Status { return &t.Status } -// GetTopLevelConditionType retrieves the happy condition of this resource. Implements the KRShaped interface. -func (t *KResource) GetTopLevelConditionType() apis.ConditionType { +// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface. +func (t *KResource) GetConditionSet() apis.ConditionSet { // Note: KResources are unmarshalled from existing resources. This will only work properly for resources that // have already been initialized to their type. if cond := t.Status.GetCondition(apis.ConditionSucceeded); cond != nil { - return apis.ConditionSucceeded + return apis.NewBatchConditionSet() } - return apis.ConditionReady + return apis.NewLivingConditionSet() } diff --git a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go index d1ceb54ed8..0d098ab606 100644 --- a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go +++ b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler_reconciler.go @@ -145,6 +145,10 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty Package: "context", Name: "Context", }), + "reflectDeepEqual": c.Universe.Package("reflect").Function("DeepEqual"), + "equalitySemantic": c.Universe.Package("k8s.io/apimachinery/pkg/api/equality").Variable("Semantic"), + "jsonMarshal": c.Universe.Package("encoding/json").Function("Marshal"), + "typesMergePatchType": c.Universe.Package("k8s.io/apimachinery/pkg/types").Constant("MergePatchType"), } sw.Do(reconcilerInterfaceFactory, m) @@ -321,7 +325,7 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro } // Synchronize the status. - if equality.Semantic.DeepEqual(original.Status, resource.Status) { + if {{.equalitySemantic|raw}}.DeepEqual(original.Status, resource.Status) { // If we didn't change anything then don't call updateStatus. // This is important because the copy we loaded from the injectionInformer's // cache may be stale and we don't want to overwrite a prior update @@ -368,7 +372,7 @@ func (r *reconcilerImpl) updateStatus(existing *{{.type|raw}}, desired *{{.type| } // If there's nothing to update, just return. - if reflect.DeepEqual(existing.Status, desired.Status) { + if {{.reflectDeepEqual|raw}}(existing.Status, desired.Status) { return nil } @@ -433,7 +437,7 @@ func (r *reconcilerImpl) updateFinalizersFiltered(ctx {{.contextContext|raw}}, r }, } - patch, err := json.Marshal(mergePatch) + patch, err := {{.jsonMarshal|raw}}(mergePatch) if err != nil { return resource, err } @@ -443,7 +447,7 @@ func (r *reconcilerImpl) updateFinalizersFiltered(ctx {{.contextContext|raw}}, r {{else}} patcher := r.Client.{{.group}}{{.version}}().{{.type|apiGroup}}(resource.Namespace) {{end}} - resource, err = patcher.Patch(resource.Name, types.MergePatchType, patch) + resource, err = patcher.Patch(resource.Name, {{.typesMergePatchType|raw}}, patch) if err != nil { r.Recorder.Eventf(resource, {{.corev1EventTypeWarning|raw}}, "FinalizerUpdateFailed", "Failed to update finalizers for %q: %v", resource.Name, err) diff --git a/vendor/knative.dev/pkg/controller/controller.go b/vendor/knative.dev/pkg/controller/controller.go index 4713335d18..15496b8a99 100644 --- a/vendor/knative.dev/pkg/controller/controller.go +++ b/vendor/knative.dev/pkg/controller/controller.go @@ -427,20 +427,20 @@ func (c *Impl) processNextWorkItem() bool { // Embed the key into the logger and attach that to the context we pass // to the Reconciler. logger := c.logger.With(zap.String(logkey.TraceId, uuid.New().String()), zap.String(logkey.Key, keyStr)) - ctx := logging.WithLogger(context.TODO(), logger) + ctx := logging.WithLogger(context.Background(), logger) // Run Reconcile, passing it the namespace/name string of the // resource to be synced. if err = c.Reconciler.Reconcile(ctx, keyStr); err != nil { c.handleErr(err, key) - logger.Infof("Reconcile failed. Time taken: %v.", time.Since(startTime)) + logger.Info("Reconcile failed. Time taken: ", time.Since(startTime)) return true } // Finally, if no error occurs we Forget this item so it does not // have any delay when another change happens. c.WorkQueue.Forget(key) - logger.Infof("Reconcile succeeded. Time taken: %v.", time.Since(startTime)) + logger.Info("Reconcile succeeded. Time taken: ", time.Since(startTime)) return true } diff --git a/vendor/knative.dev/pkg/injection/sharedmain/main.go b/vendor/knative.dev/pkg/injection/sharedmain/main.go index c169fa20c6..1410138e1f 100644 --- a/vendor/knative.dev/pkg/injection/sharedmain/main.go +++ b/vendor/knative.dev/pkg/injection/sharedmain/main.go @@ -189,7 +189,7 @@ func MainWithConfig(ctx context.Context, component string, cfg *rest.Config, cto // Set up leader election config leaderElectionConfig, err := GetLeaderElectionConfig(ctx) if err != nil { - logger.Fatalf("Error loading leader election configuration: %v", err) + logger.Fatalw("Error loading leader election configuration", zap.Error(err)) } leConfig := leaderElectionConfig.GetComponentConfig(component) @@ -343,9 +343,9 @@ func SetupConfigMapWatchOrDie(ctx context.Context, logger *zap.SugaredLogger) *c if cmLabel := system.ResourceLabel(); cmLabel != "" { req, err := configmap.FilterConfigByLabelExists(cmLabel) if err != nil { - logger.With(zap.Error(err)).Fatalf("Failed to generate requirement for label %q") + logger.Fatalw("Failed to generate requirement for label "+cmLabel, zap.Error(err)) } - logger.Infof("Setting up ConfigMap watcher with label selector %q", req) + logger.Info("Setting up ConfigMap watcher with label selector: ", req) cmLabelReqs = append(cmLabelReqs, *req) } // TODO(mattmoor): This should itself take a context and be injection-based. @@ -360,7 +360,7 @@ func WatchLoggingConfigOrDie(ctx context.Context, cmw *configmap.InformedWatcher metav1.GetOptions{}); err == nil { cmw.Watch(logging.ConfigMapName(), logging.UpdateLevelFromConfigMap(logger, atomicLevel, component)) } else if !apierrors.IsNotFound(err) { - logger.With(zap.Error(err)).Fatalf("Error reading ConfigMap %q", logging.ConfigMapName()) + logger.Fatalw("Error reading ConfigMap "+logging.ConfigMapName(), zap.Error(err)) } } @@ -374,7 +374,7 @@ func WatchObservabilityConfigOrDie(ctx context.Context, cmw *configmap.InformedW metrics.ConfigMapWatcher(component, SecretFetcher(ctx), logger), profilingHandler.UpdateFromConfigMap) } else if !apierrors.IsNotFound(err) { - logger.With(zap.Error(err)).Fatalf("Error reading ConfigMap %q", metrics.ConfigMapName()) + logger.Fatalw("Error reading ConfigMap "+metrics.ConfigMapName(), zap.Error(err)) } } @@ -457,7 +457,7 @@ func RunLeaderElected(ctx context.Context, logger *zap.SugaredLogger, run func(c EventRecorder: recorder, }) if err != nil { - logger.Fatalw("Error creating lock: %v", err) + logger.Fatalw("Error creating lock", zap.Error(err)) } // Execute the `run` function when we have the lock. @@ -469,7 +469,7 @@ func RunLeaderElected(ctx context.Context, logger *zap.SugaredLogger, run func(c Callbacks: leaderelection.LeaderCallbacks{ OnStartedLeading: run, OnStoppedLeading: func() { - logger.Fatal("leaderelection lost") + logger.Fatal("Leader election lost") }, }, ReleaseOnCancel: true, diff --git a/vendor/knative.dev/pkg/logging/config.go b/vendor/knative.dev/pkg/logging/config.go index a0fb96e275..9e6f488fdc 100644 --- a/vendor/knative.dev/pkg/logging/config.go +++ b/vendor/knative.dev/pkg/logging/config.go @@ -52,7 +52,7 @@ var ( func NewLogger(configJSON string, levelOverride string, opts ...zap.Option) (*zap.SugaredLogger, zap.AtomicLevel) { logger, atomicLevel, err := newLoggerFromConfig(configJSON, levelOverride, opts) if err == nil { - return enrichLoggerWithCommitID(logger.Sugar()), atomicLevel + return enrichLoggerWithCommitID(logger), atomicLevel } loggingCfg := zap.NewProductionConfig() @@ -66,18 +66,18 @@ func NewLogger(configJSON string, levelOverride string, opts ...zap.Option) (*za if err2 != nil { panic(err2) } - return enrichLoggerWithCommitID(logger.Named(fallbackLoggerName).Sugar()), loggingCfg.Level + return enrichLoggerWithCommitID(logger.Named(fallbackLoggerName)), loggingCfg.Level } -func enrichLoggerWithCommitID(logger *zap.SugaredLogger) *zap.SugaredLogger { +func enrichLoggerWithCommitID(logger *zap.Logger) *zap.SugaredLogger { commmitID, err := changeset.Get() if err == nil { // Enrich logs with GitHub commit ID. - return logger.With(zap.String(logkey.GitHubCommitID, commmitID)) + return logger.With(zap.String(logkey.GitHubCommitID, commmitID)).Sugar() } - logger.Infof("Fetch GitHub commit ID from kodata failed: %v", err) - return logger + logger.Info("Fetch GitHub commit ID from kodata failed", zap.Error(err)) + return logger.Sugar() } // NewLoggerFromConfig creates a logger using the provided Config diff --git a/vendor/knative.dev/pkg/logging/object_encoders.go b/vendor/knative.dev/pkg/logging/object_encoders.go new file mode 100644 index 0000000000..b6f39fe21a --- /dev/null +++ b/vendor/knative.dev/pkg/logging/object_encoders.go @@ -0,0 +1,59 @@ +/* +Copyright 2020 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package logging + +import ( + "strings" + + "go.uber.org/zap/zapcore" + + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" +) + +// This file contains the specific object encoders for use in Knative +// to optimize logging experience and performance. + +// StringSet returns a marshaler for the set of strings. +// To use this in sugared logger do: +// logger.Infow("Revision State", zap.Object("healthy", logging.StringSet(healthySet)), +// zap.Object("unhealthy", logging.StringSet(unhealthySet))) +// To use with non-sugared logger do: +// logger.Info("Revision State", zap.Object("healthy", logging.StringSet(healthySet)), +// zap.Object("unhealthy", logging.StringSet(unhealthySet))) +func StringSet(s sets.String) zapcore.ObjectMarshalerFunc { + return func(enc zapcore.ObjectEncoder) error { + enc.AddString("keys", strings.Join(s.UnsortedList(), ",")) + return nil + } +} + +// NamespacedName returns a marshaler for NamespacedName. +// To use this in sugared logger do: +// logger.Infow("Enqueuing", zap.Object("key", logging.NamespacedName(n))) +// To use with non-sugared logger do: +// logger.Info("Enqueuing", zap.Object("key", logging.NamespacedName(n))) +func NamespacedName(n types.NamespacedName) zapcore.ObjectMarshalerFunc { + return func(enc zapcore.ObjectEncoder) error { + if n.Namespace != "" { + enc.AddString("key", n.Name) + } else { + enc.AddString("key", n.Namespace+"/"+n.Name) + } + return nil + } +} diff --git a/vendor/knative.dev/pkg/reconciler/reconcile_common.go b/vendor/knative.dev/pkg/reconciler/reconcile_common.go new file mode 100644 index 0000000000..38e3bbe383 --- /dev/null +++ b/vendor/knative.dev/pkg/reconciler/reconcile_common.go @@ -0,0 +1,55 @@ +/* +Copyright 2020 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconciler + +import ( + "context" + + duckv1 "knative.dev/pkg/apis/duck/v1" + "knative.dev/pkg/logging" +) + +const failedGenerationBump = "NewObservedGenFailure" + +// PreProcessReconcile contains logic to apply before reconciliation of a resource. +func PreProcessReconcile(ctx context.Context, resource duckv1.KRShaped) { + newStatus := resource.GetStatus() + + if newStatus.ObservedGeneration != resource.GetObjectMeta().GetGeneration() { + condSet := resource.GetConditionSet() + manager := condSet.Manage(newStatus) + + // Reset Ready/Successful to unknown. The reconciler is expected to overwrite this. + manager.MarkUnknown(condSet.GetTopLevelConditionType(), failedGenerationBump, "unsuccessfully observed a new generation") + } +} + +// PostProcessReconcile contains logic to apply after reconciliation of a resource. +func PostProcessReconcile(ctx context.Context, resource duckv1.KRShaped) { + logger := logging.FromContext(ctx) + newStatus := resource.GetStatus() + mgr := resource.GetConditionSet().Manage(newStatus) + + // Bump observed generation to denote that we have processed this + // generation regardless of success or failure. + newStatus.ObservedGeneration = resource.GetObjectMeta().GetGeneration() + + rc := mgr.GetTopLevelCondition() + if rc.Reason == failedGenerationBump { + logger.Warn("A reconciler observed a new generation without updating the resource status") + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 60d231f75f..745e4f3f29 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,4 +1,5 @@ # cloud.google.com/go v0.56.0 +## explicit cloud.google.com/go cloud.google.com/go/compute/metadata cloud.google.com/go/container/apiv1 @@ -13,17 +14,20 @@ cloud.google.com/go/monitoring/apiv3 cloud.google.com/go/scheduler/apiv1 cloud.google.com/go/trace/apiv2 # cloud.google.com/go/logging v1.0.1-0.20200331222814-69e77e66e597 +## explicit cloud.google.com/go/logging cloud.google.com/go/logging/apiv2 cloud.google.com/go/logging/internal cloud.google.com/go/logging/logadmin # cloud.google.com/go/pubsub v1.3.2-0.20200331222814-69e77e66e597 +## explicit cloud.google.com/go/pubsub cloud.google.com/go/pubsub/apiv1 cloud.google.com/go/pubsub/internal/distribution cloud.google.com/go/pubsub/internal/scheduler cloud.google.com/go/pubsub/pstest # cloud.google.com/go/storage v1.6.1-0.20200331222814-69e77e66e597 +## explicit cloud.google.com/go/storage # contrib.go.opencensus.io/exporter/ocagent v0.6.0 contrib.go.opencensus.io/exporter/ocagent @@ -87,6 +91,7 @@ github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1 # github.com/cespare/xxhash/v2 v2.1.1 github.com/cespare/xxhash/v2 # github.com/cloudevents/sdk-go v1.2.0 +## explicit github.com/cloudevents/sdk-go github.com/cloudevents/sdk-go/pkg/cloudevents github.com/cloudevents/sdk-go/pkg/cloudevents/client @@ -104,6 +109,7 @@ github.com/cloudevents/sdk-go/pkg/cloudevents/transport/pubsub/context github.com/cloudevents/sdk-go/pkg/cloudevents/transport/pubsub/internal github.com/cloudevents/sdk-go/pkg/cloudevents/types # github.com/cloudevents/sdk-go/v2 v2.0.0-RC2 => github.com/cloudevents/sdk-go/v2 v2.0.0-RC1 +## explicit github.com/cloudevents/sdk-go/v2 github.com/cloudevents/sdk-go/v2/binding github.com/cloudevents/sdk-go/v2/binding/format @@ -134,6 +140,7 @@ github.com/emicklei/go-restful/log # github.com/evanphx/json-patch v4.5.0+incompatible github.com/evanphx/json-patch # github.com/fsnotify/fsnotify v1.4.9 +## explicit github.com/fsnotify/fsnotify # github.com/ghodss/yaml v1.0.0 github.com/ghodss/yaml @@ -150,6 +157,7 @@ github.com/go-openapi/swag # github.com/gobuffalo/envy v1.7.1 github.com/gobuffalo/envy # github.com/gogo/protobuf v1.3.1 => github.com/gogo/protobuf v1.3.0 +## explicit github.com/gogo/protobuf/jsonpb github.com/gogo/protobuf/proto github.com/gogo/protobuf/protoc-gen-gogo/descriptor @@ -158,6 +166,7 @@ github.com/gogo/protobuf/types # github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e github.com/golang/groupcache/lru # github.com/golang/protobuf v1.4.0 +## explicit github.com/golang/protobuf/descriptor github.com/golang/protobuf/internal/gengogrpc github.com/golang/protobuf/jsonpb @@ -172,6 +181,7 @@ github.com/golang/protobuf/ptypes/struct github.com/golang/protobuf/ptypes/timestamp github.com/golang/protobuf/ptypes/wrappers # github.com/google/go-cmp v0.4.0 +## explicit github.com/google/go-cmp/cmp github.com/google/go-cmp/cmp/cmpopts github.com/google/go-cmp/cmp/internal/diff @@ -185,12 +195,15 @@ github.com/google/gofuzz # github.com/google/subcommands v1.0.1 github.com/google/subcommands # github.com/google/uuid v1.1.1 +## explicit github.com/google/uuid # github.com/google/wire v0.4.0 +## explicit github.com/google/wire github.com/google/wire/cmd/wire github.com/google/wire/internal/wire # github.com/googleapis/gax-go/v2 v2.0.5 +## explicit github.com/googleapis/gax-go/v2 # github.com/googleapis/gnostic v0.4.0 github.com/googleapis/gnostic/OpenAPIv2 @@ -216,6 +229,7 @@ github.com/jstemmer/go-junit-report github.com/jstemmer/go-junit-report/formatter github.com/jstemmer/go-junit-report/parser # github.com/kelseyhightower/envconfig v1.4.0 +## explicit github.com/kelseyhightower/envconfig # github.com/lightstep/tracecontext.go v0.0.0-20181129014701-1757c391b1ac github.com/lightstep/tracecontext.go/traceparent @@ -240,6 +254,7 @@ github.com/openzipkin/zipkin-go/propagation github.com/openzipkin/zipkin-go/reporter github.com/openzipkin/zipkin-go/reporter/http # github.com/pkg/errors v0.9.1 => github.com/pkg/errors v0.8.1 +## explicit github.com/pkg/errors # github.com/pmezard/go-difflib v1.0.0 github.com/pmezard/go-difflib/difflib @@ -269,6 +284,7 @@ github.com/spf13/pflag github.com/stretchr/testify/assert github.com/stretchr/testify/require # go.opencensus.io v0.22.3 => go.opencensus.io v0.22.1 +## explicit go.opencensus.io go.opencensus.io/internal go.opencensus.io/internal/tagencoding @@ -290,14 +306,17 @@ go.opencensus.io/trace/internal go.opencensus.io/trace/propagation go.opencensus.io/trace/tracestate # go.opentelemetry.io/otel v0.3.0 +## explicit go.opentelemetry.io/otel/api/core go.opentelemetry.io/otel/api/propagation go.opentelemetry.io/otel/api/trace # go.uber.org/atomic v1.6.0 go.uber.org/atomic # go.uber.org/multierr v1.5.0 +## explicit go.uber.org/multierr # go.uber.org/zap v1.14.1 => go.uber.org/zap v1.9.2-0.20180814183419-67bc79d13d15 +## explicit go.uber.org/zap go.uber.org/zap/buffer go.uber.org/zap/internal/bufferpool @@ -307,6 +326,7 @@ go.uber.org/zap/internal/ztest go.uber.org/zap/zapcore go.uber.org/zap/zaptest # golang.org/x/crypto v0.0.0-20200317142112-1b76d66859c6 +## explicit golang.org/x/crypto/ssh/terminal # golang.org/x/lint v0.0.0-20200302205851-738671d3881b => golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f golang.org/x/lint @@ -391,6 +411,7 @@ gonum.org/v1/gonum/lapack/gonum gonum.org/v1/gonum/lapack/lapack64 gonum.org/v1/gonum/mat # google.golang.org/api v0.22.1-0.20200430202532-ac9be1f8f530 +## explicit google.golang.org/api/container/v1beta1 google.golang.org/api/googleapi google.golang.org/api/googleapi/transport @@ -421,6 +442,7 @@ google.golang.org/appengine/internal/urlfetch google.golang.org/appengine/socket google.golang.org/appengine/urlfetch # google.golang.org/genproto v0.0.0-20200430143042-b979b6f78d84 +## explicit google.golang.org/genproto/googleapis/api google.golang.org/genproto/googleapis/api/annotations google.golang.org/genproto/googleapis/api/distribution @@ -445,6 +467,7 @@ google.golang.org/genproto/googleapis/type/calendarperiod google.golang.org/genproto/googleapis/type/expr google.golang.org/genproto/protobuf/field_mask # google.golang.org/grpc v1.29.1 +## explicit google.golang.org/grpc google.golang.org/grpc/attributes google.golang.org/grpc/backoff @@ -496,6 +519,7 @@ google.golang.org/grpc/stats google.golang.org/grpc/status google.golang.org/grpc/tap # google.golang.org/protobuf v1.21.0 +## explicit google.golang.org/protobuf/cmd/protoc-gen-go/internal_gengo google.golang.org/protobuf/compiler/protogen google.golang.org/protobuf/encoding/protojson @@ -570,13 +594,17 @@ honnef.co/go/tools/stylecheck honnef.co/go/tools/unused honnef.co/go/tools/version # istio.io/api v0.0.0-20200227213531-891bf31f3c32 => istio.io/api v0.0.0-20200227213531-891bf31f3c32 +## explicit istio.io/api/security/v1beta1 istio.io/api/type/v1beta1 # istio.io/client-go v0.0.0-20200227214646-23b87b42e49b => istio.io/client-go v0.0.0-20200227214646-23b87b42e49b +## explicit istio.io/client-go/pkg/apis/security/v1beta1 # istio.io/gogo-genproto v0.0.0-20200130224810-a0338448499a +## explicit istio.io/gogo-genproto/googleapis/google/api # k8s.io/api v0.17.4 => k8s.io/api v0.16.4 +## explicit k8s.io/api/admission/v1beta1 k8s.io/api/admissionregistration/v1 k8s.io/api/admissionregistration/v1beta1 @@ -635,6 +663,7 @@ k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/internalint k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1 k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1 # k8s.io/apimachinery v0.18.1 => k8s.io/apimachinery v0.16.5-beta.1 +## explicit k8s.io/apimachinery/pkg/api/apitesting/fuzzer k8s.io/apimachinery/pkg/api/equality k8s.io/apimachinery/pkg/api/errors @@ -686,6 +715,7 @@ k8s.io/apimachinery/third_party/forked/golang/reflect # k8s.io/apiserver v0.17.4 k8s.io/apiserver/pkg/storage/names # k8s.io/client-go v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible => k8s.io/client-go v0.16.4 +## explicit k8s.io/client-go/discovery k8s.io/client-go/discovery/fake k8s.io/client-go/dynamic @@ -951,6 +981,7 @@ k8s.io/utils/integer k8s.io/utils/pointer k8s.io/utils/trace # knative.dev/eventing v0.14.1-0.20200506063944-e9eb527e1295 +## explicit knative.dev/eventing/pkg/apis/config knative.dev/eventing/pkg/apis/configs knative.dev/eventing/pkg/apis/configs/v1alpha1 @@ -1043,7 +1074,8 @@ knative.dev/eventing/test/test_images/logevents knative.dev/eventing/test/test_images/recordevents knative.dev/eventing/test/test_images/sendevents knative.dev/eventing/test/test_images/transformevents -# knative.dev/pkg v0.0.0-20200506142844-5b98a558168e +# knative.dev/pkg v0.0.0-20200507220045-66f1d63f1019 +## explicit knative.dev/pkg/apis knative.dev/pkg/apis/duck knative.dev/pkg/apis/duck/v1 @@ -1135,6 +1167,7 @@ knative.dev/pkg/webhook/resourcesemantics/conversion knative.dev/pkg/webhook/resourcesemantics/defaulting knative.dev/pkg/webhook/resourcesemantics/validation # knative.dev/serving v0.14.1-0.20200506171253-f2c76cb8a6dd +## explicit knative.dev/serving/pkg/apis/autoscaling knative.dev/serving/pkg/apis/autoscaling/v1alpha1 knative.dev/serving/pkg/apis/config @@ -1180,6 +1213,40 @@ knative.dev/serving/pkg/client/listers/serving/v1 knative.dev/serving/pkg/client/listers/serving/v1alpha1 knative.dev/serving/pkg/client/listers/serving/v1beta1 # knative.dev/test-infra v0.0.0-20200506193944-431dda291f8c +## explicit knative.dev/test-infra/scripts # sigs.k8s.io/yaml v1.2.0 => sigs.k8s.io/yaml v1.1.0 sigs.k8s.io/yaml +# contrib.go.opencensus.io/exporter/stackdriver => contrib.go.opencensus.io/exporter/stackdriver v0.12.9-0.20191108183826-59d068f8d8ff +# go.opencensus.io => go.opencensus.io v0.22.1 +# istio.io/api => istio.io/api v0.0.0-20200227213531-891bf31f3c32 +# istio.io/client-go => istio.io/client-go v0.0.0-20200227214646-23b87b42e49b +# k8s.io/api => k8s.io/api v0.16.4 +# k8s.io/apimachinery => k8s.io/apimachinery v0.16.5-beta.1 +# k8s.io/client-go => k8s.io/client-go v0.16.4 +# k8s.io/code-generator => k8s.io/code-generator v0.16.5-beta.1 +# k8s.io/kube-openapi => k8s.io/kube-openapi v0.0.0-20190918143330-0270cf2f1c1d +# github.com/aws/aws-sdk-go => github.com/aws/aws-sdk-go v1.25.1 +# github.com/blang/semver => github.com/blang/semver v1.1.1-0.20190414102917-ba2c2ddd8906 +# github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.0 +# github.com/imdario/mergo => github.com/imdario/mergo v0.3.7 +# github.com/jmespath/go-jmespath => github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af +# github.com/json-iterator/go => github.com/json-iterator/go v1.1.7 +# github.com/modern-go/reflect2 => github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 +# github.com/pkg/errors => github.com/pkg/errors v0.8.1 +# github.com/robfig/cron/v3 => github.com/robfig/cron/v3 v3.0.0 +# go.uber.org/zap => go.uber.org/zap v1.9.2-0.20180814183419-67bc79d13d15 +# golang.org/x/lint => golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f +# golang.org/x/net => golang.org/x/net v0.0.0-20190930134127-c5a3c61f89f3 +# golang.org/x/oauth2 => golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 +# golang.org/x/sync => golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e +# golang.org/x/sys => golang.org/x/sys v0.0.0-20190927073244-c990c680b611 +# golang.org/x/text => golang.org/x/text v0.3.2 +# golang.org/x/time => golang.org/x/time v0.0.0-20190921001708-c4c64cad1fd0 +# golang.org/x/tools => golang.org/x/tools v0.0.0-20190930152728-90aeebe84374 +# golang.org/x/xerrors => golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 +# gomodules.xyz/jsonpatch/v2 => gomodules.xyz/jsonpatch/v2 v2.0.1 +# gopkg.in/yaml.v2 => gopkg.in/yaml.v2 v2.2.2 +# honnef.co/go/tools => honnef.co/go/tools v0.0.1-2019.2.3 +# sigs.k8s.io/yaml => sigs.k8s.io/yaml v1.1.0 +# github.com/cloudevents/sdk-go/v2 => github.com/cloudevents/sdk-go/v2 v2.0.0-RC1 From c05e0784faab438869ab93bd6dab4023a2738bba Mon Sep 17 00:00:00 2001 From: capri-xiyue <52932582+capri-xiyue@users.noreply.github.com> Date: Fri, 8 May 2020 09:01:45 -0700 Subject: [PATCH 10/11] updated docs of install knative-gcp and examples of cloudstoragesource (#1004) * updated docs of install knative-gcp and examples of cloudstoragesource * modified docs * fixed format * fixed format * fixed docs * fixed docs * modified docs * fixed wrong link * fixed nits * fixed nits --- docs/examples/cloudstoragesource/README.md | 76 +++++++++++++--------- docs/install/install-knative-gcp.md | 45 ++++++++++--- 2 files changed, 80 insertions(+), 41 deletions(-) diff --git a/docs/examples/cloudstoragesource/README.md b/docs/examples/cloudstoragesource/README.md index 14537f224b..61b5fe01d1 100644 --- a/docs/examples/cloudstoragesource/README.md +++ b/docs/examples/cloudstoragesource/README.md @@ -12,12 +12,51 @@ Object Notifications for when a new object is added to Google Cloud Storage 1. [Create a Pub/Sub enabled Service Account](../../install/pubsub-service-account.md) -1. Enable the `Cloud Storage API` on your project: - - ```shell - gcloud services enable storage-component.googleapis.com - gcloud services enable storage-api.googleapis.com - ``` +1. Enable the `Cloud Storage API` on your project and give Google Cloud Storage + permissions to publish to GCP Pub/Sub. Currently, we support two methods: + automated script or manual + + - Option 1 (Recommended, via automated scripts): + Apply + [init_cloud_storage_source.sh](../../../hack/init_cloud_storage_source.sh): + ```shell + ./hack/init_cloud_storage_source.sh + ``` + **_Note_**: This script will need one parameter + 1. `PROJECT_ID`: an optional parameter to specify the project to use, default to `gcloud config get-value project` + If you want to specify the parameter `PROJECT_ID` instead of using the default one, + ```shell + ./hack/init_cloud_storage_source.sh [PROJECT_ID] + ``` + + - Option 2 (manual): + 1. Enable the `Cloud Storage API` on your project: + ```shell + gcloud services enable storage-component.googleapis.com + gcloud services enable storage-api.googleapis.com + ``` + 1. Give Google Cloud Storage permissions to publish to GCP Pub/Sub. + 1. First find the Service Account that GCS uses to publish to Pub/Sub + (Either using UI or using curl as shown below) + - Option 1: Use the steps outlined in + [Cloud Console or the JSON API](https://cloud.google.com/storage/docs/getting-service-account) + Assume the service account you found from above was + `service-XYZ@gs-project-accounts.iam.gserviceaccount.com`, you'd do: + `shell export GCS_SERVICE_ACCOUNT=service-XYZ@gs-project-accounts.iam.gserviceaccount.com` + - Option 2: Use `curl` to fetch the email: + ````shell + export GCS_SERVICE_ACCOUNT=`curl -s -X GET -H "Authorization: Bearer \`GOOGLE_APPLICATION_CREDENTIALS=./cre-pubsub.json + gcloud auth application-default print-access-token\`" + "https://www.googleapis.com/storage/v1/projects/$PROJECT_ID/serviceAccount" + | grep email_address | cut -d '"' -f 4` + ```` + + 1. Then grant rights to that Service Account to publish to GCP Pub/Sub. + ```shell + gcloud projects add-iam-policy-binding $PROJECT_ID \ + --member=serviceAccount:$GCS_SERVICE_ACCOUNT \ + --role roles/pubsub.publisher + ``` 1. Use an existing GCS Bucket, or create a new one. You can create a bucket either from the [Cloud Console](https://cloud.google.com/console) or by using @@ -27,31 +66,6 @@ Object Notifications for when a new object is added to Google Cloud Storage export BUCKET= ``` -1. Give Google Cloud Storage permissions to publish to GCP Pub/Sub. - - 1. First find the Service Account that GCS uses to publish to Pub/Sub (Either - using UI or using curl as shown below) - - 1. Use the steps outlined in - [Cloud Console or the JSON API](https://cloud.google.com/storage/docs/getting-service-account) - Assume the service account you found from above was - `service-XYZ@gs-project-accounts.iam.gserviceaccount.com`, you'd do: - `shell export GCS_SERVICE_ACCOUNT=service-XYZ@gs-project-accounts.iam.gserviceaccount.com` - - 1. Use `curl` to fetch the email: - - ```shell - export GCS_SERVICE_ACCOUNT=`curl -s -X GET -H "Authorization: Bearer \`GOOGLE_APPLICATION_CREDENTIALS=./cre-pubsub.json gcloud auth application-default print-access-token\`" "https://www.googleapis.com/storage/v1/projects/$PROJECT_ID/serviceAccount" | grep email_address | cut -d '"' -f 4` - ``` - - 1. Then grant rights to that Service Account to publish to GCP Pub/Sub. - - ```shell - gcloud projects add-iam-policy-binding $PROJECT_ID \ - --member=serviceAccount:$GCS_SERVICE_ACCOUNT \ - --role roles/pubsub.publisher - ``` - ## Deployment 1. Update `googleServiceAccount` / `secret` in the diff --git a/docs/install/install-knative-gcp.md b/docs/install/install-knative-gcp.md index ab6d703bec..63cdfd12a0 100644 --- a/docs/install/install-knative-gcp.md +++ b/docs/install/install-knative-gcp.md @@ -72,12 +72,6 @@ information about Workload Identity, please see 1. Your gcloud `CLI` are up to date. You may use `gcloud components update` to update it. -**_Note_**: Both scripts will have a step to create a Google Cloud Service -Account `cloud-run-events`. Ignore the error message if you already had this -service account (error for 'service account already exists'). -TODO([#896](https://github.com/google/knative-gcp/issues/896)) Get rid of the -error message. - **_Note_**: The configuration steps have been automated by the scripts below. If wish to configure the auth manually, refer to [manually configure authentication for GCP](./authentication-mechanisms-gcp.md), @@ -85,10 +79,9 @@ wish to configure the auth manually, refer to - Option 1 (Recommended): Use Workload Identity. **_Note:_** Now, Workload Identity for the Control Plane only works if you install the Knative-GCP Constructs from the master. If you install the Knative-GCP Constructs with our - latest release (v0.14.0) or older releases, please use option 2. - - Apply - + latest release (v0.14.0) or older releases, please use option 2. + + Apply [init_control_plane_gke.sh](../../hack/init_control_plane_gke.sh): ```shell @@ -97,6 +90,26 @@ wish to configure the auth manually, refer to **_Note_**: If you didn't enable Workload Identity when you created your cluster, this step may take a long time to finish. + **_Note_**: Optional parameters available. + + 1. `CLUSTER_NAME`: an optional parameter to specify the cluster to use, + default to + `gcloud config get-value run/cluster` + 1. `CLUSTER_LOCATION`: an optional parameter to specify the cluster location + to use, default to + `gcloud config get-value run/cluster_location` + 1. `CLUSTER_LOCATION_TYPE`: an optional parameter to specify the cluster + location type to use, default to `zonal`. CLUSTER_LOCATION_TYPE must be + `zonal` or `regional`. + 1. `PROJECT_ID`: an optional parameter to specify the project to use, default + to + `gcloud config get-value project`. + + If you want to specify the parameters instead of using the default ones, + + ```shell + ./hack/init_control_plane_gke.sh [CLUSTER_NAME] [CLUSTER_LOCATION] [CLUSTER_LOCATION_TYPE] [PROJECT_ID] + ``` * Option 2: Export service account keys and store them as Kubernetes Secrets. Apply [init_control_plane.sh](../../hack/init_control_plane.sh): @@ -104,3 +117,15 @@ wish to configure the auth manually, refer to ```shell ./hack/init_control_plane.sh ``` + + **_Note_**: Optional parameters available. + + 1. `PROJECT_ID`: an optional parameter to specify the project to use, default + to + `gcloud config get-value project`. + If you want to specify the parameter + `PROJECT_ID` instead of using the default one, + + ```shell + ./hack/init_control_plane.sh [PROJECT_ID] + ``` From 509af113b92ef519ff3bcaf6c8b1239528cc478a Mon Sep 17 00:00:00 2001 From: Chi Zhang Date: Fri, 8 May 2020 10:23:45 -0700 Subject: [PATCH 11/11] fix perf tests setup (#1025) --- .../continuous/100-broker-pubsub-setup.yaml | 24 +++++++++++++++---- .../continuous/100-channel-perf-setup.yaml | 6 ++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/test/performance/benchmarks/broker-pubsub/continuous/100-broker-pubsub-setup.yaml b/test/performance/benchmarks/broker-pubsub/continuous/100-broker-pubsub-setup.yaml index 20531ac2e0..b76d1c5799 100644 --- a/test/performance/benchmarks/broker-pubsub/continuous/100-broker-pubsub-setup.yaml +++ b/test/performance/benchmarks/broker-pubsub/continuous/100-broker-pubsub-setup.yaml @@ -12,19 +12,33 @@ # See the License for the specific language governing permissions and # limitations under the License. -apiVersion: eventing.knative.dev/v1alpha1 +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-broker +data: + channelTemplateSpec: | + apiVersion: messaging.cloud.google.com/v1beta1 + kind: Channel + +--- + +apiVersion: eventing.knative.dev/v1beta1 kind: Broker metadata: name: pubsub namespace: default + annotations: + eventing.knative.dev/broker.class: ChannelBasedBroker spec: - channelTemplateSpec: - apiVersion: messaging.cloud.google.com/v1alpha1 - kind: Channel + config: + apiVersion: v1 + kind: ConfigMap + name: config-broker --- -apiVersion: eventing.knative.dev/v1alpha1 +apiVersion: eventing.knative.dev/v1beta1 kind: Trigger metadata: name: broker-pubsub diff --git a/test/performance/benchmarks/channel-pubsub/continuous/100-channel-perf-setup.yaml b/test/performance/benchmarks/channel-pubsub/continuous/100-channel-perf-setup.yaml index ae463773a2..46bb77ec88 100644 --- a/test/performance/benchmarks/channel-pubsub/continuous/100-channel-perf-setup.yaml +++ b/test/performance/benchmarks/channel-pubsub/continuous/100-channel-perf-setup.yaml @@ -67,7 +67,7 @@ roleRef: --- -apiVersion: messaging.cloud.google.com/v1alpha1 +apiVersion: messaging.cloud.google.com/v1beta1 kind: Channel metadata: name: pubsub-test-channel @@ -76,14 +76,14 @@ metadata: --- -apiVersion: messaging.knative.dev/v1alpha1 +apiVersion: messaging.knative.dev/v1beta1 kind: Subscription metadata: name: pubsub-test-channel-sub namespace: default spec: channel: - apiVersion: messaging.cloud.google.com/v1alpha1 + apiVersion: messaging.cloud.google.com/v1beta1 kind: Channel name: pubsub-test-channel subscriber: