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

MGMT-16977: Mapping Alerts to AlarmEventRecords #73

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

danielerez
Copy link
Collaborator

Added mapping logic to the AlarmFetcher:

  • For each Alert retrieved from the AlertManager, map to an AlarmEventRecord object.
  • Use the resource server url for fetching info about resources and pools (needed for resourceID/resourceTypeID properties).

Also added an API for fetching alarm probable causes:

  • This API is not defined by O2ims spec, but we can use it for exposing a custom list of probable causes.
  • This list is available in data folder (data/alarms/probable_causes.json), and can be customized and maintained by server admins as required.

Note: this PR does not include AlarmDictionary support (will be handled separately).

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 13, 2024

@danielerez: This pull request references MGMT-16977 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.16.0" version, but no target version was set.

In response to this:

Added mapping logic to the AlarmFetcher:

  • For each Alert retrieved from the AlertManager, map to an AlarmEventRecord object.
  • Use the resource server url for fetching info about resources and pools (needed for resourceID/resourceTypeID properties).

Also added an API for fetching alarm probable causes:

  • This API is not defined by O2ims spec, but we can use it for exposing a custom list of probable causes.
  • This list is available in data folder (data/alarms/probable_causes.json), and can be customized and maintained by server admins as required.

Note: this PR does not include AlarmDictionary support (will be handled separately).

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested review from irinamihai and jhernand March 13, 2024 15:09
Copy link

openshift-ci bot commented Mar 13, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from danielerez. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

@danielerez danielerez force-pushed the alarm_handler branch 5 times, most recently from 8a4590c to 4aaaa5b Compare March 17, 2024 14:19
return
}

func (r *AlarmFetcher) FetchResourcePool(ctx context.Context, clusterId string) (resourcePool data.Object, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these functions need to be public?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really, can be private, fixed. Good catch:)

@jhernand
Copy link
Collaborator

I only have a minor question about why a function is public. Other than that it looks good. Feel free to merge.

Added mapping logic to the AlarmFetcher:
* For each Alert retrieved from the AlertManager,
  map to an AlarmEventRecord object.
* Use the resource server url for fetching info about resources and pools
  (needed for resourceID/resourceTypeID properties).

Also added an API for fetching alarm probable causes:
* This API is not defined by O2ims spec, but we can use it for exposing
  a custom list of probable causes.
* This list is available in data folder (data/alarms/probable_causes.json),
  and can be customized and maintained by server admins as required.

Note: this PR does not include AlarmDictionary support (will be handled separately).
@danielerez
Copy link
Collaborator Author

I only have a minor question about why a function is public. Other than that it looks good. Feel free to merge.

Can indeed be private, fixed. Thanks:)

@danielerez
Copy link
Collaborator Author

/retest

@danielerez danielerez merged commit 3fbe178 into openshift-kni:main Mar 18, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants