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.

Sign up for GitHub

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

Jump to bottom

Update host health state based on saptune data #1905

Merged
merged 9 commits into from
Oct 16, 2023
Merged

Update host health state based on saptune data #1905

merged 9 commits into from
Oct 16, 2023

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Oct 5, 2023 •
edited
Loading

Description

Use saptune state to change host health.

  • If SAP workload is found in the host (meaning, a database or application instance is there):
    • saptune not installed -> warning
    • unsupported version -> warning
    • not compliant -> critical
    • not tuned -> warning
    • compliant -> passing
  • If SAP workload is not found in the host:
    • passing

In order to achieve this, I have added a new is_sap_running field to the UpdateSaptuneStatus. This field is used to determine if a SAP workload is there.
Together with this, we need store the saptune_health in the aggregate and create a new event to update it. Very similar of what we do with the checks health.

How was this tested?

UT

@arbulu89 arbulu89 added the enhancement New feature or request label Oct 5, 2023
@arbulu89 arbulu89 force-pushed the saptune-health-changes branch 2 times, most recently from 4f72d8d to 2856d33 Compare October 10, 2023 15:37
@arbulu89 arbulu89 added the env Create an ephimeral environment for the pr branch label Oct 10, 2023
@arbulu89 arbulu89 force-pushed the saptune-health-changes branch 2 times, most recently from b244a01 to 90ed526 Compare October 10, 2023 16:00
@arbulu89 arbulu89 marked this pull request as ready for review October 10, 2023 16:01
@arbulu89 arbulu89 mentioned this pull request Oct 11, 2023
Merged
EMaksy
EMaksy approved these changes Oct 11, 2023
View reviewed changes
Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

Hey man great job, codewise lgtm :D

@arbulu89 arbulu89 force-pushed the saptune-health-changes branch from 90ed526 to 1bfeb3a Compare October 11, 2023 14:39
CDimonaco
CDimonaco approved these changes Oct 11, 2023
View reviewed changes
Copy link
Member

@CDimonaco CDimonaco left a comment

Choose a reason for hiding this comment

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

LGTM

rtorrero
rtorrero approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@rtorrero rtorrero left a comment

Choose a reason for hiding this comment

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

LGTM, two minor comments that are just optional changes if you agree 👍

lib/trento/application/integration/discovery/discovery.ex Outdated Show resolved Hide resolved
test/trento/domain/host/host_test.exs Outdated Show resolved Hide resolved
@arbulu89 arbulu89 merged commit 36647d7 into main Oct 16, 2023
24 checks passed
@arbulu89 arbulu89 deleted the saptune-health-changes branch October 16, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EMaksy EMaksy EMaksy approved these changes

@CDimonaco CDimonaco CDimonaco approved these changes

@rtorrero rtorrero rtorrero approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request env Create an ephimeral environment for the pr branch
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arbulu89 @rtorrero @CDimonaco @EMaksy