Skip to content

core: fix ceph health check up status check #16595

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
Merged
travisn merged 2 commits into from
Oct 14, 2025
Merged

core: fix ceph health check up status check #16595

travisn merged 2 commits into from
Oct 14, 2025

Conversation

@parth-gr
Copy link
Member

@parth-gr parth-gr commented Oct 10, 2025

currently the health checkup was reporting the
condition and reason to be always connected,
But which might not be the case,
So instead of that dont report the condition and
its phase during the health checkups

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@parth-gr parth-gr requested a review from travisn October 10, 2025 14:49
@parth-gr parth-gr changed the title ceph: fix cep health check up status check core: fix cep health check up status check Oct 10, 2025
@parth-gr parth-gr changed the title core: fix cep health check up status check core: fix ceph health check up status check Oct 10, 2025
travisn
travisn requested changes Oct 10, 2025
View reviewed changes
pkg/operator/ceph/controller/conditions.go Outdated
}
currentCondition.Status = status
currentCondition.Reason = reason
if reason != "" {
Copy link
Member

@travisn travisn Oct 10, 2025

Choose a reason for hiding this comment

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

As discussed, let's see if it will keep things simpler for the osd goroutine to have its own method for updating the status, rather than calling this method.

Copy link
Member Author

@parth-gr parth-gr Oct 13, 2025

Choose a reason for hiding this comment

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

Done

@parth-gr parth-gr requested a review from travisn October 13, 2025 14:51
@parth-gr
core: fix cep health check up status check
454c221 
currently the health checkup was reporting the
condition and reason to be always connected,
But which might not be the case,
So instead of that dont report the condition and
its phase during the health checkups

Signed-off-by: parth-gr <partharora1010@gmail.com>
travisn
travisn reviewed Oct 13, 2025
View reviewed changes
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

This looks much simpler now, thanks. Please just confirm if you could test a couple scenarios:

  1. That the conditions and phase were preserved even while the ceph status is updated here
  2. That the ceph status is preserved by the reconcile, even while the conditions and phase are updated

@parth-gr
external: add cluster connected condition in case of external mode
3f5ebe2 
external mode was dependednt on the ceph status to
update the cluster connected condition,
BUt as now the status check will only update the ceph status,
so we will explicitly update the ceph status condition in the
end of the reconcile

Signed-off-by: parth-gr <partharora1010@gmail.com>
@parth-gr
Copy link
Member Author

parth-gr commented Oct 14, 2025

This looks much simpler now, thanks. Please just confirm if you could test a couple scenarios:

  1. That the conditions and phase were preserved even while the ceph status is updated here
Status:
  Ceph:
    Capacity:
      Bytes Available:  20942438400
      Bytes Total:      20971520000
      Bytes Used:       29081600
      Last Updated:     2025-10-14T12:57:34Z
    Fsid:               3ea948c6-10b7-4164-bd4d-40a7633c3bd3
    Health:             HEALTH_OK
    Last Checked:       2025-10-14T12:57:34Z
    Versions:
      Mgr:
        ceph version 19.2.3 (c92aebb279828e9c3c1f5d24613efca272649e62) squid (stable):  1
      Mon:
        ceph version 19.2.3 (c92aebb279828e9c3c1f5d24613efca272649e62) squid (stable):  1
      Osd:
        ceph version 19.2.3 (c92aebb279828e9c3c1f5d24613efca272649e62) squid (stable):  1
      Overall:
        ceph version 19.2.3 (c92aebb279828e9c3c1f5d24613efca272649e62) squid (stable):  3
  Cephx:
    Admin:
    Ceph Exporter:
      Key Ceph Version:  19.2.3-0
      Key Generation:    1
    Crash Collector:
      Key Ceph Version:  19.2.3-0
      Key Generation:    1
    Csi:
      Key Ceph Version:  19.2.3-0
      Key Generation:    1
    Mgr:
      Key Ceph Version:  19.2.3-0
      Key Generation:    1
    Mon:
      Key Ceph Version:  19.2.3-0
      Key Generation:    1
    Osd:
      Key Ceph Version:  19.2.3-0
      Key Generation:    1
    Rbd Mirror Peer:
      Key Ceph Version:  19.2.3-0
      Key Generation:    1
  Conditions:
    Last Heartbeat Time:   2025-10-14T12:51:24Z
    Last Transition Time:  2025-10-14T12:15:50Z
    Message:               Cluster created successfully
    Reason:                ClusterCreated
    Status:                True
    Type:                  Ready
  Message:                 Cluster created successfully
  Observed Generation:     2
  Phase:                   Ready
  State:                   Created
  Storage:
    Device Classes:
      Name:  hdd
    Osd:
      Migration Status:
      Store Type:
        Bluestore:  1
  Version:
    Image:    quay.io/ceph/ceph:v19
    Version:  19.2.3-0
Events:
  Type    Reason              Age                   From                          Message
  ----    ------              ----                  ----                          -------
  Normal  ReconcileSucceeded  42m (x2 over 52m)     rook-ceph-cluster-controller  successfully configured CephCluster "rook-ceph/my-cluster"
  Normal  ReconcileSucceeded  15m                   rook-ceph-cluster-controller  successfully configured CephCluster "rook-ceph/my-cluster"
  Normal  ReconcileSucceeded  7m1s (x2 over 7m24s)  rook-ceph-cluster-controller  successfully configured CephCluster "rook-ceph/my-cluster"

We can see that status is getting updated and the conditions are not by the timestamp

@parth-gr
Copy link
Member Author

parth-gr commented Oct 14, 2025 •
edited
Loading

  1. That the ceph status is preserved by the reconcile, even while the conditions and phase are updated

It won't as the Ceph cluster reconcile will still update the current status and conditions at the same time.

@travisn travisn merged commit 96dda04 into rook:master Oct 14, 2025
56 checks passed
@mergify mergify bot mentioned this pull request Oct 14, 2025
Merged
6 tasks
travisn added a commit that referenced this pull request Oct 15, 2025
@travisn
Merge pull request #16604 from rook/mergify/bp/release-1.18/pr-16595
dd3d610 
core: fix ceph health check up status check (backport #16595)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@travisn travisn travisn approved these changes

Assignees

No one assigned

Labels

backport-release-1.18

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@parth-gr @travisn