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

enhance resource deletion flow. #111

Merged

Conversation

morvencao
Copy link
Contributor

@morvencao morvencao commented Jun 5, 2024

This PR enhance the resource deletion flow by:

  1. Soft delete the resource record when the request reach to Maestro Server
  2. Publish delete cloud event to Maestro agent by setting deletionTimeStamp
  3. Agent delete the resource from cluster and publish status update with ManifestsDeleted condition
  4. Maestro check the ManifestsDeleted condition in the status update and delete the resource record permanently.

@morvencao morvencao force-pushed the br_enhance_resource_deletion branch from 8146a09 to 3d1be1e Compare June 5, 2024 07:02
@morvencao morvencao changed the title [WIP] enhance resource deletion flow. enhance resource deletion flow. Jun 5, 2024
@morvencao
Copy link
Contributor Author

morvencao commented Jun 5, 2024

/assign @qiujian16 @clyang82 @skeeey

@morvencao morvencao force-pushed the br_enhance_resource_deletion branch from 3d1be1e to 6b844b6 Compare June 7, 2024 02:53
@morvencao morvencao force-pushed the br_enhance_resource_deletion branch from 6b844b6 to 6853f9d Compare June 7, 2024 03:18

// set the deletedAt field if the resource has been marked as deleted
if !resource.DeletedAt.Time.IsZero() {
res.DeletedAt = openapi.PtrTime(resource.DeletedAt.Time)
Copy link
Contributor

Choose a reason for hiding this comment

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

why need this check? can we just use DeletedAt: = openapi.PtrTime(resource.DeletedAt)

Copy link
Contributor Author

@morvencao morvencao Jun 7, 2024

Choose a reason for hiding this comment

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

If we don't have this check, the deleted_at for HTTP response will be time zero value, instead of null.

@clyang82
Copy link
Contributor

clyang82 commented Jun 7, 2024

ref: #101

Copy link
Contributor

@clyang82 clyang82 left a comment

Choose a reason for hiding this comment

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

LGTM

@clyang82 clyang82 merged commit 9916ae6 into openshift-online:main Jun 7, 2024
1 of 2 checks passed
@morvencao morvencao deleted the br_enhance_resource_deletion branch June 7, 2024 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants