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

mgr/dashboard: Filter alerts based on cluster fsid and do not allow to connect clusters with version less than hub cluster in multi-cluster #56579

Merged
merged 1 commit into from
May 3, 2024
Merged

mgr/dashboard: Filter alerts based on cluster fsid and do not allow to connect clusters with version less than hub cluster in multi-cluster #56579

merged 1 commit into from
May 3, 2024

Conversation

aaSharma14
Copy link
Contributor

@aaSharma14 aaSharma14 commented Mar 29, 2024 •
edited

  1. Since we have a new cluster variable in the prometheus metrics , we need to filter the alerts based on the cluster fsid.
  2. Do not allow to connect clusters with version less than 7.1 in multi-cluster

Fixes: https://tracker.ceph.com/issues/65774

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@aaSharma14 aaSharma14 requested a review from a team as a code owner March 29, 2024 10:39
@aaSharma14 aaSharma14 requested review from cloudbehl and Pegonzal and removed request for a team March 29, 2024 10:39
@aaSharma14 aaSharma14 force-pushed the fix-alerts-verison-compatability-multi-cluster branch from 40c4bf2 to f3593ca Compare April 1, 2024 07:48
@aaSharma14 aaSharma14 changed the title mgr/dashboard: Filter alerts based on cluster fsid and do not allow to connect clusters with version < 7.1 in multi-cluster mgr/dashboard: Filter alerts based on cluster fsid and do not allow to connect clusters with version less than hub cluster in multi-cluster Apr 1, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 1, 2024

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@cloudbehl
Copy link
Contributor

Merged this PR after merging this: #53679

@aaSharma14 aaSharma14 force-pushed the fix-alerts-verison-compatability-multi-cluster branch from f3593ca to 50f613a Compare April 2, 2024 05:20
@aaSharma14 aaSharma14 force-pushed the fix-alerts-verison-compatability-multi-cluster branch 7 times, most recently from 1cb699a to 5df76c2 Compare April 4, 2024 10:00
nizamial09
nizamial09 reviewed Apr 5, 2024
View reviewed changes
Copy link
Member

@nizamial09 nizamial09 left a comment

Choose a reason for hiding this comment

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

looks good

src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard-v3/dashboard/dashboard-v3.component.ts Outdated Show resolved Hide resolved
@aaSharma14 aaSharma14 force-pushed the fix-alerts-verison-compatability-multi-cluster branch from 5df76c2 to b7378eb Compare April 5, 2024 06:42
cloudbehl
cloudbehl reviewed Apr 5, 2024
View reviewed changes
src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert.service.ts
refresh() {
this.getAlerts();
refresh(clusterFilteredAlerts?: boolean) {
this.getAlerts(clusterFilteredAlerts);
this.getRules();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you check if the rules have same issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

@aaSharma14

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rules don't have this issue as they are static and do not contain anything related to the fsid variable

@afreen23
Copy link
Contributor

afreen23 commented Apr 5, 2024

Tested with two ceph clusters - one quincy and other from latest:main

  • I was unable to connect to quincy cluster, but the message that came up is not what we expect, perhaps failed earlier.
    See the screencast.(ignore last part of dev console opening)
  • Also it did not ask for fsid , did we remove it ?
  • Moreover, I tried to connect another cluster (using latest:main) and it refused for that one too. Perhaps , I need to do some additional steps, let me know in that case.

PS: It was a fresh install using this PR, so everything was updated and loaded well which I can confirm!

Screencast.from.2024-04-06.00-02-26.webm

For alerts I can see cluster filter is set true always:

Screenshot from 2024-04-05 20-17-44

I wanted to test more after connecting a cluster but due to above could not do it

Thanks

@aaSharma14 aaSharma14 force-pushed the fix-alerts-verison-compatability-multi-cluster branch 3 times, most recently from 14505ad to 71f750b Compare April 16, 2024 09:44
@aaSharma14
Copy link
Contributor Author

jenkins retest this please

@cloudbehl
Copy link
Contributor

Also, tests are failing still.

mgr/dashboard: filter alerts with cluster fsid and donot allow clusters
bca4720 
with version less than hub cluster to be added in multi-cluster

Signed-off-by: Aashish Sharma <aasharma@redhat.com>
@aaSharma14 aaSharma14 force-pushed the fix-alerts-verison-compatability-multi-cluster branch from 71f750b to bca4720 Compare April 24, 2024 07:35
@afreen23
Copy link
Contributor

When I connected a cluster not having multicluster, I was able to see the expected message
Screenshot from 2024-04-25 01-06-40
i did not see any error as mentioned above when connected both cluster supporting mult-cluster.
image

@aaSharma14 aaSharma14 merged commit e67bece into ceph:main May 3, 2024
13 checks passed
@aaSharma14 aaSharma14 deleted the fix-alerts-verison-compatability-multi-cluster branch May 3, 2024 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cloudbehl cloudbehl cloudbehl left review comments

@nizamial09 nizamial09 nizamial09 approved these changes

@Pegonzal Pegonzal Awaiting requested review from Pegonzal Pegonzal is a code owner automatically assigned from ceph/dashboard

Assignees
No one assigned
Labels
api-change dashboard pybind
Projects
Ceph-Dashboard
Archived in project
Milestone
No milestone
4 participants
@aaSharma14 @cloudbehl @afreen23 @nizamial09