From fe41fe30f3107065df45164dbd97ba6efdada75a Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 6 Oct 2020 21:40:37 -0700 Subject: [PATCH 1/9] Move e2e tests for metrics export to integration status to reduce crosstalk flakiness --- metrics/record_e2e_test.go | 627 ++++++++++++++++++++++++++++++++++ metrics/resource_view_test.go | 589 -------------------------------- test/presubmit-tests.sh | 4 +- 3 files changed, 630 insertions(+), 590 deletions(-) create mode 100644 metrics/record_e2e_test.go diff --git a/metrics/record_e2e_test.go b/metrics/record_e2e_test.go new file mode 100644 index 0000000000..d6627fdd47 --- /dev/null +++ b/metrics/record_e2e_test.go @@ -0,0 +1,627 @@ +// +build e2e + +// These tests create fake actual exporter services to verify that the metrics +// transmitted on the wire are correct. As such, we run them separately from +// the rest of the tests because of the duration and the nensitivity to +// external reconfiguration of the package-global metrics provider. + +/* +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 metrics + +import ( + "context" + "fmt" + "io" + "io/ioutil" + "net" + "net/http" + "os" + "sort" + "strings" + "sync" + "testing" + "time" + + sd "contrib.go.opencensus.io/exporter/stackdriver" + ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" + ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" + "go.opencensus.io/resource" + "go.opencensus.io/stats" + "go.opencensus.io/stats/view" + "go.opencensus.io/tag" + "k8s.io/apimachinery/pkg/util/clock" + + emptypb "github.com/golang/protobuf/ptypes/empty" + "github.com/google/go-cmp/cmp" + "google.golang.org/api/option" + metricpb "google.golang.org/genproto/googleapis/api/metric" + stackdriverpb "google.golang.org/genproto/googleapis/monitoring/v3" + "google.golang.org/grpc" + proto "google.golang.org/protobuf/proto" + + logtesting "knative.dev/pkg/logging/testing" + "knative.dev/pkg/metrics/metricskey" + "knative.dev/pkg/metrics/metricstest" +) + +var ( + NamespaceTagKey = tag.MustNewKey(metricskey.LabelNamespaceName) + ServiceTagKey = tag.MustNewKey(metricskey.LabelServiceName) + ConfigTagKey = tag.MustNewKey(metricskey.LabelConfigurationName) + RevisionTagKey = tag.MustNewKey(metricskey.LabelRevisionName) +) + +type metricExtract struct { + Name string + Labels map[string]string + Value int64 +} + +func (m metricExtract) Key() string { + return fmt.Sprintf("%s<%s>", m.Name, resource.EncodeLabels(m.Labels)) +} + +func (m metricExtract) String() string { + return fmt.Sprintf("%s:%d", m.Key(), m.Value) +} + +func initSdFake(sdFake *stackDriverFake) error { + if err := sdFake.start(); err != nil { + return err + } + conn, err := grpc.Dial(sdFake.address, grpc.WithInsecure()) + if err != nil { + return err + } + newStackdriverExporterFunc = func(o sd.Options) (view.Exporter, error) { + o.MonitoringClientOptions = append(o.MonitoringClientOptions, option.WithGRPCConn(conn)) + return newOpencensusSDExporter(o) + } + // File: must exist, be json of credentialsFile, and type must be a jwtConfig or oauth2Config + tmp, err := ioutil.TempFile("", "metrics-sd-test") + if err != nil { + return err + } + defer tmp.Close() + credentialsContent := []byte(`{"type": "service_account"}`) + if _, err := tmp.Write(credentialsContent); err != nil { + return err + } + os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", tmp.Name()) + return nil +} + +func sortMetrics() cmp.Option { + return cmp.Transformer("Sort", func(in []metricExtract) []string { + out := make([]string, 0, len(in)) + seen := map[string]int{} + for _, m := range in { + // Keep only the newest report for a key + key := m.Key() + if seen[key] == 0 { + out = append(out, m.String()) + seen[key] = len(out) // Store address+1 to avoid doubling first item. + } else { + out[seen[key]-1] = m.String() + } + } + sort.Strings(out) + return out + }) +} + +// Begin table tests for exporters +func TestMetricsExport(t *testing.T) { + TestOverrideBundleCount = 1 + t.Cleanup(func() { TestOverrideBundleCount = 0 }) + ocFake := openCensusFake{address: "localhost:12345"} + sdFake := stackDriverFake{} + prometheusPort := 19090 + configForBackend := func(backend metricsBackend) ExporterOptions { + return ExporterOptions{ + Domain: servingDomain, + Component: testComponent, + PrometheusPort: prometheusPort, + ConfigMap: map[string]string{ + BackendDestinationKey: string(backend), + collectorAddressKey: ocFake.address, + allowStackdriverCustomMetricsKey: "true", + stackdriverCustomMetricSubDomainKey: servingDomain, + reportingPeriodKey: "1", + }, + } + } + + resources := []*resource.Resource{ + { + Type: "revision", + Labels: map[string]string{ + "project": "p1", + "revision": "r1", + }, + }, + { + Type: "revision", + Labels: map[string]string{ + "project": "p1", + "revision": "r2", + }, + }, + } + gauge := stats.Int64("testing/value", "Stored value", stats.UnitDimensionless) + counter := stats.Int64("export counts", "Times through the export", stats.UnitDimensionless) + gaugeView := &view.View{ + Name: "testing/value", + Description: "Test value", + Measure: gauge, + Aggregation: view.LastValue(), + } + resourceCounter := &view.View{ + Name: "resource_global_export_count", + Description: "Count of exports via RegisterResourceView.", + Measure: counter, + Aggregation: view.Count(), + } + globalCounter := &view.View{ + Name: "global_export_counts", + Description: "Count of exports via standard OpenCensus view.", + Measure: counter, + Aggregation: view.Count(), + } + + expected := []metricExtract{ + {"knative.dev/serving/testComponent/global_export_counts", map[string]string{}, 2}, + {"knative.dev/serving/testComponent/resource_global_export_count", map[string]string{}, 2}, + {"knative.dev/serving/testComponent/testing/value", map[string]string{"project": "p1", "revision": "r1"}, 0}, + {"knative.dev/serving/testComponent/testing/value", map[string]string{"project": "p1", "revision": "r2"}, 1}, + } + + harnesses := []struct { + name string + init func() error + validate func(t *testing.T) + }{{ + name: "Prometheus", + init: func() error { + return UpdateExporter(context.Background(), configForBackend(prometheus), logtesting.TestLogger(t)) + }, + validate: func(t *testing.T) { + metricstest.EnsureRecorded() + resp, err := http.Get(fmt.Sprintf("http://localhost:%d/metrics", prometheusPort)) + if err != nil { + t.Fatalf("failed to fetch prometheus metrics: %+v", err) + } + defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatalf("failed to read prometheus response: %+v", err) + } + want := `# HELP testComponent_global_export_counts Count of exports via standard OpenCensus view. +# TYPE testComponent_global_export_counts counter +testComponent_global_export_counts 2 +# HELP testComponent_resource_global_export_count Count of exports via RegisterResourceView. +# TYPE testComponent_resource_global_export_count counter +testComponent_resource_global_export_count 2 +# HELP testComponent_testing_value Test value +# TYPE testComponent_testing_value gauge +testComponent_testing_value{project="p1",revision="r1"} 0 +testComponent_testing_value{project="p1",revision="r2"} 1 +` + if diff := cmp.Diff(want, string(body)); diff != "" { + t.Errorf("Unexpected prometheus output (-want +got):\n%s", diff) + } + }, + }, { + name: "OpenCensus", + init: func() error { + if err := ocFake.start(len(resources) + 1); err != nil { + return err + } + t.Log("Created exporter at", ocFake.address) + return UpdateExporter(context.Background(), configForBackend(openCensus), logtesting.TestLogger(t)) + }, + validate: func(t *testing.T) { + // We unregister the views because this is one of two ways to flush + // the internal aggregation buffers; the other is to have the + // internal reporting period duration tick, which is at least + // [new duration] in the future. + view.Unregister(globalCounter) + UnregisterResourceView(gaugeView, resourceCounter) + + records := []metricExtract{} + loop: + for { + select { + case record := <-ocFake.published: + if record == nil { + continue loop + } + for _, m := range record.Metrics { + if len(m.Timeseries) > 0 { + labels := map[string]string{} + if record.Resource != nil { + labels = record.Resource.Labels + } + records = append(records, metricExtract{ + Name: m.MetricDescriptor.Name, + Labels: labels, + Value: m.Timeseries[0].Points[0].GetInt64Value(), + }) + } + } + if len(records) >= len(expected) { + break loop + } + case <-time.After(4 * time.Second): + t.Error("Timeout reading input") + break loop + } + } + + if diff := cmp.Diff(expected, records, sortMetrics()); diff != "" { + t.Errorf("Unexpected OpenCensus exports (-want +got):\n%s", diff) + } + }, + }, { + name: "Stackdriver", + init: func() error { + if err := initSdFake(&sdFake); err != nil { + return err + } + return UpdateExporter(context.Background(), configForBackend(stackdriver), logtesting.TestLogger(t)) + }, + validate: func(t *testing.T) { + records := []metricExtract{} + for record := range sdFake.published { + for _, ts := range record.TimeSeries { + name := ts.Metric.Type[len("custom.googleapis.com/"):] + records = append(records, metricExtract{ + Name: name, + Labels: ts.Resource.Labels, + Value: ts.Points[0].Value.GetInt64Value(), + }) + } + if len(records) >= 4 { + // There's no way to synchronize on the internal timer used + // by metricsexport.IntervalReader, so shut down the + // exporter after the first report cycle. + FlushExporter() + sdFake.srv.GracefulStop() + } + } + if diff := cmp.Diff(expected, records, sortMetrics()); diff != "" { + t.Errorf("Unexpected Stackdriver exports (-want +got):\n%s", diff) + } + }, + }} + + for _, c := range harnesses { + t.Run(c.name, func(t *testing.T) { + ClearMetersForTest() + sdFake.t = t + if err := c.init(); err != nil { + t.Fatalf("unable to init: %+v", err) + } + + view.Register(globalCounter) + if err := RegisterResourceView(gaugeView, resourceCounter); err != nil { + t.Fatal("Unable to register views:", err) + } + t.Cleanup(func() { + view.Unregister(globalCounter) + UnregisterResourceView(gaugeView, resourceCounter) + }) + + for i, r := range resources { + ctx := context.Background() + Record(ctx, counter.M(int64(1))) + if r != nil { + ctx = metricskey.WithResource(ctx, *r) + } + Record(ctx, gauge.M(int64(i))) + } + c.validate(t) + }) + } +} + +func TestStackDriverExports(t *testing.T) { + TestOverrideBundleCount = 1 + t.Cleanup(func() { TestOverrideBundleCount = 0 }) + eo := ExporterOptions{ + Domain: servingDomain, + Component: "autoscaler", + ConfigMap: map[string]string{ + BackendDestinationKey: string(stackdriver), + reportingPeriodKey: "1", + stackdriverProjectIDKey: "foobar", + }, + } + + label1 := map[string]string{ + "cluster_name": "test-cluster", + "configuration_name": "config", + "location": "test-location", + "namespace_name": "ns", + "project_id": "foobar", + "revision_name": "revision", + "service_name": "service", + } + label2 := map[string]string{ + "cluster_name": "test-cluster", + "configuration_name": "config2", + "location": "test-location", + "namespace_name": "ns2", + "project_id": "foobar", + "revision_name": "revision2", + "service_name": "service2", + } + batchLabels := map[string]string{ + "namespace_name": "ns2", + "configuration_name": "config2", + "revision_name": "revision2", + "service_name": "service2", + } + harness := []struct { + name string + allowCustomMetrics string + expected []metricExtract + }{{ + name: "Allow custom metrics", + allowCustomMetrics: "true", + expected: []metricExtract{ + { + "knative.dev/serving/autoscaler/actual_pods", + label1, + 1, + }, + { + "knative.dev/serving/autoscaler/desired_pods", + label2, + 2, + }, + { + "custom.googleapis.com/knative.dev/autoscaler/not_ready_pods", + batchLabels, + 3, + }, + }, + }, { + name: "Don't allow custom metrics", + allowCustomMetrics: "false", + expected: []metricExtract{ + { + "knative.dev/serving/autoscaler/actual_pods", + label1, + 1, + }, + { + "knative.dev/serving/autoscaler/desired_pods", + label2, + 2, + }, + }, + }} + + for _, tc := range harness { + t.Run(tc.name, func(t *testing.T) { + eo.ConfigMap[allowStackdriverCustomMetricsKey] = tc.allowCustomMetrics + // Change the cluster name to reinitialize the exporter and pick up a new port. + eo.ConfigMap[stackdriverClusterNameKey] = tc.name + actualPodCountM := stats.Int64( + "actual_pods", + "Number of pods that are allocated currently", + stats.UnitDimensionless) + actualPodsCountView := &view.View{ + Description: "Number of pods that are allocated currently", + Measure: actualPodCountM, + Aggregation: view.LastValue(), + TagKeys: []tag.Key{NamespaceTagKey, ServiceTagKey, ConfigTagKey, RevisionTagKey}, + } + desiredPodCountM := stats.Int64( + "desired_pods", + "Number of pods that are desired", + stats.UnitDimensionless) + desiredPodsCountView := &view.View{ + Description: "Number of pods that are desired", + Measure: desiredPodCountM, + Aggregation: view.LastValue(), + } + notReadyPodCountM := stats.Int64( + "not_ready_pods", + "Number of pods that are not ready", + stats.UnitDimensionless) + customView := &view.View{ + Description: "non-knative-revision metric per KnativeRevisionMetrics", + Measure: notReadyPodCountM, + Aggregation: view.LastValue(), + } + + sdFake := stackDriverFake{t: t} + if err := initSdFake(&sdFake); err != nil { + t.Error("Init stackdriver failed", err) + } + if err := UpdateExporter(context.Background(), eo, logtesting.TestLogger(t)); err != nil { + t.Error("UpdateExporter failed", err) + } + + if err := RegisterResourceView(desiredPodsCountView, actualPodsCountView, customView); err != nil { + t.Fatalf("unable to register view: %+v", err) + } + t.Cleanup(func() { + UnregisterResourceView(desiredPodsCountView, actualPodsCountView, customView) + }) + + ctx, err := tag.New(context.Background(), tag.Upsert(NamespaceTagKey, "ns"), + tag.Upsert(ServiceTagKey, "service"), + tag.Upsert(ConfigTagKey, "config"), + tag.Upsert(RevisionTagKey, "revision")) + if err != nil { + t.Fatal("Unable to create tags", err) + } + Record(ctx, actualPodCountM.M(int64(1))) + + r := resource.Resource{ + Type: "testing", + Labels: batchLabels, + } + RecordBatch( + metricskey.WithResource(context.Background(), r), + desiredPodCountM.M(int64(2)), + notReadyPodCountM.M(int64(3))) + + records := []metricExtract{} + loop: + for { + select { + case record := <-sdFake.published: + for _, ts := range record.TimeSeries { + extracted := metricExtract{ + Name: ts.Metric.Type, + Labels: ts.Resource.Labels, + Value: ts.Points[0].Value.GetInt64Value(), + } + // Override 'cluster-name' label to reset to a fixed value + if extracted.Labels["cluster_name"] != "" { + extracted.Labels["cluster_name"] = "test-cluster" + } + records = append(records, extracted) + if strings.HasPrefix(ts.Metric.Type, "knative.dev/") { + if diff := cmp.Diff(ts.Resource.Type, metricskey.ResourceTypeKnativeRevision); diff != "" { + t.Errorf("Incorrect resource type for %q: (-want +got):\n%s", ts.Metric.Type, diff) + } + } + } + if len(records) >= len(tc.expected) { + // There's no way to synchronize on the internal timer used + // by metricsexport.IntervalReader, so shut down the + // exporter after the first report cycle. + FlushExporter() + sdFake.srv.GracefulStop() + break loop + } + case <-time.After(4 * time.Second): + t.Error("Timeout reading records from Stackdriver") + break loop + } + } + if diff := cmp.Diff(tc.expected, records, sortMetrics()); diff != "" { + t.Errorf("Unexpected stackdriver knative exports (-want +got):\n%s", diff) + } + }) + } +} + +type openCensusFake struct { + ocmetrics.UnimplementedMetricsServiceServer + address string + srv *grpc.Server + exports sync.WaitGroup + wg sync.WaitGroup + published chan *ocmetrics.ExportMetricsServiceRequest +} + +func (oc *openCensusFake) start(expectedStreams int) error { + ln, err := net.Listen("tcp", oc.address) + if err != nil { + return err + } + oc.published = make(chan *ocmetrics.ExportMetricsServiceRequest, 100) + oc.srv = grpc.NewServer() + ocmetrics.RegisterMetricsServiceServer(oc.srv, oc) + // Run the server in the background. + oc.wg.Add(1) + go func() { + oc.srv.Serve(ln) + oc.wg.Done() + oc.wg.Wait() + close(oc.published) + }() + oc.exports.Add(expectedStreams) + go oc.stop() + return nil +} + +func (oc *openCensusFake) stop() { + oc.exports.Wait() + oc.srv.Stop() +} + +func (oc *openCensusFake) Export(stream ocmetrics.MetricsService_ExportServer) error { + var streamResource *ocresource.Resource + oc.wg.Add(1) + defer oc.wg.Done() + metricSeen := false + for { + in, err := stream.Recv() + if err == io.EOF { + return nil + } + if err != nil { + return err + } + if in.Resource != nil { + // The stream is stateful, keep track of the last Resource seen. + streamResource = in.Resource + } + if len(in.Metrics) > 0 { + if in.Resource == nil { + in.Resource = streamResource + } + oc.published <- proto.Clone(in).(*ocmetrics.ExportMetricsServiceRequest) + if !metricSeen { + oc.exports.Done() + metricSeen = true + } + } + } +} + +type stackDriverFake struct { + stackdriverpb.UnimplementedMetricServiceServer + address string + srv *grpc.Server + t *testing.T + published chan *stackdriverpb.CreateTimeSeriesRequest +} + +func (sd *stackDriverFake) start() error { + sd.published = make(chan *stackdriverpb.CreateTimeSeriesRequest, 100) + ln, err := net.Listen("tcp", "localhost:0") + if err != nil { + return err + } + sd.address = ln.Addr().String() + sd.srv = grpc.NewServer() + stackdriverpb.RegisterMetricServiceServer(sd.srv, sd) + // Run the server in the background. + go func() { + sd.srv.Serve(ln) + close(sd.published) + }() + return nil +} + +func (sd *stackDriverFake) CreateTimeSeries(ctx context.Context, req *stackdriverpb.CreateTimeSeriesRequest) (*emptypb.Empty, error) { + sd.published <- req + return &emptypb.Empty{}, nil +} + +func (sd *stackDriverFake) CreateMetricDescriptor(ctx context.Context, req *stackdriverpb.CreateMetricDescriptorRequest) (*metricpb.MetricDescriptor, error) { + return req.MetricDescriptor, nil +} diff --git a/metrics/resource_view_test.go b/metrics/resource_view_test.go index e527672b88..563c92adca 100644 --- a/metrics/resource_view_test.go +++ b/metrics/resource_view_test.go @@ -17,47 +17,18 @@ limitations under the License. package metrics import ( - "context" "fmt" - "io" - "io/ioutil" - "net" - "net/http" - "os" - "sort" - "strings" - "sync" "testing" "time" - sd "contrib.go.opencensus.io/exporter/stackdriver" - ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" - ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" "go.opencensus.io/resource" "go.opencensus.io/stats" "go.opencensus.io/stats/view" - "go.opencensus.io/tag" "k8s.io/apimachinery/pkg/util/clock" - - emptypb "github.com/golang/protobuf/ptypes/empty" - "github.com/google/go-cmp/cmp" - "google.golang.org/api/option" - metricpb "google.golang.org/genproto/googleapis/api/metric" - stackdriverpb "google.golang.org/genproto/googleapis/monitoring/v3" - "google.golang.org/grpc" - proto "google.golang.org/protobuf/proto" - - logtesting "knative.dev/pkg/logging/testing" - "knative.dev/pkg/metrics/metricskey" - "knative.dev/pkg/metrics/metricstest" ) var ( r = resource.Resource{Labels: map[string]string{"foo": "bar"}} - NamespaceTagKey = tag.MustNewKey(metricskey.LabelNamespaceName) - ServiceTagKey = tag.MustNewKey(metricskey.LabelServiceName) - ConfigTagKey = tag.MustNewKey(metricskey.LabelConfigurationName) - RevisionTagKey = tag.MustNewKey(metricskey.LabelRevisionName) ) func TestRegisterResourceView(t *testing.T) { @@ -229,563 +200,3 @@ func TestResourceAsString(t *testing.T) { t.Error("Expect different resources, but got the same", s1) } } - -type metricExtract struct { - Name string - Labels map[string]string - Value int64 -} - -func (m metricExtract) Key() string { - return fmt.Sprintf("%s<%s>", m.Name, resource.EncodeLabels(m.Labels)) -} - -func (m metricExtract) String() string { - return fmt.Sprintf("%s:%d", m.Key(), m.Value) -} - -func initSdFake(sdFake *stackDriverFake) error { - if err := sdFake.start(); err != nil { - return err - } - conn, err := grpc.Dial(sdFake.address, grpc.WithInsecure()) - if err != nil { - return err - } - newStackdriverExporterFunc = func(o sd.Options) (view.Exporter, error) { - o.MonitoringClientOptions = append(o.MonitoringClientOptions, option.WithGRPCConn(conn)) - return newOpencensusSDExporter(o) - } - // File: must exist, be json of credentialsFile, and type must be a jwtConfig or oauth2Config - tmp, err := ioutil.TempFile("", "metrics-sd-test") - if err != nil { - return err - } - defer tmp.Close() - credentialsContent := []byte(`{"type": "service_account"}`) - if _, err := tmp.Write(credentialsContent); err != nil { - return err - } - os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", tmp.Name()) - return nil -} - -func sortMetrics() cmp.Option { - return cmp.Transformer("Sort", func(in []metricExtract) []string { - out := make([]string, 0, len(in)) - seen := map[string]int{} - for _, m := range in { - // Keep only the newest report for a key - key := m.Key() - if seen[key] == 0 { - out = append(out, m.String()) - seen[key] = len(out) // Store address+1 to avoid doubling first item. - } else { - out[seen[key]-1] = m.String() - } - } - sort.Strings(out) - return out - }) -} - -// Begin table tests for exporters -func TestMetricsExport(t *testing.T) { - TestOverrideBundleCount = 1 - t.Cleanup(func() { TestOverrideBundleCount = 0 }) - ocFake := openCensusFake{address: "localhost:12345"} - sdFake := stackDriverFake{} - prometheusPort := 19090 - configForBackend := func(backend metricsBackend) ExporterOptions { - return ExporterOptions{ - Domain: servingDomain, - Component: testComponent, - PrometheusPort: prometheusPort, - ConfigMap: map[string]string{ - BackendDestinationKey: string(backend), - collectorAddressKey: ocFake.address, - allowStackdriverCustomMetricsKey: "true", - stackdriverCustomMetricSubDomainKey: servingDomain, - reportingPeriodKey: "1", - }, - } - } - - resources := []*resource.Resource{ - { - Type: "revision", - Labels: map[string]string{ - "project": "p1", - "revision": "r1", - }, - }, - { - Type: "revision", - Labels: map[string]string{ - "project": "p1", - "revision": "r2", - }, - }, - } - gauge := stats.Int64("testing/value", "Stored value", stats.UnitDimensionless) - counter := stats.Int64("export counts", "Times through the export", stats.UnitDimensionless) - gaugeView := &view.View{ - Name: "testing/value", - Description: "Test value", - Measure: gauge, - Aggregation: view.LastValue(), - } - resourceCounter := &view.View{ - Name: "resource_global_export_count", - Description: "Count of exports via RegisterResourceView.", - Measure: counter, - Aggregation: view.Count(), - } - globalCounter := &view.View{ - Name: "global_export_counts", - Description: "Count of exports via standard OpenCensus view.", - Measure: counter, - Aggregation: view.Count(), - } - - expected := []metricExtract{ - {"knative.dev/serving/testComponent/global_export_counts", map[string]string{}, 2}, - {"knative.dev/serving/testComponent/resource_global_export_count", map[string]string{}, 2}, - {"knative.dev/serving/testComponent/testing/value", map[string]string{"project": "p1", "revision": "r1"}, 0}, - {"knative.dev/serving/testComponent/testing/value", map[string]string{"project": "p1", "revision": "r2"}, 1}, - } - - harnesses := []struct { - name string - init func() error - validate func(t *testing.T) - }{{ - name: "Prometheus", - init: func() error { - return UpdateExporter(context.Background(), configForBackend(prometheus), logtesting.TestLogger(t)) - }, - validate: func(t *testing.T) { - metricstest.EnsureRecorded() - resp, err := http.Get(fmt.Sprintf("http://localhost:%d/metrics", prometheusPort)) - if err != nil { - t.Fatalf("failed to fetch prometheus metrics: %+v", err) - } - defer resp.Body.Close() - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - t.Fatalf("failed to read prometheus response: %+v", err) - } - want := `# HELP testComponent_global_export_counts Count of exports via standard OpenCensus view. -# TYPE testComponent_global_export_counts counter -testComponent_global_export_counts 2 -# HELP testComponent_resource_global_export_count Count of exports via RegisterResourceView. -# TYPE testComponent_resource_global_export_count counter -testComponent_resource_global_export_count 2 -# HELP testComponent_testing_value Test value -# TYPE testComponent_testing_value gauge -testComponent_testing_value{project="p1",revision="r1"} 0 -testComponent_testing_value{project="p1",revision="r2"} 1 -` - if diff := cmp.Diff(want, string(body)); diff != "" { - t.Errorf("Unexpected prometheus output (-want +got):\n%s", diff) - } - }, - }, { - name: "OpenCensus", - init: func() error { - if err := ocFake.start(len(resources) + 1); err != nil { - return err - } - t.Log("Created exporter at", ocFake.address) - return UpdateExporter(context.Background(), configForBackend(openCensus), logtesting.TestLogger(t)) - }, - validate: func(t *testing.T) { - // We unregister the views because this is one of two ways to flush - // the internal aggregation buffers; the other is to have the - // internal reporting period duration tick, which is at least - // [new duration] in the future. - view.Unregister(globalCounter) - UnregisterResourceView(gaugeView, resourceCounter) - - records := []metricExtract{} - loop: - for { - select { - case record := <-ocFake.published: - if record == nil { - continue loop - } - for _, m := range record.Metrics { - if len(m.Timeseries) > 0 { - labels := map[string]string{} - if record.Resource != nil { - labels = record.Resource.Labels - } - records = append(records, metricExtract{ - Name: m.MetricDescriptor.Name, - Labels: labels, - Value: m.Timeseries[0].Points[0].GetInt64Value(), - }) - } - } - if len(records) >= len(expected) { - break loop - } - case <-time.After(4 * time.Second): - t.Error("Timeout reading input") - break loop - } - } - - if diff := cmp.Diff(expected, records, sortMetrics()); diff != "" { - t.Errorf("Unexpected OpenCensus exports (-want +got):\n%s", diff) - } - }, - }, { - name: "Stackdriver", - init: func() error { - if err := initSdFake(&sdFake); err != nil { - return err - } - return UpdateExporter(context.Background(), configForBackend(stackdriver), logtesting.TestLogger(t)) - }, - validate: func(t *testing.T) { - records := []metricExtract{} - for record := range sdFake.published { - for _, ts := range record.TimeSeries { - name := ts.Metric.Type[len("custom.googleapis.com/"):] - records = append(records, metricExtract{ - Name: name, - Labels: ts.Resource.Labels, - Value: ts.Points[0].Value.GetInt64Value(), - }) - } - if len(records) >= 4 { - // There's no way to synchronize on the internal timer used - // by metricsexport.IntervalReader, so shut down the - // exporter after the first report cycle. - FlushExporter() - sdFake.srv.GracefulStop() - } - } - if diff := cmp.Diff(expected, records, sortMetrics()); diff != "" { - t.Errorf("Unexpected Stackdriver exports (-want +got):\n%s", diff) - } - }, - }} - - for _, c := range harnesses { - t.Run(c.name, func(t *testing.T) { - ClearMetersForTest() - sdFake.t = t - if err := c.init(); err != nil { - t.Fatalf("unable to init: %+v", err) - } - - view.Register(globalCounter) - if err := RegisterResourceView(gaugeView, resourceCounter); err != nil { - t.Fatal("Unable to register views:", err) - } - t.Cleanup(func() { - view.Unregister(globalCounter) - UnregisterResourceView(gaugeView, resourceCounter) - }) - - for i, r := range resources { - ctx := context.Background() - Record(ctx, counter.M(int64(1))) - if r != nil { - ctx = metricskey.WithResource(ctx, *r) - } - Record(ctx, gauge.M(int64(i))) - } - c.validate(t) - }) - } -} - -func TestStackDriverExports(t *testing.T) { - TestOverrideBundleCount = 1 - t.Cleanup(func() { TestOverrideBundleCount = 0 }) - eo := ExporterOptions{ - Domain: servingDomain, - Component: "autoscaler", - ConfigMap: map[string]string{ - BackendDestinationKey: string(stackdriver), - reportingPeriodKey: "1", - stackdriverProjectIDKey: "foobar", - }, - } - - label1 := map[string]string{ - "cluster_name": "test-cluster", - "configuration_name": "config", - "location": "test-location", - "namespace_name": "ns", - "project_id": "foobar", - "revision_name": "revision", - "service_name": "service", - } - label2 := map[string]string{ - "cluster_name": "test-cluster", - "configuration_name": "config2", - "location": "test-location", - "namespace_name": "ns2", - "project_id": "foobar", - "revision_name": "revision2", - "service_name": "service2", - } - batchLabels := map[string]string{ - "namespace_name": "ns2", - "configuration_name": "config2", - "revision_name": "revision2", - "service_name": "service2", - } - harness := []struct { - name string - allowCustomMetrics string - expected []metricExtract - }{{ - name: "Allow custom metrics", - allowCustomMetrics: "true", - expected: []metricExtract{ - { - "knative.dev/serving/autoscaler/actual_pods", - label1, - 1, - }, - { - "knative.dev/serving/autoscaler/desired_pods", - label2, - 2, - }, - { - "custom.googleapis.com/knative.dev/autoscaler/not_ready_pods", - batchLabels, - 3, - }, - }, - }, { - name: "Don't allow custom metrics", - allowCustomMetrics: "false", - expected: []metricExtract{ - { - "knative.dev/serving/autoscaler/actual_pods", - label1, - 1, - }, - { - "knative.dev/serving/autoscaler/desired_pods", - label2, - 2, - }, - }, - }} - - for _, tc := range harness { - t.Run(tc.name, func(t *testing.T) { - eo.ConfigMap[allowStackdriverCustomMetricsKey] = tc.allowCustomMetrics - // Change the cluster name to reinitialize the exporter and pick up a new port. - eo.ConfigMap[stackdriverClusterNameKey] = tc.name - actualPodCountM := stats.Int64( - "actual_pods", - "Number of pods that are allocated currently", - stats.UnitDimensionless) - actualPodsCountView := &view.View{ - Description: "Number of pods that are allocated currently", - Measure: actualPodCountM, - Aggregation: view.LastValue(), - TagKeys: []tag.Key{NamespaceTagKey, ServiceTagKey, ConfigTagKey, RevisionTagKey}, - } - desiredPodCountM := stats.Int64( - "desired_pods", - "Number of pods that are desired", - stats.UnitDimensionless) - desiredPodsCountView := &view.View{ - Description: "Number of pods that are desired", - Measure: desiredPodCountM, - Aggregation: view.LastValue(), - } - notReadyPodCountM := stats.Int64( - "not_ready_pods", - "Number of pods that are not ready", - stats.UnitDimensionless) - customView := &view.View{ - Description: "non-knative-revision metric per KnativeRevisionMetrics", - Measure: notReadyPodCountM, - Aggregation: view.LastValue(), - } - - sdFake := stackDriverFake{t: t} - if err := initSdFake(&sdFake); err != nil { - t.Error("Init stackdriver failed", err) - } - if err := UpdateExporter(context.Background(), eo, logtesting.TestLogger(t)); err != nil { - t.Error("UpdateExporter failed", err) - } - - if err := RegisterResourceView(desiredPodsCountView, actualPodsCountView, customView); err != nil { - t.Fatalf("unable to register view: %+v", err) - } - t.Cleanup(func() { - UnregisterResourceView(desiredPodsCountView, actualPodsCountView, customView) - }) - - ctx, err := tag.New(context.Background(), tag.Upsert(NamespaceTagKey, "ns"), - tag.Upsert(ServiceTagKey, "service"), - tag.Upsert(ConfigTagKey, "config"), - tag.Upsert(RevisionTagKey, "revision")) - if err != nil { - t.Fatal("Unable to create tags", err) - } - Record(ctx, actualPodCountM.M(int64(1))) - - r := resource.Resource{ - Type: "testing", - Labels: batchLabels, - } - RecordBatch( - metricskey.WithResource(context.Background(), r), - desiredPodCountM.M(int64(2)), - notReadyPodCountM.M(int64(3))) - - records := []metricExtract{} - loop: - for { - select { - case record := <-sdFake.published: - for _, ts := range record.TimeSeries { - extracted := metricExtract{ - Name: ts.Metric.Type, - Labels: ts.Resource.Labels, - Value: ts.Points[0].Value.GetInt64Value(), - } - // Override 'cluster-name' label to reset to a fixed value - if extracted.Labels["cluster_name"] != "" { - extracted.Labels["cluster_name"] = "test-cluster" - } - records = append(records, extracted) - if strings.HasPrefix(ts.Metric.Type, "knative.dev/") { - if diff := cmp.Diff(ts.Resource.Type, metricskey.ResourceTypeKnativeRevision); diff != "" { - t.Errorf("Incorrect resource type for %q: (-want +got):\n%s", ts.Metric.Type, diff) - } - } - } - if len(records) >= len(tc.expected) { - // There's no way to synchronize on the internal timer used - // by metricsexport.IntervalReader, so shut down the - // exporter after the first report cycle. - FlushExporter() - sdFake.srv.GracefulStop() - break loop - } - case <-time.After(4 * time.Second): - t.Error("Timeout reading records from Stackdriver") - break loop - } - } - if diff := cmp.Diff(tc.expected, records, sortMetrics()); diff != "" { - t.Errorf("Unexpected stackdriver knative exports (-want +got):\n%s", diff) - } - }) - } -} - -type openCensusFake struct { - ocmetrics.UnimplementedMetricsServiceServer - address string - srv *grpc.Server - exports sync.WaitGroup - wg sync.WaitGroup - published chan *ocmetrics.ExportMetricsServiceRequest -} - -func (oc *openCensusFake) start(expectedStreams int) error { - ln, err := net.Listen("tcp", oc.address) - if err != nil { - return err - } - oc.published = make(chan *ocmetrics.ExportMetricsServiceRequest, 100) - oc.srv = grpc.NewServer() - ocmetrics.RegisterMetricsServiceServer(oc.srv, oc) - // Run the server in the background. - oc.wg.Add(1) - go func() { - oc.srv.Serve(ln) - oc.wg.Done() - oc.wg.Wait() - close(oc.published) - }() - oc.exports.Add(expectedStreams) - go oc.stop() - return nil -} - -func (oc *openCensusFake) stop() { - oc.exports.Wait() - oc.srv.Stop() -} - -func (oc *openCensusFake) Export(stream ocmetrics.MetricsService_ExportServer) error { - var streamResource *ocresource.Resource - oc.wg.Add(1) - defer oc.wg.Done() - metricSeen := false - for { - in, err := stream.Recv() - if err == io.EOF { - return nil - } - if err != nil { - return err - } - if in.Resource != nil { - // The stream is stateful, keep track of the last Resource seen. - streamResource = in.Resource - } - if len(in.Metrics) > 0 { - if in.Resource == nil { - in.Resource = streamResource - } - oc.published <- proto.Clone(in).(*ocmetrics.ExportMetricsServiceRequest) - if !metricSeen { - oc.exports.Done() - metricSeen = true - } - } - } -} - -type stackDriverFake struct { - stackdriverpb.UnimplementedMetricServiceServer - address string - srv *grpc.Server - t *testing.T - published chan *stackdriverpb.CreateTimeSeriesRequest -} - -func (sd *stackDriverFake) start() error { - sd.published = make(chan *stackdriverpb.CreateTimeSeriesRequest, 100) - ln, err := net.Listen("tcp", "localhost:0") - if err != nil { - return err - } - sd.address = ln.Addr().String() - sd.srv = grpc.NewServer() - stackdriverpb.RegisterMetricServiceServer(sd.srv, sd) - // Run the server in the background. - go func() { - sd.srv.Serve(ln) - close(sd.published) - }() - return nil -} - -func (sd *stackDriverFake) CreateTimeSeries(ctx context.Context, req *stackdriverpb.CreateTimeSeriesRequest) (*emptypb.Empty, error) { - sd.published <- req - return &emptypb.Empty{}, nil -} - -func (sd *stackDriverFake) CreateMetricDescriptor(ctx context.Context, req *stackdriverpb.CreateMetricDescriptorRequest) (*metricpb.MetricDescriptor, error) { - return req.MetricDescriptor, nil -} diff --git a/test/presubmit-tests.sh b/test/presubmit-tests.sh index af8199dd00..894161420f 100755 --- a/test/presubmit-tests.sh +++ b/test/presubmit-tests.sh @@ -28,7 +28,9 @@ source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/presubmit-tests.sh # TODO(#17): Write integration tests. -# We use the default build, unit and integration test runners. +# We use the integration test runners, but need to customize build and unit +# tests because pkg contains a number of tools and frameworks that operate at +# the build stage that need testing. function pre_build_tests() { # Test the custom code generators. This makes sure we can compile the output From 21c185f2469e207147f99c81653d1e03fbf127d7 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 6 Oct 2020 22:00:18 -0700 Subject: [PATCH 2/9] Fix gofmt --- metrics/record_e2e_test.go | 1 - metrics/resource_view_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/metrics/record_e2e_test.go b/metrics/record_e2e_test.go index d6627fdd47..0d4fa52407 100644 --- a/metrics/record_e2e_test.go +++ b/metrics/record_e2e_test.go @@ -44,7 +44,6 @@ import ( "go.opencensus.io/stats" "go.opencensus.io/stats/view" "go.opencensus.io/tag" - "k8s.io/apimachinery/pkg/util/clock" emptypb "github.com/golang/protobuf/ptypes/empty" "github.com/google/go-cmp/cmp" diff --git a/metrics/resource_view_test.go b/metrics/resource_view_test.go index 563c92adca..485d0ec2d9 100644 --- a/metrics/resource_view_test.go +++ b/metrics/resource_view_test.go @@ -28,7 +28,7 @@ import ( ) var ( - r = resource.Resource{Labels: map[string]string{"foo": "bar"}} + r = resource.Resource{Labels: map[string]string{"foo": "bar"}} ) func TestRegisterResourceView(t *testing.T) { From 4470e5c8fe9ee0bdb62b2eb85c9cbbbbacc2809c Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 12 Oct 2020 14:18:30 -0700 Subject: [PATCH 3/9] Add e2e script for pkg. --- test/e2e-tests.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 test/e2e-tests.sh diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh new file mode 100644 index 0000000000..671c8cff8e --- /dev/null +++ b/test/e2e-tests.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash + +# 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. + +# This script runs the end-to-end tests for the kn client. + +# If you already have the `KO_DOCKER_REPO` environment variable set and a +# cluster setup and currently selected in your kubeconfig, call the script +# with the `--run-tests` argument and it will use the cluster and run the tests. + +# Calling this script without arguments will create a new cluster in +# project $PROJECT_ID, start Knative serving, run the tests and delete +# the cluster. + +# If you call this script after configuring the environment variable +# $KNATIVE_SERVING_VERSION / $KNATIVE_EVENTING_VERSION with a valid release, +# e.g. 0.10.0, Knative Serving / Eventing of this specified version will be +# installed in the Kubernetes cluster, and all the tests will run against +# Knative Serving / Eventing of this specific version. + +source $(dirname $0)/common.sh + +# Add local dir to have access to built kn +export PATH=$PATH:${REPO_ROOT_DIR} + + +run() { + # Create cluster + initialize $@ + + # Integration test + eval integration_test || fail_test + + success +} + +integration_test() { + header "Running integration tests for Knative pkg" + + go_test_e2e -timeout=45m ./... || fail_test +} \ No newline at end of file From 73fede04a7c3ae74d65e36c22333000688b87afc Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 13 Oct 2020 12:33:48 -0700 Subject: [PATCH 4/9] Mark test executable --- test/e2e-tests.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 test/e2e-tests.sh diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh old mode 100644 new mode 100755 From 25cf33ed796c908a1cba18b6ad553cb37774d066 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 13 Oct 2020 15:56:56 -0700 Subject: [PATCH 5/9] Add common library for e2e tests --- test/e2e-common.sh | 518 +++++++++++++++++++++++++++++++++++++++++++++ test/e2e-tests.sh | 4 +- 2 files changed, 520 insertions(+), 2 deletions(-) create mode 100644 test/e2e-common.sh diff --git a/test/e2e-common.sh b/test/e2e-common.sh new file mode 100644 index 0000000000..e76a4fb384 --- /dev/null +++ b/test/e2e-common.sh @@ -0,0 +1,518 @@ +#!/usr/bin/env bash + +# Copyright 2018 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. + +# This script provides helper methods to perform cluster actions. +source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh +source $(dirname $0)/e2e-networking-library.sh + +CERT_MANAGER_VERSION="0.12.0" +# Since default is istio, make default ingress as istio +INGRESS_CLASS=${INGRESS_CLASS:-istio.ingress.networking.knative.dev} +ISTIO_VERSION="" +GLOO_VERSION="" +KOURIER_VERSION="" +AMBASSADOR_VERSION="" +CONTOUR_VERSION="" +CERTIFICATE_CLASS="" + +HTTPS=0 +MESH=0 +INSTALL_MONITORING=0 + +# List of custom YAMLs to install, if specified (space-separated). +INSTALL_CUSTOM_YAMLS="" + +UNINSTALL_LIST=() +TMP_DIR=$(mktemp -d -t ci-$(date +%Y-%m-%d-%H-%M-%S)-XXXXXXXXXX) +readonly TMP_DIR +readonly KNATIVE_DEFAULT_NAMESPACE="knative-serving" +# This the namespace used to install Knative Serving. Use generated UUID as namespace. +export SYSTEM_NAMESPACE +SYSTEM_NAMESPACE=$(uuidgen | tr 'A-Z' 'a-z') + + +# Keep this in sync with test/ha/ha.go +readonly REPLICAS=3 +readonly BUCKETS=10 +HA_COMPONENTS=() + + +# Parse our custom flags. +function parse_flags() { + case "$1" in + --istio-version) + [[ $2 =~ ^(stable|latest)$ ]] || abort "version format must be 'stable' or 'latest'" + readonly ISTIO_VERSION=$2 + readonly INGRESS_CLASS="istio.ingress.networking.knative.dev" + return 2 + ;; + --version) + [[ $2 =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]] || abort "version format must be 'v[0-9].[0-9].[0-9]'" + LATEST_SERVING_RELEASE_VERSION=$2 + return 2 + ;; + --cert-manager-version) + [[ $2 =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] || abort "version format must be '[0-9].[0-9].[0-9]'" + readonly CERT_MANAGER_VERSION=$2 + readonly CERTIFICATE_CLASS="cert-manager.certificate.networking.knative.dev" + return 2 + ;; + --mesh) + readonly MESH=1 + return 1 + ;; + --no-mesh) + readonly MESH=0 + return 1 + ;; + --https) + readonly HTTPS=1 + return 1 + ;; + --install-monitoring) + readonly INSTALL_MONITORING=1 + return 1 + ;; + --custom-yamls) + [[ -z "$2" ]] && fail_test "Missing argument to --custom-yamls" + # Expect a list of comma-separated YAMLs. + INSTALL_CUSTOM_YAMLS="${2//,/ }" + readonly INSTALL_CUSTOM_YAMLS + return 2 + ;; + --gloo-version) + # currently, the value of --gloo-version is ignored + # latest version of Gloo pinned in third_party will be installed + readonly GLOO_VERSION=$2 + readonly INGRESS_CLASS="gloo.ingress.networking.knative.dev" + return 2 + ;; + --kourier-version) + # currently, the value of --kourier-version is ignored + # latest version of Kourier pinned in third_party will be installed + readonly KOURIER_VERSION=$2 + readonly INGRESS_CLASS="kourier.ingress.networking.knative.dev" + return 2 + ;; + --ambassador-version) + # currently, the value of --ambassador-version is ignored + # latest version of Ambassador pinned in third_party will be installed + readonly AMBASSADOR_VERSION=$2 + readonly INGRESS_CLASS="ambassador.ingress.networking.knative.dev" + return 2 + ;; + --contour-version) + # currently, the value of --contour-version is ignored + # latest version of Contour pinned in third_party will be installed + readonly CONTOUR_VERSION=$2 + readonly INGRESS_CLASS="contour.ingress.networking.knative.dev" + return 2 + ;; + --kong-version) + # currently, the value of --kong-version is ignored + # latest version of Kong pinned in third_party will be installed + readonly KONG_VERSION=$2 + readonly INGRESS_CLASS="kong" + return 2 + ;; + --system-namespace) + [[ -z "$2" ]] || [[ $2 = --* ]] && fail_test "Missing argument to --system-namespace" + export SYSTEM_NAMESPACE=$2 + return 2 + ;; + esac + return 0 +} + +# Create all manifests required to install Knative Serving. +# This will build everything from the current source. +# All generated YAMLs will be available and pointed by the corresponding +# environment variables as set in /hack/generate-yamls.sh. +function build_knative_from_source() { + local YAML_LIST="$(mktemp)" + + # Generate manifests, capture environment variables pointing to the YAML files. + local FULL_OUTPUT="$( \ + source $(dirname $0)/../hack/generate-yamls.sh ${REPO_ROOT_DIR} ${YAML_LIST} ; \ + set | grep _YAML=/)" + local LOG_OUTPUT="$(echo "${FULL_OUTPUT}" | grep -v _YAML=/)" + local ENV_OUTPUT="$(echo "${FULL_OUTPUT}" | grep '^[_0-9A-Z]\+_YAML=/')" + [[ -z "${LOG_OUTPUT}" || -z "${ENV_OUTPUT}" ]] && fail_test "Error generating manifests" + # Only import the environment variables pointing to the YAML files. + echo "${LOG_OUTPUT}" + echo -e "Generated manifests:\n${ENV_OUTPUT}" + eval "${ENV_OUTPUT}" +} + +# Installs Knative Serving in the current cluster. +# If no parameters are passed, installs the current source-based build, unless custom +# YAML files were passed using the --custom-yamls flag. +# Parameters: $1 - Knative Serving version "HEAD" or "latest-release". Default is "HEAD". +# $2 - Knative Monitoring YAML file (optional) +function install_knative_serving() { + local version=${1:-"HEAD"} + if [[ -z "${INSTALL_CUSTOM_YAMLS}" ]]; then + install_knative_serving_standard "$version" "$2" + return + fi + echo ">> Installing Knative serving from custom YAMLs" + echo "Custom YAML files: ${INSTALL_CUSTOM_YAMLS}" + for yaml in ${INSTALL_CUSTOM_YAMLS}; do + local YAML_NAME=${TMP_DIR}/${yaml##*/} + sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${yaml} > ${YAML_NAME} + echo "Installing '${YAML_NAME}'" + kubectl create -f "${YAML_NAME}" || return 1 + done +} + +# Installs Knative Serving in the current cluster. +# If no parameters are passed, installs the current source-based build. +# Parameters: $1 - Knative Serving version "HEAD" or "latest-release". +# $2 - Knative Monitoring YAML file (optional) +function install_knative_serving_standard() { + echo ">> Creating ${SYSTEM_NAMESPACE} namespace if it does not exist" + kubectl get ns ${SYSTEM_NAMESPACE} || kubectl create namespace ${SYSTEM_NAMESPACE} + if (( MESH )); then + kubectl label namespace ${SYSTEM_NAMESPACE} istio-injection=enabled + fi + # Delete the test namespace + add_trap "kubectl delete namespace ${SYSTEM_NAMESPACE} --ignore-not-found=true" SIGKILL SIGTERM SIGQUIT + + echo ">> Installing Knative CRD" + SERVING_RELEASE_YAML="" + SERVING_POST_INSTALL_JOBS_YAML="" + if [[ "$1" == "HEAD" ]]; then + # If we need to build from source, then kick that off first. + build_knative_from_source + + echo "CRD YAML: ${SERVING_CRD_YAML}" + kubectl apply -f "${SERVING_CRD_YAML}" || return 1 + UNINSTALL_LIST+=( "${SERVING_CRD_YAML}" ) + else + # Download the latest release of Knative Serving. + local url="https://github.com/knative/serving/releases/download/${LATEST_SERVING_RELEASE_VERSION}" + + local SERVING_RELEASE_YAML=${TMP_DIR}/"serving-${LATEST_SERVING_RELEASE_VERSION}.yaml" + local SERVING_POST_INSTALL_JOBS_YAML=${TMP_DIR}/"serving-${LATEST_SERVING_RELEASE_VERSION}-post-install-jobs.yaml" + + wget "${url}/serving-crds.yaml" -O "${SERVING_RELEASE_YAML}" \ + || fail_test "Unable to download latest knative/serving CRD file." + wget "${url}/serving-core.yaml" -O ->> "${SERVING_RELEASE_YAML}" \ + || fail_test "Unable to download latest knative/serving core file." + # TODO - switch to upgrade yaml (SERVING_POST_INSTALL_JOBS_YAML) after 0.16 is released + wget "${url}/serving-storage-version-migration.yaml" -O "${SERVING_POST_INSTALL_JOBS_YAML}" \ + || fail_test "Unable to download latest knative/serving post install file." + + # Replace the default system namespace with the test's system namespace. + sed -i "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_RELEASE_YAML} + sed -i "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_POST_INSTALL_JOBS_YAML} + + echo "Knative YAML: ${SERVING_RELEASE_YAML}" + ko apply -f "${SERVING_RELEASE_YAML}" --selector=knative.dev/crd-install=true || return 1 + fi + + echo ">> Installing Ingress" + if [[ -n "${GLOO_VERSION}" ]]; then + install_gloo || return 1 + elif [[ -n "${KOURIER_VERSION}" ]]; then + install_kourier || return 1 + elif [[ -n "${AMBASSADOR_VERSION}" ]]; then + install_ambassador || return 1 + elif [[ -n "${CONTOUR_VERSION}" ]]; then + install_contour || return 1 + elif [[ -n "${KONG_VERSION}" ]]; then + install_kong || return 1 + else + if [[ "$1" == "HEAD" ]]; then + install_istio "./third_party/net-istio.yaml" || return 1 + else + # Download the latest release of net-istio. + local url="https://github.com/knative/net-istio/releases/download/${LATEST_NET_ISTIO_RELEASE_VERSION}" + local yaml="net-istio.yaml" + local YAML_NAME=${TMP_DIR}/"net-istio-${LATEST_NET_ISTIO_RELEASE_VERSION}.yaml" + wget "${url}/${yaml}" -O "${YAML_NAME}" \ + || fail_test "Unable to download latest knative/net-istio release." + echo "net-istio YAML: ${YAML_NAME}" + install_istio $YAML_NAME || return 1 + fi + fi + + echo ">> Installing Cert-Manager" + readonly INSTALL_CERT_MANAGER_YAML="./third_party/cert-manager-${CERT_MANAGER_VERSION}/cert-manager.yaml" + echo "Cert Manager YAML: ${INSTALL_CERT_MANAGER_YAML}" + kubectl apply -f "${INSTALL_CERT_MANAGER_YAML}" --validate=false || return 1 + UNINSTALL_LIST+=( "${INSTALL_CERT_MANAGER_YAML}" ) + readonly NET_CERTMANAGER_YAML="./third_party/cert-manager-${CERT_MANAGER_VERSION}/net-certmanager.yaml" + echo "net-certmanager YAML: ${NET_CERTMANAGER_YAML}" + local CERT_YAML_NAME=${TMP_DIR}/${NET_CERTMANAGER_YAML##*/} + sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${NET_CERTMANAGER_YAML} > ${CERT_YAML_NAME} + kubectl apply \ + -f "${CERT_YAML_NAME}" || return 1 + UNINSTALL_LIST+=( "${CERT_YAML_NAME}" ) + + echo ">> Installing Knative serving" + HA_COMPONENTS+=( "controller" "webhook" "autoscaler-hpa" ) + if [[ "$1" == "HEAD" ]]; then + local CORE_YAML_NAME=${TMP_DIR}/${SERVING_CORE_YAML##*/} + sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_CORE_YAML} > ${CORE_YAML_NAME} + local HPA_YAML_NAME=${TMP_DIR}/${SERVING_HPA_YAML##*/} + sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_HPA_YAML} > ${HPA_YAML_NAME} + local POST_INSTALL_JOBS_YAML_NAME=${TMP_DIR}/${SERVING_POST_INSTALL_JOBS_YAML##*/} + sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_POST_INSTALL_JOBS_YAML} > ${POST_INSTALL_JOBS_YAML_NAME} + + echo "Knative YAML: ${CORE_YAML_NAME} and ${HPA_YAML_NAME}" + kubectl apply \ + -f "${CORE_YAML_NAME}" \ + -f "${HPA_YAML_NAME}" || return 1 + UNINSTALL_LIST+=( "${CORE_YAML_NAME}" "${HPA_YAML_NAME}" ) + kubectl create -f ${POST_INSTALL_JOBS_YAML_NAME} + + if (( INSTALL_MONITORING )); then + echo ">> Installing Monitoring" + echo "Knative Monitoring YAML: ${MONITORING_YAML}" + kubectl apply -f "${MONITORING_YAML}" || return 1 + UNINSTALL_LIST+=( "${MONITORING_YAML}" ) + fi + else + echo "Knative YAML: ${SERVING_RELEASE_YAML}" + # We use ko because it has better filtering support for CRDs. + ko apply -f "${SERVING_RELEASE_YAML}" || return 1 + ko create -f "${SERVING_POST_INSTALL_JOBS_YAML}" || return 1 + UNINSTALL_LIST+=( "${SERVING_RELEASE_YAML}" ) + + if (( INSTALL_MONITORING )); then + echo ">> Installing Monitoring" + echo "Knative Monitoring YAML: ${2}" + kubectl apply -f "${2}" || return 1 + UNINSTALL_LIST+=( "${2}" ) + fi + fi + + echo ">> Configuring the default Ingress: ${INGRESS_CLASS}" + cat <> Turning on profiling.enable" + cat <> Patching activator HPA" + # We set min replicas to 15 for testing multiple activator pods. + kubectl -n ${SYSTEM_NAMESPACE} patch hpa activator --patch '{"spec":{"minReplicas":15}}' || return 1 +} + +# Check if we should use --resolvabledomain. In case the ingress only has +# hostname, we doesn't yet have a way to support resolvable domain in tests. +function use_resolvable_domain() { + # Temporarily turning off xip.io tests, as DNS errors aren't always retried. + echo "false" +} + +# Check if we should specify --ingressClass +function ingress_class() { + if [[ -z "${INGRESS_CLASS}" ]]; then + echo "" + else + echo "--ingressClass=${INGRESS_CLASS}" + fi +} + +# Check if we should specify --certificateClass +function certificate_class() { + if [[ -z "${CERTIFICATE_CLASS}" ]]; then + echo "" + else + echo "--certificateClass=${CERTIFICATE_CLASS}" + fi +} + +# Uninstalls Knative Serving from the current cluster. +function knative_teardown() { + if [[ -z "${INSTALL_CUSTOM_YAMLS}" && -z "${UNINSTALL_LIST[@]}" ]]; then + echo "install_knative_serving() was not called, nothing to uninstall" + return 0 + fi + if [[ -n "${INSTALL_CUSTOM_YAMLS}" ]]; then + echo ">> Uninstalling Knative serving from custom YAMLs" + for yaml in ${INSTALL_CUSTOM_YAMLS}; do + echo "Uninstalling '${yaml}'" + kubectl delete --ignore-not-found=true -f "${yaml}" || return 1 + done + else + echo ">> Uninstalling Knative serving" + for i in ${!UNINSTALL_LIST[@]}; do + # We uninstall elements in the reverse of the order they were installed. + local YAML="${UNINSTALL_LIST[$(( ${#array[@]} - $i ))]}" + echo ">> Bringing down YAML: ${YAML}" + kubectl delete --ignore-not-found=true -f "${YAML}" || return 1 + done + fi +} + +# Create test resources and images +function test_setup() { + echo ">> Replacing ${KNATIVE_DEFAULT_NAMESPACE} with the actual namespace for Knative Serving..." + local TEST_DIR=${TMP_DIR}/test + mkdir -p ${TEST_DIR} + cp -r test/* ${TEST_DIR} + find ${TEST_DIR} -type f -name "*.yaml" -exec sed -i "s/${KNATIVE_DEFAULT_NAMESPACE}/${SYSTEM_NAMESPACE}/g" {} + + + echo ">> Setting up logging..." + + # Install kail if needed. + if ! which kail > /dev/null; then + bash <( curl -sfL https://raw.githubusercontent.com/boz/kail/master/godownloader.sh) -b "$GOPATH/bin" + fi + + # Capture all logs. + kail > ${ARTIFACTS}/k8s.log-$(basename ${E2E_SCRIPT}).txt & + local kail_pid=$! + # Clean up kail so it doesn't interfere with job shutting down + add_trap "kill $kail_pid || true" EXIT + + echo ">> Waiting for Serving components to be running..." + wait_until_pods_running ${SYSTEM_NAMESPACE} || return 1 + + local TEST_CONFIG_DIR=${TEST_DIR}/config + echo ">> Creating test resources (${TEST_CONFIG_DIR}/)" + ko apply ${KO_FLAGS} -f ${TEST_CONFIG_DIR}/ || return 1 + if (( MESH )); then + kubectl label namespace serving-tests istio-injection=enabled + kubectl label namespace serving-tests-alt istio-injection=enabled + kubectl label namespace serving-tests-security istio-injection=enabled + ko apply ${KO_FLAGS} -f ${TEST_CONFIG_DIR}/security/ --selector=test.knative.dev/dependency=istio-sidecar || return 1 + fi + + echo ">> Uploading test images..." + ${REPO_ROOT_DIR}/test/upload-test-images.sh || return 1 + + echo ">> Waiting for Cert Manager components to be running..." + wait_until_pods_running cert-manager || return 1 + + echo ">> Waiting for Ingress provider to be running..." + wait_until_ingress_running || return 1 + + if (( INSTALL_MONITORING )); then + echo ">> Waiting for Monitoring to be running..." + wait_until_pods_running knative-monitoring || return 1 + fi +} + +# Delete test resources +function test_teardown() { + local TEST_CONFIG_DIR=${TMP_DIR}/test/config + echo ">> Removing test resources (${TEST_CONFIG_DIR}/)" + ko delete --ignore-not-found=true --now -f ${TEST_CONFIG_DIR}/ + if (( MESH )); then + ko delete --ignore-not-found=true --now -f ${TEST_CONFIG_DIR}/security/ + fi + echo ">> Ensuring test namespaces are clean" + kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests + kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests + kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests-alt + kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests-alt + kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests-security + kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests-security +} + +# Dump more information when test fails. +function dump_extra_cluster_state() { + echo ">>> Routes:" + kubectl get routes -o yaml --all-namespaces + echo ">>> Configurations:" + kubectl get configurations -o yaml --all-namespaces + echo ">>> Revisions:" + kubectl get revisions -o yaml --all-namespaces + echo ">>> PodAutoscalers:" + kubectl get podautoscalers -o yaml --all-namespaces + echo ">>> SKSs:" + kubectl get serverlessservices -o yaml --all-namespaces +} + +function wait_for_leader_controller() { + echo -n "Waiting for a leader Controller" + for i in {1..150}; do # timeout after 5 minutes + local leader=$(kubectl get lease -n "${SYSTEM_NAMESPACE}" -ojsonpath='{range .items[*].spec}{"\n"}{.holderIdentity}' | cut -d"_" -f1 | grep "^controller-" | head -1) + # Make sure the leader pod exists. + if [ -n "${leader}" ] && kubectl get pod "${leader}" -n "${SYSTEM_NAMESPACE}" >/dev/null 2>&1; then + echo -e "\nNew leader Controller has been elected" + return 0 + fi + echo -n "." + sleep 2 + done + echo -e "\n\nERROR: timeout waiting for leader controller" + return 1 +} + +function toggle_feature() { + local FEATURE="$1" + local STATE="$2" + local CONFIG="${3:-config-features}" + echo -n "Setting feature ${FEATURE} to ${STATE}" + kubectl patch cm "${CONFIG}" -n "${SYSTEM_NAMESPACE}" -p '{"data":{"'${FEATURE}'":"'${STATE}'"}}' + # We don't have a good mechanism for positive handoff so sleep :( + echo "Waiting 30s for change to get picked up." + sleep 30 +} + +function immediate_gc() { + echo -n "Setting config-gc to immediate garbage collection" + local DATA='{"data":{'` + `'"retain-since-create-time":"disabled",'` + `'"retain-since-last-active-time":"disabled",'` + `'"min-non-active-revisions":"0",'` + `'"max-non-active-revisions":"0"'` + `"}}" + kubectl patch cm "config-gc" -n "${SYSTEM_NAMESPACE}" -p "${DATA}" + echo "Waiting 30s for change to get picked up." + sleep 30 +} + +function scale_controlplane() { + for deployment in "$@"; do + # Make sure all pods run in leader-elected mode. + kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "$deployment" --replicas=0 || fail_test + # Give it time to kill the pods. + sleep 5 + # Scale up components for HA tests + kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "$deployment" --replicas="${REPLICAS}" || fail_test + done +} + +function disable_chaosduck() { + kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "chaosduck" --replicas=0 || fail_test +} + +function enable_chaosduck() { + kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "chaosduck" --replicas=1 || fail_test +} diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 671c8cff8e..dcd35946bb 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -30,7 +30,7 @@ # installed in the Kubernetes cluster, and all the tests will run against # Knative Serving / Eventing of this specific version. -source $(dirname $0)/common.sh +source $(dirname $0)/e2e-common.sh # Add local dir to have access to built kn export PATH=$PATH:${REPO_ROOT_DIR} @@ -50,4 +50,4 @@ integration_test() { header "Running integration tests for Knative pkg" go_test_e2e -timeout=45m ./... || fail_test -} \ No newline at end of file +} From 60c3c499d7f5c905ed455cbed13368c32469131a Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 14 Oct 2020 16:12:36 -0700 Subject: [PATCH 6/9] Don't need serving's common library at all --- test/e2e-common.sh | 518 --------------------------------------------- test/e2e-tests.sh | 3 - 2 files changed, 521 deletions(-) delete mode 100644 test/e2e-common.sh diff --git a/test/e2e-common.sh b/test/e2e-common.sh deleted file mode 100644 index e76a4fb384..0000000000 --- a/test/e2e-common.sh +++ /dev/null @@ -1,518 +0,0 @@ -#!/usr/bin/env bash - -# Copyright 2018 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. - -# This script provides helper methods to perform cluster actions. -source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh -source $(dirname $0)/e2e-networking-library.sh - -CERT_MANAGER_VERSION="0.12.0" -# Since default is istio, make default ingress as istio -INGRESS_CLASS=${INGRESS_CLASS:-istio.ingress.networking.knative.dev} -ISTIO_VERSION="" -GLOO_VERSION="" -KOURIER_VERSION="" -AMBASSADOR_VERSION="" -CONTOUR_VERSION="" -CERTIFICATE_CLASS="" - -HTTPS=0 -MESH=0 -INSTALL_MONITORING=0 - -# List of custom YAMLs to install, if specified (space-separated). -INSTALL_CUSTOM_YAMLS="" - -UNINSTALL_LIST=() -TMP_DIR=$(mktemp -d -t ci-$(date +%Y-%m-%d-%H-%M-%S)-XXXXXXXXXX) -readonly TMP_DIR -readonly KNATIVE_DEFAULT_NAMESPACE="knative-serving" -# This the namespace used to install Knative Serving. Use generated UUID as namespace. -export SYSTEM_NAMESPACE -SYSTEM_NAMESPACE=$(uuidgen | tr 'A-Z' 'a-z') - - -# Keep this in sync with test/ha/ha.go -readonly REPLICAS=3 -readonly BUCKETS=10 -HA_COMPONENTS=() - - -# Parse our custom flags. -function parse_flags() { - case "$1" in - --istio-version) - [[ $2 =~ ^(stable|latest)$ ]] || abort "version format must be 'stable' or 'latest'" - readonly ISTIO_VERSION=$2 - readonly INGRESS_CLASS="istio.ingress.networking.knative.dev" - return 2 - ;; - --version) - [[ $2 =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]] || abort "version format must be 'v[0-9].[0-9].[0-9]'" - LATEST_SERVING_RELEASE_VERSION=$2 - return 2 - ;; - --cert-manager-version) - [[ $2 =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] || abort "version format must be '[0-9].[0-9].[0-9]'" - readonly CERT_MANAGER_VERSION=$2 - readonly CERTIFICATE_CLASS="cert-manager.certificate.networking.knative.dev" - return 2 - ;; - --mesh) - readonly MESH=1 - return 1 - ;; - --no-mesh) - readonly MESH=0 - return 1 - ;; - --https) - readonly HTTPS=1 - return 1 - ;; - --install-monitoring) - readonly INSTALL_MONITORING=1 - return 1 - ;; - --custom-yamls) - [[ -z "$2" ]] && fail_test "Missing argument to --custom-yamls" - # Expect a list of comma-separated YAMLs. - INSTALL_CUSTOM_YAMLS="${2//,/ }" - readonly INSTALL_CUSTOM_YAMLS - return 2 - ;; - --gloo-version) - # currently, the value of --gloo-version is ignored - # latest version of Gloo pinned in third_party will be installed - readonly GLOO_VERSION=$2 - readonly INGRESS_CLASS="gloo.ingress.networking.knative.dev" - return 2 - ;; - --kourier-version) - # currently, the value of --kourier-version is ignored - # latest version of Kourier pinned in third_party will be installed - readonly KOURIER_VERSION=$2 - readonly INGRESS_CLASS="kourier.ingress.networking.knative.dev" - return 2 - ;; - --ambassador-version) - # currently, the value of --ambassador-version is ignored - # latest version of Ambassador pinned in third_party will be installed - readonly AMBASSADOR_VERSION=$2 - readonly INGRESS_CLASS="ambassador.ingress.networking.knative.dev" - return 2 - ;; - --contour-version) - # currently, the value of --contour-version is ignored - # latest version of Contour pinned in third_party will be installed - readonly CONTOUR_VERSION=$2 - readonly INGRESS_CLASS="contour.ingress.networking.knative.dev" - return 2 - ;; - --kong-version) - # currently, the value of --kong-version is ignored - # latest version of Kong pinned in third_party will be installed - readonly KONG_VERSION=$2 - readonly INGRESS_CLASS="kong" - return 2 - ;; - --system-namespace) - [[ -z "$2" ]] || [[ $2 = --* ]] && fail_test "Missing argument to --system-namespace" - export SYSTEM_NAMESPACE=$2 - return 2 - ;; - esac - return 0 -} - -# Create all manifests required to install Knative Serving. -# This will build everything from the current source. -# All generated YAMLs will be available and pointed by the corresponding -# environment variables as set in /hack/generate-yamls.sh. -function build_knative_from_source() { - local YAML_LIST="$(mktemp)" - - # Generate manifests, capture environment variables pointing to the YAML files. - local FULL_OUTPUT="$( \ - source $(dirname $0)/../hack/generate-yamls.sh ${REPO_ROOT_DIR} ${YAML_LIST} ; \ - set | grep _YAML=/)" - local LOG_OUTPUT="$(echo "${FULL_OUTPUT}" | grep -v _YAML=/)" - local ENV_OUTPUT="$(echo "${FULL_OUTPUT}" | grep '^[_0-9A-Z]\+_YAML=/')" - [[ -z "${LOG_OUTPUT}" || -z "${ENV_OUTPUT}" ]] && fail_test "Error generating manifests" - # Only import the environment variables pointing to the YAML files. - echo "${LOG_OUTPUT}" - echo -e "Generated manifests:\n${ENV_OUTPUT}" - eval "${ENV_OUTPUT}" -} - -# Installs Knative Serving in the current cluster. -# If no parameters are passed, installs the current source-based build, unless custom -# YAML files were passed using the --custom-yamls flag. -# Parameters: $1 - Knative Serving version "HEAD" or "latest-release". Default is "HEAD". -# $2 - Knative Monitoring YAML file (optional) -function install_knative_serving() { - local version=${1:-"HEAD"} - if [[ -z "${INSTALL_CUSTOM_YAMLS}" ]]; then - install_knative_serving_standard "$version" "$2" - return - fi - echo ">> Installing Knative serving from custom YAMLs" - echo "Custom YAML files: ${INSTALL_CUSTOM_YAMLS}" - for yaml in ${INSTALL_CUSTOM_YAMLS}; do - local YAML_NAME=${TMP_DIR}/${yaml##*/} - sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${yaml} > ${YAML_NAME} - echo "Installing '${YAML_NAME}'" - kubectl create -f "${YAML_NAME}" || return 1 - done -} - -# Installs Knative Serving in the current cluster. -# If no parameters are passed, installs the current source-based build. -# Parameters: $1 - Knative Serving version "HEAD" or "latest-release". -# $2 - Knative Monitoring YAML file (optional) -function install_knative_serving_standard() { - echo ">> Creating ${SYSTEM_NAMESPACE} namespace if it does not exist" - kubectl get ns ${SYSTEM_NAMESPACE} || kubectl create namespace ${SYSTEM_NAMESPACE} - if (( MESH )); then - kubectl label namespace ${SYSTEM_NAMESPACE} istio-injection=enabled - fi - # Delete the test namespace - add_trap "kubectl delete namespace ${SYSTEM_NAMESPACE} --ignore-not-found=true" SIGKILL SIGTERM SIGQUIT - - echo ">> Installing Knative CRD" - SERVING_RELEASE_YAML="" - SERVING_POST_INSTALL_JOBS_YAML="" - if [[ "$1" == "HEAD" ]]; then - # If we need to build from source, then kick that off first. - build_knative_from_source - - echo "CRD YAML: ${SERVING_CRD_YAML}" - kubectl apply -f "${SERVING_CRD_YAML}" || return 1 - UNINSTALL_LIST+=( "${SERVING_CRD_YAML}" ) - else - # Download the latest release of Knative Serving. - local url="https://github.com/knative/serving/releases/download/${LATEST_SERVING_RELEASE_VERSION}" - - local SERVING_RELEASE_YAML=${TMP_DIR}/"serving-${LATEST_SERVING_RELEASE_VERSION}.yaml" - local SERVING_POST_INSTALL_JOBS_YAML=${TMP_DIR}/"serving-${LATEST_SERVING_RELEASE_VERSION}-post-install-jobs.yaml" - - wget "${url}/serving-crds.yaml" -O "${SERVING_RELEASE_YAML}" \ - || fail_test "Unable to download latest knative/serving CRD file." - wget "${url}/serving-core.yaml" -O ->> "${SERVING_RELEASE_YAML}" \ - || fail_test "Unable to download latest knative/serving core file." - # TODO - switch to upgrade yaml (SERVING_POST_INSTALL_JOBS_YAML) after 0.16 is released - wget "${url}/serving-storage-version-migration.yaml" -O "${SERVING_POST_INSTALL_JOBS_YAML}" \ - || fail_test "Unable to download latest knative/serving post install file." - - # Replace the default system namespace with the test's system namespace. - sed -i "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_RELEASE_YAML} - sed -i "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_POST_INSTALL_JOBS_YAML} - - echo "Knative YAML: ${SERVING_RELEASE_YAML}" - ko apply -f "${SERVING_RELEASE_YAML}" --selector=knative.dev/crd-install=true || return 1 - fi - - echo ">> Installing Ingress" - if [[ -n "${GLOO_VERSION}" ]]; then - install_gloo || return 1 - elif [[ -n "${KOURIER_VERSION}" ]]; then - install_kourier || return 1 - elif [[ -n "${AMBASSADOR_VERSION}" ]]; then - install_ambassador || return 1 - elif [[ -n "${CONTOUR_VERSION}" ]]; then - install_contour || return 1 - elif [[ -n "${KONG_VERSION}" ]]; then - install_kong || return 1 - else - if [[ "$1" == "HEAD" ]]; then - install_istio "./third_party/net-istio.yaml" || return 1 - else - # Download the latest release of net-istio. - local url="https://github.com/knative/net-istio/releases/download/${LATEST_NET_ISTIO_RELEASE_VERSION}" - local yaml="net-istio.yaml" - local YAML_NAME=${TMP_DIR}/"net-istio-${LATEST_NET_ISTIO_RELEASE_VERSION}.yaml" - wget "${url}/${yaml}" -O "${YAML_NAME}" \ - || fail_test "Unable to download latest knative/net-istio release." - echo "net-istio YAML: ${YAML_NAME}" - install_istio $YAML_NAME || return 1 - fi - fi - - echo ">> Installing Cert-Manager" - readonly INSTALL_CERT_MANAGER_YAML="./third_party/cert-manager-${CERT_MANAGER_VERSION}/cert-manager.yaml" - echo "Cert Manager YAML: ${INSTALL_CERT_MANAGER_YAML}" - kubectl apply -f "${INSTALL_CERT_MANAGER_YAML}" --validate=false || return 1 - UNINSTALL_LIST+=( "${INSTALL_CERT_MANAGER_YAML}" ) - readonly NET_CERTMANAGER_YAML="./third_party/cert-manager-${CERT_MANAGER_VERSION}/net-certmanager.yaml" - echo "net-certmanager YAML: ${NET_CERTMANAGER_YAML}" - local CERT_YAML_NAME=${TMP_DIR}/${NET_CERTMANAGER_YAML##*/} - sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${NET_CERTMANAGER_YAML} > ${CERT_YAML_NAME} - kubectl apply \ - -f "${CERT_YAML_NAME}" || return 1 - UNINSTALL_LIST+=( "${CERT_YAML_NAME}" ) - - echo ">> Installing Knative serving" - HA_COMPONENTS+=( "controller" "webhook" "autoscaler-hpa" ) - if [[ "$1" == "HEAD" ]]; then - local CORE_YAML_NAME=${TMP_DIR}/${SERVING_CORE_YAML##*/} - sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_CORE_YAML} > ${CORE_YAML_NAME} - local HPA_YAML_NAME=${TMP_DIR}/${SERVING_HPA_YAML##*/} - sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_HPA_YAML} > ${HPA_YAML_NAME} - local POST_INSTALL_JOBS_YAML_NAME=${TMP_DIR}/${SERVING_POST_INSTALL_JOBS_YAML##*/} - sed "s/namespace: ${KNATIVE_DEFAULT_NAMESPACE}/namespace: ${SYSTEM_NAMESPACE}/g" ${SERVING_POST_INSTALL_JOBS_YAML} > ${POST_INSTALL_JOBS_YAML_NAME} - - echo "Knative YAML: ${CORE_YAML_NAME} and ${HPA_YAML_NAME}" - kubectl apply \ - -f "${CORE_YAML_NAME}" \ - -f "${HPA_YAML_NAME}" || return 1 - UNINSTALL_LIST+=( "${CORE_YAML_NAME}" "${HPA_YAML_NAME}" ) - kubectl create -f ${POST_INSTALL_JOBS_YAML_NAME} - - if (( INSTALL_MONITORING )); then - echo ">> Installing Monitoring" - echo "Knative Monitoring YAML: ${MONITORING_YAML}" - kubectl apply -f "${MONITORING_YAML}" || return 1 - UNINSTALL_LIST+=( "${MONITORING_YAML}" ) - fi - else - echo "Knative YAML: ${SERVING_RELEASE_YAML}" - # We use ko because it has better filtering support for CRDs. - ko apply -f "${SERVING_RELEASE_YAML}" || return 1 - ko create -f "${SERVING_POST_INSTALL_JOBS_YAML}" || return 1 - UNINSTALL_LIST+=( "${SERVING_RELEASE_YAML}" ) - - if (( INSTALL_MONITORING )); then - echo ">> Installing Monitoring" - echo "Knative Monitoring YAML: ${2}" - kubectl apply -f "${2}" || return 1 - UNINSTALL_LIST+=( "${2}" ) - fi - fi - - echo ">> Configuring the default Ingress: ${INGRESS_CLASS}" - cat <> Turning on profiling.enable" - cat <> Patching activator HPA" - # We set min replicas to 15 for testing multiple activator pods. - kubectl -n ${SYSTEM_NAMESPACE} patch hpa activator --patch '{"spec":{"minReplicas":15}}' || return 1 -} - -# Check if we should use --resolvabledomain. In case the ingress only has -# hostname, we doesn't yet have a way to support resolvable domain in tests. -function use_resolvable_domain() { - # Temporarily turning off xip.io tests, as DNS errors aren't always retried. - echo "false" -} - -# Check if we should specify --ingressClass -function ingress_class() { - if [[ -z "${INGRESS_CLASS}" ]]; then - echo "" - else - echo "--ingressClass=${INGRESS_CLASS}" - fi -} - -# Check if we should specify --certificateClass -function certificate_class() { - if [[ -z "${CERTIFICATE_CLASS}" ]]; then - echo "" - else - echo "--certificateClass=${CERTIFICATE_CLASS}" - fi -} - -# Uninstalls Knative Serving from the current cluster. -function knative_teardown() { - if [[ -z "${INSTALL_CUSTOM_YAMLS}" && -z "${UNINSTALL_LIST[@]}" ]]; then - echo "install_knative_serving() was not called, nothing to uninstall" - return 0 - fi - if [[ -n "${INSTALL_CUSTOM_YAMLS}" ]]; then - echo ">> Uninstalling Knative serving from custom YAMLs" - for yaml in ${INSTALL_CUSTOM_YAMLS}; do - echo "Uninstalling '${yaml}'" - kubectl delete --ignore-not-found=true -f "${yaml}" || return 1 - done - else - echo ">> Uninstalling Knative serving" - for i in ${!UNINSTALL_LIST[@]}; do - # We uninstall elements in the reverse of the order they were installed. - local YAML="${UNINSTALL_LIST[$(( ${#array[@]} - $i ))]}" - echo ">> Bringing down YAML: ${YAML}" - kubectl delete --ignore-not-found=true -f "${YAML}" || return 1 - done - fi -} - -# Create test resources and images -function test_setup() { - echo ">> Replacing ${KNATIVE_DEFAULT_NAMESPACE} with the actual namespace for Knative Serving..." - local TEST_DIR=${TMP_DIR}/test - mkdir -p ${TEST_DIR} - cp -r test/* ${TEST_DIR} - find ${TEST_DIR} -type f -name "*.yaml" -exec sed -i "s/${KNATIVE_DEFAULT_NAMESPACE}/${SYSTEM_NAMESPACE}/g" {} + - - echo ">> Setting up logging..." - - # Install kail if needed. - if ! which kail > /dev/null; then - bash <( curl -sfL https://raw.githubusercontent.com/boz/kail/master/godownloader.sh) -b "$GOPATH/bin" - fi - - # Capture all logs. - kail > ${ARTIFACTS}/k8s.log-$(basename ${E2E_SCRIPT}).txt & - local kail_pid=$! - # Clean up kail so it doesn't interfere with job shutting down - add_trap "kill $kail_pid || true" EXIT - - echo ">> Waiting for Serving components to be running..." - wait_until_pods_running ${SYSTEM_NAMESPACE} || return 1 - - local TEST_CONFIG_DIR=${TEST_DIR}/config - echo ">> Creating test resources (${TEST_CONFIG_DIR}/)" - ko apply ${KO_FLAGS} -f ${TEST_CONFIG_DIR}/ || return 1 - if (( MESH )); then - kubectl label namespace serving-tests istio-injection=enabled - kubectl label namespace serving-tests-alt istio-injection=enabled - kubectl label namespace serving-tests-security istio-injection=enabled - ko apply ${KO_FLAGS} -f ${TEST_CONFIG_DIR}/security/ --selector=test.knative.dev/dependency=istio-sidecar || return 1 - fi - - echo ">> Uploading test images..." - ${REPO_ROOT_DIR}/test/upload-test-images.sh || return 1 - - echo ">> Waiting for Cert Manager components to be running..." - wait_until_pods_running cert-manager || return 1 - - echo ">> Waiting for Ingress provider to be running..." - wait_until_ingress_running || return 1 - - if (( INSTALL_MONITORING )); then - echo ">> Waiting for Monitoring to be running..." - wait_until_pods_running knative-monitoring || return 1 - fi -} - -# Delete test resources -function test_teardown() { - local TEST_CONFIG_DIR=${TMP_DIR}/test/config - echo ">> Removing test resources (${TEST_CONFIG_DIR}/)" - ko delete --ignore-not-found=true --now -f ${TEST_CONFIG_DIR}/ - if (( MESH )); then - ko delete --ignore-not-found=true --now -f ${TEST_CONFIG_DIR}/security/ - fi - echo ">> Ensuring test namespaces are clean" - kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests - kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests - kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests-alt - kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests-alt - kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests-security - kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests-security -} - -# Dump more information when test fails. -function dump_extra_cluster_state() { - echo ">>> Routes:" - kubectl get routes -o yaml --all-namespaces - echo ">>> Configurations:" - kubectl get configurations -o yaml --all-namespaces - echo ">>> Revisions:" - kubectl get revisions -o yaml --all-namespaces - echo ">>> PodAutoscalers:" - kubectl get podautoscalers -o yaml --all-namespaces - echo ">>> SKSs:" - kubectl get serverlessservices -o yaml --all-namespaces -} - -function wait_for_leader_controller() { - echo -n "Waiting for a leader Controller" - for i in {1..150}; do # timeout after 5 minutes - local leader=$(kubectl get lease -n "${SYSTEM_NAMESPACE}" -ojsonpath='{range .items[*].spec}{"\n"}{.holderIdentity}' | cut -d"_" -f1 | grep "^controller-" | head -1) - # Make sure the leader pod exists. - if [ -n "${leader}" ] && kubectl get pod "${leader}" -n "${SYSTEM_NAMESPACE}" >/dev/null 2>&1; then - echo -e "\nNew leader Controller has been elected" - return 0 - fi - echo -n "." - sleep 2 - done - echo -e "\n\nERROR: timeout waiting for leader controller" - return 1 -} - -function toggle_feature() { - local FEATURE="$1" - local STATE="$2" - local CONFIG="${3:-config-features}" - echo -n "Setting feature ${FEATURE} to ${STATE}" - kubectl patch cm "${CONFIG}" -n "${SYSTEM_NAMESPACE}" -p '{"data":{"'${FEATURE}'":"'${STATE}'"}}' - # We don't have a good mechanism for positive handoff so sleep :( - echo "Waiting 30s for change to get picked up." - sleep 30 -} - -function immediate_gc() { - echo -n "Setting config-gc to immediate garbage collection" - local DATA='{"data":{'` - `'"retain-since-create-time":"disabled",'` - `'"retain-since-last-active-time":"disabled",'` - `'"min-non-active-revisions":"0",'` - `'"max-non-active-revisions":"0"'` - `"}}" - kubectl patch cm "config-gc" -n "${SYSTEM_NAMESPACE}" -p "${DATA}" - echo "Waiting 30s for change to get picked up." - sleep 30 -} - -function scale_controlplane() { - for deployment in "$@"; do - # Make sure all pods run in leader-elected mode. - kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "$deployment" --replicas=0 || fail_test - # Give it time to kill the pods. - sleep 5 - # Scale up components for HA tests - kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "$deployment" --replicas="${REPLICAS}" || fail_test - done -} - -function disable_chaosduck() { - kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "chaosduck" --replicas=0 || fail_test -} - -function enable_chaosduck() { - kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "chaosduck" --replicas=1 || fail_test -} diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index dcd35946bb..0fd2ae3456 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -30,12 +30,9 @@ # installed in the Kubernetes cluster, and all the tests will run against # Knative Serving / Eventing of this specific version. -source $(dirname $0)/e2e-common.sh - # Add local dir to have access to built kn export PATH=$PATH:${REPO_ROOT_DIR} - run() { # Create cluster initialize $@ From e7d5ebe292480327a22473d60f4d4d0e28fd7a84 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 21 Oct 2020 12:01:03 -0700 Subject: [PATCH 7/9] Address Victor's feedback. Fix one bug where prometheus wouldn't work if only the port changed. Fix timeout to work correctly in the record_e2e_test. Fix one bug in the record_e2e_test when multiple measures came in for the same metric. --- metrics/exporter.go | 4 ++ metrics/record_e2e_test.go | 111 +++++++++++++++++++------------------ metrics/resource_view.go | 3 +- test/e2e-tests.sh | 38 +++---------- 4 files changed, 71 insertions(+), 85 deletions(-) diff --git a/metrics/exporter.go b/metrics/exporter.go index a606c8ad26..4607ff5c14 100644 --- a/metrics/exporter.go +++ b/metrics/exporter.go @@ -187,6 +187,10 @@ func isNewExporterRequired(newConfig *metricsConfig) bool { return newConfig.collectorAddress != cc.collectorAddress || newConfig.requireSecure != cc.requireSecure } + if newConfig.backendDestination == prometheus { + return newConfig.prometheusPort != cc.prometheusPort + } + return newConfig.backendDestination == stackdriver && newConfig.stackdriverClientConfig != cc.stackdriverClientConfig } diff --git a/metrics/record_e2e_test.go b/metrics/record_e2e_test.go index 0d4fa52407..01c077a45d 100644 --- a/metrics/record_e2e_test.go +++ b/metrics/record_e2e_test.go @@ -40,6 +40,7 @@ import ( sd "contrib.go.opencensus.io/exporter/stackdriver" ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" + "go.opencensus.io/metric/metricproducer" "go.opencensus.io/resource" "go.opencensus.io/stats" "go.opencensus.io/stats/view" @@ -59,10 +60,12 @@ import ( ) var ( - NamespaceTagKey = tag.MustNewKey(metricskey.LabelNamespaceName) - ServiceTagKey = tag.MustNewKey(metricskey.LabelServiceName) - ConfigTagKey = tag.MustNewKey(metricskey.LabelConfigurationName) - RevisionTagKey = tag.MustNewKey(metricskey.LabelRevisionName) + namespaceTagKey = tag.MustNewKey(metricskey.LabelNamespaceName) + serviceTagKey = tag.MustNewKey(metricskey.LabelServiceName) + configTagKey = tag.MustNewKey(metricskey.LabelConfigurationName) + revisionTagKey = tag.MustNewKey(metricskey.LabelRevisionName) + + sortMetricsOption = cmp.Transformer("Sort", sortMetrics) ) type metricExtract struct { @@ -105,23 +108,21 @@ func initSdFake(sdFake *stackDriverFake) error { return nil } -func sortMetrics() cmp.Option { - return cmp.Transformer("Sort", func(in []metricExtract) []string { - out := make([]string, 0, len(in)) - seen := map[string]int{} - for _, m := range in { - // Keep only the newest report for a key - key := m.Key() - if seen[key] == 0 { - out = append(out, m.String()) - seen[key] = len(out) // Store address+1 to avoid doubling first item. - } else { - out[seen[key]-1] = m.String() - } +func sortMetrics(in []metricExtract) []string { + out := make([]string, 0, len(in)) + seen := make(map[string]int, len(in)) + for _, m := range in { + // Keep only the newest report for a key + key := m.Key() + if idx := seen[key]; idx == 0 { + out = append(out, m.String()) + seen[key] = len(out) // Store address+1 to avoid doubling first item. + } else { + out[idx-1] = m.String() } - sort.Strings(out) - return out - }) + } + sort.Strings(out) + return out } // Begin table tests for exporters @@ -146,21 +147,19 @@ func TestMetricsExport(t *testing.T) { } } - resources := []*resource.Resource{ - { - Type: "revision", - Labels: map[string]string{ - "project": "p1", - "revision": "r1", - }, + resources := []*resource.Resource{{ + Type: "revision", + Labels: map[string]string{ + "project": "p1", + "revision": "r1", }, - { - Type: "revision", - Labels: map[string]string{ - "project": "p1", - "revision": "r2", - }, + }, { + Type: "revision", + Labels: map[string]string{ + "project": "p1", + "revision": "r2", }, + }, } gauge := stats.Int64("testing/value", "Stored value", stats.UnitDimensionless) counter := stats.Int64("export counts", "Times through the export", stats.UnitDimensionless) @@ -210,7 +209,7 @@ func TestMetricsExport(t *testing.T) { if err != nil { t.Fatalf("failed to read prometheus response: %+v", err) } - want := `# HELP testComponent_global_export_counts Count of exports via standard OpenCensus view. + const want = `# HELP testComponent_global_export_counts Count of exports via standard OpenCensus view. # TYPE testComponent_global_export_counts counter testComponent_global_export_counts 2 # HELP testComponent_resource_global_export_count Count of exports via RegisterResourceView. @@ -235,17 +234,26 @@ testComponent_testing_value{project="p1",revision="r2"} 1 return UpdateExporter(context.Background(), configForBackend(openCensus), logtesting.TestLogger(t)) }, validate: func(t *testing.T) { - // We unregister the views because this is one of two ways to flush - // the internal aggregation buffers; the other is to have the - // internal reporting period duration tick, which is at least - // [new duration] in the future. - view.Unregister(globalCounter) - UnregisterResourceView(gaugeView, resourceCounter) - records := []metricExtract{} + + timeout := time.After(8 * time.Second) loop: for { select { + case <-timeout: + allMeters.lock.Lock() + t.Log("allMeters.meters: ", allMeters.meters) + for k, v := range allMeters.meters { + // t.Logf(" Exporter for %q: %+v", k, v.e) + if v.m != nil { + meter := v.m.(metricproducer.Producer) + t.Logf(" %q --> %+v", k, meter.Read()) + } + } + allMeters.lock.Unlock() + t.Error("Timeout reading input") + break loop + case record := <-ocFake.published: if record == nil { continue loop @@ -263,16 +271,13 @@ testComponent_testing_value{project="p1",revision="r2"} 1 }) } } - if len(records) >= len(expected) { + if len(sortMetrics(records)) >= len(expected) { break loop } - case <-time.After(4 * time.Second): - t.Error("Timeout reading input") - break loop } } - if diff := cmp.Diff(expected, records, sortMetrics()); diff != "" { + if diff := cmp.Diff(expected, records, sortMetricsOption); diff != "" { t.Errorf("Unexpected OpenCensus exports (-want +got):\n%s", diff) } }, @@ -303,7 +308,7 @@ testComponent_testing_value{project="p1",revision="r2"} 1 sdFake.srv.GracefulStop() } } - if diff := cmp.Diff(expected, records, sortMetrics()); diff != "" { + if diff := cmp.Diff(expected, records, sortMetricsOption); diff != "" { t.Errorf("Unexpected Stackdriver exports (-want +got):\n%s", diff) } }, @@ -430,7 +435,7 @@ func TestStackDriverExports(t *testing.T) { Description: "Number of pods that are allocated currently", Measure: actualPodCountM, Aggregation: view.LastValue(), - TagKeys: []tag.Key{NamespaceTagKey, ServiceTagKey, ConfigTagKey, RevisionTagKey}, + TagKeys: []tag.Key{namespaceTagKey, serviceTagKey, configTagKey, revisionTagKey}, } desiredPodCountM := stats.Int64( "desired_pods", @@ -466,10 +471,10 @@ func TestStackDriverExports(t *testing.T) { UnregisterResourceView(desiredPodsCountView, actualPodsCountView, customView) }) - ctx, err := tag.New(context.Background(), tag.Upsert(NamespaceTagKey, "ns"), - tag.Upsert(ServiceTagKey, "service"), - tag.Upsert(ConfigTagKey, "config"), - tag.Upsert(RevisionTagKey, "revision")) + ctx, err := tag.New(context.Background(), tag.Upsert(namespaceTagKey, "ns"), + tag.Upsert(serviceTagKey, "service"), + tag.Upsert(configTagKey, "config"), + tag.Upsert(revisionTagKey, "revision")) if err != nil { t.Fatal("Unable to create tags", err) } @@ -519,7 +524,7 @@ func TestStackDriverExports(t *testing.T) { break loop } } - if diff := cmp.Diff(tc.expected, records, sortMetrics()); diff != "" { + if diff := cmp.Diff(tc.expected, records, sortMetricsOption); diff != "" { t.Errorf("Unexpected stackdriver knative exports (-want +got):\n%s", diff) } }) diff --git a/metrics/resource_view.go b/metrics/resource_view.go index 57e54a6f00..d01e16339b 100644 --- a/metrics/resource_view.go +++ b/metrics/resource_view.go @@ -379,7 +379,8 @@ var defaultMeter = meterExporter{ } func (*defaultMeterImpl) Record(*tag.Map, interface{}, map[string]interface{}) { - // using an empty option prevents this from being called + // using an empty option prevents this from being called, which would cause a loop. + panic("Attempted to call Record() method of default Meter.") } // Find calls view.Find diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 0fd2ae3456..c7107c45a7 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -14,37 +14,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -# This script runs the end-to-end tests for the kn client. +# This script runs specific integration tests which may be large and +# not play nicely with other -# If you already have the `KO_DOCKER_REPO` environment variable set and a -# cluster setup and currently selected in your kubeconfig, call the script -# with the `--run-tests` argument and it will use the cluster and run the tests. +source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh -# Calling this script without arguments will create a new cluster in -# project $PROJECT_ID, start Knative serving, run the tests and delete -# the cluster. +e2e_test_dirs=("metrics") +for module in ${e2e_test_dirs[@]}; do + go_test_e2e -timeout=15m ./${module} +done -# If you call this script after configuring the environment variable -# $KNATIVE_SERVING_VERSION / $KNATIVE_EVENTING_VERSION with a valid release, -# e.g. 0.10.0, Knative Serving / Eventing of this specified version will be -# installed in the Kubernetes cluster, and all the tests will run against -# Knative Serving / Eventing of this specific version. - -# Add local dir to have access to built kn -export PATH=$PATH:${REPO_ROOT_DIR} - -run() { - # Create cluster - initialize $@ - - # Integration test - eval integration_test || fail_test - - success -} - -integration_test() { - header "Running integration tests for Knative pkg" - - go_test_e2e -timeout=45m ./... || fail_test -} From 211b2be58ab8bb590824fb7546caf0199af88d00 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Fri, 23 Oct 2020 09:37:37 -0700 Subject: [PATCH 8/9] Clean up the factory/exporter registration lifecycle. It's fine to Unregister(nil), but not to Register(nil). --- metrics/opencensus_exporter.go | 1 - metrics/resource_view.go | 15 ++++++++++----- metrics/resource_view_test.go | 2 ++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/metrics/opencensus_exporter.go b/metrics/opencensus_exporter.go index 403449e238..52e19a0ce6 100644 --- a/metrics/opencensus_exporter.go +++ b/metrics/opencensus_exporter.go @@ -54,7 +54,6 @@ func newOpenCensusExporter(config *metricsConfig, logger *zap.SugaredLogger) (vi return nil, nil, err } logger.Infow("Created OpenCensus exporter with config:", zap.Any("config", *config)) - view.RegisterExporter(e) return e, getFactory(e, opts), nil } diff --git a/metrics/resource_view.go b/metrics/resource_view.go index d01e16339b..8fc08ab38e 100644 --- a/metrics/resource_view.go +++ b/metrics/resource_view.go @@ -17,7 +17,6 @@ limitations under the License. package metrics import ( - "errors" "fmt" "sort" "strings" @@ -183,7 +182,9 @@ func UnregisterResourceView(views ...*view.View) { func setFactory(f ResourceExporterFactory) error { if f == nil { - return errors.New("do not setFactory(nil)") + f = func(*resource.Resource) (view.Exporter, error) { + return nil, nil + } } allMeters.lock.Lock() @@ -200,7 +201,9 @@ func setFactory(f ResourceExporterFactory) error { continue // Keep trying to clean up remaining Meters. } meter.m.UnregisterExporter(meter.e) - meter.m.RegisterExporter(e) + if e != nil { + meter.m.RegisterExporter(e) + } meter.e = e } return retErr @@ -240,6 +243,7 @@ func ClearMetersForTest() { if k == "" { continue } + meter.m.UnregisterExporter(meter.e) meter.m.Stop() delete(allMeters.meters, k) } @@ -305,8 +309,9 @@ func optionForResource(r *resource.Resource) (stats.Options, error) { if err != nil { return nil, err } - - mE.m.RegisterExporter(exporter) + if exporter != nil { + mE.m.RegisterExporter(exporter) + } mE.e = exporter return mE.o, nil } diff --git a/metrics/resource_view_test.go b/metrics/resource_view_test.go index 22922400f8..9e6f2bf23d 100644 --- a/metrics/resource_view_test.go +++ b/metrics/resource_view_test.go @@ -87,6 +87,8 @@ func TestSetFactory(t *testing.T) { resource123.Labels["id"] = "123" setFactory(fakeFactory) + defer setFactory(nil) + defer ClearMetersForTest() // Create the new meter and apply the factory _, err := optionForResource(&resource123) if err != nil { From c7d694fbce877ae93dd20ef1a901b737d27af74d Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Fri, 23 Oct 2020 09:41:25 -0700 Subject: [PATCH 9/9] Clean up isNewExporterRequired --- metrics/exporter.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/metrics/exporter.go b/metrics/exporter.go index 37602d5824..910bc9a8d2 100644 --- a/metrics/exporter.go +++ b/metrics/exporter.go @@ -181,17 +181,17 @@ func isNewExporterRequired(newConfig *metricsConfig) bool { return true } - // If the OpenCensus address has changed, restart the exporter. - // TODO(evankanderson): Should we just always restart the opencensus agent? - if newConfig.backendDestination == openCensus { + switch newConfig.backendDestination { + case openCensus: + // If the OpenCensus address has changed, restart the exporter. + // TODO(evankanderson): Should we just always restart the opencensus agent? return newConfig.collectorAddress != cc.collectorAddress || newConfig.requireSecure != cc.requireSecure - } - - if newConfig.backendDestination == prometheus { + case prometheus: return newConfig.prometheusPort != cc.prometheusPort + case stackdriver: + return newConfig.stackdriverClientConfig != cc.stackdriverClientConfig } - - return newConfig.backendDestination == stackdriver && newConfig.stackdriverClientConfig != cc.stackdriverClientConfig + return false } // newMetricsExporter gets a metrics exporter based on the config.