Skip to content

Commit

Permalink
Backports for client v0.15.2 release
Browse files Browse the repository at this point in the history
 - Updates the vendor/modules.txt
 - Update CHANGELOG
 - Squashed commits for
   knative#866
   knative#880
   knative#883
  • Loading branch information
navidshaikh committed Jun 16, 2020
1 parent 0cac223 commit 88f15b4
Show file tree
Hide file tree
Showing 12 changed files with 194 additions and 39 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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%"]
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ replace (
k8s.io/code-generator => k8s.io/code-generator v0.16.4
)

go 1.13
go 1.14
32 changes: 19 additions & 13 deletions hack/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 0 additions & 3 deletions hack/verify-codegen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
9 changes: 9 additions & 0 deletions lib/test/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/kn/commands/service/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
20 changes: 16 additions & 4 deletions pkg/kn/traffic/compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
16 changes: 14 additions & 2 deletions pkg/kn/traffic/compute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
})
}
Expand Down
13 changes: 13 additions & 0 deletions test/e2e/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 88f15b4

Please sign in to comment.