-
Notifications
You must be signed in to change notification settings - Fork 1.3k
🌱 Fake client should set CreationTimestamp #3404
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,7 +95,8 @@ type fakeClient struct { | |
| // indexesLock must be held when accessing indexes. | ||
| indexesLock sync.RWMutex | ||
|
|
||
| returnManagedFields bool | ||
| returnManagedFields bool | ||
| setCreationTimestamp bool | ||
| } | ||
|
|
||
| var _ client.WithWatch = &fakeClient{} | ||
|
|
@@ -131,6 +132,7 @@ type ClientBuilder struct { | |
| interceptorFuncs *interceptor.Funcs | ||
| typeConverters []managedfields.TypeConverter | ||
| returnManagedFields bool | ||
| setCreationTimestamp bool | ||
| isBuilt bool | ||
|
|
||
| // indexes maps each GroupVersionKind (GVK) to the indexes registered for that GVK. | ||
|
|
@@ -256,6 +258,13 @@ func (f *ClientBuilder) WithReturnManagedFields() *ClientBuilder { | |
| return f | ||
| } | ||
|
|
||
| // WithSetCreationTimestamp configures the fake client to set metadata.creationTimestamp on Create and first Apply. | ||
| // on objects. | ||
| func (f *ClientBuilder) WithSetCreationTimestamp() *ClientBuilder { | ||
| f.setCreationTimestamp = true | ||
| return f | ||
| } | ||
|
|
||
| // Build builds and returns a new fake client. | ||
| func (f *ClientBuilder) Build() client.WithWatch { | ||
| if f.isBuilt { | ||
|
|
@@ -307,6 +316,7 @@ func (f *ClientBuilder) Build() client.WithWatch { | |
| scheme: f.scheme, | ||
| withStatusSubresource: withStatusSubResource, | ||
| usesFieldManagedObjectTracker: usesFieldManagedObjectTracker, | ||
| setCreationTimestamp: f.setCreationTimestamp, | ||
| } | ||
|
|
||
| for _, obj := range f.initObject { | ||
|
|
@@ -332,6 +342,7 @@ func (f *ClientBuilder) Build() client.WithWatch { | |
| indexes: f.indexes, | ||
| withStatusSubresource: withStatusSubResource, | ||
| returnManagedFields: f.returnManagedFields, | ||
| setCreationTimestamp: f.setCreationTimestamp, | ||
| } | ||
|
|
||
| if f.interceptorFuncs != nil { | ||
|
|
@@ -933,6 +944,16 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client | |
| // Overwrite it unconditionally, this matches the apiserver behavior | ||
| // which allows to set it on create, but will then ignore it. | ||
| obj.SetResourceVersion("1") | ||
|
|
||
| if c.setCreationTimestamp { | ||
| now, err := time.Parse(time.RFC3339, time.Now().Format(time.RFC3339)) | ||
| if err != nil { | ||
| return apierrors.NewInternalError(err) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe mention what failed so its easier to understand? |
||
| } | ||
| obj.SetCreationTimestamp(metav1.Time{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the behavior of the KAS to just override it if its set? |
||
| Time: now, | ||
| }) | ||
| } | ||
| } else { | ||
| // SSA deletionTimestamp updates are silently ignored | ||
| obj.SetDeletionTimestamp(oldAccessor.GetDeletionTimestamp()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -414,6 +414,7 @@ var _ = Describe("Fake client", func() { | |||||
| Expect(err).ToNot(HaveOccurred()) | ||||||
| Expect(obj).To(Equal(newcm)) | ||||||
| Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1")) | ||||||
| Expect(obj.ObjectMeta.CreationTimestamp.IsZero()).To(BeTrue()) | ||||||
| }) | ||||||
|
|
||||||
| It("should error on create with set resourceVersion", func(ctx SpecContext) { | ||||||
|
|
@@ -1229,7 +1230,6 @@ var _ = Describe("Fake client", func() { | |||||
| Expect(err).ToNot(HaveOccurred()) | ||||||
| Expect(newObj.Finalizers).To(BeEmpty()) | ||||||
| }) | ||||||
|
|
||||||
| } | ||||||
|
|
||||||
| Context("with default scheme.Scheme", func() { | ||||||
|
|
@@ -1473,6 +1473,48 @@ var _ = Describe("Fake client", func() { | |||||
| }) | ||||||
| }) | ||||||
|
|
||||||
| Context("with SetCreationTimestamp option", func() { | ||||||
| BeforeEach(func() { | ||||||
| cl = NewClientBuilder(). | ||||||
| WithSetCreationTimestamp(). | ||||||
| Build() | ||||||
| }) | ||||||
|
|
||||||
| It("should be able to Create and set metadata.CreationTimestamp", func(ctx SpecContext) { | ||||||
| By("Creating a new configmap") | ||||||
| newcm := &corev1.ConfigMap{ | ||||||
| ObjectMeta: metav1.ObjectMeta{ | ||||||
| Name: "test-cm-with-creation-timestamp", | ||||||
| Namespace: "ns2", | ||||||
| }, | ||||||
| } | ||||||
| err := cl.Create(ctx, newcm) | ||||||
| Expect(err).ToNot(HaveOccurred()) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe also check here that its unset? |
||||||
|
|
||||||
| By("Getting the new configmap") | ||||||
| namespacedName := types.NamespacedName{ | ||||||
| Name: "test-cm-with-creation-timestamp", | ||||||
| Namespace: "ns2", | ||||||
| } | ||||||
| obj := &corev1.ConfigMap{} | ||||||
| err = cl.Get(ctx, namespacedName, obj) | ||||||
| Expect(err).ToNot(HaveOccurred()) | ||||||
| Expect(obj).To(Equal(newcm)) | ||||||
| Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1")) | ||||||
| Expect(obj.ObjectMeta.CreationTimestamp.IsZero()).ToNot(BeTrue()) | ||||||
| }) | ||||||
|
|
||||||
| It("sets creatioTimestamp on SSA create when required to do so", func(ctx SpecContext) { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo and that its required is implicit from the
Suggested change
|
||||||
| obj := corev1applyconfigurations. | ||||||
| ConfigMap("foo-with-creation-timestamp", "default"). | ||||||
| WithData(map[string]string{"some": "data"}) | ||||||
|
|
||||||
| cl := NewClientBuilder().WithSetCreationTimestamp().Build() | ||||||
| Expect(cl.Apply(ctx, obj, client.FieldOwner("foo"))).NotTo(HaveOccurred()) | ||||||
| Expect(obj.CreationTimestamp.IsZero()).To(BeFalse()) | ||||||
| }) | ||||||
| }) | ||||||
|
|
||||||
| It("should set the ResourceVersion to 999 when adding an object to the tracker", func(ctx SpecContext) { | ||||||
| cl := NewClientBuilder().WithObjects(&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "cm"}}).Build() | ||||||
|
|
||||||
|
|
@@ -3068,6 +3110,16 @@ var _ = Describe("Fake client", func() { | |||||
| Expect(obj.ResourceVersion).To(BeEquivalentTo(ptr.To("1"))) | ||||||
| }) | ||||||
|
|
||||||
| It("does not set creatioTimestamp on SSA create", func(ctx SpecContext) { | ||||||
| obj := corev1applyconfigurations. | ||||||
| ConfigMap("foo", "default"). | ||||||
| WithData(map[string]string{"some": "data"}) | ||||||
|
|
||||||
| cl := NewClientBuilder().Build() | ||||||
| Expect(cl.Apply(ctx, obj, client.FieldOwner("foo"))).NotTo(HaveOccurred()) | ||||||
| Expect(obj.CreationTimestamp.IsZero()).To(BeTrue()) | ||||||
| }) | ||||||
|
|
||||||
| It("ignores a passed resourceVersion on SSA create", func(ctx SpecContext) { | ||||||
| obj := corev1applyconfigurations. | ||||||
| ConfigMap("foo", "default"). | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remainder?