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

Log MIWI/SP install type to AsyncQoS; also log successful cluster deletions #3934

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

ventifus
Copy link
Collaborator

@ventifus ventifus commented Oct 31, 2024

Which issue this PR addresses:

Fixes https://issues.redhat.com/browse/ARO-11772
Enables https://issues.redhat.com/browse/ARO-7841

What this PR does / why we need it:

For dashboarding purposes, we're missing a few things:

We wish to include cluster deletion success/failure rates in a dashboard. This is not possible because currently we don't log the completion of successful cluster deletions. That's because the logs (and other metrics) get emitted by endLease(), and the lease doesn't get released for deletions because the cluster doc gets deleted instead.

We wish to improve our cluster creation dashboards by distinguishing MIWI clusters installs from Service Principal installs. To do that we need a new field in AsyncQoS that records the cluster identity type requested.

Test plan for issue:

Manual testing will be required, as E2E does not exercise this as part of cluster deletion.

When manually testing, check that the appropriate "long running operation succeeded" log is emitted for cluster creation. Check that "long running operation succeeded" now gets logged on deletion. Check that the clusterIdentity field is emitted in AsyncQoS logs.

Is there any documentation that needs to be updated for this PR?

How do you know this will function as expected in production?

@ventifus
Copy link
Collaborator Author

/azp run ci,e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ventifus
Copy link
Collaborator Author

ventifus commented Nov 1, 2024

In the e2e logs for deletion, it does not appear to delete via backend operation, so it's not testing this change.

edisonLcardenas
edisonLcardenas previously approved these changes Nov 5, 2024
bitoku
bitoku previously approved these changes Nov 7, 2024
@github-actions github-actions bot added the needs-rebase branch needs a rebase label Nov 14, 2024
Copy link

Please rebase pull request.

@ventifus ventifus force-pushed the ventifus/ARO-11772-log-delete-completion branch from 16a54b5 to 8b8aecf Compare January 17, 2025 19:41
@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Jan 17, 2025
@ventifus
Copy link
Collaborator Author

/azp run e2e,ci

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ventifus ventifus changed the title Log successful cluster deletions Log MIWI/SP install type to AsyncQoS; also log successful cluster deletions Jan 24, 2025
Copy link
Collaborator

@rajdeepc2792 rajdeepc2792 left a comment

Choose a reason for hiding this comment

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

lgtm, thank you!!

@cadenmarchese cadenmarchese added next-release To be included in the next RP release rollout chainsaw Pull requests or issues owned by Team Chainsaw labels Jan 24, 2025
@cadenmarchese cadenmarchese merged commit fcc1bdf into master Jan 27, 2025
23 checks passed
LiniSusan pushed a commit that referenced this pull request Jan 30, 2025
…etions (#3934)

* Add terminal-state log and metrics to successful cluster deletion operations.

* ARO-7841: Add "clusterIdentity" field to AsyncQoS logs
@kimorris27 kimorris27 deleted the ventifus/ARO-11772-log-delete-completion branch January 31, 2025 15:21
ArrisLee pushed a commit that referenced this pull request Feb 9, 2025
…etions (#3934)

* Add terminal-state log and metrics to successful cluster deletion operations.

* ARO-7841: Add "clusterIdentity" field to AsyncQoS logs
s-fairchild pushed a commit that referenced this pull request Feb 19, 2025
…etions (#3934)

* Add terminal-state log and metrics to successful cluster deletion operations.

* ARO-7841: Add "clusterIdentity" field to AsyncQoS logs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chainsaw Pull requests or issues owned by Team Chainsaw next-release To be included in the next RP release rollout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants