knative
/
client
mirror of https://github.com/knative/client.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix exit code on service delete and revision delete (#971) 

* Fix exit code 0 upon service delete

* Mentioned bug fix in CHANGELOG.adoc

* Add error check test for service not found

* Fix for kn revision delete failure exit code and add test cases

* Testing changes in test cases for failure in intergration tests

* Fix test case error causing integration test failure
This commit is contained in:
Himanshu Ranjan 2020-08-06 16:11:28 +05:30 committed by GitHub
parent 7f2de9eb9d
commit 980a6a9469
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 94 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.adoc
View File

@ -34,6 +34,10 @@
| Fix Missing NAMESPACE column header for 'kn source list -A'
| https://github.com/knative/client/pull/951[#951]
| 🐛
| Fix exit code for `kn service delete` and `kn revision delete` failures
| https://github.com/knative/client/pull/971[#971]
|===
## v0.16.0 (2020-07-14)

4
lib/test/revision.go
View File

@ -60,11 +60,11 @@ func RevisionMultipleDelete(r *KnRunResultCollector, existRevision1, existRevisi
assert.Check(r.T(), strings.Contains(out.Stdout, existRevision2), "Required revision2 does not exist")
out = r.KnTest().Kn().Run("revision", "delete", existRevision1, existRevision2, nonexistRevision)
r.AssertNoError(out)
r.AssertError(out)
assert.Check(r.T(), util.ContainsAll(out.Stdout, "Revision", existRevision1, "deleted", "namespace", r.KnTest().Kn().Namespace()), "Failed to get 'deleted' first revision message")
assert.Check(r.T(), util.ContainsAll(out.Stdout, "Revision", existRevision2, "deleted", "namespace", r.KnTest().Kn().Namespace()), "Failed to get 'deleted' second revision message")
assert.Check(r.T(), util.ContainsAll(out.Stdout, "revisions.serving.knative.dev", nonexistRevision, "not found"), "Failed to get 'not found' error")
assert.Check(r.T(), util.ContainsAll(out.Stderr, "revisions.serving.knative.dev", nonexistRevision, "not found"), "Failed to get 'not found' error")
}
// RevisionDescribeWithPrintFlags verifies describing given revision using print flag '--output=name'

7
pkg/kn/commands/revision/delete.go
View File

@ -17,6 +17,7 @@ package revision
import (
"errors"
"fmt"
"strings"
"time"
"github.com/spf13/cobra"
@ -47,6 +48,7 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command {
return err
}
errs := []string{}
for _, name := range args {
timeout := time.Duration(0)
if waitFlags.Wait {
@ -54,11 +56,14 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command {
}
err = client.DeleteRevision(name, timeout)
if err != nil {
fmt.Fprintf(cmd.OutOrStdout(), "%s.\n", err)
errs = append(errs, err.Error())
} else {
fmt.Fprintf(cmd.OutOrStdout(), "Revision '%s' deleted in namespace '%s'.\n", name, namespace)
}
}
if len(errs) > 0 {
return errors.New("Error: " + strings.Join(errs, "\nError: "))
}
return nil
},
}

22
pkg/kn/commands/revision/delete_test.go
View File

@ -25,7 +25,9 @@ import (
clienttesting "k8s.io/client-go/testing"
"knative.dev/client/pkg/kn/commands"
clientservingv1 "knative.dev/client/pkg/serving/v1"
"knative.dev/client/pkg/util"
"knative.dev/client/pkg/util/mock"
"knative.dev/client/pkg/wait"
servingv1 "knative.dev/serving/pkg/apis/serving/v1"
)
@ -102,3 +104,23 @@ func getRevisionDeleteEvents(name string) []watch.Event {
{watch.Deleted, &servingv1.Revision{ObjectMeta: metav1.ObjectMeta{Name: name}}},
}
}
func TestRevisionDeleteCheckErrorForNotFoundRevisionsMock(t *testing.T) {
// New mock client
client := clientservingv1.NewMockKnServiceClient(t)
// Recording:
r := client.Recorder()
r.DeleteRevision("foo", mock.Any(), nil)
r.DeleteRevision("bar", mock.Any(), errors.New("revisions.serving.knative.dev \"bar\" not found."))
r.DeleteRevision("baz", mock.Any(), errors.New("revisions.serving.knative.dev \"baz\" not found."))
output, err := executeRevisionCommand(client, "delete", "foo", "bar", "baz")
if err == nil {
t.Fatal("Expected revision not found error, returned nil")
}
assert.Assert(t, util.ContainsAll(output, "'foo' deleted", "\"bar\" not found", "\"baz\" not found"))
r.Validate()
}

29
pkg/kn/commands/revision/revision_test.go
View File

@ -15,15 +15,24 @@
package revision
import (
"bytes"
"strings"
"testing"
"github.com/spf13/cobra"
"gotest.tools/assert"
"k8s.io/client-go/tools/clientcmd"
knflags "knative.dev/client/pkg/kn/flags"
servingv1 "knative.dev/serving/pkg/apis/serving/v1"
"knative.dev/client/pkg/kn/commands"
clientservingv1 "knative.dev/client/pkg/serving/v1"
"knative.dev/client/pkg/util"
)
// Helper methods
var blankConfig clientcmd.ClientConfig
func TestExtractTrafficAndTag(t *testing.T) {
service := &servingv1.Service{
@ -52,3 +61,23 @@ func createTarget(rev string, percent int64, tag string) servingv1.TrafficTarget
Percent: &percent,
}
}
func executeRevisionCommand(client clientservingv1.KnServingClient, args ...string) (string, error) {
knParams := &commands.KnParams{}
knParams.ClientConfig = blankConfig
output := new(bytes.Buffer)
knParams.Output = output
knParams.NewServingClient = func(namespace string) (clientservingv1.KnServingClient, error) {
return client, nil
}
cmd := NewRevisionCommand(knParams)
cmd.SetArgs(args)
cmd.SetOutput(output)
cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
return knflags.ReconcileBoolFlags(cmd.Flags())
}
err := cmd.Execute()
return output.String(), err
}

7
pkg/kn/commands/service/delete.go
View File

@ -17,6 +17,7 @@ package service
import (
"errors"
"fmt"
"strings"
"time"
"github.com/spf13/cobra"
@ -77,6 +78,7 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
}
}
errs := []string{}
for _, name := range args {
timeout := time.Duration(0)
if waitFlags.Wait {
@ -84,11 +86,14 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
}
err = client.DeleteService(name, timeout)
if err != nil {
fmt.Fprintf(cmd.OutOrStdout(), "%s.\n", err)
errs = append(errs, err.Error())
} else {
fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully deleted in namespace '%s'.\n", name, namespace)
}
}
if len(errs) > 0 {
return errors.New("Error: " + strings.Join(errs, "\nError: "))
}
return nil
},
}

21
pkg/kn/commands/service/delete_mock_test.go
View File

@ -15,6 +15,7 @@
package service
import (
"errors"
"testing"
"gotest.tools/assert"
@ -141,3 +142,23 @@ func TestServiceDeleteNoSvcNameMock(t *testing.T) {
r.Validate()
}
func TestServiceDeleteCheckErrorForNotFoundServicesMock(t *testing.T) {
// New mock client
client := clientservingv1.NewMockKnServiceClient(t)
// Recording:
r := client.Recorder()
r.DeleteService("foo", mock.Any(), nil)
r.DeleteService("bar", mock.Any(), errors.New("services.serving.knative.dev \"bar\" not found."))
r.DeleteService("baz", mock.Any(), errors.New("services.serving.knative.dev \"baz\" not found."))
output, err := executeServiceCommand(client, "delete", "foo", "bar", "baz")
if err == nil {
t.Fatal("Expected service not found error, returned nil")
}
assert.Assert(t, util.ContainsAll(output, "'foo' successfully deleted", "\"bar\" not found", "\"baz\" not found"))
r.Validate()
}

8
test/e2e/service_test.go
View File

@ -125,8 +125,8 @@ func serviceDeleteNonexistent(r *test.KnRunResultCollector, serviceName string)
assert.Check(r.T(), !strings.Contains(out.Stdout, serviceName), "The service exists")
out = r.KnTest().Kn().Run("service", "delete", serviceName)
r.AssertNoError(out)
assert.Check(r.T(), util.ContainsAll(out.Stdout, "hello", "not found"), "Failed to get 'not found' error")
r.AssertError(out)
assert.Check(r.T(), util.ContainsAll(out.Stderr, "hello", "not found"), "Failed to get 'not found' error")
}
func serviceMultipleDelete(r *test.KnRunResultCollector, existService, nonexistService string) {
@ -136,12 +136,12 @@ func serviceMultipleDelete(r *test.KnRunResultCollector, existService, nonexistS
assert.Check(r.T(), !strings.Contains(out.Stdout, nonexistService), "The service", nonexistService, " exists (but is supposed to be not)")
out = r.KnTest().Kn().Run("service", "delete", existService, nonexistService)
r.AssertNoError(out)
r.AssertError(out)
expectedSuccess := fmt.Sprintf(`Service '%s' successfully deleted in namespace '%s'.`, existService, r.KnTest().Kn().Namespace())
expectedErr := fmt.Sprintf(`services.serving.knative.dev "%s" not found`, nonexistService)
assert.Check(r.T(), strings.Contains(out.Stdout, expectedSuccess), "Failed to get 'successfully deleted' message")
assert.Check(r.T(), strings.Contains(out.Stdout, expectedErr), "Failed to get 'not found' error")
assert.Check(r.T(), strings.Contains(out.Stderr, expectedErr), "Failed to get 'not found' error")
}
func serviceUntagTagThatDoesNotExist(r *test.KnRunResultCollector, serviceName string) {