From cb143125a02367a1542f5d365b3976657f3c94b0 Mon Sep 17 00:00:00 2001 From: Szilard Parrag Date: Wed, 24 Jan 2024 09:05:29 +0100 Subject: [PATCH 01/18] feat(ci): add check-diff target Signed-off-by: Szilard Parrag --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 4f705a6..ef06ff7 100644 --- a/Makefile +++ b/Makefile @@ -190,3 +190,7 @@ tidy: ## Tidy Go modules .PHONY: e2e-test e2e-test: ## Run e2e tests, make sure subscription operator is running somewhere cd e2e && timeout --foreground 15m ./e2e_test.sh || (echo "E2E test failed"; exit 1) + +.PHONY: check-diff +check-diff: generate + git diff --exit-code From de1dc1c349500daa9c82de8e08b9055882160032 Mon Sep 17 00:00:00 2001 From: Szilard Parrag Date: Wed, 24 Jan 2024 09:05:52 +0100 Subject: [PATCH 02/18] feat(ci): add basic gh actions ci workflow Signed-off-by: Szilard Parrag --- .github/workflows/ci.yaml | 44 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 .github/workflows/ci.yaml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..14de3ea --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,44 @@ +name: CI + +on: + push: + branches: main + pull_request: + +jobs: + test: + name: Test + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version: '1.21' + + - name: Check diff + run: make check-diff + + - name: Test + run: make test + + lint: + name: Lint + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version: '1.21' + + - name: Lint + run: make lint + env: + LINTER_FLAGS: '--timeout 5m' \ No newline at end of file From c8108f968ca8cb3f5126eb520ffe7f2cb7344c51 Mon Sep 17 00:00:00 2001 From: Szilard Parrag Date: Wed, 24 Jan 2024 09:11:11 +0100 Subject: [PATCH 03/18] fix(ci): make lint-fix results Signed-off-by: Szilard Parrag --- Makefile | 2 +- api/telemetry/v1alpha1/otlp_config.go | 2 +- internal/controller/telemetry/otel_conf_gen.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index ef06ff7..91ff0a6 100644 --- a/Makefile +++ b/Makefile @@ -65,7 +65,7 @@ test: manifests generate fmt vet envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test -v ./... -coverprofile cover.out GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint -GOLANGCI_LINT_VERSION ?= v1.54.2 +GOLANGCI_LINT_VERSION ?= v1.55.2 golangci-lint: @[ -f $(GOLANGCI_LINT) ] || { \ set -e ;\ diff --git a/api/telemetry/v1alpha1/otlp_config.go b/api/telemetry/v1alpha1/otlp_config.go index 0e2fab4..f9a93bf 100644 --- a/api/telemetry/v1alpha1/otlp_config.go +++ b/api/telemetry/v1alpha1/otlp_config.go @@ -95,7 +95,7 @@ type GRPCClientSettings struct { Authority string `json:"authority,omitempty"` // Auth configuration for outgoing RPCs. - Auth string `json:"auth,omitempty"` //TODO this is a refernece *configauth.Authentication + Auth string `json:"auth,omitempty"` //TODO this is a reference *configauth.Authentication } // TLSClientSetting contains TLS configurations that are specific to client diff --git a/internal/controller/telemetry/otel_conf_gen.go b/internal/controller/telemetry/otel_conf_gen.go index 8650ecc..d5694d3 100644 --- a/internal/controller/telemetry/otel_conf_gen.go +++ b/internal/controller/telemetry/otel_conf_gen.go @@ -23,7 +23,7 @@ import ( "gopkg.in/yaml.v3" ) -// TODO move this to its appropiate place +// TODO move this to its appropriate place type OtelColConfigInput struct { Tenants []v1alpha1.Tenant Subscriptions []v1alpha1.Subscription From 4b11b6da59e27c836ac44d8ca5dd2e2be1e364e9 Mon Sep 17 00:00:00 2001 From: Szilard Parrag Date: Wed, 24 Jan 2024 09:23:30 +0100 Subject: [PATCH 04/18] fix(lint): trivial lint fixes Signed-off-by: Szilard Parrag --- .../telemetry/collector_controller.go | 44 ++++++++++++------- .../controller/telemetry/otel_conf_gen.go | 7 ++- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/internal/controller/telemetry/collector_controller.go b/internal/controller/telemetry/collector_controller.go index 0e5bdd3..1651d24 100644 --- a/internal/controller/telemetry/collector_controller.go +++ b/internal/controller/telemetry/collector_controller.go @@ -76,7 +76,10 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( collector.Status.Tenants = tenantNames - r.Status().Update(ctx, collector) + if err := r.Status().Update(ctx, collector); err != nil { + return ctrl.Result{}, err + } + logger.Info("Setting collector status") subscriptions := []v1alpha1.Subscription{} @@ -112,7 +115,10 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( slices.Sort(logsourceNamespacesForTenant) tenant.Status.LogSourceNamespaces = logsourceNamespacesForTenant - r.Status().Update(ctx, &tenant) + if err := r.Status().Update(ctx, &tenant); err != nil { + return ctrl.Result{}, err + } + logger.Info("Setting tenant status") } @@ -190,7 +196,9 @@ func (r *CollectorReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( }, } - ctrl.SetControllerReference(collector, &otelCollector, r.Scheme) + if err := ctrl.SetControllerReference(collector, &otelCollector, r.Scheme); err != nil { + return ctrl.Result{}, err + } resourceReconciler := reconciler.NewReconcilerWith(r.Client, reconciler.WithLog(logger)) @@ -235,7 +243,9 @@ func (r *CollectorReconciler) reconcileServiceAccount(ctx context.Context, colle }, } - ctrl.SetControllerReference(collector, &serviceAccount, r.Scheme) + if err := ctrl.SetControllerReference(collector, &serviceAccount, r.Scheme); err != nil { + return v1alpha1.NamespacedName{}, err + } resourceReconciler := reconciler.NewReconcilerWith(r.Client, reconciler.WithLog(logger)) @@ -265,7 +275,9 @@ func (r *CollectorReconciler) reconcileClusterRoleBinding(ctx context.Context, c }, } - ctrl.SetControllerReference(collector, &clusterRoleBinding, r.Scheme) + if err := ctrl.SetControllerReference(collector, &clusterRoleBinding, r.Scheme); err != nil { + return err + } resourceReconciler := reconciler.NewReconcilerWith(r.Client, reconciler.WithLog(logger)) @@ -295,7 +307,9 @@ func (r *CollectorReconciler) reconcileClusterRole(ctx context.Context, collecto }, } - ctrl.SetControllerReference(collector, &clusterRole, r.Scheme) + if err := ctrl.SetControllerReference(collector, &clusterRole, r.Scheme); err != nil { + return err + } resourceReconciler := reconciler.NewReconcilerWith(r.Client, reconciler.WithLog(logger)) @@ -305,18 +319,18 @@ func (r *CollectorReconciler) reconcileClusterRole(ctx context.Context, collecto } func getTenantNamesFromTenants(tenants []v1alpha1.Tenant) []string { - var tenantNames []string - for _, tenant := range tenants { - tenantNames = append(tenantNames, tenant.Name) + tenantNames := make([]string, len(tenants)) + for i, tenant := range tenants { + tenantNames[i] = tenant.Name } return tenantNames } func getSubscriptionNamesFromSubscription(subscriptions []v1alpha1.Subscription) []v1alpha1.NamespacedName { - var subscriptionNames []v1alpha1.NamespacedName - for _, subscription := range subscriptions { - subscriptionNames = append(subscriptionNames, subscription.NamespacedName()) + subscriptionNames := make([]v1alpha1.NamespacedName, len(subscriptions)) + for i, subscription := range subscriptions { + subscriptionNames[i] = subscription.NamespacedName() } return subscriptionNames @@ -434,10 +448,10 @@ func (r *CollectorReconciler) getLogsourceNamespaceNamesForTenant(ctx context.Co return nil, err } - var namespaceNames []string + namespaceNames := make([]string, len(namespaces)) - for _, namespace := range namespaces { - namespaceNames = append(namespaceNames, namespace.Name) + for i, namespace := range namespaces { + namespaceNames[i] = namespace.Name } return namespaceNames, nil diff --git a/internal/controller/telemetry/otel_conf_gen.go b/internal/controller/telemetry/otel_conf_gen.go index d5694d3..2ed08aa 100644 --- a/internal/controller/telemetry/otel_conf_gen.go +++ b/internal/controller/telemetry/otel_conf_gen.go @@ -130,11 +130,10 @@ func newRoutingConnector(name string, defaultPipelines []string) RoutingConnecto } func buildRoutingTableItemForTenant(tenant v1alpha1.Tenant) RoutingConnectorTableItem { + conditions := make([]string, len(tenant.Status.LogSourceNamespaces)) - var conditions []string - - for _, namespace := range tenant.Status.LogSourceNamespaces { - conditions = append(conditions, fmt.Sprintf(`IsMatch(attributes["k8s.namespace.name"], %q)`, namespace)) + for i, namespace := range tenant.Status.LogSourceNamespaces { + conditions[i] = fmt.Sprintf(`IsMatch(attributes["k8s.namespace.name"], %q)`, namespace) } conditionString := strings.Join(conditions, " or ") From 5c634b92c20de0120f3e89533d3f6cc683f87576 Mon Sep 17 00:00:00 2001 From: Szilard Parrag Date: Wed, 24 Jan 2024 13:58:55 +0100 Subject: [PATCH 05/18] fix(test): followup on unit tests Signed-off-by: Szilard Parrag --- .../otel_col_conf_test_fixtures/complex.yaml | 261 +++++----- .../telemetry/otel_conf_gen_test.go | 465 +++++++++++------- 2 files changed, 404 insertions(+), 322 deletions(-) diff --git a/internal/controller/telemetry/otel_col_conf_test_fixtures/complex.yaml b/internal/controller/telemetry/otel_col_conf_test_fixtures/complex.yaml index 3d476a9..df9ee8c 100644 --- a/internal/controller/telemetry/otel_col_conf_test_fixtures/complex.yaml +++ b/internal/controller/telemetry/otel_col_conf_test_fixtures/complex.yaml @@ -1,143 +1,128 @@ receivers: - file/in: - path: - /dev/stdin - + filelog/kubernetes: + exclude: + - /var/log/pods/*/otc-container/*.log + include: + - /var/log/pods/*/*/*.log + include_file_name: false + include_file_path: true + operators: + - id: get-format + routes: + - expr: body matches "^\\{" + output: parser-docker + - expr: body matches "^[^ Z]+Z" + output: parser-containerd + type: router + - id: parser-containerd + output: extract_metadata_from_filepath + regex: ^(?P