Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-2.12] Add support for mcoa-managed observability capabilities #1470

Merged
merged 28 commits into from
Aug 6, 2024

Conversation

periklis
Copy link
Contributor

@periklis periklis commented Jun 5, 2024

The following PR adds support to reconcile further observability capabilities via the multicluster-observability-addon. The present implementation adds:

  1. Support of the addon as separate MCO operand.
  2. Extends the MultiClusterObservability CRD spec by a new section named capabilities as discussed here (TODO: Add link to DDR here).
  3. For logs data collection it adds support for platform and user workloads.
  4. For tracing data collection/instrumentation it adds support for user workloads.

Special notes for your reviewer:
To the test the following PR one needs to publish an image of the referenced dependency PR and applying the following instructions:

  1. Install the Advanced Cluster Management for Kubernetes from the OperatorHub.
  2. Create a MultiClusterHub resource.
  3. Set the MultiClusterHub resource to pause reconciliation via:
    kubectl -n open-cluster-management annotate mch multiclusterhub --overwrite mch-pause=true
  4. Build and publish the MCO operator image from this branch:
make docker-build-local IMAGE_TAG_BASE=quay.io/$YOUR_ORG/multicluster-observability-operator

docker push quay.io/$YOUR_ORG/multicluster-observability-operator:latest
  1. Edit the image for the multicluster-observability-operator deployment
    kubectl -n open-cluster-management edit deployment multicluster-observability-operator
  2. Update the ClusterRole to add support for create/update/delete/patch for AddonDeploymentConfig resources:
    kubectl edit clusterrole open-cluster-management:multicluster-observability-operator:multicluster-observability-operator
  3. Update the CRD using the one in this branch to add support for the capabilities section.
  4. Finally create an MultiClusterObservability custom resource as the following:
apiVersion: observability.open-cluster-management.io/v1beta2
kind: MultiClusterObservability
metadata:
  annotations:
    mco-multicluster_observability_addon-image: quay.io/YOUR_ORG_HERE/multicluster-observability-addon:v0.0.1
  name: observability
spec:
  capabilities:
    platform:
      collection:
        logs:
          enabled: true
    userWorkloads:
      logs:
        collection:
          clusterLogForwarder:
            enabled: true
      traces:
        collection:
          instrumentation:
            enabled: true
          openTelemetryCollector:
            enabled: false
  observabilityAddonSpec: {}
  storageConfig:
    metricObjectStorage:
      name: thanos-object-storage
      key: thanos.yaml

Dependencies:

Copy link

openshift-ci bot commented Jun 5, 2024

Hi @periklis. Thanks for your PR.

I'm waiting for a stolostron member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Contributor

@JoaoBraveCoding JoaoBraveCoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good! I'm just concerned with how we will package the CRDs since without them users will not be able to create stanzas unless they install the operators

periklis and others added 12 commits August 1, 2024 13:32
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
…r-observability-addon/deployment.yaml

Co-authored-by: Joao Marcal <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Signed-off-by: Periklis Tsirakidis <[email protected]>
Copy link

sonarqubecloud bot commented Aug 1, 2024

@periklis
Copy link
Contributor Author

periklis commented Aug 1, 2024

/test test-e2e

Copy link
Contributor

@philipgough philipgough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Copy link

openshift-ci bot commented Aug 6, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: periklis, philipgough

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [periklis,philipgough]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit f347ca9 into stolostron:main Aug 6, 2024
16 checks passed
JoaoBraveCoding added a commit to JoaoBraveCoding/multicluster-observability-operator that referenced this pull request Aug 27, 2024
ClusterManagementAddOn

In
stolostron#1470
we added support for deploying and configuring MCOA with the
capabilities field. However if we enabled a field in capabilities and
then disable it MCO would not reconcile the AddOnDeploymentConfig
correctly due to DeepDerivative, that would return true instead of
false.
JoaoBraveCoding added a commit to JoaoBraveCoding/multicluster-observability-operator that referenced this pull request Aug 28, 2024
ClusterManagementAddOn

In
stolostron#1470
we added support for deploying and configuring MCOA with the
capabilities field. However if we enabled a field in capabilities and
then disable it MCO would not reconcile the AddOnDeploymentConfig
correctly due to DeepDerivative, that would return true instead of
false.

Signed-off-by: Joao Marcal <[email protected]>
JoaoBraveCoding added a commit to JoaoBraveCoding/multicluster-observability-operator that referenced this pull request Aug 28, 2024
ClusterManagementAddOn

In
stolostron#1470
we added support for deploying and configuring MCOA with the
capabilities field. However if we enabled a field in capabilities and
then disable it MCO would not reconcile the AddOnDeploymentConfig
correctly due to DeepDerivative, that would return true instead of
false.

Signed-off-by: Joao Marcal <[email protected]>
openshift-merge-bot bot pushed a commit that referenced this pull request Sep 9, 2024
ClusterManagementAddOn

In
#1470
we added support for deploying and configuring MCOA with the
capabilities field. However if we enabled a field in capabilities and
then disable it MCO would not reconcile the AddOnDeploymentConfig
correctly due to DeepDerivative, that would return true instead of
false.

Signed-off-by: Joao Marcal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants