Skip to content

Commit 8e7d38e

Browse files
committed
using bg goroutine to handle work deleting
Signed-off-by: Wei Liu <liuweixa@redhat.com>
1 parent 5b730d7 commit 8e7d38e

3 files changed

Lines changed: 409 additions & 93 deletions

File tree

pkg/cloudevents/clients/work/agent/client/manifestwork.go

Lines changed: 75 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import (
66
"net/http"
77
"strconv"
88
"sync"
9+
"time"
910

1011
"k8s.io/apimachinery/pkg/api/meta"
1112

1213
"k8s.io/apimachinery/pkg/api/errors"
1314
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1415
kubetypes "k8s.io/apimachinery/pkg/types"
16+
"k8s.io/apimachinery/pkg/util/wait"
1517
"k8s.io/apimachinery/pkg/watch"
1618
"k8s.io/klog/v2"
1719

@@ -22,11 +24,17 @@ import (
2224
cloudeventserrors "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/errors"
2325
"open-cluster-management.io/sdk-go/pkg/cloudevents/clients/store"
2426
"open-cluster-management.io/sdk-go/pkg/cloudevents/clients/utils"
27+
"open-cluster-management.io/sdk-go/pkg/cloudevents/clients/work/payload"
2528
"open-cluster-management.io/sdk-go/pkg/cloudevents/generic"
2629
"open-cluster-management.io/sdk-go/pkg/cloudevents/generic/metrics"
2730
"open-cluster-management.io/sdk-go/pkg/cloudevents/generic/types"
2831
)
2932

33+
const (
34+
// workDeletionCheckInterval defines how often to check for works that need deletion
35+
workDeletionCheckInterval = 2 * time.Second
36+
)
37+
3038
// ManifestWorkAgentClient implements the ManifestWorkInterface. It sends the manifestworks status back to source by
3139
// CloudEventAgentClient.
3240
type ManifestWorkAgentClient struct {
@@ -42,14 +50,38 @@ type ManifestWorkAgentClient struct {
4250
var _ workv1client.ManifestWorkInterface = &ManifestWorkAgentClient{}
4351

4452
func NewManifestWorkAgentClient(
45-
_ string,
53+
ctx context.Context,
4654
watcherStore store.ClientWatcherStore[*workv1.ManifestWork],
4755
cloudEventsClient generic.CloudEventsClient[*workv1.ManifestWork],
4856
) *ManifestWorkAgentClient {
49-
return &ManifestWorkAgentClient{
57+
58+
client := &ManifestWorkAgentClient{
5059
cloudEventsClient: cloudEventsClient,
5160
watcherStore: watcherStore,
5261
}
62+
63+
// Start a background goroutine to periodically check for works that need deletion.
64+
// This ensures that works with deletion timestamps and no finalizers are properly
65+
// cleaned up and their deletion status is sent back to the source.
66+
go wait.UntilWithContext(ctx, func(ctx context.Context) {
67+
logger := klog.FromContext(ctx)
68+
69+
// List all works and check if any need to be deleted
70+
works, err := watcherStore.ListAll(ctx)
71+
if err != nil {
72+
logger.Error(err, "failed to list all works for deletion check")
73+
return
74+
}
75+
76+
// Process each work for potential deletion
77+
for _, work := range works {
78+
if err := client.deleteWork(ctx, work); err != nil {
79+
logger.Error(err, "failed to delete work", "namespace", work.Namespace, "name", work.Name)
80+
}
81+
}
82+
}, workDeletionCheckInterval)
83+
84+
return client
5385
}
5486

5587
func (c *ManifestWorkAgentClient) SetNamespace(namespace string) {
@@ -187,18 +219,7 @@ func (c *ManifestWorkAgentClient) Patch(ctx context.Context, name string, pt kub
187219

188220
newWork := patchedWork.DeepCopy()
189221

190-
isDeleted := !newWork.DeletionTimestamp.IsZero() && len(newWork.Finalizers) == 0
191-
192-
if utils.IsStatusPatch(subresources) || isDeleted {
193-
if isDeleted {
194-
meta.SetStatusCondition(&newWork.Status.Conditions, metav1.Condition{
195-
Type: common.ResourceDeleted,
196-
Status: metav1.ConditionTrue,
197-
Reason: "ManifestsDeleted",
198-
Message: fmt.Sprintf("The manifests are deleted from the cluster %s", newWork.Namespace),
199-
})
200-
}
201-
222+
if utils.IsStatusPatch(subresources) {
202223
// Set work's resource version to remote resource version for publishing
203224
workToPublish := newWork.DeepCopy()
204225
workToPublish.ResourceVersion = ""
@@ -211,19 +232,6 @@ func (c *ManifestWorkAgentClient) Patch(ctx context.Context, name string, pt kub
211232
}
212233
}
213234

214-
// the finalizers of a deleting manifestwork are removed, marking the manifestwork status to deleted and sending
215-
// it back to source
216-
if isDeleted {
217-
if err := c.watcherStore.Delete(newWork); err != nil {
218-
returnErr := errors.NewInternalError(err)
219-
metrics.IncreaseWorkProcessedCounter("delete", string(returnErr.ErrStatus.Reason))
220-
return nil, returnErr
221-
}
222-
223-
metrics.IncreaseWorkProcessedCounter("delete", metav1.StatusSuccess)
224-
return newWork, nil
225-
}
226-
227235
// Fetch the latest work from the store and verify the resource version to avoid updating the store
228236
// with outdated work. Return a conflict error if the resource version is outdated.
229237
// Due to the lack of read-modify-write guarantees in the store, race conditions may occur between
@@ -248,6 +256,46 @@ func (c *ManifestWorkAgentClient) Patch(ctx context.Context, name string, pt kub
248256
return newWork, nil
249257
}
250258

259+
// deleteWork handles the cleanup of a manifestwork that is being deleted. It checks if the work
260+
// has a deletion timestamp and all finalizers have been removed. If so, it marks the manifestwork
261+
// status as deleted, publishes the deletion event to the source, and removes the work from the cache.
262+
func (c *ManifestWorkAgentClient) deleteWork(ctx context.Context, work *workv1.ManifestWork) error {
263+
if work.DeletionTimestamp.IsZero() || len(work.Finalizers) != 0 {
264+
// not ready for deletion (has finalizers or no deletion timestamp)
265+
return nil
266+
}
267+
268+
eventType := types.CloudEventsType{
269+
CloudEventsDataType: payload.ManifestBundleEventDataType,
270+
SubResource: types.SubResourceStatus,
271+
Action: types.UpdateRequestAction,
272+
}
273+
274+
workToPublish := work.DeepCopy()
275+
workToPublish.ResourceVersion = ""
276+
meta.SetStatusCondition(&workToPublish.Status.Conditions, metav1.Condition{
277+
Type: common.ResourceDeleted,
278+
Status: metav1.ConditionTrue,
279+
Reason: "ManifestsDeleted",
280+
Message: fmt.Sprintf("The manifests are deleted from the cluster %s", work.Namespace),
281+
})
282+
283+
if err := c.cloudEventsClient.Publish(ctx, eventType, workToPublish); err != nil {
284+
return cloudeventserrors.ToStatusError(common.ManifestWorkGR, work.Name, err)
285+
}
286+
287+
c.Lock()
288+
defer c.Unlock()
289+
if err := c.watcherStore.Delete(work); err != nil {
290+
returnErr := errors.NewInternalError(err)
291+
metrics.IncreaseWorkProcessedCounter("delete", string(returnErr.ErrStatus.Reason))
292+
return returnErr
293+
}
294+
295+
metrics.IncreaseWorkProcessedCounter("delete", metav1.StatusSuccess)
296+
return nil
297+
}
298+
251299
func versionCompare(new, old *workv1.ManifestWork) *errors.StatusError {
252300
// Resource version 0 means force conflict.
253301
if new.GetResourceVersion() == "0" {

0 commit comments

Comments
 (0)