From 57bef4a42dcd6f17661ce5e305995ae706dce4ef Mon Sep 17 00:00:00 2001 From: Periklis Tsirakidis Date: Tue, 4 Jun 2024 10:42:49 +0200 Subject: [PATCH 1/6] Deploy ManifestWorks based on MCO-managed AddonDeploymentConfig --- deploy/resources/addondeploymentconfig.yaml | 15 ++- internal/addon/helm/values.go | 55 +++-------- internal/addon/helm/values_test.go | 17 +--- internal/addon/options.go | 101 ++++++++++++++++++++ internal/addon/var.go | 14 +-- internal/logging/handlers/handler.go | 23 +++-- internal/logging/helm_test.go | 6 +- internal/logging/manifests/logging.go | 50 +++++++--- internal/logging/manifests/logging_test.go | 20 +--- internal/logging/manifests/options.go | 9 +- internal/tracing/handlers/handler.go | 20 ++-- internal/tracing/helm_test.go | 4 +- internal/tracing/manifests/options.go | 1 + 13 files changed, 213 insertions(+), 122 deletions(-) create mode 100644 internal/addon/options.go diff --git a/deploy/resources/addondeploymentconfig.yaml b/deploy/resources/addondeploymentconfig.yaml index 90b1a358..2398994a 100644 --- a/deploy/resources/addondeploymentconfig.yaml +++ b/deploy/resources/addondeploymentconfig.yaml @@ -5,5 +5,16 @@ metadata: namespace: open-cluster-management spec: customizedVariables: - - name: loggingSubscriptionChannel - value: stable-5.9 \ No newline at end of file + # Operator Subscription Channels + - name: openshiftLoggingChannel + value: stable-5.9 + # Platform Observability + - name: platformLogsCollection + value: clusterlogforwarders.v1.logging.openshift.io + # User Workloads Observability + - name: userWorkloadLogsCollection + value: clusterlogforwarders.v1.logging.openshift.io + - name: userWorkloadTracesCollection + value: opentelemetrycollectors.v1beta1.opentelemetry.io + - name: userWorkloadInstrumentation + value: instrumentations.v1alpha1.opentelemetry.io diff --git a/internal/addon/helm/values.go b/internal/addon/helm/values.go index fbf3f73d..00af279c 100644 --- a/internal/addon/helm/values.go +++ b/internal/addon/helm/values.go @@ -2,14 +2,12 @@ package helm import ( "context" - "strconv" "github.com/rhobs/multicluster-observability-addon/internal/addon" lhandlers "github.com/rhobs/multicluster-observability-addon/internal/logging/handlers" lmanifests "github.com/rhobs/multicluster-observability-addon/internal/logging/manifests" thandlers "github.com/rhobs/multicluster-observability-addon/internal/tracing/handlers" tmanifests "github.com/rhobs/multicluster-observability-addon/internal/tracing/manifests" - "k8s.io/klog/v2" "open-cluster-management.io/addon-framework/pkg/addonfactory" addonutils "open-cluster-management.io/addon-framework/pkg/utils" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" @@ -25,36 +23,35 @@ type HelmChartValues struct { Tracing tmanifests.TracingValues `json:"tracing"` } -type Options struct { - LoggingDisabled bool - TracingDisabled bool -} - func GetValuesFunc(ctx context.Context, k8s client.Client) addonfactory.GetValuesFunc { return func( cluster *clusterv1.ManagedCluster, - addon *addonapiv1alpha1.ManagedClusterAddOn, + mcAddon *addonapiv1alpha1.ManagedClusterAddOn, ) (addonfactory.Values, error) { // if hub cluster, then don't install anything if isHubCluster(cluster) { return addonfactory.JsonStructToValues(HelmChartValues{}) } - aodc, err := getAddOnDeploymentConfig(ctx, k8s, addon) + aodc, err := getAddOnDeploymentConfig(ctx, k8s, mcAddon) if err != nil { return nil, err } - opts, err := buildOptions(aodc) + opts, err := addon.BuildOptions(aodc) if err != nil { return nil, err } + if !opts.Platform.Enabled && !opts.UserWorkloads.Enabled { + return addonfactory.JsonStructToValues(HelmChartValues{}) + } + userValues := HelmChartValues{ Enabled: true, } - if !opts.LoggingDisabled { - loggingOpts, err := lhandlers.BuildOptions(ctx, k8s, addon, aodc) + if opts.Platform.Logs.CollectionEnabled || opts.UserWorkloads.Logs.CollectionEnabled { + loggingOpts, err := lhandlers.BuildOptions(ctx, k8s, mcAddon, opts.Platform.Logs, opts.Platform.Logs) if err != nil { return nil, err } @@ -66,9 +63,8 @@ func GetValuesFunc(ctx context.Context, k8s client.Client) addonfactory.GetValue userValues.Logging = *logging } - if !opts.TracingDisabled { - klog.Info("Tracing enabled") - tracingOpts, err := thandlers.BuildOptions(ctx, k8s, addon, aodc) + if opts.UserWorkloads.Traces.CollectionEnabled { + tracingOpts, err := thandlers.BuildOptions(ctx, k8s, mcAddon, opts.UserWorkloads.Traces) if err != nil { return nil, err } @@ -94,35 +90,6 @@ func getAddOnDeploymentConfig(ctx context.Context, k8s client.Client, mcAddon *a return addOnDeployment, nil } -func buildOptions(addOnDeployment *addonapiv1alpha1.AddOnDeploymentConfig) (Options, error) { - var opts Options - if addOnDeployment == nil { - return opts, nil - } - - if addOnDeployment.Spec.CustomizedVariables == nil { - return opts, nil - } - - for _, keyvalue := range addOnDeployment.Spec.CustomizedVariables { - switch keyvalue.Name { - case addon.AdcLoggingDisabledKey: - value, err := strconv.ParseBool(keyvalue.Value) - if err != nil { - return opts, err - } - opts.LoggingDisabled = value - case addon.AdcTracingisabledKey: - value, err := strconv.ParseBool(keyvalue.Value) - if err != nil { - return opts, err - } - opts.TracingDisabled = value - } - } - return opts, nil -} - func isHubCluster(cluster *clusterv1.ManagedCluster) bool { val, ok := cluster.Labels[annotationLocalCluster] if !ok { diff --git a/internal/addon/helm/values_test.go b/internal/addon/helm/values_test.go index 7cc898c3..f198d0c8 100644 --- a/internal/addon/helm/values_test.go +++ b/internal/addon/helm/values_test.go @@ -59,16 +59,7 @@ func Test_Mcoa_Disable_Charts(t *testing.T) { Namespace: "open-cluster-management", }, Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ - CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ - { - Name: "loggingDisabled", - Value: "true", - }, - { - Name: "tracingDisabled", - Value: "true", - }, - }, + CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{}, }, } @@ -77,7 +68,7 @@ func Test_Mcoa_Disable_Charts(t *testing.T) { WithObjects(addOnDeploymentConfig). Build() - loggingAgentAddon, err := addonfactory.NewAgentAddonFactory(addon.Name, addon.FS, addon.McoaChartDir). + agentAddon, err := addonfactory.NewAgentAddonFactory(addon.Name, addon.FS, addon.McoaChartDir). WithGetValuesFuncs(GetValuesFunc(context.TODO(), fakeKubeClient)). WithAgentRegistrationOption(&agent.RegistrationOption{}). WithScheme(scheme.Scheme). @@ -86,9 +77,9 @@ func Test_Mcoa_Disable_Charts(t *testing.T) { klog.Fatalf("failed to build agent %v", err) } - objects, err := loggingAgentAddon.Manifests(managedCluster, managedClusterAddOn) + objects, err := agentAddon.Manifests(managedCluster, managedClusterAddOn) require.NoError(t, err) - require.Equal(t, 2, len(objects)) + require.Empty(t, objects) } func Test_Mcoa_Disable_Chart_Hub(t *testing.T) { diff --git a/internal/addon/options.go b/internal/addon/options.go new file mode 100644 index 00000000..26d04a14 --- /dev/null +++ b/internal/addon/options.go @@ -0,0 +1,101 @@ +package addon + +import ( + addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" +) + +const ( + // Operator Subscription Channels + KeyOpenShiftLoggingChannel = "openshiftLoggingChannel" + + // Platform Observability Keys + KeyPlatformLogsCollection = "platformLogsCollection" + + // User Workloads Observability Keys + KeyUserWorkloadLogsCollection = "userWorkloadLogsCollection" + KeyUserWorkloadTracesCollection = "userWorkloadTracesCollection" + KeyUserWorkloadTracesInstrumentation = "userWorkloadTracesInstrumentation" +) + +type CollectionKind string + +const ( + ClusterLogForwarderV1 CollectionKind = "clusterlogforwarders.v1.logging.openshift.io" + OpenTelemetryCollectorV1beta1 CollectionKind = "opentelemetrycollectors.v1beta1.opentelemetry.io" +) + +type InstrumentationKind string + +const ( + InstrumentationV1alpha1 InstrumentationKind = "instrumentations.v1alpha1.opentelemetry.io" +) + +type LogsOptions struct { + CollectionEnabled bool + SubscriptionChannel string +} + +type TracesOptions struct { + CollectionEnabled bool + InstrumentationEnabled bool + SubscriptionChannel string +} + +type PlatformOptions struct { + Enabled bool + Logs LogsOptions +} + +type UserWorkloadOptions struct { + Enabled bool + Logs LogsOptions + Traces TracesOptions +} + +type Options struct { + Platform PlatformOptions + UserWorkloads UserWorkloadOptions +} + +func BuildOptions(addOnDeployment *addonapiv1alpha1.AddOnDeploymentConfig) (Options, error) { + var opts Options + if addOnDeployment == nil { + return opts, nil + } + + if addOnDeployment.Spec.CustomizedVariables == nil { + return opts, nil + } + + for _, keyvalue := range addOnDeployment.Spec.CustomizedVariables { + switch keyvalue.Name { + // Operator Subscriptions + case KeyOpenShiftLoggingChannel: + opts.Platform.Logs.SubscriptionChannel = keyvalue.Value + opts.UserWorkloads.Logs.SubscriptionChannel = keyvalue.Value + // Platform Observability Options + case KeyPlatformLogsCollection: + if keyvalue.Value == string(ClusterLogForwarderV1) { + opts.Platform.Enabled = true + opts.Platform.Logs.CollectionEnabled = true + } + // User Workload Observability Options + case KeyUserWorkloadLogsCollection: + if keyvalue.Value == string(ClusterLogForwarderV1) { + opts.UserWorkloads.Enabled = true + opts.UserWorkloads.Logs.CollectionEnabled = true + } + case KeyUserWorkloadTracesCollection: + if keyvalue.Value == string(OpenTelemetryCollectorV1beta1) { + opts.UserWorkloads.Enabled = true + opts.UserWorkloads.Traces.CollectionEnabled = true + } + case KeyUserWorkloadTracesInstrumentation: + if keyvalue.Value == string(InstrumentationV1alpha1) { + opts.UserWorkloads.Enabled = true + opts.UserWorkloads.Traces.InstrumentationEnabled = true + } + } + } + return opts, nil +} diff --git a/internal/addon/var.go b/internal/addon/var.go index 01f91999..c59b635b 100644 --- a/internal/addon/var.go +++ b/internal/addon/var.go @@ -13,15 +13,11 @@ const ( TracingChartDir = "manifests/charts/mcoa/charts/tracing" AddonDeploymentConfigResource = "addondeploymentconfigs" - - AdcLoggingDisabledKey = "loggingDisabled" - AdcTracingisabledKey = "tracingDisabled" - - ClusterLogForwardersResource = "clusterlogforwarders" - SpokeCLFName = "mcoa-instance" - SpokeCLFNamespace = "openshift-logging" - clfProbeKey = "isReady" - clfProbePath = ".status.conditions[?(@.type==\"Ready\")].status" + ClusterLogForwardersResource = "clusterlogforwarders" + SpokeCLFName = "mcoa-instance" + SpokeCLFNamespace = "openshift-logging" + clfProbeKey = "isReady" + clfProbePath = ".status.conditions[?(@.type==\"Ready\")].status" OpenTelemetryCollectorsResource = "opentelemetrycollectors" SpokeOTELColName = "mcoa-instance" diff --git a/internal/logging/handlers/handler.go b/internal/logging/handlers/handler.go index 437ad2bf..e50aa8df 100644 --- a/internal/logging/handlers/handler.go +++ b/internal/logging/handlers/handler.go @@ -10,17 +10,24 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alpha1.ManagedClusterAddOn, adoc *addonapiv1alpha1.AddOnDeploymentConfig) (manifests.Options, error) { - resources := manifests.Options{ - AddOnDeploymentConfig: adoc, +func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alpha1.ManagedClusterAddOn, platform, userWorkloads addon.LogsOptions) (manifests.Options, error) { + opts := manifests.Options{ + Platform: platform, + UserWorkloads: userWorkloads, + } + + if platform.SubscriptionChannel != "" { + opts.SubscriptionChannel = platform.SubscriptionChannel + } else { + opts.SubscriptionChannel = userWorkloads.SubscriptionChannel } key := addon.GetObjectKey(mcAddon.Status.ConfigReferences, loggingv1.GroupVersion.Group, addon.ClusterLogForwardersResource) clf := &loggingv1.ClusterLogForwarder{} if err := k8s.Get(ctx, key, clf, &client.GetOptions{}); err != nil { - return resources, err + return opts, err } - resources.ClusterLogForwarder = clf + opts.ClusterLogForwarder = clf targetSecretName := make(map[addon.Endpoint]string) for _, output := range clf.Spec.Outputs { @@ -29,9 +36,9 @@ func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alp targetSecrets, err := addon.GetSecrets(ctx, k8s, clf.Namespace, mcAddon.Namespace, targetSecretName) if err != nil { - return resources, err + return opts, err } - resources.Secrets = targetSecrets + opts.Secrets = targetSecrets - return resources, nil + return opts, nil } diff --git a/internal/logging/helm_test.go b/internal/logging/helm_test.go index 4c894aa6..89ab23fe 100644 --- a/internal/logging/helm_test.go +++ b/internal/logging/helm_test.go @@ -36,10 +36,10 @@ var ( func fakeGetValues(k8s client.Client) addonfactory.GetValuesFunc { return func( - cluster *clusterv1.ManagedCluster, - addon *addonapiv1alpha1.ManagedClusterAddOn, + _ *clusterv1.ManagedCluster, + mcAddon *addonapiv1alpha1.ManagedClusterAddOn, ) (addonfactory.Values, error) { - opts, err := handlers.BuildOptions(context.TODO(), k8s, addon, nil) + opts, err := handlers.BuildOptions(context.TODO(), k8s, mcAddon, addon.LogsOptions{}, addon.LogsOptions{}) if err != nil { return nil, err } diff --git a/internal/logging/manifests/logging.go b/internal/logging/manifests/logging.go index d3525d47..cbfda374 100644 --- a/internal/logging/manifests/logging.go +++ b/internal/logging/manifests/logging.go @@ -2,22 +2,21 @@ package manifests import ( "encoding/json" + "errors" "slices" loggingv1 "github.com/openshift/cluster-logging-operator/apis/logging/v1" "github.com/rhobs/multicluster-observability-addon/internal/addon" ) -func buildSubscriptionChannel(resources Options) string { - adoc := resources.AddOnDeploymentConfig - if adoc == nil || len(adoc.Spec.CustomizedVariables) == 0 { - return defaultLoggingVersion - } +var ( + errPlatformLogsNotDefined = errors.New("Platform logs not defined") + errUserWorkloadLogsNotDefined = errors.New("User Workloads logs not defined") +) - for _, keyvalue := range adoc.Spec.CustomizedVariables { - if keyvalue.Name == subscriptionChannelValueKey { - return keyvalue.Value - } +func buildSubscriptionChannel(resources Options) string { + if resources.SubscriptionChannel != "" { + return resources.SubscriptionChannel } return defaultLoggingVersion } @@ -46,9 +45,38 @@ func buildSecrets(resources Options) ([]SecretValue, error) { return secretsValue, nil } -func buildClusterLogForwarderSpec(resources Options) (*loggingv1.ClusterLogForwarderSpec, error) { - clf := resources.ClusterLogForwarder +func buildClusterLogForwarderSpec(opts Options) (*loggingv1.ClusterLogForwarderSpec, error) { + clf := opts.ClusterLogForwarder clf.Spec.ServiceAccountName = "mcoa-logcollector" + // Validate Platform Logs enabled + var ( + platformDetected bool + userWorkloadsDetected bool + ) + for _, pipeline := range clf.Spec.Pipelines { + // Consider pipelines without outputs invalid + if pipeline.OutputRefs == nil { + continue + } + + for _, ref := range pipeline.InputRefs { + if ref == loggingv1.InputNameInfrastructure || ref == loggingv1.InputNameAudit { + platformDetected = true + } + if ref == loggingv1.InputNameApplication { + userWorkloadsDetected = true + } + } + } + + if opts.Platform.CollectionEnabled && !platformDetected { + return nil, errPlatformLogsNotDefined + } + + if opts.UserWorkloads.CollectionEnabled && !userWorkloadsDetected { + return nil, errUserWorkloadLogsNotDefined + } + return &clf.Spec, nil } diff --git a/internal/logging/manifests/logging_test.go b/internal/logging/manifests/logging_test.go index fbed2641..0d5b0ba0 100644 --- a/internal/logging/manifests/logging_test.go +++ b/internal/logging/manifests/logging_test.go @@ -22,31 +22,19 @@ func Test_BuildSubscriptionChannel(t *testing.T) { subChannel string }{ { - name: "unknown key", - key: "test", - value: "stable-1.0", + name: "not set", subChannel: "stable-5.9", }, { - name: "known key", - key: "loggingSubscriptionChannel", + name: "user set", value: "stable-5.7", subChannel: "stable-5.7", }, } { + tc := tc t.Run(tc.name, func(t *testing.T) { - adoc := &addonapiv1alpha1.AddOnDeploymentConfig{ - Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ - CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ - { - Name: tc.key, - Value: tc.value, - }, - }, - }, - } resources := Options{ - AddOnDeploymentConfig: adoc, + SubscriptionChannel: tc.value, } subChannel := buildSubscriptionChannel(resources) require.Equal(t, tc.subChannel, subChannel) diff --git a/internal/logging/manifests/options.go b/internal/logging/manifests/options.go index d9793180..8b91c24b 100644 --- a/internal/logging/manifests/options.go +++ b/internal/logging/manifests/options.go @@ -4,11 +4,12 @@ import ( loggingv1 "github.com/openshift/cluster-logging-operator/apis/logging/v1" "github.com/rhobs/multicluster-observability-addon/internal/addon" corev1 "k8s.io/api/core/v1" - addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" ) type Options struct { - Secrets map[addon.Endpoint]corev1.Secret - ClusterLogForwarder *loggingv1.ClusterLogForwarder - AddOnDeploymentConfig *addonapiv1alpha1.AddOnDeploymentConfig + Secrets map[addon.Endpoint]corev1.Secret + ClusterLogForwarder *loggingv1.ClusterLogForwarder + Platform addon.LogsOptions + UserWorkloads addon.LogsOptions + SubscriptionChannel string } diff --git a/internal/tracing/handlers/handler.go b/internal/tracing/handlers/handler.go index 5cd68b02..43929c99 100644 --- a/internal/tracing/handlers/handler.go +++ b/internal/tracing/handlers/handler.go @@ -20,33 +20,33 @@ var ( errNoVolumeMountForSecret = fmt.Errorf("no volumemount found for secret") ) -func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alpha1.ManagedClusterAddOn, adoc *addonapiv1alpha1.AddOnDeploymentConfig) (manifests.Options, error) { - resources := manifests.Options{ - AddOnDeploymentConfig: adoc, - ClusterName: mcAddon.Namespace, +func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alpha1.ManagedClusterAddOn, userWorkloads addon.TracesOptions) (manifests.Options, error) { + opts := manifests.Options{ + ClusterName: mcAddon.Namespace, + UserWorkloads: userWorkloads, } klog.Info("Retrieving OpenTelemetry Collector template") key := addon.GetObjectKey(mcAddon.Status.ConfigReferences, otelv1beta1.GroupVersion.Group, addon.OpenTelemetryCollectorsResource) otelCol := &otelv1beta1.OpenTelemetryCollector{} if err := k8s.Get(ctx, key, otelCol, &client.GetOptions{}); err != nil { - return resources, err + return opts, err } - resources.OpenTelemetryCollector = otelCol + opts.OpenTelemetryCollector = otelCol klog.Info("OpenTelemetry Collector template found") targetSecretName, err := buildExportersSecrets(otelCol) if err != nil { - return resources, nil + return opts, nil } targetSecrets, err := addon.GetSecrets(ctx, k8s, otelCol.Namespace, mcAddon.Namespace, targetSecretName) if err != nil { - return resources, err + return opts, err } - resources.Secrets = targetSecrets + opts.Secrets = targetSecrets - return resources, nil + return opts, nil } func buildExportersSecrets(otelCol *otelv1beta1.OpenTelemetryCollector) (map[addon.Endpoint]string, error) { diff --git a/internal/tracing/helm_test.go b/internal/tracing/helm_test.go index 41661394..20cad8b8 100644 --- a/internal/tracing/helm_test.go +++ b/internal/tracing/helm_test.go @@ -34,9 +34,9 @@ var ( func fakeGetValues(k8s client.Client) addonfactory.GetValuesFunc { return func( cluster *clusterv1.ManagedCluster, - addon *addonapiv1alpha1.ManagedClusterAddOn, + mcAddon *addonapiv1alpha1.ManagedClusterAddOn, ) (addonfactory.Values, error) { - opts, err := handlers.BuildOptions(context.TODO(), k8s, addon, nil) + opts, err := handlers.BuildOptions(context.TODO(), k8s, mcAddon, addon.TracesOptions{}) if err != nil { return nil, err } diff --git a/internal/tracing/manifests/options.go b/internal/tracing/manifests/options.go index b2738c7a..806ff2dc 100644 --- a/internal/tracing/manifests/options.go +++ b/internal/tracing/manifests/options.go @@ -13,4 +13,5 @@ type Options struct { ConfigMaps []corev1.ConfigMap OpenTelemetryCollector *otelv1beta1.OpenTelemetryCollector AddOnDeploymentConfig *addonapiv1alpha1.AddOnDeploymentConfig + UserWorkloads addon.TracesOptions } From 8ff828924967041bfa57d5a3bdcc62472611a8e3 Mon Sep 17 00:00:00 2001 From: Periklis Tsirakidis Date: Tue, 4 Jun 2024 11:07:31 +0200 Subject: [PATCH 2/6] Change addon namespace to open-cluster-management-observability --- CONTRIBUTING.md | 39 ++++++++++++++----- .../templates/clusterlogforwarder.yaml | 4 +- .../templates/logging-auth-configmap.yaml | 4 +- .../tracing/templates/otel-collector.yaml | 2 +- .../templates/tracing-auth-configmap.yaml | 4 +- .../templates/managed-cluster-addon.yaml | 6 +-- .../templates/logging-static-auth.yaml | 4 +- deploy/kustomization.yaml | 2 +- deploy/resources/addondeploymentconfig.yaml | 2 +- .../resources/cluster-management-addon.yaml | 6 +-- .../templates/aws-secret-default.yaml | 2 +- .../addon-install/templates/clf-instance.yaml | 2 +- .../templates/instance-default.yaml | 4 +- .../templates/otelcol-instance.yaml | 2 +- internal/addon/helm/values_test.go | 17 ++------ internal/addon/var.go | 2 +- internal/logging/helm_test.go | 10 ++--- internal/logging/manifests/logging_test.go | 4 +- internal/tracing/helm_test.go | 12 +++--- 19 files changed, 70 insertions(+), 58 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc10d0f6..2a6a7b57 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -66,28 +66,40 @@ Then every time you want to test a new version, you can just: ```shell make oci # Delete the mcoa pod which will make the Deployment pull the new image -oc -n open-cluster-management delete pod -l app=multicluster-observability-addon-manager +oc -n open-cluster-management-observability delete pod -l app=multicluster-observability-addon-manager ``` -### Disabeling specific signals +### Enable specific Observability Capabilities -The addon supports disabling signals using the resource `AddOnDeploymentConfig`. For instance, to disable the logging signal create the following resource on the hub cluster: +The addon supports enabling observability capabilities using the resource `AddOnDeploymentConfig`. For instance, to enable platform and user workloads logging/tracing/instrumentation create the following resource on the hub cluster: ```yaml apiVersion: addon.open-cluster-management.io/v1alpha1 kind: AddOnDeploymentConfig metadata: name: multicluster-observability-addon - namespace: open-cluster-management + namespace: open-cluster-management-observability spec: customizedVariables: - - name: loggingDisabled - value: "true" + # Platform Observability + - name: platformLogsCollection + value: clusterlogforwarders.v1.logging.openshift.io + # User Workloads Observability + - name: userWorkloadLogsCollection + value: clusterlogforwarders.v1.logging.openshift.io + - name: userWorkloadTracesCollection + value: opentelemetrycollectors.v1alpha1.opentelemetry.io + - name: userWorkloadInstrumentation + value: instrumentations.v1alpha1.opentelemetry.io ``` -Supported keys are `metricsDisabled`, `loggingDisabled` and `tracingDisabled` +Supported keys are: +- `platformLogsCollection`: Supports values `clusterlogforwarders.v1.logging.openshift.io` +- `userWorkloadLogsCollection`: Supports values `clusterlogforwarders.v1.logging.openshift.io` +- `userWorkloadTracesCollection`: Supports values `opentelemetrycollectors.v1alpha1.opentelemetry.io` +- `userWorkloadTracesInstrumentation`: Supports values `instrumentations.v1alpha1.opentelemetry.io` -## Install the addon on a Spoke Cluster +__Note__: Keys can hold multiple values separated by semicolon, e.g. `clusterlogforwarders.v1.logging.openshift.io;opentelemetrycollectors.v1alpha1.opentelemetry.io`. The addon installation is managed by the addon-manager. This means that users don't need to explicetelly create resources to install the addon on spoke @@ -118,7 +130,16 @@ This MCOA supports all outputs defined in [OpenShift Documentation](https://docs Note: the service account used by the `ClusterLogForwarder` deployed by MCOA is `openshift-logging/mcoa-logcollector`, this information is esential when using the AWS STS authentication. -### Configuring Traces +### Configuring User Workloads Observability Capabilities + +#### Logs Collection + +Currently the addon supports configuration to send logs either to: + +- CloudWatch: requires the auth configmap to be specified +- Loki: requires the auth configmap, the url configmap and optionally the inject ca configmap + +### Traces Collection & Instrumentation Currently MCOA supports deploying a single instance of `OpenTelemetryCollector` templated with the stanza created in the hub cluster. The instance deployed in diff --git a/demo/addon-config/charts/logging/templates/clusterlogforwarder.yaml b/demo/addon-config/charts/logging/templates/clusterlogforwarder.yaml index 0ea0932c..14f1396c 100644 --- a/demo/addon-config/charts/logging/templates/clusterlogforwarder.yaml +++ b/demo/addon-config/charts/logging/templates/clusterlogforwarder.yaml @@ -2,7 +2,7 @@ apiVersion: logging.openshift.io/v1 kind: ClusterLogForwarder metadata: name: instance - namespace: open-cluster-management + namespace: open-cluster-management-observability spec: outputs: {{- range $_, $dic := .Values.outputs }} @@ -38,4 +38,4 @@ spec: {{- end }} outputRefs: - {{ $dic.name }} -{{- end }} \ No newline at end of file +{{- end }} diff --git a/demo/addon-config/charts/logging/templates/logging-auth-configmap.yaml b/demo/addon-config/charts/logging/templates/logging-auth-configmap.yaml index 1d897a51..315ccd2f 100644 --- a/demo/addon-config/charts/logging/templates/logging-auth-configmap.yaml +++ b/demo/addon-config/charts/logging/templates/logging-auth-configmap.yaml @@ -3,7 +3,7 @@ apiVersion: v1 kind: ConfigMap metadata: name: logging-auth - namespace: open-cluster-management + namespace: open-cluster-management-observability labels: mcoa.openshift.io/signal: logging data: @@ -15,4 +15,4 @@ data: {{- end }} {{- end }} --- -{{- end }} \ No newline at end of file +{{- end }} diff --git a/demo/addon-config/charts/tracing/templates/otel-collector.yaml b/demo/addon-config/charts/tracing/templates/otel-collector.yaml index 92a70029..f0f93d1d 100644 --- a/demo/addon-config/charts/tracing/templates/otel-collector.yaml +++ b/demo/addon-config/charts/tracing/templates/otel-collector.yaml @@ -2,7 +2,7 @@ apiVersion: opentelemetry.io/v1alpha1 kind: OpenTelemetryCollector metadata: name: spoke-otelcol - namespace: open-cluster-management + namespace: open-cluster-management-observability spec: config: | receivers: diff --git a/demo/addon-config/charts/tracing/templates/tracing-auth-configmap.yaml b/demo/addon-config/charts/tracing/templates/tracing-auth-configmap.yaml index 266d0a63..9e2d9d8a 100644 --- a/demo/addon-config/charts/tracing/templates/tracing-auth-configmap.yaml +++ b/demo/addon-config/charts/tracing/templates/tracing-auth-configmap.yaml @@ -3,9 +3,9 @@ apiVersion: v1 kind: ConfigMap metadata: name: tracing-auth - namespace: open-cluster-management + namespace: open-cluster-management-observability labels: mcoa.openshift.io/signal: tracing data: otlp: mTLS -{{- end }} \ No newline at end of file +{{- end }} diff --git a/demo/addon-install/templates/managed-cluster-addon.yaml b/demo/addon-install/templates/managed-cluster-addon.yaml index 83b17508..ca8e6d7d 100644 --- a/demo/addon-install/templates/managed-cluster-addon.yaml +++ b/demo/addon-install/templates/managed-cluster-addon.yaml @@ -11,7 +11,7 @@ spec: # Logging Auth ConfigMap - resource: configmaps name: logging-auth - namespace: open-cluster-management + namespace: open-cluster-management-observability # Logging URLs for Loki ConfigMap {{- range $_, $dic := $.Values.logging.outputs }} {{- if eq $dic.type "loki" }} @@ -38,10 +38,10 @@ spec: # Tracing Auth ConfigMap - resource: configmaps name: tracing-auth - namespace: open-cluster-management + namespace: open-cluster-management-observability # Tracing ca-bundle configmap - resource: secrets name: otel-gateway namespace: observability {{- end }} -{{- end }} \ No newline at end of file +{{- end }} diff --git a/demo/mcoa-demo/templates/logging-static-auth.yaml b/demo/mcoa-demo/templates/logging-static-auth.yaml index 873b3e61..47cfe38f 100644 --- a/demo/mcoa-demo/templates/logging-static-auth.yaml +++ b/demo/mcoa-demo/templates/logging-static-auth.yaml @@ -3,8 +3,8 @@ apiVersion: v1 kind: Secret metadata: name: static-authentication - namespace: open-cluster-management + namespace: open-cluster-management-observability data: aws_access_key_id: {{ .Values.logging.aws.keyID | b64enc }} aws_secret_access_key: {{ .Values.logging.aws.keySecret | b64enc }} -{{- end }} \ No newline at end of file +{{- end }} diff --git a/deploy/kustomization.yaml b/deploy/kustomization.yaml index ed0049a0..10b78851 100644 --- a/deploy/kustomization.yaml +++ b/deploy/kustomization.yaml @@ -3,7 +3,7 @@ images: newName: quay.io/rhobs/multicluster-observability-addon newTag: v0.0.1 -namespace: open-cluster-management +namespace: open-cluster-management-observability resources: - resources/cluster_role_binding.yaml diff --git a/deploy/resources/addondeploymentconfig.yaml b/deploy/resources/addondeploymentconfig.yaml index 2398994a..6f87645f 100644 --- a/deploy/resources/addondeploymentconfig.yaml +++ b/deploy/resources/addondeploymentconfig.yaml @@ -2,7 +2,7 @@ apiVersion: addon.open-cluster-management.io/v1alpha1 kind: AddOnDeploymentConfig metadata: name: multicluster-observability-addon - namespace: open-cluster-management + namespace: open-cluster-management-observability spec: customizedVariables: # Operator Subscription Channels diff --git a/deploy/resources/cluster-management-addon.yaml b/deploy/resources/cluster-management-addon.yaml index 9703da35..6da07c7f 100644 --- a/deploy/resources/cluster-management-addon.yaml +++ b/deploy/resources/cluster-management-addon.yaml @@ -16,7 +16,7 @@ spec: resource: addondeploymentconfigs defaultConfig: name: multicluster-observability-addon - namespace: open-cluster-management + namespace: open-cluster-management-observability # Describes the default log forwarding outputs for each log type applied to all managed clusters. - group: logging.openshift.io resource: clusterlogforwarders @@ -32,8 +32,8 @@ spec: - group: logging.openshift.io resource: clusterlogforwarders name: instance - namespace: open-cluster-management + namespace: open-cluster-management-observability - group: opentelemetry.io resource: opentelemetrycollectors name: instance - namespace: open-cluster-management + namespace: open-cluster-management-observability diff --git a/hack/addon-install/templates/aws-secret-default.yaml b/hack/addon-install/templates/aws-secret-default.yaml index 8acee2a0..13b36ed7 100644 --- a/hack/addon-install/templates/aws-secret-default.yaml +++ b/hack/addon-install/templates/aws-secret-default.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: Secret metadata: name: aws-credentials-default - namespace: open-cluster-management + namespace: open-cluster-management-observability type: Opaque data: aws_access_key_id: {{ .Values.awsCredentials.accessKeyID | b64enc }} diff --git a/hack/addon-install/templates/clf-instance.yaml b/hack/addon-install/templates/clf-instance.yaml index 378b250b..d125439d 100644 --- a/hack/addon-install/templates/clf-instance.yaml +++ b/hack/addon-install/templates/clf-instance.yaml @@ -2,7 +2,7 @@ apiVersion: logging.openshift.io/v1 kind: ClusterLogForwarder metadata: name: instance - namespace: open-cluster-management + namespace: open-cluster-management-observability spec: outputs: - cloudwatch: diff --git a/hack/addon-install/templates/instance-default.yaml b/hack/addon-install/templates/instance-default.yaml index 348bf904..29cbc9f9 100644 --- a/hack/addon-install/templates/instance-default.yaml +++ b/hack/addon-install/templates/instance-default.yaml @@ -2,7 +2,7 @@ apiVersion: logging.openshift.io/v1 kind: ClusterLogForwarder metadata: name: instance-default - namespace: open-cluster-management + namespace: open-cluster-management-observability spec: outputs: - cloudwatch: @@ -17,4 +17,4 @@ spec: inputRefs: - infrastructure outputRefs: - - cw \ No newline at end of file + - cw diff --git a/hack/addon-install/templates/otelcol-instance.yaml b/hack/addon-install/templates/otelcol-instance.yaml index 2b42bd2f..7b8a8353 100644 --- a/hack/addon-install/templates/otelcol-instance.yaml +++ b/hack/addon-install/templates/otelcol-instance.yaml @@ -2,7 +2,7 @@ apiVersion: opentelemetry.io/v1beta1 kind: OpenTelemetryCollector metadata: name: instance - namespace: open-cluster-management + namespace: open-cluster-management-observability spec: config: exporters: diff --git a/internal/addon/helm/values_test.go b/internal/addon/helm/values_test.go index f198d0c8..013ff20b 100644 --- a/internal/addon/helm/values_test.go +++ b/internal/addon/helm/values_test.go @@ -47,7 +47,7 @@ func Test_Mcoa_Disable_Charts(t *testing.T) { Resource: "addondeploymentconfigs", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", Name: "multicluster-observability-addon", }, }, @@ -56,7 +56,7 @@ func Test_Mcoa_Disable_Charts(t *testing.T) { addOnDeploymentConfig = &addonapiv1alpha1.AddOnDeploymentConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "multicluster-observability-addon", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{}, @@ -111,19 +111,10 @@ func Test_Mcoa_Disable_Chart_Hub(t *testing.T) { addOnDeploymentConfig = &addonapiv1alpha1.AddOnDeploymentConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "multicluster-observability-addon", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ - CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ - { - Name: "loggingDisabled", - Value: "true", - }, - { - Name: "tracingDisabled", - Value: "true", - }, - }, + CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{}, }, } diff --git a/internal/addon/var.go b/internal/addon/var.go index c59b635b..8c5b5e62 100644 --- a/internal/addon/var.go +++ b/internal/addon/var.go @@ -6,7 +6,7 @@ import ( const ( Name = "multicluster-observability-addon" - InstallNamespace = "open-cluster-management" + InstallNamespace = "open-cluster-management-observability" McoaChartDir = "manifests/charts/mcoa" LoggingChartDir = "manifests/charts/mcoa/charts/logging" diff --git a/internal/logging/helm_test.go b/internal/logging/helm_test.go index 89ab23fe..6c621900 100644 --- a/internal/logging/helm_test.go +++ b/internal/logging/helm_test.go @@ -81,7 +81,7 @@ func Test_Logging_AllConfigsTogether_AllResources(t *testing.T) { Resource: "addondeploymentconfigs", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", Name: "multicluster-observability-addon", }, }, @@ -91,7 +91,7 @@ func Test_Logging_AllConfigsTogether_AllResources(t *testing.T) { Resource: "clusterlogforwarders", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", Name: "mcoa-instance", }, }, @@ -101,7 +101,7 @@ func Test_Logging_AllConfigsTogether_AllResources(t *testing.T) { clf = &loggingv1.ClusterLogForwarder{ ObjectMeta: metav1.ObjectMeta{ Name: "mcoa-instance", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Spec: loggingv1.ClusterLogForwarderSpec{ Inputs: []loggingv1.InputSpec{ @@ -162,7 +162,7 @@ func Test_Logging_AllConfigsTogether_AllResources(t *testing.T) { staticCred = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "static-authentication", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Data: map[string][]byte{ "key": []byte("data"), @@ -173,7 +173,7 @@ func Test_Logging_AllConfigsTogether_AllResources(t *testing.T) { addOnDeploymentConfig = &addonapiv1alpha1.AddOnDeploymentConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "multicluster-observability-addon", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ diff --git a/internal/logging/manifests/logging_test.go b/internal/logging/manifests/logging_test.go index 0d5b0ba0..25e9f062 100644 --- a/internal/logging/manifests/logging_test.go +++ b/internal/logging/manifests/logging_test.go @@ -101,7 +101,7 @@ func Test_BuildCLFSpec(t *testing.T) { Resource: "clusterlogforwarders", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", Name: "mcoa-instance", }, }, @@ -111,7 +111,7 @@ func Test_BuildCLFSpec(t *testing.T) { clf = &loggingv1.ClusterLogForwarder{ ObjectMeta: metav1.ObjectMeta{ Name: "mcoa-instance", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Spec: loggingv1.ClusterLogForwarderSpec{ Inputs: []loggingv1.InputSpec{ diff --git a/internal/tracing/helm_test.go b/internal/tracing/helm_test.go index 20cad8b8..4077682f 100644 --- a/internal/tracing/helm_test.go +++ b/internal/tracing/helm_test.go @@ -77,7 +77,7 @@ func Test_Tracing_AllConfigsTogether_AllResources(t *testing.T) { Resource: "configmaps", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", Name: "tracing-auth", }, }, @@ -89,7 +89,7 @@ func Test_Tracing_AllConfigsTogether_AllResources(t *testing.T) { Resource: "addondeploymentconfigs", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", Name: "multicluster-observability-addon", }, }, @@ -99,7 +99,7 @@ func Test_Tracing_AllConfigsTogether_AllResources(t *testing.T) { Resource: "opentelemetrycollectors", }, ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", Name: "mcoa-instance", }, }, @@ -109,7 +109,7 @@ func Test_Tracing_AllConfigsTogether_AllResources(t *testing.T) { otelCol := otelv1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Name: "mcoa-instance", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Spec: otelv1beta1.OpenTelemetryCollectorSpec{ Config: otelv1beta1.Config{ @@ -130,7 +130,7 @@ func Test_Tracing_AllConfigsTogether_AllResources(t *testing.T) { addOnDeploymentConfig = &addonapiv1alpha1.AddOnDeploymentConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "multicluster-observability-addon", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{}, } @@ -138,7 +138,7 @@ func Test_Tracing_AllConfigsTogether_AllResources(t *testing.T) { authCM = &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "tracing-auth", - Namespace: "open-cluster-management", + Namespace: "open-cluster-management-observability", }, Data: map[string]string{ "otlphttp": "mTLS", From a2a30df0dc6851cfa57cfcd389bc74a751eaed02 Mon Sep 17 00:00:00 2001 From: Periklis Tsirakidis Date: Tue, 4 Jun 2024 20:05:22 +0200 Subject: [PATCH 3/6] Fix typo --- internal/addon/options.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/addon/options.go b/internal/addon/options.go index 26d04a14..e63b5d2a 100644 --- a/internal/addon/options.go +++ b/internal/addon/options.go @@ -12,9 +12,9 @@ const ( KeyPlatformLogsCollection = "platformLogsCollection" // User Workloads Observability Keys - KeyUserWorkloadLogsCollection = "userWorkloadLogsCollection" - KeyUserWorkloadTracesCollection = "userWorkloadTracesCollection" - KeyUserWorkloadTracesInstrumentation = "userWorkloadTracesInstrumentation" + KeyUserWorkloadLogsCollection = "userWorkloadLogsCollection" + KeyUserWorkloadTracesCollection = "userWorkloadTracesCollection" + KeyUserWorkloadInstrumentation = "userWorkloadInstrumentation" ) type CollectionKind string @@ -90,7 +90,7 @@ func BuildOptions(addOnDeployment *addonapiv1alpha1.AddOnDeploymentConfig) (Opti opts.UserWorkloads.Enabled = true opts.UserWorkloads.Traces.CollectionEnabled = true } - case KeyUserWorkloadTracesInstrumentation: + case KeyUserWorkloadInstrumentation: if keyvalue.Value == string(InstrumentationV1alpha1) { opts.UserWorkloads.Enabled = true opts.UserWorkloads.Traces.InstrumentationEnabled = true From f0b4815b1c75cfc9c04e54c0e89a71fb53c6da2e Mon Sep 17 00:00:00 2001 From: Periklis Tsirakidis Date: Thu, 6 Jun 2024 19:38:31 +0200 Subject: [PATCH 4/6] Address code review suggestions --- CONTRIBUTING.md | 2 +- internal/addon/helm/values.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a6a7b57..b44caa50 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -139,7 +139,7 @@ Currently the addon supports configuration to send logs either to: - CloudWatch: requires the auth configmap to be specified - Loki: requires the auth configmap, the url configmap and optionally the inject ca configmap -### Traces Collection & Instrumentation +### Traces Collection Currently MCOA supports deploying a single instance of `OpenTelemetryCollector` templated with the stanza created in the hub cluster. The instance deployed in diff --git a/internal/addon/helm/values.go b/internal/addon/helm/values.go index 00af279c..76bc1cc8 100644 --- a/internal/addon/helm/values.go +++ b/internal/addon/helm/values.go @@ -51,7 +51,7 @@ func GetValuesFunc(ctx context.Context, k8s client.Client) addonfactory.GetValue } if opts.Platform.Logs.CollectionEnabled || opts.UserWorkloads.Logs.CollectionEnabled { - loggingOpts, err := lhandlers.BuildOptions(ctx, k8s, mcAddon, opts.Platform.Logs, opts.Platform.Logs) + loggingOpts, err := lhandlers.BuildOptions(ctx, k8s, mcAddon, opts.Platform.Logs, opts.UserWorkloads.Logs) if err != nil { return nil, err } From 618426a51f507fd2a98cfd6890f3e813ae377236 Mon Sep 17 00:00:00 2001 From: Periklis Tsirakidis Date: Mon, 10 Jun 2024 15:36:09 +0200 Subject: [PATCH 5/6] Address code review suggestions --- CONTRIBUTING.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b44caa50..e4202e7a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -99,7 +99,20 @@ Supported keys are: - `userWorkloadTracesCollection`: Supports values `opentelemetrycollectors.v1alpha1.opentelemetry.io` - `userWorkloadTracesInstrumentation`: Supports values `instrumentations.v1alpha1.opentelemetry.io` -__Note__: Keys can hold multiple values separated by semicolon, e.g. `clusterlogforwarders.v1.logging.openshift.io;opentelemetrycollectors.v1alpha1.opentelemetry.io`. +__Note__: Some keys can hold multiple values separated by semicolon to support multiple data collection capabilities in parallel, e.g: + +```yaml +apiVersion: addon.open-cluster-management.io/v1alpha1 +kind: AddOnDeploymentConfig +metadata: + name: multicluster-observability-addon + namespace: open-cluster-management-observability +spec: + customizedVariables: + # User Workloads Observability with multiple collectors + - name: userWorkloadLogsCollection + value: clusterlogforwarders.v1.logging.openshift.io;opentelemetrycollectors.v1alpha1.opentelemetry.io +``` The addon installation is managed by the addon-manager. This means that users don't need to explicetelly create resources to install the addon on spoke From 861354e1fbfcbae60ff034e14dcb5157f5207f38 Mon Sep 17 00:00:00 2001 From: Periklis Tsirakidis Date: Tue, 11 Jun 2024 13:20:43 +0200 Subject: [PATCH 6/6] Apply code review suggestions --- CONTRIBUTING.md | 15 +++----------- internal/addon/helm/values_test.go | 11 ++++++---- internal/logging/manifests/logging.go | 30 ++++++++++++++++++++++++++- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e4202e7a..e1c100c4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -88,7 +88,7 @@ spec: - name: userWorkloadLogsCollection value: clusterlogforwarders.v1.logging.openshift.io - name: userWorkloadTracesCollection - value: opentelemetrycollectors.v1alpha1.opentelemetry.io + value: opentelemetrycollectors.v1beta1.opentelemetry.io - name: userWorkloadInstrumentation value: instrumentations.v1alpha1.opentelemetry.io ``` @@ -96,7 +96,7 @@ spec: Supported keys are: - `platformLogsCollection`: Supports values `clusterlogforwarders.v1.logging.openshift.io` - `userWorkloadLogsCollection`: Supports values `clusterlogforwarders.v1.logging.openshift.io` -- `userWorkloadTracesCollection`: Supports values `opentelemetrycollectors.v1alpha1.opentelemetry.io` +- `userWorkloadTracesCollection`: Supports values `opentelemetrycollectors.v1beta1.opentelemetry.io` - `userWorkloadTracesInstrumentation`: Supports values `instrumentations.v1alpha1.opentelemetry.io` __Note__: Some keys can hold multiple values separated by semicolon to support multiple data collection capabilities in parallel, e.g: @@ -111,7 +111,7 @@ spec: customizedVariables: # User Workloads Observability with multiple collectors - name: userWorkloadLogsCollection - value: clusterlogforwarders.v1.logging.openshift.io;opentelemetrycollectors.v1alpha1.opentelemetry.io + value: clusterlogforwarders.v1.logging.openshift.io;opentelemetrycollectors.v1beta1.opentelemetry.io ``` The addon installation is managed by the addon-manager. This means that users @@ -143,15 +143,6 @@ This MCOA supports all outputs defined in [OpenShift Documentation](https://docs Note: the service account used by the `ClusterLogForwarder` deployed by MCOA is `openshift-logging/mcoa-logcollector`, this information is esential when using the AWS STS authentication. -### Configuring User Workloads Observability Capabilities - -#### Logs Collection - -Currently the addon supports configuration to send logs either to: - -- CloudWatch: requires the auth configmap to be specified -- Loki: requires the auth configmap, the url configmap and optionally the inject ca configmap - ### Traces Collection Currently MCOA supports deploying a single instance of `OpenTelemetryCollector` diff --git a/internal/addon/helm/values_test.go b/internal/addon/helm/values_test.go index 013ff20b..f1d62bef 100644 --- a/internal/addon/helm/values_test.go +++ b/internal/addon/helm/values_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - loggingapis "github.com/openshift/cluster-logging-operator/apis" operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -23,7 +22,6 @@ import ( ) var ( - _ = loggingapis.AddToScheme(scheme.Scheme) _ = operatorsv1.AddToScheme(scheme.Scheme) _ = operatorsv1alpha1.AddToScheme(scheme.Scheme) _ = addonapiv1alpha1.AddToScheme(scheme.Scheme) @@ -114,7 +112,12 @@ func Test_Mcoa_Disable_Chart_Hub(t *testing.T) { Namespace: "open-cluster-management-observability", }, Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ - CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{}, + CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ + { + Name: addon.KeyPlatformLogsCollection, + Value: string(addon.ClusterLogForwarderV1), + }, + }, }, } @@ -134,5 +137,5 @@ func Test_Mcoa_Disable_Chart_Hub(t *testing.T) { objects, err := loggingAgentAddon.Manifests(managedCluster, managedClusterAddOn) require.NoError(t, err) - require.Equal(t, 0, len(objects)) + require.Empty(t, objects) } diff --git a/internal/logging/manifests/logging.go b/internal/logging/manifests/logging.go index cbfda374..cfa52ee0 100644 --- a/internal/logging/manifests/logging.go +++ b/internal/logging/manifests/logging.go @@ -51,16 +51,44 @@ func buildClusterLogForwarderSpec(opts Options) (*loggingv1.ClusterLogForwarderS // Validate Platform Logs enabled var ( - platformDetected bool + platformInputRefs []string + platformDetected bool + + userWorkloadInputRefs []string userWorkloadsDetected bool ) + + for _, input := range clf.Spec.Inputs { + if input.Application != nil { + userWorkloadInputRefs = append(userWorkloadInputRefs, input.Name) + } + if input.Infrastructure != nil || input.Audit != nil { + platformInputRefs = append(platformInputRefs, input.Name) + } + } + for _, pipeline := range clf.Spec.Pipelines { // Consider pipelines without outputs invalid if pipeline.OutputRefs == nil { continue } + outer: for _, ref := range pipeline.InputRefs { + for _, input := range platformInputRefs { + if input == ref { + platformDetected = true + continue outer + } + } + + for _, input := range userWorkloadInputRefs { + if input == ref { + userWorkloadsDetected = true + continue outer + } + } + if ref == loggingv1.InputNameInfrastructure || ref == loggingv1.InputNameAudit { platformDetected = true }