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

external: Defer secret hash update until all resources are successfully created #2904

Merged

Conversation

parth-gr
Copy link
Member

@parth-gr parth-gr commented Nov 21, 2024

This update ensures that the secret hash in the storage cluster status is updated only after all resources are created successfully.

Previously, the status was updated prematurely, preventing the reconcile loop from re-running whether there was a failure or success. This change enables the reconcile process to continue retrying until all resources are successfully created.

@parth-gr
Copy link
Member Author

/assign @iamniting @travisn

@parth-gr parth-gr changed the title external: fix reconcile for external mode external: return success only after all resources created Nov 21, 2024
@parth-gr
Copy link
Member Author

Testing:
Resources were created successfully

[rider@localhost rook]$ oc get cephBlockPoolRadosNamespace 
NAME         PHASE   BLOCKPOOL   AGE
namespace1   Ready   rbd         23s
[rider@localhost rook]$ oc get sc
NAME                                                   PROVISIONER                             RECLAIMPOLICY   VOLUMEBINDINGMODE      ALLOWVOLUMEEXPANSION   AGE
ocs-external-storagecluster-ceph-rbd                   openshift-storage.rbd.csi.ceph.com      Delete          Immediate              true                   2d10h
ocs-external-storagecluster-ceph-rbd-rados-namespace   openshift-storage.rbd.csi.ceph.com      Delete          Immediate              true                   25s
ocs-external-storagecluster-ceph-rgw                   openshift-storage.ceph.rook.io/bucket   Delete          Immediate              false                  2d10h
ocs-external-storagecluster-cephfs                     openshift-storage.cephfs.csi.ceph.com   Delete          Immediate              true                   2d4h
openshift-storage.noobaa.io                            openshift-storage.noobaa.io/obc         Delete          Immediate              false                  2d10h
thin-csi (default)                                     csi.vsphere.vmware.com                  Delete          WaitForFirstConsumer   true                   2d10h
[rider@localhost rook]$ 

Copy link
Member

@iamniting iamniting left a comment

Choose a reason for hiding this comment

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

Changes looks good to me, though we need to work on a commit msg.

Defer secret hash update until all resources are successfully created

This update ensures that the secret hash in the storage cluster status is updated only after all resources are created successfully.

Previously, the status was updated prematurely, preventing the reconcile loop from re-running whether there was a failure or success. This change enables the reconcile process to continue retrying until all resources are successfully created.

@parth-gr parth-gr force-pushed the fix-external-reconcile branch 2 times, most recently from dbbeb04 to a9a1a2f Compare November 22, 2024 06:33
@parth-gr parth-gr changed the title external: return success only after all resources created external: Defer secret hash update until all resources are successfully created Nov 22, 2024
@parth-gr parth-gr force-pushed the fix-external-reconcile branch 2 times, most recently from f8e21fd to bd21d8b Compare November 22, 2024 06:36
This update ensures that the secret hash in the storage
cluster status is updated only after all resources are created
successfully.
Previously, the status was updated prematurely, preventing the
reconcile loop from re-running whether there was a failure or success.
This change enables the reconcile process to continue retrying
until all resources are successfully created.

Signed-off-by: parth-gr <paarora@redhat.com>
@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 22, 2024
Copy link
Contributor

@malayparida2000 malayparida2000 left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Contributor

openshift-ci bot commented Nov 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iamniting, malayparida2000, parth-gr

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

The pull request process is described 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

@openshift-merge-bot openshift-merge-bot bot merged commit 3ef212d into red-hat-storage:main Nov 22, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants