Allow Create/Update/Delete kubectl commands to handle arbitrary objects

* Ensure kubectl uses abstractions from other parts of Kube
* Begin adding abstractions that allow arbitrary objects
* Refactor "update" to more closely match allowed behavior

pkg/kubectl/cmd/cmd.go

@@ -27,12 +27,20 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 )
 
+type Factory struct {
+	Mapper meta.RESTMapper
+	Typer  runtime.ObjectTyper
+	Client func(*cobra.Command, *meta.RESTMapping) (kubectl.RESTClient, error)
+}
+
 func RunKubectl(out io.Writer) {
 	// Parent command to which all subcommands are added.
 	cmds := &cobra.Command{
@@ -44,6 +52,15 @@ Find more information at https://github.com/GoogleCloudPlatform/kubernetes.`,
 		Run: runHelp,
 	}
 
+	factory := &Factory{
+		Mapper: latest.NewDefaultRESTMapper(),
+		Typer:  api.Scheme,
+		Client: func(cmd *cobra.Command, mapping *meta.RESTMapping) (kubectl.RESTClient, error) {
+			// Will handle all resources defined by the command
+			return getKubeClient(cmd), nil
+		},
+	}
+
 	// Globally persistent flags across all subcommands.
 	// TODO Change flag names to consts to allow safer lookup from subcommands.
 	// TODO Add a verbose flag that turns on glog logging. Probably need a way
@@ -63,9 +80,11 @@ Find more information at https://github.com/GoogleCloudPlatform/kubernetes.`,
 	cmds.AddCommand(NewCmdProxy(out))
 	cmds.AddCommand(NewCmdGet(out))
 	cmds.AddCommand(NewCmdDescribe(out))
-	cmds.AddCommand(NewCmdCreate(out))
+
-	cmds.AddCommand(NewCmdUpdate(out))
+	cmds.AddCommand(factory.NewCmdCreate(out))
-	cmds.AddCommand(NewCmdDelete(out))
+	cmds.AddCommand(factory.NewCmdUpdate(out))
+	cmds.AddCommand(factory.NewCmdDelete(out))
+
 	cmds.AddCommand(NewCmdNamespace(out))
 	cmds.AddCommand(NewCmdLog(out))
 	cmds.AddCommand(NewCmdCreateAll(out))

pkg/kubectl/cmd/create.go

@@ -17,13 +17,14 @@ limitations under the License.
 package cmd
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"
 	"github.com/spf13/cobra"
 )
 
-func NewCmdCreate(out io.Writer) *cobra.Command {
+func (f *Factory) NewCmdCreate(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "create -f filename",
 		Short: "Create a resource by filename or stdin",
@@ -40,13 +41,15 @@ Examples:
 		Run: func(cmd *cobra.Command, args []string) {
 			filename := getFlagString(cmd, "filename")
 			if len(filename) == 0 {
-				usageError(cmd, "Must pass a filename to update")
+				usageError(cmd, "Must specify filename to create")
 			}
-			data, err := readConfigData(filename)
+			mapping, namespace, name, data := ResourceFromFile(filename, f.Typer, f.Mapper)
+			client, err := f.Client(cmd, mapping)
 			checkErr(err)
 
-			err = kubectl.Modify(out, getKubeClient(cmd).RESTClient, getKubeNamespace(cmd), kubectl.ModifyCreate, data)
+			err = kubectl.NewRESTModifier(client, mapping).Create(namespace, data)
 			checkErr(err)
+			fmt.Fprintf(out, "%s\n", name)
 		},
 	}
 	cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to use to create the resource")

pkg/kubectl/cmd/delete.go

@@ -17,13 +17,14 @@ limitations under the License.
 package cmd
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"
 	"github.com/spf13/cobra"
 )
 
-func NewCmdDelete(out io.Writer) *cobra.Command {
+func (f *Factory) NewCmdDelete(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "delete ([-f filename] | (<resource> <id>))",
 		Short: "Delete a resource by filename, stdin or resource and id",
@@ -48,31 +49,14 @@ Examples:
   $ kubectl delete pod 1234-56-7890-234234-456456
   <delete a pod with ID 1234-56-7890-234234-456456>`,
 		Run: func(cmd *cobra.Command, args []string) {
-			// If command line args are passed in, use those preferentially.
-			if len(args) > 0 && len(args) != 2 {
-				usageError(cmd, "If passing in command line parameters, must be resource and name")
-			}
-
-			var data []byte
-			var err error
-
-			if len(args) == 2 {
-				data, err = kubectl.CreateResource(args[0], args[1])
-			} else {
 			filename := getFlagString(cmd, "filename")
-				if len(filename) > 0 {
+			mapping, namespace, name := ResourceFromArgsOrFile(cmd, args, filename, f.Typer, f.Mapper)
-					data, err = readConfigData(getFlagString(cmd, "filename"))
+			client, err := f.Client(cmd, mapping)
-				}
-			}
 			checkErr(err)
 
-			if len(data) == 0 {
+			err = kubectl.NewRESTModifier(client, mapping).Delete(namespace, name)
-				usageError(cmd, "Must specify filename or command line params")
-			}
-
-			// TODO Add ability to require a resource-version check for delete.
-			err = kubectl.Modify(out, getKubeClient(cmd).RESTClient, getKubeNamespace(cmd), kubectl.ModifyDelete, data)
 			checkErr(err)
+			fmt.Fprintf(out, "%s\n", name)
 		},
 	}
 	cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to use to delete the resource")

pkg/kubectl/cmd/resource.go

@@ -0,0 +1,92 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cmd
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+)
+
+// ResourceFromArgsOrFile expects two arguments or a valid file with a given type, and extracts
+// the fields necessary to uniquely locate a resource. Displays a usageError if that contract is
+// not satisfied, or a generic error if any other problems occur.
+func ResourceFromArgsOrFile(cmd *cobra.Command, args []string, filename string, typer runtime.ObjectTyper, mapper meta.RESTMapper) (mapping *meta.RESTMapping, namespace, name string) {
+	// If command line args are passed in, use those preferentially.
+	if len(args) > 0 && len(args) != 2 {
+		usageError(cmd, "If passing in command line parameters, must be resource and name")
+	}
+
+	if len(args) == 2 {
+		resource := args[0]
+		namespace = api.NamespaceDefault
+		name = args[1]
+		if len(name) == 0 || len(resource) == 0 {
+			usageError(cmd, "Must specify filename or command line params")
+		}
+
+		version, kind, err := mapper.VersionAndKindForResource(resource)
+		checkErr(err)
+
+		mapping, err = mapper.RESTMapping(version, kind)
+		checkErr(err)
+		return
+	}
+
+	if len(filename) == 0 {
+		usageError(cmd, "Must specify filename or command line params")
+	}
+
+	mapping, namespace, name, _ = ResourceFromFile(filename, typer, mapper)
+	if len(name) == 0 {
+		checkErr(fmt.Errorf("The resource in the provided file has no name (or ID) defined"))
+	}
+
+	return
+}
+
+func ResourceFromFile(filename string, typer runtime.ObjectTyper, mapper meta.RESTMapper) (mapping *meta.RESTMapping, namespace, name string, data []byte) {
+	configData, err := readConfigData(filename)
+	checkErr(err)
+	data = configData
+
+	version, kind, err := typer.DataVersionAndKind(data)
+	checkErr(err)
+
+	// TODO: allow unversioned objects?
+	if len(version) == 0 {
+		checkErr(fmt.Errorf("The resource in the provided file has no apiVersion defined"))
+	}
+
+	mapping, err = mapper.RESTMapping(version, kind)
+	checkErr(err)
+
+	obj, err := mapping.Codec.Decode(data)
+	checkErr(err)
+
+	meta := mapping.MetadataAccessor
+	namespace, err = meta.Namespace(obj)
+	checkErr(err)
+	name, err = meta.Name(obj)
+	checkErr(err)
+
+	return
+}

pkg/kubectl/cmd/update.go

@@ -17,13 +17,14 @@ limitations under the License.
 package cmd
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"
 	"github.com/spf13/cobra"
 )
 
-func NewCmdUpdate(out io.Writer) *cobra.Command {
+func (f *Factory) NewCmdUpdate(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "update -f filename",
 		Short: "Update a resource by filename or stdin",
@@ -40,14 +41,15 @@ Examples:
 		Run: func(cmd *cobra.Command, args []string) {
 			filename := getFlagString(cmd, "filename")
 			if len(filename) == 0 {
-				usageError(cmd, "Must pass a filename to update")
+				usageError(cmd, "Must specify filename to update")
 			}
-
+			mapping, namespace, name, data := ResourceFromFile(filename, f.Typer, f.Mapper)
-			data, err := readConfigData(filename)
+			client, err := f.Client(cmd, mapping)
 			checkErr(err)
 
-			err = kubectl.Modify(out, getKubeClient(cmd).RESTClient, getKubeNamespace(cmd), kubectl.ModifyUpdate, data)
+			err = kubectl.NewRESTModifier(client, mapping).Update(namespace, name, true, data)
 			checkErr(err)
+			fmt.Fprintf(out, "%s\n", name)
 		},
 	}
 	cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to use to update the resource")

pkg/kubectl/interfaces.go

@@ -0,0 +1,30 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kubectl
+
+import (
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+)
+
+// RESTClient is a client helper for dealing with RESTful resources
+// in a generic way.
+type RESTClient interface {
+	Get() *client.Request
+	Post() *client.Request
+	Delete() *client.Request
+	Put() *client.Request
+}

pkg/kubectl/kubectl.go

@@ -27,12 +27,11 @@ import (
 	"strings"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version"
+
 	"gopkg.in/v1/yaml"
 )
 
@@ -160,21 +159,6 @@ func makeImageList(manifest api.ContainerManifest) string {
 	return strings.Join(images, ",")
 }
 
-// Takes input 'data' as either json or yaml and attemps to decode it into the
-// supplied object.
-func dataToObject(data []byte) (runtime.Object, error) {
-	// This seems hacky but we can't get the codec from kubeClient.
-	versionInterfaces, err := latest.InterfacesFor(apiVersionToUse)
-	if err != nil {
-		return nil, err
-	}
-	obj, err := versionInterfaces.Codec.Decode(data)
-	if err != nil {
-		return nil, err
-	}
-	return obj, nil
-}
-
 const (
 	resolveToPath = "path"
 	resolveToKind = "kind"

pkg/kubectl/modify.go

@@ -17,150 +17,76 @@ limitations under the License.
 package kubectl
 
 import (
-	"fmt"
-	"io"
-
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 )
 
-type ModifyAction string
+// RESTModifier provides methods for mutating a known or unknown
+// RESTful resource.
+type RESTModifier struct {
+	Resource string
+	// A RESTClient capable of mutating this resource
+	RESTClient RESTClient
+	// A codec for decoding and encoding objects of this resource type.
+	Codec runtime.Codec
+	// An interface for reading or writing the resource version of this
+	// type.
+	Versioner runtime.ResourceVersioner
+}
 
-const (
+// NewRESTModifier creates a RESTModifier from a RESTMapping
-	ModifyCreate = ModifyAction("create")
+func NewRESTModifier(client RESTClient, mapping *meta.RESTMapping) *RESTModifier {
-	ModifyUpdate = ModifyAction("update")
+	return &RESTModifier{
-	ModifyDelete = ModifyAction("delete")
+		RESTClient: client,
-)
+		Resource:   mapping.Resource,
+		Codec:      mapping.Codec,
+		Versioner:  mapping.MetadataAccessor,
+	}
+}
 
-func Modify(w io.Writer, c *client.RESTClient, namespace string, action ModifyAction, data []byte) error {
+func (m *RESTModifier) Delete(namespace, name string) error {
-	if action != ModifyCreate && action != ModifyUpdate && action != ModifyDelete {
+	return m.RESTClient.Delete().Path(m.Resource).Path(name).Do().Error()
-		return fmt.Errorf("Action not recognized")
+}
+
+func (m *RESTModifier) Create(namespace string, data []byte) error {
+	return m.RESTClient.Post().Path(m.Resource).Body(data).Do().Error()
+}
+
+func (m *RESTModifier) Update(namespace, name string, overwrite bool, data []byte) error {
+	c := m.RESTClient
+
+	obj, err := m.Codec.Decode(data)
+	if err != nil {
+		// We don't know how to handle this object, but update it anyway
+		return c.Put().Path(m.Resource).Path(name).Body(data).Do().Error()
 	}
 
-	// TODO Support multiple API versions.
+	// Attempt to version the object based on client logic.
-	version, kind, err := versionAndKind(data)
+	version, err := m.Versioner.ResourceVersion(obj)
+	if err != nil {
+		// We don't know how to version this object, so send it to the server as is
+		return c.Put().Path(m.Resource).Path(name).Body(data).Do().Error()
+	}
+	if version == "" && overwrite {
+		// Retrieve the current version of the object to overwrite the server object
+		serverObj, err := c.Get().Path(m.Resource).Path(name).Do().Get()
+		if err != nil {
+			// The object does not exist, but we want it to be created
+			return c.Put().Path(m.Resource).Path(name).Body(data).Do().Error()
+		}
+		serverVersion, err := m.Versioner.ResourceVersion(serverObj)
 		if err != nil {
 			return err
 		}
-
+		if err := m.Versioner.SetResourceVersion(obj, serverVersion); err != nil {
-	if version != apiVersionToUse {
-		return fmt.Errorf("Only supporting API version '%s' for now (version '%s' specified)", apiVersionToUse, version)
-	}
-
-	obj, err := dataToObject(data)
-	if err != nil {
-		if err.Error() == "No type '' for version ''" {
-			return fmt.Errorf("Object could not be decoded. Make sure it has the Kind field defined.")
-		}
 			return err
 		}
-
+		newData, err := m.Codec.Encode(obj)
-	resource, err := resolveKindToResource(kind)
 		if err != nil {
 			return err
 		}
-
+		data = newData
-	var id string
-	switch action {
-	case "create":
-		id, err = doCreate(c, namespace, resource, data)
-	case "update":
-		id, err = doUpdate(c, namespace, resource, obj)
-	case "delete":
-		id, err = doDelete(c, namespace, resource, obj)
 	}
 
-	if err != nil {
+	return c.Put().Path(m.Resource).Path(name).Body(data).Do().Error()
-		return err
-	}
-
-	fmt.Fprintf(w, "%s\n", id)
-	return nil
-}
-
-// Creates the object then returns the ID of the newly created object.
-func doCreate(c *client.RESTClient, namespace string, resource string, data []byte) (string, error) {
-	obj, err := c.Post().Namespace(namespace).Path(resource).Body(data).Do().Get()
-	if err != nil {
-		return "", err
-	}
-	return getIDFromObj(obj)
-}
-
-// Creates the object then returns the ID of the newly created object.
-func doUpdate(c *client.RESTClient, namespace string, resource string, obj runtime.Object) (string, error) {
-	// Figure out the ID of the object to update by introspecting into the
-	// object.
-	id, err := getIDFromObj(obj)
-	if err != nil {
-		return "", fmt.Errorf("Name not retrievable from object for update: %v", err)
-	}
-
-	// Get the object from the server to find out its current resource
-	// version to prevent race conditions in updating the object.
-	serverObj, err := c.Get().Namespace(namespace).Path(resource).Path(id).Do().Get()
-	if err != nil {
-		return "", fmt.Errorf("Item Name %s does not exist for update: %v", id, err)
-	}
-	version, err := getResourceVersionFromObj(serverObj)
-	if err != nil {
-		return "", err
-	}
-
-	// Update the object we are trying to send to the server with the
-	// correct resource version.
-	meta, err := meta.Accessor(obj)
-	if err != nil {
-		return "", err
-	}
-	meta.SetResourceVersion(version)
-
-	// Convert object with updated resourceVersion to data for PUT.
-	data, err := c.Codec.Encode(obj)
-	if err != nil {
-		return "", err
-	}
-
-	// Do the update.
-	err = c.Put().Namespace(namespace).Path(resource).Path(id).Body(data).Do().Error()
-	fmt.Printf("r: %q, i: %q, d: %s", resource, id, data)
-	if err != nil {
-		return "", err
-	}
-
-	return id, nil
-}
-
-func doDelete(c *client.RESTClient, namespace string, resource string, obj runtime.Object) (string, error) {
-	id, err := getIDFromObj(obj)
-	if err != nil {
-		return "", fmt.Errorf("Name not retrievable from object for delete: %v", err)
-	}
-	if id == "" {
-		return "", fmt.Errorf("The supplied resource has no Name and cannot be deleted")
-	}
-
-	err = c.Delete().Namespace(namespace).Path(resource).Path(id).Do().Error()
-	if err != nil {
-		return "", err
-	}
-
-	return id, nil
-}
-
-func getIDFromObj(obj runtime.Object) (string, error) {
-	meta, err := meta.Accessor(obj)
-	if err != nil {
-		return "", err
-	}
-	return meta.Name(), nil
-}
-
-func getResourceVersionFromObj(obj runtime.Object) (string, error) {
-	meta, err := meta.Accessor(obj)
-	if err != nil {
-		return "", err
-	}
-	return meta.ResourceVersion(), nil
 }

pkg/kubectl/modify_test.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kubectl
+
+import (
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+)
+
+type FakeRESTClient struct{}
+
+func (c *FakeRESTClient) Get() *client.Request {
+	return &client.Request{}
+}
+func (c *FakeRESTClient) Put() *client.Request {
+	return &client.Request{}
+}
+func (c *FakeRESTClient) Post() *client.Request {
+	return &client.Request{}
+}
+func (c *FakeRESTClient) Delete() *client.Request {
+	return &client.Request{}
+}
+
+func TestRESTModifierDelete(t *testing.T) {
+	tests := []struct {
+		Err bool
+	}{
+	/*{
+		Err: true,
+	},*/
+	}
+	for _, test := range tests {
+		client := &FakeRESTClient{}
+		modifier := &RESTModifier{
+			RESTClient: client,
+		}
+		err := modifier.Delete("bar", "foo")
+		switch {
+		case err == nil && test.Err:
+			t.Errorf("Unexpected non-error")
+			continue
+		case err != nil && !test.Err:
+			t.Errorf("Unexpected error: %v", err)
+			continue
+		}
+	}
+}

pkg/runtime/scheme.go

@@ -176,7 +176,7 @@ func (s *Scheme) KnownTypes(version string) map[string]reflect.Type {
 }
 
 // DataVersionAndKind will return the APIVersion and Kind of the given wire-format
-// enconding of an API Object, or an error.
+// encoding of an API Object, or an error.
 func (s *Scheme) DataVersionAndKind(data []byte) (version, kind string, err error) {
 	return s.raw.DataVersionAndKind(data)
 }