Merge pull request #812 from alexeldeib/ace/unstruct

🐛 respect context in unstructured client

Author: k8s-ci-robot

pkg/client/client.go

@@ -19,14 +19,12 @@ package client
 import (
 	"context"
 	"fmt"
-	"reflect"
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -70,25 +68,22 @@ func New(config *rest.Config, options Options) (Client, error) {
 		}
 	}
 
-	dynamicClient, err := dynamic.NewForConfig(config)
-	if err != nil {
-		return nil, err
+	clientcache := &clientCache{
+		config:         config,
+		scheme:         options.Scheme,
+		mapper:         options.Mapper,
+		codecs:         serializer.NewCodecFactory(options.Scheme),
+		resourceByType: make(map[schema.GroupVersionKind]*resourceMeta),
 	}
 
 	c := &client{
 		typedClient: typedClient{
-			cache: clientCache{
-				config:         config,
-				scheme:         options.Scheme,
-				mapper:         options.Mapper,
-				codecs:         serializer.NewCodecFactory(options.Scheme),
-				resourceByType: make(map[reflect.Type]*resourceMeta),
-			},
+			cache:      clientcache,
 			paramCodec: runtime.NewParameterCodec(options.Scheme),
 		},
 		unstructuredClient: unstructuredClient{
-			client:     dynamicClient,
-			restMapper: options.Mapper,
+			cache:      clientcache,
+			paramCodec: noConversionParamCodec{},
 		},
 	}
 

pkg/client/client_cache.go

@@ -17,7 +17,6 @@ limitations under the License.
 package client
 
 import (
-	"reflect"
 	"strings"
 	"sync"
 
@@ -45,19 +44,14 @@ type clientCache struct {
 	codecs serializer.CodecFactory
 
 	// resourceByType caches type metadata
-	resourceByType map[reflect.Type]*resourceMeta
+	resourceByType map[schema.GroupVersionKind]*resourceMeta
 	mu             sync.RWMutex
 }
 
 // newResource maps obj to a Kubernetes Resource and constructs a client for that Resource.
 // If the object is a list, the resource represents the item's type instead.
-func (c *clientCache) newResource(obj runtime.Object) (*resourceMeta, error) {
-	gvk, err := apiutil.GVKForObject(obj, c.scheme)
-	if err != nil {
-		return nil, err
-	}
-
-	if strings.HasSuffix(gvk.Kind, "List") && meta.IsListType(obj) {
+func (c *clientCache) newResource(gvk schema.GroupVersionKind, isList bool) (*resourceMeta, error) {
+	if strings.HasSuffix(gvk.Kind, "List") && isList {
 		// if this was a list, treat it as a request for the item's resource
 		gvk.Kind = gvk.Kind[:len(gvk.Kind)-4]
 	}
@@ -76,12 +70,15 @@ func (c *clientCache) newResource(obj runtime.Object) (*resourceMeta, error) {
 // getResource returns the resource meta information for the given type of object.
 // If the object is a list, the resource represents the item's type instead.
 func (c *clientCache) getResource(obj runtime.Object) (*resourceMeta, error) {
-	typ := reflect.TypeOf(obj)
+	gvk, err := apiutil.GVKForObject(obj, c.scheme)
+	if err != nil {
+		return nil, err
+	}
 
 	// It's better to do creation work twice than to not let multiple
 	// people make requests at once
 	c.mu.RLock()
-	r, known := c.resourceByType[typ]
+	r, known := c.resourceByType[gvk]
 	c.mu.RUnlock()
 
 	if known {
@@ -91,11 +88,11 @@ func (c *clientCache) getResource(obj runtime.Object) (*resourceMeta, error) {
 	// Initialize a new Client
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	r, err := c.newResource(obj)
+	r, err = c.newResource(gvk, meta.IsListType(obj))
 	if err != nil {
 		return nil, err
 	}
-	c.resourceByType[typ] = r
+	c.resourceByType[gvk] = r
 	return r, err
 }
 

pkg/client/client_test.go

@@ -827,6 +827,57 @@ var _ = Describe("Client", func() {
 				close(done)
 			})
 
+			It("should update status and preserve type information", 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
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
+				err = cl.Status().Update(context.TODO(), u)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("validating updated Deployment has type information")
+				Expect(u.GroupVersionKind()).To(Equal(depGvk))
+
+				close(done)
+			})
+
+			It("should patch status and preserve type information", 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("patching the status of Deployment")
+				u := &unstructured.Unstructured{}
+				depPatch := client.MergeFrom(dep.DeepCopy())
+				dep.Status.Replicas = 1
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
+				err = cl.Status().Patch(context.TODO(), u, depPatch)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("validating updated Deployment has type information")
+				Expect(u.GroupVersionKind()).To(Equal(depGvk))
+
+				By("validating patched 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())

pkg/client/codec.go

@@ -0,0 +1,24 @@
+package client
+
+import (
+	"errors"
+	"net/url"
+
+	"k8s.io/apimachinery/pkg/conversion/queryparams"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+var _ runtime.ParameterCodec = noConversionParamCodec{}
+
+// noConversionParamCodec is a no-conversion codec for serializing parameters into URL query strings.
+// it's useful in scenarios with the unstructured client and arbitrary resouces.
+type noConversionParamCodec struct{}
+
+func (noConversionParamCodec) EncodeParameters(obj runtime.Object, to schema.GroupVersion) (url.Values, error) {
+	return queryparams.Convert(obj)
+}
+
+func (noConversionParamCodec) DecodeParameters(parameters url.Values, from schema.GroupVersion, into runtime.Object) error {
+	return errors.New("DecodeParameters not implemented on noConversionParamCodec")
+}

pkg/client/typed_client.go

@@ -25,7 +25,7 @@ import (
 // client is a client.Client that reads and writes directly from/to an API server.  It lazily initializes
 // new clients at the time they are used, and caches the client.
 type typedClient struct {
-	cache      clientCache
+	cache      *clientCache
 	paramCodec runtime.ParameterCodec
 }