Skip to content

client: update status subresource support #96

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,36 @@ func (c *client) List(ctx context.Context, opts *ListOptions, obj runtime.Object
Do().
Into(obj)
}

// Status implements client.StatusClient
func (c *client) Status() StatusWriter {
return &statusWriter{client: c}
}

// statusWriter is client.StatusWriter that writes status subresource
type statusWriter struct {
client *client
}

// ensure statusWriter implements client.StatusWriter
var _ StatusWriter = &statusWriter{}

// Update implements client.StatusWriter
func (sw *statusWriter) Update(_ context.Context, obj runtime.Object) error {
o, err := sw.client.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 ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could always check discovery to see if the /status subresource appeared in discovery first, if we really wanted to...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tip. Not critical for now, so not addressing this in this PR.

// 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)
}
141 changes: 128 additions & 13 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,15 @@ var _ = Describe("Client", func() {
var pod *corev1.Pod
var node *corev1.Node
var count uint64 = 0
var replicaCount int32 = 2
var ns = "default"

BeforeEach(func(done Done) {
atomic.AddUint64(&count, 1)
dep = &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("deployment-name-%v", count), Namespace: ns},
Spec: appsv1.DeploymentSpec{
Replicas: &replicaCount,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo": "bar"},
},
Expand Down Expand Up @@ -143,7 +145,7 @@ var _ = Describe("Client", func() {
close(done)
})

It("should use the provided Mapper if provided", func() {
PIt("should use the provided Mapper if provided", func() {

})

Expand Down Expand Up @@ -267,7 +269,7 @@ var _ = Describe("Client", func() {
Expect(err.Error()).To(ContainSubstring("no kind is registered for the type"))
})

It("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() {
// TODO(seans3): implement these
// Example: ListOptions
})
Expand Down Expand Up @@ -358,11 +360,11 @@ var _ = Describe("Client", func() {
close(done)
})

It("should fail if the object does not pass server-side validation", func() {
PIt("should fail if the object does not pass server-side validation", func() {

})

It("should fail if the object doesn't have meta", func() {
PIt("should fail if the object doesn't have meta", func() {

})

Expand All @@ -386,7 +388,120 @@ var _ = Describe("Client", func() {
close(done)
})

It("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() {

})
})

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())

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("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")
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))

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")
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))

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")
err = cl.Status().Update(context.TODO(), dep)
Expect(err).To(HaveOccurred())

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())

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"))

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() {

})
})
Expand Down Expand Up @@ -471,7 +586,7 @@ var _ = Describe("Client", func() {
close(done)
})

It("should fail if the object doesn't have meta", func() {
PIt("should fail if the object doesn't have meta", func() {

})

Expand All @@ -494,7 +609,7 @@ var _ = Describe("Client", func() {
close(done)
})

It("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() {

})
})
Expand Down Expand Up @@ -609,7 +724,7 @@ var _ = Describe("Client", func() {
close(done)
})

It("should fail if the object doesn't have meta", func() {
PIt("should fail if the object doesn't have meta", func() {

})

Expand All @@ -632,7 +747,7 @@ var _ = Describe("Client", func() {
Expect(err.Error()).To(ContainSubstring("no kind is registered for the type"))
})

It("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() {

})
})
Expand Down Expand Up @@ -846,19 +961,19 @@ var _ = Describe("Client", func() {
close(done)
}, serverSideTimeoutSeconds)

It("should fail if it cannot get a client", func() {
PIt("should fail if it cannot get a client", func() {

})

It("should fail if the object doesn't have meta", func() {
PIt("should fail if the object doesn't have meta", func() {

})

It("should fail if the object cannot be mapped to a GVK", func() {
PIt("should fail if the object cannot be mapped to a GVK", func() {

})

It("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() {

})
})
Expand Down
14 changes: 14 additions & 0 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ func (c *fakeClient) Update(ctx context.Context, obj runtime.Object) error {
return c.tracker.Update(gvr, obj, accessor.GetNamespace())
}

func (c *fakeClient) Status() client.StatusWriter {
return &fakeStatusWriter{client: c}
}

func getGVRFromObject(obj runtime.Object) (schema.GroupVersionResource, error) {
gvk, err := apiutil.GVKForObject(obj, scheme.Scheme)
if err != nil {
Expand All @@ -137,3 +141,13 @@ func getGVRFromObject(obj runtime.Object) (schema.GroupVersionResource, error) {
gvr, _ := meta.UnsafeGuessKindToResource(gvk)
return gvr, nil
}

type fakeStatusWriter struct {
client *fakeClient
}

func (sw *fakeStatusWriter) Update(ctx context.Context, obj runtime.Object) error {
// TODO(droot): This results in full update of the obj (spec + status). Need
// a way to update status field only.
return sw.client.Update(ctx, obj)
}
15 changes: 15 additions & 0 deletions pkg/client/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,25 @@ type Writer interface {
Update(ctx context.Context, obj runtime.Object) error
}

// StatusClient knows how to create a client which can update status subresource
// for kubernetes objects.
type StatusClient interface {
Status() StatusWriter
}

// StatusWriter knows how to update status subresource of a Kubernetes object.
type StatusWriter interface {
// Update updates the fields corresponding to the status subresource for the
// given obj. obj must be a struct pointer so that obj can be updated
// with the content returned by the Server.
Update(ctx context.Context, obj runtime.Object) error
}

// Client knows how to perform CRUD operations on Kubernetes objects.
type Client interface {
Reader
Writer
StatusClient
}

// IndexerFunc knows how to take an object and turn it into a series
Expand Down
3 changes: 2 additions & 1 deletion pkg/client/split.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ limitations under the License.
package client

// DelegatingClient forms an interface Client by composing separate
// read and write interfaces. This way, you can have an Client that
// reader, writer and statusclient interfaces. This way, you can have an Client that
// reads from a cache and writes to the API server.
type DelegatingClient struct {
Reader
Writer
StatusClient
}
3 changes: 1 addition & 2 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ func New(config *rest.Config, options Options) (Manager, error) {
cache, err := options.newCache(config, cache.Options{Scheme: options.Scheme, Mapper: mapper, Resync: options.SyncPeriod})
if err != nil {
return nil, err

}
// Create the recorder provider to inject event recorders for the components.
recorderProvider, err := options.newRecorderProvider(config, options.Scheme)
Expand All @@ -144,7 +143,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
errChan: make(chan error),
cache: cache,
fieldIndexes: cache,
client: client.DelegatingClient{Reader: cache, Writer: writeObj},
client: client.DelegatingClient{Reader: cache, Writer: writeObj, StatusClient: writeObj},
recorderProvider: recorderProvider,
}, nil
}
Expand Down