v0.15.2 release backports

CHANGELOG.adoc

@@ -12,6 +12,25 @@
 | https://github.com/knative/client/pull/[#]
 ////
 
+## v0.15.2 (2020-06-16)
+
+[cols="1,10,3", options="header", width="100%"]
+|===
+| | Description | PR
+
+| 🐛
+| Fix build.sh for macOS users
+| https://github.com/knative/client/pull/883[#883]
+
+| 🐛
+| Return error message when using --untag with nonexistent tag
+| https://github.com/knative/client/pull/880[#880]
+
+| ✨
+| Update go.mod to specify the module is go1.14
+| https://github.com/knative/client/pull/866[#866]
+|===
+
 ## v0.15.1 (2020-06-03)
 
 [cols="1,10,3", options="header", width="100%"]

go.mod

@@ -32,4 +32,4 @@ replace (
 	k8s.io/code-generator => k8s.io/code-generator v0.16.4
 )
 
-go 1.13
+go 1.14

hack/build.sh

@@ -110,21 +110,27 @@ go_fmt() {
   find $(echo $source_dirs) -name "*.go" -print0 | xargs -0 gofmt -s -w
 }
 
+# Run a go tool, get it first if necessary.
+run_go_tool() {
+  local tool=$2
+  local install_failed=0
+  if [ -z "$(which ${tool})" ]; then
+    local temp_dir="$(mktemp -d)"
+    pushd "${temp_dir}" > /dev/null 2>&1
+    GOFLAGS="" go get "$1" || install_failed=1
+    popd > /dev/null 2>&1
+    rm -rf "${temp_dir}"
+  fi
+  (( install_failed )) && return ${install_failed}
+  shift 2
+  ${tool} "$@"
+}
+
+
 source_format() {
   set +e
-  which goimports >/dev/null 2>&1
-  if [ $? -ne 0 ]; then
-     echo "✋ No 'goimports' found. Please use"
-     echo "✋   go install golang.org/x/tools/cmd/goimports"
-     echo "✋ to enable import cleanup. Import cleanup skipped."
-
-     # Run go fmt instead
-     go_fmt
-  else
-     echo "🧽 ${X}Format"
-     goimports -w $(echo $source_dirs)
-     find $(echo $source_dirs) -name "*.go" -print0 | xargs -0 gofmt -s -w
-  fi
+  run_go_tool golang.org/x/tools/cmd/goimports goimports -w $(echo $source_dirs)
+  find $(echo $source_dirs) -name "*.go" -print0 | xargs -0 gofmt -s -w
   set -e
 }
 

hack/verify-codegen.sh

@@ -20,9 +20,6 @@ set -o pipefail
 
 source $(dirname $0)/../scripts/test-infra/library.sh
 
-# Needed later
-go install golang.org/x/tools/cmd/goimports
-
 "${REPO_ROOT_DIR}"/hack/build.sh --codegen
 if output="$(git status --porcelain)" && [ -z "$output" ]; then
   echo "${REPO_ROOT_DIR} is up to date."

lib/test/service.go

@@ -72,6 +72,15 @@ func ServiceUpdate(r *KnRunResultCollector, serviceName string, args ...string)
 	assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, "updating", "service", serviceName, "ready"))
 }
 
+// ServiceUpdateWithError verifies service update operation with given arguments in sync mode
+// when expecting an error
+func ServiceUpdateWithError(r *KnRunResultCollector, serviceName string, args ...string) {
+	fullArgs := append([]string{}, "service", "update", serviceName)
+	fullArgs = append(fullArgs, args...)
+	out := r.KnTest().Kn().Run(fullArgs...)
+	r.AssertError(out)
+}
+
 // ServiceDelete verifies service deletion in sync mode
 func ServiceDelete(r *KnRunResultCollector, serviceName string) {
 	out := r.KnTest().Kn().Run("service", "delete", "--wait", serviceName)

pkg/kn/commands/service/update.go

@@ -92,7 +92,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
 				}
 
 				if trafficFlags.Changed(cmd) {
-					traffic, err := traffic.Compute(cmd, service.Spec.Traffic, &trafficFlags)
+					traffic, err := traffic.Compute(cmd, service.Spec.Traffic, &trafficFlags, service.Name)
 					if err != nil {
 						return nil, err
 					}

pkg/kn/commands/service/update_test.go

@@ -737,6 +737,15 @@ func TestServiceUpdateDeletionTimestampNotNil(t *testing.T) {
 	assert.ErrorContains(t, err, "service")
 }
 
+func TestServiceUpdateTagDoesNotExist(t *testing.T) {
+	orig := newEmptyService()
+
+	_, _, _, err := fakeServiceUpdate(orig, []string{
+		"service", "update", "foo", "--untag", "foo", "--no-wait"})
+
+	assert.Assert(t, util.ContainsAll(err.Error(), "tag(s)", "foo", "not present", "service", "foo"))
+}
+
 func newEmptyService() *servingv1.Service {
 	ret := &servingv1.Service{
 		TypeMeta: metav1.TypeMeta{

pkg/kn/traffic/compute.go

@@ -83,13 +83,15 @@ func (e ServiceTraffic) isTagPresent(tag string) bool {
 	return false
 }
 
-func (e ServiceTraffic) untagRevision(tag string) {
+func (e ServiceTraffic) untagRevision(tag string, serviceName string) bool {
 	for i, target := range e {
 		if target.Tag == tag {
 			e[i].Tag = ""
-			break
+			return true
 		}
 	}
+
+	return false
 }
 
 func (e ServiceTraffic) isRevisionPresent(revision string) bool {
@@ -267,7 +269,8 @@ func verifyInputSanity(trafficFlags *flags.Traffic) error {
 }
 
 // Compute takes service traffic targets and updates per given traffic flags
-func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget, trafficFlags *flags.Traffic) ([]servingv1.TrafficTarget, error) {
+func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget,
+	trafficFlags *flags.Traffic, serviceName string) ([]servingv1.TrafficTarget, error) {
 	err := verifyInputSanity(trafficFlags)
 	if err != nil {
 		return nil, err
@@ -276,8 +279,17 @@ func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget, trafficFlags
 	traffic := newServiceTraffic(targets)
 
 	// First precedence: Untag revisions
+	var errTagNames []string
 	for _, tag := range trafficFlags.UntagRevisions {
-		traffic.untagRevision(tag)
+		tagExists := traffic.untagRevision(tag, serviceName)
+		if !tagExists {
+			errTagNames = append(errTagNames, tag)
+		}
+	}
+
+	// Return all errors from untagging revisions
+	if len(errTagNames) > 0 {
+		return nil, fmt.Errorf("tag(s) %s not present for any revisions of service %s", strings.Join(errTagNames, ", "), serviceName)
 	}
 
 	for _, each := range trafficFlags.RevisionsTags {

pkg/kn/traffic/compute_test.go

@@ -193,7 +193,7 @@ func TestCompute(t *testing.T) {
 			testCmd, tFlags := newTestTrafficCommand()
 			testCmd.SetArgs(testCase.inputFlags)
 			testCmd.Execute()
-			targets, err := Compute(testCmd, testCase.existingTraffic, tFlags)
+			targets, err := Compute(testCmd, testCase.existingTraffic, tFlags, "serviceName")
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -278,12 +278,24 @@ func TestComputeErrMsg(t *testing.T) {
 			[]string{"--traffic", "echo-v1=40", "--traffic", "echo-v1=60"},
 			"repetition of revision reference echo-v1 is not allowed, use only once with --traffic flag",
 		},
+		{
+			"untag single tag that does not exist",
+			append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
+			[]string{"--untag", "foo"},
+			"tag(s) foo not present for any revisions of service serviceName",
+		},
+		{
+			"untag multiple tags that do not exist",
+			append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
+			[]string{"--untag", "foo", "--untag", "bar"},
+			"tag(s) foo, bar not present for any revisions of service serviceName",
+		},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
 			testCmd, tFlags := newTestTrafficCommand()
 			testCmd.SetArgs(testCase.inputFlags)
 			testCmd.Execute()
-			_, err := Compute(testCmd, testCase.existingTraffic, tFlags)
+			_, err := Compute(testCmd, testCase.existingTraffic, tFlags, "serviceName")
 			assert.Error(t, err, testCase.errMsg)
 		})
 	}

test/e2e/service_test.go

@@ -59,6 +59,10 @@ func TestService(t *testing.T) {
 	t.Log("create service private and make public")
 	serviceCreatePrivateUpdatePublic(r, "hello-private-public")
 
+	t.Log("error message from --untag with tag that doesn't exist")
+	test.ServiceCreate(r, "untag")
+	serviceUntagTagThatDoesNotExist(r, "untag")
+
 	t.Log("delete all services in a namespace")
 	test.ServiceCreate(r, "svc1")
 	test.ServiceCreate(r, "service2")
@@ -140,6 +144,15 @@ func serviceMultipleDelete(r *test.KnRunResultCollector, existService, nonexistS
 	assert.Check(r.T(), strings.Contains(out.Stdout, expectedErr), "Failed to get 'not found' error")
 }
 
+func serviceUntagTagThatDoesNotExist(r *test.KnRunResultCollector, serviceName string) {
+	out := r.KnTest().Kn().Run("service", "list", serviceName)
+	r.AssertNoError(out)
+	assert.Check(r.T(), strings.Contains(out.Stdout, serviceName), "Service "+serviceName+" does not exist for test (but should exist)")
+
+	out = r.KnTest().Kn().Run("service", "update", serviceName, "--untag", "foo", "--no-wait")
+	assert.Check(r.T(), util.ContainsAll(out.Stderr, "tag(s)", "foo", "not present", "service", "untag"), "Expected error message for using --untag with nonexistent tag")
+}
+
 func serviceDeleteAll(r *test.KnRunResultCollector) {
 	out := r.KnTest().Kn().Run("service", "list")
 	r.AssertNoError(out)