Skip to content

Commit 3144e8a

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

13 files changed

Lines changed: 83 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: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package store
22

33
import (
44
"context"
5+
"k8s.io/client-go/tools/cache"
56
"testing"
67
"time"
78

@@ -20,23 +21,23 @@ func TestVersioner(t *testing.T) {
2021
t.Run("increment version for new resource", func(t *testing.T) {
2122
v := newVersioner()
2223
version := v.increment("test-work")
23-
if version != 0 {
24-
t.Errorf("expected version 0 for new resource, got %d", version)
24+
if version != 1 {
25+
t.Errorf("expected version 1 for new resource, got %d", version)
2526
}
2627
})
2728

2829
t.Run("increment version for existing resource", func(t *testing.T) {
2930
v := newVersioner()
3031
v.increment("test-work")
3132
version := v.increment("test-work")
32-
if version != 1 {
33-
t.Errorf("expected version 1 for existing resource, got %d", version)
33+
if version != 2 {
34+
t.Errorf("expected version 2 for existing resource, got %d", version)
3435
}
3536
})
3637

3738
t.Run("increment multiple times", func(t *testing.T) {
3839
v := newVersioner()
39-
for i := 0; i < 5; i++ {
40+
for i := 1; i <= 5; i++ {
4041
version := v.increment("test-work")
4142
if version != int64(i) {
4243
t.Errorf("expected version %d, got %d", i, version)
@@ -49,8 +50,8 @@ func TestVersioner(t *testing.T) {
4950
v.increment("test-work")
5051
v.delete("test-work")
5152
version := v.increment("test-work")
52-
if version != 0 {
53-
t.Errorf("expected version 0 after delete, got %d", version)
53+
if version != 1 {
54+
t.Errorf("expected version 1 after delete, got %d", version)
5455
}
5556
})
5657

@@ -70,14 +71,15 @@ func TestVersioner(t *testing.T) {
7071
}
7172

7273
finalVersion := v.increment("test-work")
73-
if finalVersion != 10 {
74-
t.Errorf("expected version 10 after 10 concurrent increments, got %d", finalVersion)
74+
if finalVersion != 11 {
75+
t.Errorf("expected version 11 after 10 concurrent increments, got %d", finalVersion)
7576
}
7677
})
7778
}
7879

7980
func TestAgentInformerWatcherStore_Add(t *testing.T) {
8081
store := NewAgentInformerWatcherStore()
82+
store.Store = cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, cache.Indexers{})
8183

8284
// Start consuming events to prevent blocking
8385
watcher, err := store.GetWatcher("", metav1.ListOptions{})
@@ -118,8 +120,8 @@ func TestAgentInformerWatcherStore_Add(t *testing.T) {
118120
t.Fatalf("unexpected error adding work: %v", err)
119121
}
120122

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

125127
// Add another work
@@ -135,13 +137,14 @@ func TestAgentInformerWatcherStore_Add(t *testing.T) {
135137
t.Fatalf("unexpected error adding work: %v", err)
136138
}
137139

138-
if work2.ResourceVersion != "1" {
139-
t.Errorf("expected resource version 1 for second add, got %s", work2.ResourceVersion)
140+
if work2.ResourceVersion != "2" {
141+
t.Errorf("expected resource version 2 for second add, got %s", work2.ResourceVersion)
140142
}
141143
}
142144

143145
func TestAgentInformerWatcherStore_Update(t *testing.T) {
144146
store := NewAgentInformerWatcherStore()
147+
store.Store = cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, cache.Indexers{})
145148

146149
// Start consuming events to prevent blocking
147150
watcher, err := store.GetWatcher("", metav1.ListOptions{})
@@ -198,8 +201,8 @@ func TestAgentInformerWatcherStore_Update(t *testing.T) {
198201
t.Fatalf("unexpected error updating work: %v", err)
199202
}
200203

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

205208
// Update again
@@ -208,13 +211,14 @@ func TestAgentInformerWatcherStore_Update(t *testing.T) {
208211
t.Fatalf("unexpected error updating work: %v", err)
209212
}
210213

211-
if updatedWork.ResourceVersion != "2" {
212-
t.Errorf("expected resource version 2 after second update, got %s", updatedWork.ResourceVersion)
214+
if updatedWork.ResourceVersion != "3" {
215+
t.Errorf("expected resource version 3 after second update, got %s", updatedWork.ResourceVersion)
213216
}
214217
}
215218

216219
func TestAgentInformerWatcherStore_Delete(t *testing.T) {
217220
store := NewAgentInformerWatcherStore()
221+
store.Store = cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, cache.Indexers{})
218222

219223
// Start consuming events to prevent blocking
220224
watcher, err := store.GetWatcher("", metav1.ListOptions{})
@@ -267,7 +271,7 @@ func TestAgentInformerWatcherStore_Delete(t *testing.T) {
267271
t.Fatalf("unexpected error deleting work: %v", err)
268272
}
269273

270-
// Add the same work again, version should be reset to 0
274+
// Add the same work again, version should be reset to 1
271275
newWork := &workv1.ManifestWork{
272276
ObjectMeta: metav1.ObjectMeta{
273277
Name: "test-work",
@@ -280,13 +284,14 @@ func TestAgentInformerWatcherStore_Delete(t *testing.T) {
280284
t.Fatalf("unexpected error adding work after delete: %v", err)
281285
}
282286

283-
if newWork.ResourceVersion != "0" {
284-
t.Errorf("expected resource version 0 after delete and re-add, got %s", newWork.ResourceVersion)
287+
if newWork.ResourceVersion != "1" {
288+
t.Errorf("expected resource version 1 after delete and re-add, got %s", newWork.ResourceVersion)
285289
}
286290
}
287291

288292
func TestAgentInformerWatcherStore_ResourceVersionIncrement(t *testing.T) {
289293
store := NewAgentInformerWatcherStore()
294+
store.Store = cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, cache.Indexers{})
290295

291296
// Start consuming events to prevent blocking
292297
watcher, err := store.GetWatcher("", metav1.ListOptions{})
@@ -323,7 +328,7 @@ func TestAgentInformerWatcherStore_ResourceVersionIncrement(t *testing.T) {
323328
}
324329

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

328333
// Add
329334
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

0 commit comments

Comments
 (0)