Skip to content

Commit 1698cbf

Browse files
committed
Add getGeneration in the interface
Signed-off-by: Jian Qiu <jqiu@redhat.com>
1 parent a853869 commit 1698cbf

13 files changed

Lines changed: 78 additions & 84 deletions

File tree

pkg/cloudevents/clients/store/informer.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,17 @@ func NewAgentInformerWatcherStore[T generic.ResourceObject]() *AgentInformerWatc
3737

3838
func (s *AgentInformerWatcherStore[T]) Add(resource runtime.Object) error {
3939
s.Watcher.Receive(watch.Event{Type: watch.Added, Object: resource})
40-
return nil
40+
return s.Store.Add(resource)
4141
}
4242

4343
func (s *AgentInformerWatcherStore[T]) Update(resource runtime.Object) error {
4444
s.Watcher.Receive(watch.Event{Type: watch.Modified, Object: resource})
45-
return nil
45+
return s.Store.Update(resource)
4646
}
4747

4848
func (s *AgentInformerWatcherStore[T]) Delete(resource runtime.Object) error {
4949
s.Watcher.Receive(watch.Event{Type: watch.Deleted, Object: resource})
50-
return nil
50+
return s.Store.Delete(resource)
5151
}
5252

5353
func (s *AgentInformerWatcherStore[T]) HandleReceivedResource(action types.ResourceAction, resource T) error {

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ func (c *ManifestWorkAgentClient) Watch(ctx context.Context, opts metav1.ListOpt
125125
}
126126

127127
func (c *ManifestWorkAgentClient) Patch(ctx context.Context, name string, pt kubetypes.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (result *workv1.ManifestWork, err error) {
128-
klog.V(4).Infof("patching manifestwork %s/%s", c.namespace, name)
128+
logger := klog.FromContext(ctx)
129+
logger.V(4).Info("patching manifestwork")
129130

130131
// avoid race conditions among the agent's go routines
131132
c.Lock()
@@ -175,8 +176,7 @@ func (c *ManifestWorkAgentClient) Patch(ctx context.Context, name string, pt kub
175176
Action: types.UpdateRequestAction,
176177
}
177178

178-
// we first compare local resource version
179-
if returnErr = versoinCompare(patchedWork, lastWork); returnErr != nil {
179+
if returnErr = versionCompare(patchedWork, lastWork); returnErr != nil {
180180
return nil, returnErr
181181
}
182182

@@ -232,7 +232,8 @@ func (c *ManifestWorkAgentClient) Patch(ctx context.Context, name string, pt kub
232232
returnErr = errors.NewNotFound(common.ManifestWorkGR, name)
233233
return nil, returnErr
234234
}
235-
if returnErr = versoinCompare(patchedWork, latestWork); returnErr != nil {
235+
236+
if returnErr = versionCompare(patchedWork, latestWork); returnErr != nil {
236237
return nil, returnErr
237238
}
238239
if err := c.watcherStore.Update(newWork); err != nil {
@@ -242,7 +243,7 @@ func (c *ManifestWorkAgentClient) Patch(ctx context.Context, name string, pt kub
242243
return newWork, nil
243244
}
244245

245-
func versoinCompare(new, old *workv1.ManifestWork) *errors.StatusError {
246+
func versionCompare(new, old *workv1.ManifestWork) *errors.StatusError {
246247
// If resource version is empty or 0, skip the comparison (e.g., when publishing status updates)
247248
if new.GetResourceVersion() == "" || new.GetResourceVersion() == "0" {
248249
return nil
@@ -256,6 +257,7 @@ func versoinCompare(new, old *workv1.ManifestWork) *errors.StatusError {
256257
if err != nil {
257258
return errors.NewInternalError(err)
258259
}
260+
259261
// ensure the resource version of the work is not outdated
260262
if newResourceVersion < lastResourceVersion {
261263
// It's safe to return a conflict error here, even if the status update event

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func TestVersionCompare(t *testing.T) {
111111
},
112112
}
113113

114-
err := versoinCompare(newWork, oldWork)
114+
err := versionCompare(newWork, oldWork)
115115

116116
if tt.expectError {
117117
if err == nil {

pkg/cloudevents/clients/work/agent/codec/manifestbundle.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package codec
33
import (
44
"encoding/json"
55
"fmt"
6-
"strconv"
7-
86
"github.com/bwmarrin/snowflake"
97
cloudevents "github.com/cloudevents/sdk-go/v2"
108
cloudeventstypes "github.com/cloudevents/sdk-go/v2/types"
@@ -50,16 +48,6 @@ func (c *ManifestBundleCodec) Encode(source string, eventType types.CloudEventsT
5048
return nil, fmt.Errorf("unsupported cloudevents data type %s", eventType.CloudEventsDataType)
5149
}
5250

53-
var resourceVersion int64
54-
// If resource version is empty, use 0 as default (e.g., when publishing status updates)
55-
if work.ResourceVersion != "" {
56-
var err error
57-
resourceVersion, err = strconv.ParseInt(work.ResourceVersion, 10, 64)
58-
if err != nil {
59-
return nil, fmt.Errorf("failed to parse the resourceversion of the work %s, %v", work.UID, err)
60-
}
61-
}
62-
6351
originalSource, ok := work.Labels[common.CloudEventsOriginalSourceLabelKey]
6452
if !ok {
6553
return nil, fmt.Errorf("failed to find originalsource from the work %s", work.UID)
@@ -68,7 +56,7 @@ func (c *ManifestBundleCodec) Encode(source string, eventType types.CloudEventsT
6856
evt := types.NewEventBuilder(source, eventType).
6957
WithResourceID(string(work.UID)).
7058
WithStatusUpdateSequenceID(sequenceGenerator.Generate().String()).
71-
WithResourceVersion(resourceVersion).
59+
WithResourceVersion(work.Generation).
7260
WithClusterName(work.Namespace).
7361
WithOriginalSource(originalSource).
7462
NewEvent()
@@ -144,7 +132,7 @@ func (c *ManifestBundleCodec) Decode(evt *cloudevents.Event) (*workv1.ManifestWo
144132
metaObj.Name = resourceID
145133
}
146134
metaObj.Namespace = clusterName
147-
// This is explicitly set to empy since it will be managed by local client.
135+
// This is explicitly set to empty since it will be managed by local client.
148136
metaObj.ResourceVersion = ""
149137
// The resourceVersion in cloudevent actually sematically equals to generation, since it increments when
150138
// spec changes

pkg/cloudevents/clients/work/store/informer.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func (v *versioner) increment(name string) int64 {
111111
defer v.lock.Unlock()
112112

113113
if _, ok := v.versions[name]; !ok {
114-
v.versions[name] = 0
114+
v.versions[name] = 1
115115
} else {
116116
v.versions[name] = v.versions[name] + 1
117117
}
@@ -184,9 +184,7 @@ func (s *AgentInformerWatcherStore) HandleReceivedResource(action types.Resource
184184

185185
updatedWork := work.DeepCopy()
186186

187-
// restore the fields that are maintained by local agent
188-
updatedWork.Labels = lastWork.Labels
189-
updatedWork.Annotations = lastWork.Annotations
187+
// restore the fields that are maintained by local agent.
190188
updatedWork.Finalizers = lastWork.Finalizers
191189
updatedWork.Status = lastWork.Status
192190

@@ -201,9 +199,8 @@ func (s *AgentInformerWatcherStore) HandleReceivedResource(action types.Resource
201199
return nil
202200
}
203201

202+
// we should only update the deletionTimestamp or the local work
204203
updatedWork := lastWork.DeepCopy()
205-
updatedWork.Generation = work.Generation
206-
updatedWork.ResourceVersion = work.ResourceVersion
207204
updatedWork.DeletionTimestamp = work.DeletionTimestamp
208205
return s.Update(updatedWork)
209206
default:

pkg/cloudevents/clients/work/store/informer_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,23 @@ func TestVersioner(t *testing.T) {
2020
t.Run("increment version for new resource", func(t *testing.T) {
2121
v := newVersioner()
2222
version := v.increment("test-work")
23-
if version != 0 {
24-
t.Errorf("expected version 0 for new resource, got %d", version)
23+
if version != 1 {
24+
t.Errorf("expected version 1 for new resource, got %d", version)
2525
}
2626
})
2727

2828
t.Run("increment version for existing resource", func(t *testing.T) {
2929
v := newVersioner()
3030
v.increment("test-work")
3131
version := v.increment("test-work")
32-
if version != 1 {
33-
t.Errorf("expected version 1 for existing resource, got %d", version)
32+
if version != 2 {
33+
t.Errorf("expected version 2 for existing resource, got %d", version)
3434
}
3535
})
3636

3737
t.Run("increment multiple times", func(t *testing.T) {
3838
v := newVersioner()
39-
for i := 0; i < 5; i++ {
39+
for i := 1; i <= 5; i++ {
4040
version := v.increment("test-work")
4141
if version != int64(i) {
4242
t.Errorf("expected version %d, got %d", i, version)
@@ -49,8 +49,8 @@ func TestVersioner(t *testing.T) {
4949
v.increment("test-work")
5050
v.delete("test-work")
5151
version := v.increment("test-work")
52-
if version != 0 {
53-
t.Errorf("expected version 0 after delete, got %d", version)
52+
if version != 1 {
53+
t.Errorf("expected version 1 after delete, got %d", version)
5454
}
5555
})
5656

@@ -70,8 +70,8 @@ func TestVersioner(t *testing.T) {
7070
}
7171

7272
finalVersion := v.increment("test-work")
73-
if finalVersion != 10 {
74-
t.Errorf("expected version 10 after 10 concurrent increments, got %d", finalVersion)
73+
if finalVersion != 11 {
74+
t.Errorf("expected version 11 after 10 concurrent increments, got %d", finalVersion)
7575
}
7676
})
7777
}
@@ -118,8 +118,8 @@ func TestAgentInformerWatcherStore_Add(t *testing.T) {
118118
t.Fatalf("unexpected error adding work: %v", err)
119119
}
120120

121-
if work.ResourceVersion != "0" {
122-
t.Errorf("expected resource version 0, got %s", work.ResourceVersion)
121+
if work.ResourceVersion != "1" {
122+
t.Errorf("expected resource version 1, got %s", work.ResourceVersion)
123123
}
124124

125125
// Add another work
@@ -135,8 +135,8 @@ func TestAgentInformerWatcherStore_Add(t *testing.T) {
135135
t.Fatalf("unexpected error adding work: %v", err)
136136
}
137137

138-
if work2.ResourceVersion != "1" {
139-
t.Errorf("expected resource version 1 for second add, got %s", work2.ResourceVersion)
138+
if work2.ResourceVersion != "2" {
139+
t.Errorf("expected resource version 2 for second add, got %s", work2.ResourceVersion)
140140
}
141141
}
142142

@@ -198,8 +198,8 @@ func TestAgentInformerWatcherStore_Update(t *testing.T) {
198198
t.Fatalf("unexpected error updating work: %v", err)
199199
}
200200

201-
if updatedWork.ResourceVersion != "1" {
202-
t.Errorf("expected resource version 1 after update, got %s", updatedWork.ResourceVersion)
201+
if updatedWork.ResourceVersion != "2" {
202+
t.Errorf("expected resource version 2 after update, got %s", updatedWork.ResourceVersion)
203203
}
204204

205205
// Update again
@@ -208,8 +208,8 @@ func TestAgentInformerWatcherStore_Update(t *testing.T) {
208208
t.Fatalf("unexpected error updating work: %v", err)
209209
}
210210

211-
if updatedWork.ResourceVersion != "2" {
212-
t.Errorf("expected resource version 2 after second update, got %s", updatedWork.ResourceVersion)
211+
if updatedWork.ResourceVersion != "3" {
212+
t.Errorf("expected resource version 3 after second update, got %s", updatedWork.ResourceVersion)
213213
}
214214
}
215215

@@ -267,7 +267,7 @@ func TestAgentInformerWatcherStore_Delete(t *testing.T) {
267267
t.Fatalf("unexpected error deleting work: %v", err)
268268
}
269269

270-
// Add the same work again, version should be reset to 0
270+
// Add the same work again, version should be reset to 1
271271
newWork := &workv1.ManifestWork{
272272
ObjectMeta: metav1.ObjectMeta{
273273
Name: "test-work",
@@ -280,8 +280,8 @@ func TestAgentInformerWatcherStore_Delete(t *testing.T) {
280280
t.Fatalf("unexpected error adding work after delete: %v", err)
281281
}
282282

283-
if newWork.ResourceVersion != "0" {
284-
t.Errorf("expected resource version 0 after delete and re-add, got %s", newWork.ResourceVersion)
283+
if newWork.ResourceVersion != "1" {
284+
t.Errorf("expected resource version 1 after delete and re-add, got %s", newWork.ResourceVersion)
285285
}
286286
}
287287

@@ -323,7 +323,7 @@ func TestAgentInformerWatcherStore_ResourceVersionIncrement(t *testing.T) {
323323
}
324324

325325
// Verify resource versions increment properly
326-
expectedVersions := []string{"0", "1", "2", "3"}
326+
expectedVersions := []string{"1", "2", "3", "4"}
327327

328328
// Add
329329
err = store.Add(work)

pkg/cloudevents/generic/clients/agentclient.go

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package clients
33
import (
44
"context"
55
"fmt"
6-
"strconv"
76
"time"
87

98
cloudevents "github.com/cloudevents/sdk-go/v2"
@@ -84,19 +83,10 @@ func (c *CloudEventAgentClient[T]) Resync(ctx context.Context, source string) er
8483

8584
resources := &payload.ResourceVersionList{Versions: make([]payload.ResourceVersion, len(objs))}
8685
for i, obj := range objs {
87-
var resourceVersion int64
88-
// If resource version is empty, use 0 as default
89-
if obj.GetResourceVersion() != "" {
90-
var err error
91-
resourceVersion, err = strconv.ParseInt(obj.GetResourceVersion(), 10, 64)
92-
if err != nil {
93-
return err
94-
}
95-
}
96-
9786
resources.Versions[i] = payload.ResourceVersion{
98-
ResourceID: string(obj.GetUID()),
99-
ResourceVersion: resourceVersion,
87+
ResourceID: string(obj.GetUID()),
88+
// this should be set as generation, since the resource version of the object is local version.
89+
ResourceVersion: obj.GetGeneration(),
10090
}
10191
}
10292

@@ -304,27 +294,11 @@ func (c *CloudEventAgentClient[T]) specAction(
304294

305295
// if both the current and the last object have the resource version "0" or empty, then object
306296
// is considered as modified, the message broker guarantees the order of the messages
307-
if (obj.GetResourceVersion() == "0" || obj.GetResourceVersion() == "") &&
308-
(lastObj.GetResourceVersion() == "0" || lastObj.GetResourceVersion() == "") {
297+
if lastObj.GetGeneration() == 0 && obj.GetGeneration() == 0 {
309298
return types.Modified, nil
310299
}
311300

312-
// If resource version is empty, treat as modified (cannot compare versions)
313-
if obj.GetResourceVersion() == "" || lastObj.GetResourceVersion() == "" {
314-
return types.Modified, nil
315-
}
316-
317-
resourceVersion, err := strconv.ParseInt(obj.GetResourceVersion(), 10, 64)
318-
if err != nil {
319-
return evt, err
320-
}
321-
322-
lastResourceVersion, err := strconv.ParseInt(lastObj.GetResourceVersion(), 10, 64)
323-
if err != nil {
324-
return evt, err
325-
}
326-
327-
if resourceVersion <= lastResourceVersion {
301+
if obj.GetGeneration() <= lastObj.GetGeneration() {
328302
return evt, nil
329303
}
330304

pkg/cloudevents/generic/clients/agentclient_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ func TestAgentPublish(t *testing.T) {
122122
clusterName: "cluster1",
123123
resources: &generictesting.MockResource{
124124
UID: kubetypes.UID("1234"),
125+
Generation: 2,
125126
ResourceVersion: "2",
126127
Status: "test-status",
127128
Namespace: "cluster1",
@@ -409,6 +410,7 @@ func TestReceiveResourceSpec(t *testing.T) {
409410
eventType,
410411
&generictesting.MockResource{
411412
UID: kubetypes.UID("test1"),
413+
Generation: 1,
412414
ResourceVersion: "1",
413415
Namespace: "cluster1",
414416
})
@@ -435,6 +437,7 @@ func TestReceiveResourceSpec(t *testing.T) {
435437
eventType,
436438
&generictesting.MockResource{
437439
UID: kubetypes.UID("test1"),
440+
Generation: 2,
438441
ResourceVersion: "2",
439442
Namespace: "cluster1",
440443
})
@@ -505,14 +508,15 @@ func TestReceiveResourceSpec(t *testing.T) {
505508
eventType,
506509
&generictesting.MockResource{
507510
UID: kubetypes.UID("test1"),
511+
Generation: 2,
508512
ResourceVersion: "2",
509513
Namespace: "cluster1",
510514
})
511515
return *evt
512516
}(),
513517
resources: []*generictesting.MockResource{
514-
{UID: kubetypes.UID("test1"), ResourceVersion: "2", Namespace: "cluster1"},
515-
{UID: kubetypes.UID("test2"), ResourceVersion: "1", Namespace: "cluster1"},
518+
{UID: kubetypes.UID("test1"), Generation: 2, ResourceVersion: "2", Namespace: "cluster1"},
519+
{UID: kubetypes.UID("test2"), Generation: 1, ResourceVersion: "1", Namespace: "cluster1"},
516520
},
517521
validate: func(event types.ResourceAction, resource *generictesting.MockResource) {
518522
if len(event) != 0 {

pkg/cloudevents/generic/clients/sourceclient_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ func TestSourcePublish(t *testing.T) {
105105
resources: &generictesting.MockResource{
106106
UID: kubetypes.UID("1234"),
107107
ResourceVersion: "2",
108+
Generation: 2,
108109
Spec: "test-spec",
109110
},
110111
eventType: types.CloudEventsType{

pkg/cloudevents/generic/interface.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,12 @@ type ResourceObject interface {
2525
// GetResourceVersion returns the resource version of this object. The resource version is a required int64 sequence
2626
// number property that must be incremented by the source whenever this resource changes.
2727
// The source should guarantee its incremental nature.
28+
// Deprecated: use GetGeneration() instead.
2829
GetResourceVersion() string
2930

31+
// GetGeneration returns the generation number of this object to reflect the spec change of the resource.
32+
GetGeneration() int64
33+
3034
// GetDeletionTimestamp returns the deletion timestamp of this object. The deletiontimestamp is an optional
3135
// timestamp property representing the resource is deleting from the source, the agent needs to clean up the
3236
// resource from its cluster.

0 commit comments

Comments
 (0)