improved create service error message (#312)

* improved create service error message

* error pkg

* revisions error handling

* adding license

* project rebuild

* unit test improvements

* build updates

pkg/errors/errors.go

@@ -0,0 +1,28 @@
+// Copyright © 2019 The Knative Authors
+//
+// 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 errors
+
+import (
+	"fmt"
+	"strings"
+)
+
+func newInvalidCRD(apiGroup string) *KNError {
+	parts := strings.Split(apiGroup, ".")
+	name := parts[0]
+	msg := fmt.Sprintf("no Knative %s API found on the backend. Please verify the installation.", name)
+
+	return NewKNError(msg)
+}

pkg/errors/errors_test.go

@@ -0,0 +1,33 @@
+// Copyright © 2019 The Knative Authors
+//
+// 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 errors
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+)
+
+func TestNewInvalidCRD(t *testing.T) {
+	err := newInvalidCRD("serving.knative.dev")
+	assert.Error(t, err, "no Knative serving API found on the backend. Please verify the installation.")
+
+	err = newInvalidCRD("serving")
+	assert.Error(t, err, "no Knative serving API found on the backend. Please verify the installation.")
+
+	err = newInvalidCRD("")
+	assert.Error(t, err, "no Knative  API found on the backend. Please verify the installation.")
+
+}

pkg/errors/factory.go

@@ -0,0 +1,51 @@
+// Copyright © 2019 The Knative Authors
+//
+// 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 errors
+
+import (
+	"strings"
+
+	api_errors "k8s.io/apimachinery/pkg/api/errors"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func isCRDError(status api_errors.APIStatus) bool {
+	for _, cause := range status.Status().Details.Causes {
+		if strings.HasPrefix(cause.Message, "404") && cause.Type == v1.CauseTypeUnexpectedServerResponse {
+			return true
+		}
+	}
+
+	return false
+}
+
+//Retrieves a custom error struct based on the original error APIStatus struct
+//Returns the original error struct in case it can't identify the kind of APIStatus error
+func GetError(err error) error {
+	apiStatus, ok := err.(api_errors.APIStatus)
+	if !ok {
+		return err
+	}
+
+	var knerr *KNError
+
+	if isCRDError(apiStatus) {
+		knerr = newInvalidCRD(apiStatus.Status().Details.Group)
+		knerr.Status = apiStatus
+		return knerr
+	}
+
+	return err
+}

pkg/errors/factory_test.go

@@ -0,0 +1,94 @@
+// Copyright © 2019 The Knative Authors
+//
+// 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 errors
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+	api_errors "k8s.io/apimachinery/pkg/api/errors"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+func TestBuild(t *testing.T) {
+	cases := []struct {
+		Name        string
+		Schema      schema.GroupResource
+		StatusError func(schema.GroupResource) *api_errors.StatusError
+		ExpectedMsg string
+		Validate    func(t *testing.T, err error, msg string)
+	}{
+		{
+			Name: "Should get a missing serving api error",
+			Schema: schema.GroupResource{
+				Group:    "serving.knative.dev",
+				Resource: "service",
+			},
+			StatusError: func(resource schema.GroupResource) *api_errors.StatusError {
+				statusError := api_errors.NewNotFound(resource, "serv")
+				statusError.Status().Details.Causes = []v1.StatusCause{
+					{
+						Type:    "UnexpectedServerResponse",
+						Message: "404 page not found",
+					},
+				}
+				return statusError
+			},
+			ExpectedMsg: "no Knative serving API found on the backend. Please verify the installation.",
+			Validate: func(t *testing.T, err error, msg string) {
+				assert.Error(t, err, msg)
+			},
+		},
+		{
+			Name: "Should get the default not found error",
+			Schema: schema.GroupResource{
+				Group:    "serving.knative.dev",
+				Resource: "service",
+			},
+			StatusError: func(resource schema.GroupResource) *api_errors.StatusError {
+				return api_errors.NewNotFound(resource, "serv")
+			},
+			ExpectedMsg: "service.serving.knative.dev \"serv\" not found",
+			Validate: func(t *testing.T, err error, msg string) {
+				assert.Error(t, err, msg)
+			},
+		},
+		{
+			Name: "Should return the original error",
+			Schema: schema.GroupResource{
+				Group:    "serving.knative.dev",
+				Resource: "service",
+			},
+			StatusError: func(resource schema.GroupResource) *api_errors.StatusError {
+				return api_errors.NewAlreadyExists(resource, "serv")
+			},
+			ExpectedMsg: "service.serving.knative.dev \"serv\" already exists",
+			Validate: func(t *testing.T, err error, msg string) {
+				assert.Error(t, err, msg)
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Parallel()
+			statusError := tc.StatusError(tc.Schema)
+			err := GetError(statusError)
+			tc.Validate(t, err, tc.ExpectedMsg)
+		})
+	}
+}

pkg/errors/knerror.go

@@ -0,0 +1,25 @@
+// Copyright © 2019 The Knative Authors
+//
+// 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 errors
+
+func NewKNError(msg string) *KNError {
+	return &KNError{
+		msg: msg,
+	}
+}
+
+func (kne *KNError) Error() string {
+	return kne.msg
+}

pkg/errors/knerror_test.go

@@ -0,0 +1,37 @@
+// Copyright © 2019 The Knative Authors
+//
+// 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 errors
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+)
+
+func TestNewKNError(t *testing.T) {
+	err := NewKNError("myerror")
+	assert.Error(t, err, "myerror")
+
+	err = NewKNError("")
+	assert.Error(t, err, "")
+}
+
+func TestKNError_Error(t *testing.T) {
+	err := NewKNError("myerror")
+	assert.Equal(t, err.Error(), "myerror")
+
+	err = NewKNError("")
+	assert.Equal(t, err.Error(), "")
+}

pkg/errors/types.go

@@ -0,0 +1,22 @@
+// Copyright © 2019 The Knative Authors
+//
+// 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 errors
+
+import api_errors "k8s.io/apimachinery/pkg/api/errors"
+
+type KNError struct {
+	Status api_errors.APIStatus
+	msg    string
+}

pkg/serving/v1alpha1/client.go

@@ -24,6 +24,7 @@ import (
 	"github.com/knative/client/pkg/serving"
 	"github.com/knative/client/pkg/wait"
 
+	kn_errors "github.com/knative/client/pkg/errors"
 	api_serving "github.com/knative/serving/pkg/apis/serving"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	client_v1alpha1 "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
@@ -133,7 +134,7 @@ func NewKnServingClient(client client_v1alpha1.ServingV1alpha1Interface, namespa
 func (cl *knClient) GetService(name string) (*v1alpha1.Service, error) {
 	service, err := cl.client.Services(cl.namespace).Get(name, v1.GetOptions{})
 	if err != nil {
-		return nil, err
+		return nil, kn_errors.GetError(err)
 	}
 	err = serving.UpdateGroupVersionKind(service, v1alpha1.SchemeGroupVersion)
 	if err != nil {
@@ -146,7 +147,7 @@ func (cl *knClient) GetService(name string) (*v1alpha1.Service, error) {
 func (cl *knClient) ListServices(config ...ListConfig) (*v1alpha1.ServiceList, error) {
 	serviceList, err := cl.client.Services(cl.namespace).List(ListConfigs(config).toListOptions())
 	if err != nil {
-		return nil, err
+		return nil, kn_errors.GetError(err)
 	}
 	serviceListNew := serviceList.DeepCopy()
 	err = updateServingGvk(serviceListNew)
@@ -170,7 +171,7 @@ func (cl *knClient) ListServices(config ...ListConfig) (*v1alpha1.ServiceList, e
 func (cl *knClient) CreateService(service *v1alpha1.Service) error {
 	_, err := cl.client.Services(cl.namespace).Create(service)
 	if err != nil {
-		return err
+		return kn_errors.GetError(err)
 	}
 	return updateServingGvk(service)
 }
@@ -186,10 +187,15 @@ func (cl *knClient) UpdateService(service *v1alpha1.Service) error {
 
 // Delete a service by name
 func (cl *knClient) DeleteService(serviceName string) error {
-	return cl.client.Services(cl.namespace).Delete(
+	err := cl.client.Services(cl.namespace).Delete(
 		serviceName,
 		&v1.DeleteOptions{},
 	)
+	if err != nil {
+		return kn_errors.GetError(err)
+	}
+
+	return nil
 }
 
 // Wait for a service to become ready, but not longer than provided timeout
@@ -215,7 +221,7 @@ func (cl *knClient) GetConfiguration(name string) (*v1alpha1.Configuration, erro
 func (cl *knClient) GetRevision(name string) (*v1alpha1.Revision, error) {
 	revision, err := cl.client.Revisions(cl.namespace).Get(name, v1.GetOptions{})
 	if err != nil {
-		return nil, err
+		return nil, kn_errors.GetError(err)
 	}
 	err = updateServingGvk(revision)
 	if err != nil {
@@ -226,14 +232,19 @@ func (cl *knClient) GetRevision(name string) (*v1alpha1.Revision, error) {
 
 // Delete a revision by name
 func (cl *knClient) DeleteRevision(name string) error {
-	return cl.client.Revisions(cl.namespace).Delete(name, &v1.DeleteOptions{})
+	err := cl.client.Revisions(cl.namespace).Delete(name, &v1.DeleteOptions{})
+	if err != nil {
+		return kn_errors.GetError(err)
+	}
+
+	return nil
 }
 
 // List revisions
 func (cl *knClient) ListRevisions(config ...ListConfig) (*v1alpha1.RevisionList, error) {
 	revisionList, err := cl.client.Revisions(cl.namespace).List(ListConfigs(config).toListOptions())
 	if err != nil {
-		return nil, err
+		return nil, kn_errors.GetError(err)
 	}
 	return updateServingGvkForRevisionList(revisionList)
 }