From 7471369bdb842fa2a85bc8a3e3ac250e8346801c Mon Sep 17 00:00:00 2001 From: Shawn Hurley Date: Wed, 19 Sep 2018 15:09:59 -0400 Subject: [PATCH] Status Writer working with unstructured as well as typed objects --- pkg/client/client.go | 21 +-- pkg/client/client_test.go | 270 +++++++++++++++++++++--------- pkg/client/typed_client.go | 30 +++- pkg/client/unstructured_client.go | 27 ++- 4 files changed, 239 insertions(+), 109 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index fa2b09eaf5..05b9eba2b4 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -153,21 +153,10 @@ type statusWriter struct { var _ StatusWriter = &statusWriter{} // Update implements client.StatusWriter -func (sw *statusWriter) Update(_ context.Context, obj runtime.Object) error { - o, err := sw.client.typedClient.cache.getObjMeta(obj) - if err != nil { - return err +func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object) error { + _, ok := obj.(*unstructured.Unstructured) + if ok { + return sw.client.unstructuredClient.UpdateStatus(ctx, obj) } - // TODO(droot): examine the returned error and check if it error needs to be - // wrapped to improve the UX ? - // It will be nice to receive an error saying the object doesn't implement - // status subresource and check CRD definition - return o.Put(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). - Resource(o.resource()). - Name(o.GetName()). - SubResource("status"). - Body(obj). - Do(). - Into(obj) + return sw.client.typedClient.UpdateStatus(ctx, obj) } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 4389fd8daa..3ab5c12de7 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -541,114 +541,218 @@ var _ = Describe("Client", func() { }) Describe("StatusClient", func() { - It("should update status of an existing object", func(done Done) { - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - Expect(cl).NotTo(BeNil()) + Context("with structured objects", func() { + It("should update status of an existing object", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) - By("initially creating a Deployment") - dep, err := clientset.AppsV1().Deployments(ns).Create(dep) - Expect(err).NotTo(HaveOccurred()) + By("initially creating a Deployment") + dep, err := clientset.AppsV1().Deployments(ns).Create(dep) + Expect(err).NotTo(HaveOccurred()) - By("updating the status of Deployment") - dep.Status.Replicas = 1 - err = cl.Status().Update(context.TODO(), dep) - Expect(err).NotTo(HaveOccurred()) + By("updating the status of Deployment") + dep.Status.Replicas = 1 + err = cl.Status().Update(context.TODO(), dep) + Expect(err).NotTo(HaveOccurred()) - By("validating updated Deployment has new status") - actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) - Expect(err).NotTo(HaveOccurred()) - Expect(actual).NotTo(BeNil()) - Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) + By("validating updated Deployment has new status") + actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(actual).NotTo(BeNil()) + Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) - close(done) - }) + close(done) + }) - It("should not update spec of an existing object", func(done Done) { - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - Expect(cl).NotTo(BeNil()) + It("should not update spec of an existing object", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) - By("initially creating a Deployment") - dep, err := clientset.AppsV1().Deployments(ns).Create(dep) - Expect(err).NotTo(HaveOccurred()) + By("initially creating a Deployment") + dep, err := clientset.AppsV1().Deployments(ns).Create(dep) + Expect(err).NotTo(HaveOccurred()) - By("updating the spec and status of Deployment") - var rc int32 = 1 - dep.Status.Replicas = 1 - dep.Spec.Replicas = &rc - err = cl.Status().Update(context.TODO(), dep) - Expect(err).NotTo(HaveOccurred()) + By("updating the spec and status of Deployment") + var rc int32 = 1 + dep.Status.Replicas = 1 + dep.Spec.Replicas = &rc + err = cl.Status().Update(context.TODO(), dep) + Expect(err).NotTo(HaveOccurred()) - By("validating updated Deployment has new status and unchanged spec") - actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) - Expect(err).NotTo(HaveOccurred()) - Expect(actual).NotTo(BeNil()) - Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) - Expect(*actual.Spec.Replicas).To(BeEquivalentTo(replicaCount)) + By("validating updated Deployment has new status and unchanged spec") + actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(actual).NotTo(BeNil()) + Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) + Expect(*actual.Spec.Replicas).To(BeEquivalentTo(replicaCount)) - close(done) - }) + close(done) + }) - It("should update an existing object non-namespace object", func(done Done) { - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - Expect(cl).NotTo(BeNil()) + It("should update an existing object non-namespace object", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) - node, err := clientset.CoreV1().Nodes().Create(node) - Expect(err).NotTo(HaveOccurred()) + node, err := clientset.CoreV1().Nodes().Create(node) + Expect(err).NotTo(HaveOccurred()) - By("updating status of the object") - node.Status.Phase = corev1.NodeRunning - err = cl.Status().Update(context.TODO(), node) - Expect(err).NotTo(HaveOccurred()) + By("updating status of the object") + node.Status.Phase = corev1.NodeRunning + err = cl.Status().Update(context.TODO(), node) + Expect(err).NotTo(HaveOccurred()) - By("validate updated Node had new annotation") - actual, err := clientset.CoreV1().Nodes().Get(node.Name, metav1.GetOptions{}) - Expect(err).NotTo(HaveOccurred()) - Expect(actual).NotTo(BeNil()) - Expect(actual.Status.Phase).To(Equal(corev1.NodeRunning)) + By("validate updated Node had new annotation") + actual, err := clientset.CoreV1().Nodes().Get(node.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(actual).NotTo(BeNil()) + Expect(actual.Status.Phase).To(Equal(corev1.NodeRunning)) - close(done) - }) + close(done) + }) - It("should fail if the object does not exists", func(done Done) { - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - Expect(cl).NotTo(BeNil()) + It("should fail if the object does not exists", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) - By("updating status of a non-existent object") - err = cl.Status().Update(context.TODO(), dep) - Expect(err).To(HaveOccurred()) + By("updating status of a non-existent object") + err = cl.Status().Update(context.TODO(), dep) + Expect(err).To(HaveOccurred()) - close(done) - }) + close(done) + }) - It("should fail if the object cannot be mapped to a GVK", func(done Done) { - By("creating client with empty Scheme") - emptyScheme := runtime.NewScheme() - cl, err := client.New(cfg, client.Options{Scheme: emptyScheme}) - Expect(err).NotTo(HaveOccurred()) - Expect(cl).NotTo(BeNil()) + It("should fail if the object cannot be mapped to a GVK", func(done Done) { + By("creating client with empty Scheme") + emptyScheme := runtime.NewScheme() + cl, err := client.New(cfg, client.Options{Scheme: emptyScheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) - By("initially creating a Deployment") - dep, err := clientset.AppsV1().Deployments(ns).Create(dep) - Expect(err).NotTo(HaveOccurred()) + By("initially creating a Deployment") + dep, err := clientset.AppsV1().Deployments(ns).Create(dep) + Expect(err).NotTo(HaveOccurred()) - By("updating status of the Deployment") - dep.Status.Replicas = 1 - err = cl.Status().Update(context.TODO(), dep) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("no kind is registered for the type")) + By("updating status of the Deployment") + dep.Status.Replicas = 1 + err = cl.Status().Update(context.TODO(), dep) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no kind is registered for the type")) - close(done) - }) + close(done) + }) - PIt("should fail if the GVK cannot be mapped to a Resource", func() { + PIt("should fail if the GVK cannot be mapped to a Resource", func() { + + }) + PIt("should fail if an API does not implement Status subresource", func() { + + }) }) - PIt("should fail if an API does not implement Status subresource", func() { + Context("with unstructured objects", func() { + It("should update status of an existing object", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + By("initially creating a Deployment") + dep, err := clientset.AppsV1().Deployments(ns).Create(dep) + Expect(err).NotTo(HaveOccurred()) + + By("updating the status of Deployment") + u := &unstructured.Unstructured{} + dep.Status.Replicas = 1 + scheme.Convert(dep, u, nil) + err = cl.Status().Update(context.TODO(), u) + Expect(err).NotTo(HaveOccurred()) + + By("validating updated Deployment has new status") + actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(actual).NotTo(BeNil()) + Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) + + close(done) + }) + + It("should not update spec of an existing object", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + By("initially creating a Deployment") + dep, err := clientset.AppsV1().Deployments(ns).Create(dep) + Expect(err).NotTo(HaveOccurred()) + + By("updating the spec and status of Deployment") + u := &unstructured.Unstructured{} + var rc int32 = 1 + dep.Status.Replicas = 1 + dep.Spec.Replicas = &rc + scheme.Convert(dep, u, nil) + err = cl.Status().Update(context.TODO(), u) + Expect(err).NotTo(HaveOccurred()) + + By("validating updated Deployment has new status and unchanged spec") + actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(actual).NotTo(BeNil()) + Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) + Expect(*actual.Spec.Replicas).To(BeEquivalentTo(replicaCount)) + + close(done) + }) + + It("should update an existing object non-namespace object", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + node, err := clientset.CoreV1().Nodes().Create(node) + Expect(err).NotTo(HaveOccurred()) + + By("updating status of the object") + u := &unstructured.Unstructured{} + node.Status.Phase = corev1.NodeRunning + scheme.Convert(node, u, nil) + err = cl.Status().Update(context.TODO(), u) + Expect(err).NotTo(HaveOccurred()) + + By("validate updated Node had new annotation") + actual, err := clientset.CoreV1().Nodes().Get(node.Name, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(actual).NotTo(BeNil()) + Expect(actual.Status.Phase).To(Equal(corev1.NodeRunning)) + + close(done) + }) + + It("should fail if the object does not exists", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + By("updating status of a non-existent object") + u := &unstructured.Unstructured{} + scheme.Convert(dep, u, nil) + err = cl.Status().Update(context.TODO(), u) + Expect(err).To(HaveOccurred()) + + close(done) + }) + + PIt("should fail if the GVK cannot be mapped to a Resource", func() { + + }) + + PIt("should fail if an API does not implement Status subresource", func() { + + }) }) }) diff --git a/pkg/client/typed_client.go b/pkg/client/typed_client.go index 9834863e52..63c232fa92 100644 --- a/pkg/client/typed_client.go +++ b/pkg/client/typed_client.go @@ -30,7 +30,7 @@ type typedClient struct { } // Create implements client.Client -func (c *typedClient) Create(ctx context.Context, obj runtime.Object) error { +func (c *typedClient) Create(_ context.Context, obj runtime.Object) error { o, err := c.cache.getObjMeta(obj) if err != nil { return err @@ -44,7 +44,7 @@ func (c *typedClient) Create(ctx context.Context, obj runtime.Object) error { } // Update implements client.Client -func (c *typedClient) Update(ctx context.Context, obj runtime.Object) error { +func (c *typedClient) Update(_ context.Context, obj runtime.Object) error { o, err := c.cache.getObjMeta(obj) if err != nil { return err @@ -59,7 +59,7 @@ func (c *typedClient) Update(ctx context.Context, obj runtime.Object) error { } // Delete implements client.Client -func (c *typedClient) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOptionFunc) error { +func (c *typedClient) Delete(_ context.Context, obj runtime.Object, opts ...DeleteOptionFunc) error { o, err := c.cache.getObjMeta(obj) if err != nil { return err @@ -76,7 +76,7 @@ func (c *typedClient) Delete(ctx context.Context, obj runtime.Object, opts ...De } // Get implements client.Client -func (c *typedClient) Get(ctx context.Context, key ObjectKey, obj runtime.Object) error { +func (c *typedClient) Get(_ context.Context, key ObjectKey, obj runtime.Object) error { r, err := c.cache.getResource(obj) if err != nil { return err @@ -88,7 +88,7 @@ func (c *typedClient) Get(ctx context.Context, key ObjectKey, obj runtime.Object } // List implements client.Client -func (c *typedClient) List(ctx context.Context, opts *ListOptions, obj runtime.Object) error { +func (c *typedClient) List(_ context.Context, opts *ListOptions, obj runtime.Object) error { r, err := c.cache.getResource(obj) if err != nil { return err @@ -105,3 +105,23 @@ func (c *typedClient) List(ctx context.Context, opts *ListOptions, obj runtime.O Do(). Into(obj) } + +// UpdateStatus used by StatusWriter to write status. +func (c *typedClient) UpdateStatus(_ context.Context, obj runtime.Object) error { + o, err := c.cache.getObjMeta(obj) + if err != nil { + return err + } + // TODO(droot): examine the returned error and check if it error needs to be + // wrapped to improve the UX ? + // It will be nice to receive an error saying the object doesn't implement + // status subresource and check CRD definition + return o.Put(). + NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + Resource(o.resource()). + Name(o.GetName()). + SubResource("status"). + Body(obj). + Do(). + Into(obj) +} diff --git a/pkg/client/unstructured_client.go b/pkg/client/unstructured_client.go index 17494da252..13217b07da 100644 --- a/pkg/client/unstructured_client.go +++ b/pkg/client/unstructured_client.go @@ -37,7 +37,7 @@ type unstructuredClient struct { } // Create implements client.Client -func (uc *unstructuredClient) Create(ctx context.Context, obj runtime.Object) error { +func (uc *unstructuredClient) Create(_ context.Context, obj runtime.Object) error { u, ok := obj.(*unstructured.Unstructured) if !ok { return fmt.Errorf("unstructured client did not understand object: %T", obj) @@ -55,7 +55,7 @@ func (uc *unstructuredClient) Create(ctx context.Context, obj runtime.Object) er } // Update implements client.Client -func (uc *unstructuredClient) Update(ctx context.Context, obj runtime.Object) error { +func (uc *unstructuredClient) Update(_ context.Context, obj runtime.Object) error { u, ok := obj.(*unstructured.Unstructured) if !ok { return fmt.Errorf("unstructured client did not understand object: %T", obj) @@ -73,7 +73,7 @@ func (uc *unstructuredClient) Update(ctx context.Context, obj runtime.Object) er } // Delete implements client.Client -func (uc *unstructuredClient) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOptionFunc) error { +func (uc *unstructuredClient) Delete(_ context.Context, obj runtime.Object, opts ...DeleteOptionFunc) error { u, ok := obj.(*unstructured.Unstructured) if !ok { return fmt.Errorf("unstructured client did not understand object: %T", obj) @@ -88,7 +88,7 @@ func (uc *unstructuredClient) Delete(ctx context.Context, obj runtime.Object, op } // Get implements client.Client -func (uc *unstructuredClient) Get(ctx context.Context, key ObjectKey, obj runtime.Object) error { +func (uc *unstructuredClient) Get(_ context.Context, key ObjectKey, obj runtime.Object) error { u, ok := obj.(*unstructured.Unstructured) if !ok { return fmt.Errorf("unstructured client did not understand object: %T", obj) @@ -106,7 +106,7 @@ func (uc *unstructuredClient) Get(ctx context.Context, key ObjectKey, obj runtim } // List implements client.Client -func (uc *unstructuredClient) List(ctx context.Context, opts *ListOptions, obj runtime.Object) error { +func (uc *unstructuredClient) List(_ context.Context, opts *ListOptions, obj runtime.Object) error { u, ok := obj.(*unstructured.UnstructuredList) if !ok { return fmt.Errorf("unstructured client did not understand object: %T", obj) @@ -133,6 +133,23 @@ func (uc *unstructuredClient) List(ctx context.Context, opts *ListOptions, obj r return nil } +func (uc *unstructuredClient) UpdateStatus(_ context.Context, obj runtime.Object) error { + u, ok := obj.(*unstructured.Unstructured) + if !ok { + return fmt.Errorf("unstructured client did not understand object: %T", obj) + } + r, err := uc.getResourceInterface(u.GroupVersionKind(), u.GetNamespace()) + if err != nil { + return err + } + i, err := r.UpdateStatus(u) + if err != nil { + return err + } + u.Object = i.Object + return nil +} + func (uc *unstructuredClient) getResourceInterface(gvk schema.GroupVersionKind, ns string) (dynamic.ResourceInterface, error) { mapping, err := uc.restMapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil {