Merge pull request #7030 from liggitt/skip_fetch_delete

Skip fetching from server when deleting a named resource

pkg/kubectl/cmd/delete.go

@@ -87,7 +87,7 @@ func RunDelete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str
 		FilenameParam(filenames...).
 		SelectorParam(cmdutil.GetFlagString(cmd, "selector")).
 		SelectAllParam(cmdutil.GetFlagBool(cmd, "all")).
-		ResourceTypeOrNameArgs(false, args...).
+		ResourceTypeOrNameArgs(false, args...).RequireObject(false).
 		Flatten().
 		Do()
 	err = r.Err()

pkg/kubectl/cmd/delete_test.go

@@ -27,6 +27,66 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 )
 
+func TestDeleteObjectByTuple(t *testing.T) {
+	_, _, rc := testData()
+
+	f, tf, codec := NewAPIFactory()
+	tf.Printer = &testPrinter{}
+	tf.Client = &client.FakeRESTClient{
+		Codec: codec,
+		Client: client.HTTPClientFunc(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == "/namespaces/test/replicationcontrollers/redis-master-controller" && m == "DELETE":
+				return &http.Response{StatusCode: 200, Body: objBody(codec, &rc.Items[0])}, nil
+			default:
+				// Ensures no GET is performed when deleting by name
+				t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
+				return nil, nil
+			}
+		}),
+	}
+	tf.Namespace = "test"
+	buf := bytes.NewBuffer([]byte{})
+
+	cmd := NewCmdDelete(f, buf)
+	cmd.Flags().Set("namespace", "test")
+	cmd.Run(cmd, []string{"replicationcontrollers/redis-master-controller"})
+
+	if buf.String() != "replicationControllers/redis-master-controller\n" {
+		t.Errorf("unexpected output: %s", buf.String())
+	}
+}
+
+func TestDeleteNamedObject(t *testing.T) {
+	_, _, rc := testData()
+
+	f, tf, codec := NewAPIFactory()
+	tf.Printer = &testPrinter{}
+	tf.Client = &client.FakeRESTClient{
+		Codec: codec,
+		Client: client.HTTPClientFunc(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == "/namespaces/test/replicationcontrollers/redis-master-controller" && m == "DELETE":
+				return &http.Response{StatusCode: 200, Body: objBody(codec, &rc.Items[0])}, nil
+			default:
+				// Ensures no GET is performed when deleting by name
+				t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
+				return nil, nil
+			}
+		}),
+	}
+	tf.Namespace = "test"
+	buf := bytes.NewBuffer([]byte{})
+
+	cmd := NewCmdDelete(f, buf)
+	cmd.Flags().Set("namespace", "test")
+	cmd.Run(cmd, []string{"replicationcontrollers", "redis-master-controller"})
+
+	if buf.String() != "replicationControllers/redis-master-controller\n" {
+		t.Errorf("unexpected output: %s", buf.String())
+	}
+}
+
 func TestDeleteObject(t *testing.T) {
 	_, _, rc := testData()
 

pkg/kubectl/resource/builder.go

@@ -58,6 +58,8 @@ type Builder struct {
 	flatten bool
 	latest  bool
 
+	requireObject bool
+
 	singleResourceType bool
 	continueOnError    bool
 }
@@ -70,7 +72,8 @@ type resourceTuple struct {
 // NewBuilder creates a builder that operates on generic objects.
 func NewBuilder(mapper meta.RESTMapper, typer runtime.ObjectTyper, clientMapper ClientMapper) *Builder {
 	return &Builder{
-		mapper: &Mapper{typer, mapper, clientMapper},
+		mapper:        &Mapper{typer, mapper, clientMapper},
+		requireObject: true,
 	}
 }
 
@@ -327,6 +330,12 @@ func (b *Builder) Latest() *Builder {
 	return b
 }
 
+// RequireObject ensures that resulting infos have an object set. If false, resulting info may not have an object set.
+func (b *Builder) RequireObject(require bool) *Builder {
+	b.requireObject = require
+	return b
+}
+
 // ContinueOnError will attempt to load and visit as many objects as possible, even if some visits
 // return errors or some objects cannot be loaded. The default behavior is to terminate after
 // the first error is returned from a VisitorFunc.
@@ -537,9 +546,6 @@ func (b *Builder) visitorResult() *Result {
 		visitors := []Visitor{}
 		for _, name := range b.names {
 			info := NewInfo(client, mapping, selectorNamespace, name)
-			if err := info.Get(); err != nil {
-				return &Result{singular: isSingular, err: err}
-			}
 			visitors = append(visitors, info)
 		}
 		return &Result{singular: isSingular, visitor: VisitorList(visitors), sources: visitors}
@@ -593,7 +599,7 @@ func (b *Builder) Do() *Result {
 		helpers = append(helpers, RequireNamespace(b.namespace))
 	}
 	helpers = append(helpers, FilterNamespace)
-	if b.latest {
+	if b.requireObject {
 		helpers = append(helpers, RetrieveLazy)
 	}
 	r.visitor = NewDecoratedVisitor(r.visitor, helpers...)

pkg/kubectl/resource/builder_test.go

@@ -58,7 +58,7 @@ func fakeClient() ClientMapper {
 	})
 }
 
-func fakeClientWith(t *testing.T, data map[string]string) ClientMapper {
+func fakeClientWith(testName string, t *testing.T, data map[string]string) ClientMapper {
 	return ClientMapperFunc(func(*meta.RESTMapping) (RESTClient, error) {
 		return &client.FakeRESTClient{
 			Codec: latest.Codec,
@@ -70,7 +70,7 @@ func fakeClientWith(t *testing.T, data map[string]string) ClientMapper {
 				}
 				body, ok := data[p]
 				if !ok {
-					t.Fatalf("unexpected request: %s (%s)\n%#v", p, req.URL, req)
+					t.Fatalf("%s: unexpected request: %s (%s)\n%#v", testName, p, req.URL, req)
 				}
 				return &http.Response{
 					StatusCode: http.StatusOK,
@@ -305,7 +305,7 @@ func TestURLBuilderRequireNamespace(t *testing.T) {
 
 func TestResourceByName(t *testing.T) {
 	pods, _ := testData()
-	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{
 		"/namespaces/test/pods/foo": runtime.EncodeOrDie(latest.Codec, &pods.Items[0]),
 	})).
 		NamespaceParam("test")
@@ -336,9 +336,42 @@ func TestResourceByName(t *testing.T) {
 	}
 }
 
+func TestResourceByNameWithoutRequireObject(t *testing.T) {
+	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{})).
+		NamespaceParam("test")
+
+	test := &testVisitor{}
+	singular := false
+
+	if b.Do().Err() == nil {
+		t.Errorf("unexpected non-error")
+	}
+
+	b.ResourceTypeOrNameArgs(true, "pods", "foo").RequireObject(false)
+
+	err := b.Do().IntoSingular(&singular).Visit(test.Handle)
+	if err != nil || !singular || len(test.Infos) != 1 {
+		t.Fatalf("unexpected response: %v %t %#v", err, singular, test.Infos)
+	}
+	if test.Infos[0].Name != "foo" {
+		t.Errorf("unexpected name: %#v", test.Infos[0].Name)
+	}
+	if test.Infos[0].Object != nil {
+		t.Errorf("unexpected object: %#v", test.Infos[0].Object)
+	}
+
+	mapping, err := b.Do().ResourceMapping()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if mapping.Kind != "Pod" || mapping.Resource != "pods" {
+		t.Errorf("unexpected resource mapping: %#v", mapping)
+	}
+}
+
 func TestResourceByNameAndEmptySelector(t *testing.T) {
 	pods, _ := testData()
-	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{
 		"/namespaces/test/pods/foo": runtime.EncodeOrDie(latest.Codec, &pods.Items[0]),
 	})).
 		NamespaceParam("test").
@@ -366,7 +399,7 @@ func TestResourceByNameAndEmptySelector(t *testing.T) {
 func TestSelector(t *testing.T) {
 	pods, svc := testData()
 	labelKey := api.LabelSelectorQueryParam(testapi.Version())
-	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{
 		"/namespaces/test/pods?" + labelKey + "=a%3Db":     runtime.EncodeOrDie(latest.Codec, pods),
 		"/namespaces/test/services?" + labelKey + "=a%3Db": runtime.EncodeOrDie(latest.Codec, svc),
 	})).
@@ -467,35 +500,43 @@ func TestResourceTuple(t *testing.T) {
 		},
 	}
 	for k, testCase := range testCases {
-		pods, _ := testData()
+		for _, requireObject := range []bool{true, false} {
-		b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+			expectedRequests := map[string]string{}
-			"/namespaces/test/pods/foo": runtime.EncodeOrDie(latest.Codec, &pods.Items[0]),
+			if requireObject {
-			"/namespaces/test/pods/bar": runtime.EncodeOrDie(latest.Codec, &pods.Items[0]),
+				pods, _ := testData()
-			"/nodes/foo":                runtime.EncodeOrDie(latest.Codec, &api.Node{ObjectMeta: api.ObjectMeta{Name: "foo"}}),
+				expectedRequests = map[string]string{
-		})).
+					"/namespaces/test/pods/foo": runtime.EncodeOrDie(latest.Codec, &pods.Items[0]),
-			NamespaceParam("test").DefaultNamespace().
+					"/namespaces/test/pods/bar": runtime.EncodeOrDie(latest.Codec, &pods.Items[0]),
-			ResourceTypeOrNameArgs(true, testCase.args...)
+					"/nodes/foo":                runtime.EncodeOrDie(latest.Codec, &api.Node{ObjectMeta: api.ObjectMeta{Name: "foo"}}),
+					"/minions/foo":              runtime.EncodeOrDie(latest.Codec, &api.Node{ObjectMeta: api.ObjectMeta{Name: "foo"}}),
+				}
+			}
 
-		r := b.Do()
+			b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(k, t, expectedRequests)).
+				NamespaceParam("test").DefaultNamespace().
+				ResourceTypeOrNameArgs(true, testCase.args...).RequireObject(requireObject)
 
-		if !testCase.errFn(r.Err()) {
+			r := b.Do()
-			t.Errorf("%s: unexpected error: %v", k, r.Err())
+
-		}
+			if !testCase.errFn(r.Err()) {
-		if r.Err() != nil {
+				t.Errorf("%s: unexpected error: %v", k, r.Err())
-			continue
+			}
-		}
+			if r.Err() != nil {
-		switch {
+				continue
-		case (r.singular && len(testCase.args) != 1),
+			}
-			(!r.singular && len(testCase.args) == 1):
+			switch {
-			t.Errorf("%s: result had unexpected singular value", k)
+			case (r.singular && len(testCase.args) != 1),
-		}
+				(!r.singular && len(testCase.args) == 1):
-		info, err := r.Infos()
+				t.Errorf("%s: result had unexpected singular value", k)
-		if err != nil {
+			}
-			// test error
+			info, err := r.Infos()
-			continue
+			if err != nil {
-		}
+				// test error
-		if len(info) != len(testCase.args) {
+				continue
-			t.Errorf("%s: unexpected number of infos returned: %#v", k, info)
+			}
+			if len(info) != len(testCase.args) {
+				t.Errorf("%s: unexpected number of infos returned: %#v", k, info)
+			}
 		}
 	}
 }
@@ -579,7 +620,7 @@ func TestSingularObject(t *testing.T) {
 func TestListObject(t *testing.T) {
 	pods, _ := testData()
 	labelKey := api.LabelSelectorQueryParam(testapi.Version())
-	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{
 		"/namespaces/test/pods?" + labelKey + "=a%3Db": runtime.EncodeOrDie(latest.Codec, pods),
 	})).
 		SelectorParam("a=b").
@@ -612,7 +653,7 @@ func TestListObject(t *testing.T) {
 func TestListObjectWithDifferentVersions(t *testing.T) {
 	pods, svc := testData()
 	labelKey := api.LabelSelectorQueryParam(testapi.Version())
-	obj, err := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+	obj, err := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{
 		"/namespaces/test/pods?" + labelKey + "=a%3Db":     runtime.EncodeOrDie(latest.Codec, pods),
 		"/namespaces/test/services?" + labelKey + "=a%3Db": runtime.EncodeOrDie(latest.Codec, svc),
 	})).
@@ -638,7 +679,7 @@ func TestListObjectWithDifferentVersions(t *testing.T) {
 
 func TestWatch(t *testing.T) {
 	_, svc := testData()
-	w, err := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+	w, err := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{
 		"/watch/namespaces/test/services/redis-master?resourceVersion=12": watchBody(watch.Event{
 			Type:   watch.Added,
 			Object: &svc.Items[0],
@@ -693,7 +734,7 @@ func TestLatest(t *testing.T) {
 		ObjectMeta: api.ObjectMeta{Name: "baz", Namespace: "test", ResourceVersion: "15"},
 	}
 
-	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith(t, map[string]string{
+	b := NewBuilder(latest.RESTMapper, api.Scheme, fakeClientWith("", t, map[string]string{
 		"/namespaces/test/pods/foo":     runtime.EncodeOrDie(latest.Codec, newPod),
 		"/namespaces/test/pods/bar":     runtime.EncodeOrDie(latest.Codec, newPod2),
 		"/namespaces/test/services/baz": runtime.EncodeOrDie(latest.Codec, newSvc),