Skip to content

Conversation

@jor2
Copy link
Member

@jor2 jor2 commented May 19, 2025

Description

#65

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@jor2 jor2 self-assigned this May 19, 2025
@jor2 jor2 requested a review from Aashiq-J as a code owner May 19, 2025 22:13
@jor2 jor2 mentioned this pull request May 20, 2025
6 tasks
@jor2
Copy link
Member Author

jor2 commented May 28, 2025

Screenshot 2025-05-28 at 15 27 05 Screenshot 2025-05-28 at 15 27 57

@jor2
Copy link
Member Author

jor2 commented May 28, 2025

@jor2
Copy link
Member Author

jor2 commented May 28, 2025

removed that null value

values used

variable "metrics_filter" {
  type = list(object({
    include = optional(string)
    exclude = optional(string)
  }))
  description = "To filter on custom metrics, specify the IBM Cloud Monitoring metrics to include or exclude. [Learn more](https://cloud.ibm.com/docs/monitoring?topic=monitoring-change_kube_agent#change_kube_agent_inc_exc_metrics) and [here](https://github.com/terraform-ibm-modules/terraform-ibm-monitoring-agent/tree/main/solutions/fully-configurable/DA-types.md)."
  default = [{ exclude = "all" }, { include = "kubernetes.*" }]
}
Screenshot 2025-05-28 at 16 40 31

@jor2
Copy link
Member Author

jor2 commented May 28, 2025

values passed from helm

Screenshot 2025-05-29 at 00 39 46

@jor2
Copy link
Member Author

jor2 commented May 28, 2025

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

see comments

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

Why does diagram say "Existing Monitoring Agent"? The agents are deployed by the DA?

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

Final few comments @jor2

@jor2
Copy link
Member Author

jor2 commented Jun 10, 2025

/run pipeline

@jor2 jor2 requested a review from ocofaigh June 10, 2025 11:06
Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

see latest comments

variables.tf Outdated

variable "access_key_secret_name" {
type = string
description = "The name of a Kubernetes or OpenShift Secret that contains the Sysdig agent access key under the key `access-key`. This variable allows you to reference an existing secret in your cluster, rather than providing the access key directly in your Terraform configuration."
Copy link
Contributor

Choose a reason for hiding this comment

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

This is still not accurate - I don't know why you are saying this. The access key is passed in as a terraform input, and that is passed to the helm chart which creates the secret. The name of that secret is the value of this input.

The description also needs to mention what the default is is left as null - perhaps we should just add a default value and match the default thats in the chart?

Copy link
Member Author

Choose a reason for hiding this comment

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

I cannot find a naming value for the secret, this was renamed to existing_access_key_secret_name make it more clear with validation added that both cannot be set.

@jor2 jor2 requested a review from ocofaigh June 10, 2025 14:13
@jor2
Copy link
Member Author

jor2 commented Jun 10, 2025

/run pipeline

@jor2 jor2 closed this Jun 10, 2025
@jor2 jor2 reopened this Jun 10, 2025
@jor2
Copy link
Member Author

jor2 commented Jun 10, 2025

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Jun 11, 2025

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Jun 11, 2025

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Jun 11, 2025

tolerations being applied to agent

Screenshot 2025-06-10 at 11 24 16

@ocofaigh ocofaigh merged commit 0d835ff into main Jun 11, 2025
2 checks passed
@ocofaigh ocofaigh deleted the missing-logic branch June 11, 2025 13:34
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants