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

Calculate traffic split when N-1 revisions are specified (#1483) 

* Calculate traffic split when N-1 revisions are specified

* Added test cases for traffic split

* enhanced error messages and added example

* added e2e test

* refactored verifyInput function

* Update docs/cmd/kn_service_update.md

Co-authored-by: Navid Shaikh <shaikhnavid14@gmail.com>

* Update pkg/kn/commands/service/update.go

Co-authored-by: Navid Shaikh <shaikhnavid14@gmail.com>

* added unit test for revision list error

Co-authored-by: Navid Shaikh <shaikhnavid14@gmail.com>
This commit is contained in:
Gunjan Vyas 2021-10-30 01:54:59 +05:30 committed by GitHub
parent 6c2f202d56
commit d8d0ee1e6e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 463 additions and 197 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
docs/cmd/kn_service_update.md
View File

@ -30,6 +30,10 @@ kn service update NAME
# Add tag 'test' to echo-v3 revision with 10% traffic and rest to latest ready revision of service # Add tag 'test' to echo-v3 revision with 10% traffic and rest to latest ready revision of service
kn service update svc --tag echo-v3=test --traffic test=10,@latest=90 kn service update svc --tag echo-v3=test --traffic test=10,@latest=90
# Split 50% traffic to stable, 40% traffic to staging and the
# rest will automatically be directed to echo-v3 (the remaining revision)
kn service update svc --traffic stable=50,staging=40
# Update the service in offline mode instead of kubernetes cluster # Update the service in offline mode instead of kubernetes cluster
kn service update gitopstest -n test-ns --env KEY1=VALUE1 --target=/user/knfiles kn service update gitopstest -n test-ns --env KEY1=VALUE1 --target=/user/knfiles
kn service update gitopstest --env KEY1=VALUE1 --target=/user/knfiles/test.yaml kn service update gitopstest --env KEY1=VALUE1 --target=/user/knfiles/test.yaml

10
pkg/kn/commands/service/update.go
View File

@ -51,6 +51,10 @@ var updateExample = `
# Add tag 'test' to echo-v3 revision with 10% traffic and rest to latest ready revision of service # Add tag 'test' to echo-v3 revision with 10% traffic and rest to latest ready revision of service
kn service update svc --tag echo-v3=test --traffic test=10,@latest=90 kn service update svc --tag echo-v3=test --traffic test=10,@latest=90
# Split 50% traffic to stable, 40% traffic to staging and the
# rest will automatically be directed to echo-v3 (the remaining revision)
kn service update svc --traffic stable=50,staging=40
# Update the service in offline mode instead of kubernetes cluster # Update the service in offline mode instead of kubernetes cluster
kn service update gitopstest -n test-ns --env KEY1=VALUE1 --target=/user/knfiles kn service update gitopstest -n test-ns --env KEY1=VALUE1 --target=/user/knfiles
kn service update gitopstest --env KEY1=VALUE1 --target=/user/knfiles/test.yaml kn service update gitopstest --env KEY1=VALUE1 --target=/user/knfiles/test.yaml
@ -99,7 +103,11 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
} }
if trafficFlags.Changed(cmd) { if trafficFlags.Changed(cmd) {
traffic, err := traffic.Compute(cmd, service.Spec.Traffic, &trafficFlags, service.Name) revisions, err := client.ListRevisions(cmd.Context(), clientservingv1.WithService(service.Name))
if err != nil {
return nil, err
}
traffic, err := traffic.Compute(cmd, service.Spec.Traffic, &trafficFlags, service.Name, revisions.Items)
if err != nil { if err != nil {
return nil, err return nil, err
} }

32
pkg/kn/commands/service/update_test.go
View File

@ -98,6 +98,30 @@ func fakeServiceUpdate(original *servingv1.Service, args []string) (
} }
return true, rev, nil return true, rev, nil
}) })
fakeServing.AddReactor("list", "revisions",
func(a clienttesting.Action) (bool, runtime.Object, error) {
var err error
listAction, ok := a.(clienttesting.ListAction)
if !ok {
return true, nil, fmt.Errorf("wrong kind of action %v", action)
}
if reqs, _ := listAction.GetListRestrictions().Labels.Requirements(); len(reqs) == 1 {
if strings.Contains(reqs[0].String(), "foo-err") {
err = fmt.Errorf("revision list error")
}
}
rev := &servingv1.Revision{}
rev.Spec = original.Spec.Template.Spec
rev.ObjectMeta = original.Spec.Template.ObjectMeta
rev.Name = original.Status.LatestCreatedRevisionName
rev.Status.ContainerStatuses = []servingv1.ContainerStatus{
{ImageDigest: exampleImageByDigest, Name: "user-container"},
}
revList := &servingv1.RevisionList{
Items: []servingv1.Revision{*rev},
}
return true, revList, err
})
if sync { if sync {
fakeServing.AddWatchReactor("services", fakeServing.AddWatchReactor("services",
@ -1019,6 +1043,14 @@ func TestServiceUpdateTagDoesNotExist(t *testing.T) {
assert.Assert(t, util.ContainsAll(err.Error(), "tag(s)", "foo", "not present", "service", "foo")) assert.Assert(t, util.ContainsAll(err.Error(), "tag(s)", "foo", "not present", "service", "foo"))
} }
func TestServiceUpdateRevisionListError(t *testing.T) {
orig := newEmptyService()
orig.Name = "foo-err"
_, _, _, err := fakeServiceUpdate(orig, []string{"service", "update", "foo-err", "--traffic", "@latest=100"})
assert.Assert(t, util.ContainsAll(err.Error(), "revision list error"))
}
func newEmptyService() *servingv1.Service { func newEmptyService() *servingv1.Service {
ret := &servingv1.Service{ ret := &servingv1.Service{
TypeMeta: metav1.TypeMeta{ TypeMeta: metav1.TypeMeta{

156
pkg/kn/traffic/compute.go
View File

@ -20,6 +20,7 @@ import (
"strings" "strings"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"knative.dev/client/pkg/kn/commands/revision"
"knative.dev/pkg/ptr" "knative.dev/pkg/ptr"
servingv1 "knative.dev/serving/pkg/apis/serving/v1" servingv1 "knative.dev/serving/pkg/apis/serving/v1"
@ -28,6 +29,12 @@ import (
var latestRevisionRef = "@latest" var latestRevisionRef = "@latest"
const (
errorDistributionRevisionCount = iota
errorDistributionLatestTag
errorDistributionRevisionNotFound
)
// ServiceTraffic type for operating on service traffic targets // ServiceTraffic type for operating on service traffic targets
type ServiceTraffic []servingv1.TrafficTarget type ServiceTraffic []servingv1.TrafficTarget
@ -212,14 +219,116 @@ func errorRepeatingRevision(forFlag string, name string) error {
"is not allowed, use only once with %s flag", name, forFlag) "is not allowed, use only once with %s flag", name, forFlag)
} }
func errorTrafficDistribution(sum int, reason int) error {
errMsg := ""
switch reason {
case errorDistributionRevisionCount:
errMsg = "incorrect number of revisions specified. Only 1 revision should be missing"
case errorDistributionLatestTag:
errMsg = "cannot determine traffic split when @latest tag is specified"
case errorDistributionRevisionNotFound:
errMsg = "cannot determine the missing revision"
}
return fmt.Errorf("unable to allocate the remaining traffic %d. %s", 100-sum, errMsg)
}
func errorSumGreaterThan100(sum int) error {
return fmt.Errorf("given traffic percents sum to %d, want <=100", sum)
}
// verifies if user has repeated @latest field in --tag or --traffic flags // verifies if user has repeated @latest field in --tag or --traffic flags
// verifyInput checks: // verifyInput checks:
// - if user has repeated @latest field in --tag or --traffic flags // - if user has repeated @latest field in --tag or --traffic flags
// - if provided traffic portion are integers // - if provided traffic portion are integers
func verifyInput(trafficFlags *flags.Traffic) error { func verifyInput(trafficFlags *flags.Traffic, revisions []servingv1.Revision) error {
var latestRevisionTag = false // check if traffic is being sent to @latest tag
var sum = 0 var latestRefTraffic bool
// number of revisions
var revisionCount = len(revisions)
err := verifyLatestTag(trafficFlags)
if err != nil {
return err
}
revisionRefMap, sum, err := verifyRevisionSumAndReferences(trafficFlags)
if err != nil {
return err
}
if _, ok := revisionRefMap[latestRevisionRef]; ok {
// traffic has been routed to @latest tag
latestRefTraffic = true
}
// number of revisions specified in traffic flags
specRevPercentCount := len(trafficFlags.RevisionsPercentages)
// equivalent check for `cmd.Flags().Changed("traffic")` as we don't have `cmd` in this function
if specRevPercentCount > 0 {
if sum > 100 {
return errorSumGreaterThan100(sum)
}
if sum < 100 && specRevPercentCount != revisionCount-1 {
return errorTrafficDistribution(sum, errorDistributionRevisionCount)
}
if sum < 100 && specRevPercentCount == revisionCount-1 {
if latestRefTraffic {
return errorTrafficDistribution(sum, errorDistributionLatestTag)
}
for _, rev := range revisions {
if !checkRevisionPresent(revisionRefMap, rev) {
trafficFlags.RevisionsPercentages = append(trafficFlags.RevisionsPercentages, fmt.Sprintf("%s=%d", rev.Name, 100-sum))
return nil
}
}
return errorTrafficDistribution(sum, errorDistributionRevisionNotFound)
}
}
return nil
}
func verifyRevisionSumAndReferences(trafficFlags *flags.Traffic) (revisionRefMap map[string]int, sum int, err error) {
revisionRefMap = make(map[string]int)
for i, each := range trafficFlags.RevisionsPercentages {
var revisionRef, percent string
revisionRef, percent, err = splitByEqualSign(each)
if err != nil {
return
}
// To check if there are duplicate revision names in traffic flags
if _, exist := revisionRefMap[revisionRef]; exist {
err = errorRepeatingRevision("--traffic", revisionRef)
return
}
revisionRefMap[revisionRef] = i
var percentInt int
percentInt, err = strconv.Atoi(percent)
if err != nil {
err = errorParsingInteger(percent)
return
}
if percentInt < 0 || percentInt > 100 {
err = fmt.Errorf("invalid value for traffic percent %d, expected 0 <= percent <= 100", percentInt)
return
}
sum += percentInt
}
return
}
func errorParsingInteger(percent string) error {
return fmt.Errorf("error converting given %s to integer value for traffic distribution", percent)
}
func verifyLatestTag(trafficFlags *flags.Traffic) error {
var latestRevisionTag bool
for _, each := range trafficFlags.RevisionsTags { for _, each := range trafficFlags.RevisionsTags {
revision, _, err := splitByEqualSign(each) revision, _, err := splitByEqualSign(each)
if err != nil { if err != nil {
@ -234,44 +343,19 @@ func verifyInput(trafficFlags *flags.Traffic) error {
latestRevisionTag = true latestRevisionTag = true
} }
} }
revisionRefMap := make(map[string]int)
for i, each := range trafficFlags.RevisionsPercentages {
revisionRef, percent, err := splitByEqualSign(each)
if err != nil {
return err
}
// To check if there are duplicate revision names in traffic flags
if _, exist := revisionRefMap[revisionRef]; exist {
return errorRepeatingRevision("--traffic", revisionRef)
}
revisionRefMap[revisionRef] = i
percentInt, err := strconv.Atoi(percent)
if err != nil {
return fmt.Errorf("error converting given %s to integer value for traffic distribution", percent)
}
if percentInt < 0 || percentInt > 100 {
return fmt.Errorf("invalid value for traffic percent %d, expected 0 <= percent <= 100", percentInt)
}
sum += percentInt
}
// equivalent check for `cmd.Flags().Changed("traffic")` as we don't have `cmd` in this function
if len(trafficFlags.RevisionsPercentages) > 0 && sum != 100 {
return fmt.Errorf("given traffic percents sum to %d, want 100", sum)
}
return nil return nil
} }
func checkRevisionPresent(refMap map[string]int, rev servingv1.Revision) bool {
_, nameExists := refMap[rev.Name]
_, tagExists := refMap[rev.Annotations[revision.RevisionTagsAnnotation]]
return tagExists || nameExists
}
// Compute takes service traffic targets and updates per given traffic flags // Compute takes service traffic targets and updates per given traffic flags
func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget, func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget,
trafficFlags *flags.Traffic, serviceName string) ([]servingv1.TrafficTarget, error) { trafficFlags *flags.Traffic, serviceName string, revisions []servingv1.Revision) ([]servingv1.TrafficTarget, error) {
err := verifyInput(trafficFlags) err := verifyInput(trafficFlags, revisions)
if err != nil { if err != nil {
return nil, err return nil, err
} }

421
pkg/kn/traffic/compute_test.go
View File

@ -16,6 +16,8 @@ package traffic
import ( import (
"gotest.tools/v3/assert" "gotest.tools/v3/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/client/pkg/kn/commands/revision"
servingv1 "knative.dev/serving/pkg/apis/serving/v1" servingv1 "knative.dev/serving/pkg/apis/serving/v1"
"testing" "testing"
@ -32,6 +34,7 @@ type trafficTestCase struct {
desiredRevisions []string desiredRevisions []string
desiredTags []string desiredTags []string
desiredPercents []int64 desiredPercents []int64
existingRevisions []servingv1.Revision
} }
type trafficErrorTestCase struct { type trafficErrorTestCase struct {
@ -39,6 +42,7 @@ type trafficErrorTestCase struct {
existingTraffic []servingv1.TrafficTarget existingTraffic []servingv1.TrafficTarget
inputFlags []string inputFlags []string
errMsg string errMsg string
existingRevisions []servingv1.Revision
} }
func newTestTrafficCommand() (*cobra.Command, *flags.Traffic) { func newTestTrafficCommand() (*cobra.Command, *flags.Traffic) {
@ -55,133 +59,163 @@ func newTestTrafficCommand() (*cobra.Command, *flags.Traffic) {
func TestCompute(t *testing.T) { func TestCompute(t *testing.T) {
for _, testCase := range []trafficTestCase{ for _, testCase := range []trafficTestCase{
{ {
"assign 'latest' tag to @latest revision", name: "assign 'latest' tag to @latest revision",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--tag", "@latest=latest"}, inputFlags: []string{"--tag", "@latest=latest"},
[]string{"@latest"}, desiredRevisions: []string{"@latest"},
[]string{"latest"}, desiredTags: []string{"latest"},
[]int64{100}, desiredPercents: []int64{100},
}, },
{ {
"assign tag to revision", name: "assign tag to revision",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "echo-v1", 100, false)),
[]string{"--tag", "echo-v1=stable"}, inputFlags: []string{"--tag", "echo-v1=stable"},
[]string{"echo-v1"}, desiredRevisions: []string{"echo-v1"},
[]string{"stable"}, desiredTags: []string{"stable"},
[]int64{100}, desiredPercents: []int64{100},
}, },
{ {
"re-assign same tag to same revision (unchanged)", name: "re-assign same tag to same revision (unchanged)",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("current", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("current", "", 100, true)),
[]string{"--tag", "@latest=current"}, inputFlags: []string{"--tag", "@latest=current"},
[]string{""}, desiredRevisions: []string{""},
[]string{"current"}, desiredTags: []string{"current"},
[]int64{100}, desiredPercents: []int64{100},
}, },
{ {
"split traffic to tags", name: "split traffic to tags",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true), newTarget("", "rev-v1", 0, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true), newTarget("", "rev-v1", 0, false)),
[]string{"--traffic", "@latest=10,rev-v1=90"}, inputFlags: []string{"--traffic", "@latest=10,rev-v1=90"},
[]string{"@latest", "rev-v1"}, desiredRevisions: []string{"@latest", "rev-v1"},
[]string{"", ""}, desiredTags: []string{"", ""},
[]int64{10, 90}, desiredPercents: []int64{10, 90},
}, },
{ {
"split traffic to tags with '%' suffix", name: "split traffic to tags with '%' suffix",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true), newTarget("", "rev-v1", 0, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true), newTarget("", "rev-v1", 0, false)),
[]string{"--traffic", "@latest=10%,rev-v1=90%"}, inputFlags: []string{"--traffic", "@latest=10%,rev-v1=90%"},
[]string{"@latest", "rev-v1"}, desiredRevisions: []string{"@latest", "rev-v1"},
[]string{"", ""}, desiredTags: []string{"", ""},
[]int64{10, 90}, desiredPercents: []int64{10, 90},
}, },
{ {
"add 2 more tagged revisions without giving them traffic portions", name: "add 2 more tagged revisions without giving them traffic portions",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "", 100, true)),
[]string{"--tag", "echo-v0=stale,echo-v1=old"}, inputFlags: []string{"--tag", "echo-v0=stale,echo-v1=old"},
[]string{"@latest", "echo-v0", "echo-v1"}, desiredRevisions: []string{"@latest", "echo-v0", "echo-v1"},
[]string{"latest", "stale", "old"}, desiredTags: []string{"latest", "stale", "old"},
[]int64{100, 0, 0}, desiredPercents: []int64{100, 0, 0},
}, },
{ {
"re-assign same tag to 'echo-v1' revision", name: "re-assign same tag to 'echo-v1' revision",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
[]string{"--tag", "echo-v1=latest"}, inputFlags: []string{"--tag", "echo-v1=latest"},
[]string{"echo-v1"}, desiredRevisions: []string{"echo-v1"},
[]string{"latest"}, desiredTags: []string{"latest"},
[]int64{100}, desiredPercents: []int64{100},
}, },
{ {
"set 2% traffic to latest revision by appending it in traffic block", name: "set 2% traffic to latest revision by appending it in traffic block",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
[]string{"--traffic", "@latest=2,echo-v1=98"}, inputFlags: []string{"--traffic", "@latest=2,echo-v1=98"},
[]string{"echo-v1", "@latest"}, desiredRevisions: []string{"echo-v1", "@latest"},
[]string{"latest", ""}, desiredTags: []string{"latest", ""},
[]int64{98, 2}, desiredPercents: []int64{98, 2},
}, },
{ {
"set 2% to @latest with tag (append it in traffic block)", name: "set 2% to @latest with tag (append it in traffic block)",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
[]string{"--traffic", "@latest=2,echo-v1=98", "--tag", "@latest=testing"}, inputFlags: []string{"--traffic", "@latest=2,echo-v1=98", "--tag", "@latest=testing"},
[]string{"echo-v1", "@latest"}, desiredRevisions: []string{"echo-v1", "@latest"},
[]string{"latest", "testing"}, desiredTags: []string{"latest", "testing"},
[]int64{98, 2}, desiredPercents: []int64{98, 2},
}, },
{ {
"change traffic percent of an existing revision in traffic block, add new revision with traffic share", name: "change traffic percent of an existing revision in traffic block, add new revision with traffic share",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("v1", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("v1", "echo-v1", 100, false)),
[]string{"--tag", "echo-v2=v2", "--traffic", "v1=10,v2=90"}, inputFlags: []string{"--tag", "echo-v2=v2", "--traffic", "v1=10,v2=90"},
[]string{"echo-v1", "echo-v2"}, desiredRevisions: []string{"echo-v1", "echo-v2"},
[]string{"v1", "v2"}, desiredTags: []string{"v1", "v2"},
[]int64{10, 90}, //default value, desiredPercents: []int64{10, 90}, //default value,
}, },
{ {
"untag 'latest' tag from 'echo-v1' revision", name: "untag 'latest' tag from 'echo-v1' revision",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
[]string{"--untag", "latest"}, inputFlags: []string{"--untag", "latest"},
[]string{"echo-v1"}, desiredRevisions: []string{"echo-v1"},
[]string{""}, desiredTags: []string{""},
[]int64{100}, desiredPercents: []int64{100},
}, },
{ {
"replace revision pointing to 'latest' tag from 'echo-v1' to 'echo-v2' revision", name: "replace revision pointing to 'latest' tag from 'echo-v1' to 'echo-v2' revision",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 50, false), newTarget("", "echo-v2", 50, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 50, false), newTarget("", "echo-v2", 50, false)),
[]string{"--untag", "latest", "--tag", "echo-v1=old,echo-v2=latest"}, inputFlags: []string{"--untag", "latest", "--tag", "echo-v1=old,echo-v2=latest"},
[]string{"echo-v1", "echo-v2"}, desiredRevisions: []string{"echo-v1", "echo-v2"},
[]string{"old", "latest"}, desiredTags: []string{"old", "latest"},
[]int64{50, 50}, desiredPercents: []int64{50, 50},
}, },
{ {
"have multiple tags for a revision, revision present in traffic block", name: "have multiple tags for a revision, revision present in traffic block",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 50, false), newTarget("", "echo-v2", 50, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 50, false), newTarget("", "echo-v2", 50, false)),
[]string{"--tag", "echo-v1=latest,echo-v1=current"}, inputFlags: []string{"--tag", "echo-v1=latest,echo-v1=current"},
[]string{"echo-v1", "echo-v2", "echo-v1"}, // appends a new target desiredRevisions: []string{"echo-v1", "echo-v2", "echo-v1"}, // appends a new target
[]string{"latest", "", "current"}, // with new tag requested desiredTags: []string{"latest", "", "current"}, // with new tag requested
[]int64{50, 50, 0}, // and assign 0% to it desiredPercents: []int64{50, 50, 0}, // and assign 0% to it
}, },
{ {
"have multiple tags for a revision, revision absent in traffic block", name: "have multiple tags for a revision, revision absent in traffic block",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "echo-v2", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "echo-v2", 100, false)),
[]string{"--tag", "echo-v1=latest,echo-v1=current"}, inputFlags: []string{"--tag", "echo-v1=latest,echo-v1=current"},
[]string{"echo-v2", "echo-v1", "echo-v1"}, // appends two new targets desiredRevisions: []string{"echo-v2", "echo-v1", "echo-v1"}, // appends two new targets
[]string{"", "latest", "current"}, // with new tags requested desiredTags: []string{"", "latest", "current"}, // with new tags requested
[]int64{100, 0, 0}, // and assign 0% to each desiredPercents: []int64{100, 0, 0}, // and assign 0% to each
}, },
{ {
"re-assign same tag 'current' to @latest", name: "re-assign same tag 'current' to @latest",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("current", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("current", "", 100, true)),
[]string{"--tag", "@latest=current"}, inputFlags: []string{"--tag", "@latest=current"},
[]string{""}, desiredRevisions: []string{""},
[]string{"current"}, // since no change, no error desiredTags: []string{"current"}, // since no change, no error
[]int64{100}, desiredPercents: []int64{100},
}, },
{ {
"assign echo-v1 10% traffic adjusting rest to @latest, echo-v1 isn't present in existing traffic block", name: "assign echo-v1 10% traffic adjusting rest to @latest, echo-v1 isn't present in existing traffic block",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "echo-v1=10,@latest=90"}, inputFlags: []string{"--traffic", "echo-v1=10,@latest=90"},
[]string{"", "echo-v1"}, desiredRevisions: []string{"", "echo-v1"},
[]string{"", ""}, // since no change, no error desiredTags: []string{"", ""}, // since no change, no error
[]int64{90, 10}, desiredPercents: []int64{90, 10},
},
{
name: "traffic split with sum < 100 should work for n-1 revisions",
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "rev-00001", 0, false), newTarget("", "rev-00002", 0, false), newTarget("", "rev-00003", 100, false)),
inputFlags: []string{"--traffic", "rev-00001=10,rev-00002=10"},
desiredRevisions: []string{"rev-00001", "rev-00002", "rev-00003"},
desiredPercents: []int64{10, 10, 80},
desiredTags: []string{"", "", ""},
existingRevisions: []servingv1.Revision{{
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00001",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00002",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00003",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}},
}, },
} { } {
t.Run(testCase.name, func(t *testing.T) { t.Run(testCase.name, func(t *testing.T) {
@ -193,7 +227,7 @@ func TestCompute(t *testing.T) {
testCmd, tFlags := newTestTrafficCommand() testCmd, tFlags := newTestTrafficCommand()
testCmd.SetArgs(testCase.inputFlags) testCmd.SetArgs(testCase.inputFlags)
testCmd.Execute() testCmd.Execute()
targets, err := Compute(testCmd, testCase.existingTraffic, tFlags, "serviceName") targets, err := Compute(testCmd, testCase.existingTraffic, tFlags, "serviceName", testCase.existingRevisions)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -213,89 +247,176 @@ func TestCompute(t *testing.T) {
func TestComputeErrMsg(t *testing.T) { func TestComputeErrMsg(t *testing.T) {
for _, testCase := range []trafficErrorTestCase{ for _, testCase := range []trafficErrorTestCase{
{ {
"invalid format for --traffic option", name: "invalid format for --traffic option",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "@latest=100=latest"}, inputFlags: []string{"--traffic", "@latest=100=latest"},
"expecting the value format in value1=value2, given @latest=100=latest", errMsg: "expecting the value format in value1=value2, given @latest=100=latest",
}, },
{ {
"invalid format for --tag option", name: "invalid format for --tag option",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--tag", "@latest="}, inputFlags: []string{"--tag", "@latest="},
"expecting the value format in value1=value2, given @latest=", errMsg: "expecting the value format in value1=value2, given @latest=",
}, },
{ {
"repeatedly splitting traffic to @latest revision", name: "repeatedly splitting traffic to @latest revision",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "@latest=90,@latest=10"}, inputFlags: []string{"--traffic", "@latest=90,@latest=10"},
"repetition of identifier @latest is not allowed, use only once with --traffic flag", errMsg: "repetition of identifier @latest is not allowed, use only once with --traffic flag",
}, },
{ {
"repeatedly tagging to @latest revision not allowed", name: "repeatedly tagging to @latest revision not allowed",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--tag", "@latest=latest,@latest=2"}, inputFlags: []string{"--tag", "@latest=latest,@latest=2"},
"repetition of identifier @latest is not allowed, use only once with --tag flag", errMsg: "repetition of identifier @latest is not allowed, use only once with --tag flag",
}, },
{ {
"overwriting tag not allowed, to @latest from other revision", name: "overwriting tag not allowed, to @latest from other revision",
append(append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "", 2, true)), newTarget("stable", "echo-v2", 98, false)), existingTraffic: append(append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "", 2, true)), newTarget("stable", "echo-v2", 98, false)),
[]string{"--tag", "@latest=stable"}, inputFlags: []string{"--tag", "@latest=stable"},
"refusing to overwrite existing tag in service, add flag '--untag stable' in command to untag it", errMsg: "refusing to overwrite existing tag in service, add flag '--untag stable' in command to untag it",
}, },
{ {
"overwriting tag not allowed, to a revision from other revision", name: "overwriting tag not allowed, to a revision from other revision",
append(append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "", 2, true)), newTarget("stable", "echo-v2", 98, false)), existingTraffic: append(append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "", 2, true)), newTarget("stable", "echo-v2", 98, false)),
[]string{"--tag", "echo-v2=latest"}, inputFlags: []string{"--tag", "echo-v2=latest"},
"refusing to overwrite existing tag in service, add flag '--untag latest' in command to untag it", errMsg: "refusing to overwrite existing tag in service, add flag '--untag latest' in command to untag it",
}, },
{ {
"overwriting tag of @latest not allowed, existing != requested", name: "overwriting tag of @latest not allowed, existing != requested",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("candidate", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("candidate", "", 100, true)),
[]string{"--tag", "@latest=current"}, inputFlags: []string{"--tag", "@latest=current"},
"tag 'candidate' exists on latest ready revision of service, refusing to overwrite existing tag with 'current', add flag '--untag candidate' in command to untag it", errMsg: "tag 'candidate' exists on latest ready revision of service, refusing to overwrite existing tag with 'current', add flag '--untag candidate' in command to untag it",
}, },
{ {
"verify error for non integer values given to percent", name: "verify error for non integer values given to percent",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "@latest=100p"}, inputFlags: []string{"--traffic", "@latest=100p"},
"error converting given 100p to integer value for traffic distribution", errMsg: errorParsingInteger("100p").Error(),
}, },
{ {
"verify error for traffic sum not equal to 100", name: "verify error for traffic sum not equal to 100",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "@latest=19,echo-v1=71"}, inputFlags: []string{"--traffic", "@latest=40,echo-v1=70"},
"given traffic percents sum to 90, want 100", errMsg: errorSumGreaterThan100(110).Error(),
}, },
{ {
"verify error for values out of range given to percent", name: "verify error for values out of range given to percent",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "@latest=-100"}, inputFlags: []string{"--traffic", "@latest=-100"},
"invalid value for traffic percent -100, expected 0 <= percent <= 100", errMsg: "invalid value for traffic percent -100, expected 0 <= percent <= 100",
}, },
{ {
"repeatedly splitting traffic to the same revision", name: "repeatedly splitting traffic to the same revision",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "echo-v1=40", "--traffic", "echo-v1=60"}, inputFlags: []string{"--traffic", "echo-v1=40", "--traffic", "echo-v1=60"},
"repetition of revision reference echo-v1 is not allowed, use only once with --traffic flag", errMsg: "repetition of revision reference echo-v1 is not allowed, use only once with --traffic flag",
}, },
{ {
"untag single tag that does not exist", name: "untag single tag that does not exist",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
[]string{"--untag", "foo"}, inputFlags: []string{"--untag", "foo"},
"tag(s) foo not present for any revisions of service serviceName", errMsg: "tag(s) foo not present for any revisions of service serviceName",
}, },
{ {
"untag multiple tags that do not exist", name: "untag multiple tags that do not exist",
append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)), existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("latest", "echo-v1", 100, false)),
[]string{"--untag", "foo", "--untag", "bar"}, inputFlags: []string{"--untag", "foo", "--untag", "bar"},
"tag(s) foo, bar not present for any revisions of service serviceName", errMsg: "tag(s) foo, bar not present for any revisions of service serviceName",
},
{
name: "traffic split sum < 100 should have N-1 revisions specified",
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "rev-00001", 0, false), newTarget("", "rev-00002", 0, false), newTarget("", "rev-00003", 100, false)),
inputFlags: []string{"--traffic", "rev-00001=10"},
errMsg: errorTrafficDistribution(10, errorDistributionRevisionCount).Error(),
existingRevisions: []servingv1.Revision{{
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00001",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00002",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00003",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}},
},
{
name: "traffic split sum < 100 should not have @latest specified",
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "rev-00001", 0, false), newTarget("", "rev-00002", 0, false), newTarget("", "rev-00003", 100, true)),
inputFlags: []string{"--traffic", "rev-00001=10,@latest=20"},
errMsg: errorTrafficDistribution(30, errorDistributionLatestTag).Error(),
existingRevisions: []servingv1.Revision{{
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00001",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00002",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00003",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}},
},
{
name: "traffic split sum < 100 error when remaining revision not found",
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("rev-00003", "rev-00001", 0, false), newTarget("", "rev-00002", 0, false), newTarget("", "rev-00003", 100, true)),
inputFlags: []string{"--traffic", "rev-00003=10,rev-00002=20"},
errMsg: errorTrafficDistribution(30, errorDistributionRevisionNotFound).Error(),
existingRevisions: []servingv1.Revision{{
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00001",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
Annotations: map[string]string{
revision.RevisionTagsAnnotation: "rev-00003",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00002",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}, {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-00003",
Labels: map[string]string{
"serving.knative.dev/service": "serviceName",
},
},
}},
}, },
} { } {
t.Run(testCase.name, func(t *testing.T) { t.Run(testCase.name, func(t *testing.T) {
testCmd, tFlags := newTestTrafficCommand() testCmd, tFlags := newTestTrafficCommand()
testCmd.SetArgs(testCase.inputFlags) testCmd.SetArgs(testCase.inputFlags)
testCmd.Execute() testCmd.Execute()
_, err := Compute(testCmd, testCase.existingTraffic, tFlags, "serviceName") _, err := Compute(testCmd, testCase.existingTraffic, tFlags, "serviceName", testCase.existingRevisions)
assert.Error(t, err, testCase.errMsg) assert.Error(t, err, testCase.errMsg)
}) })
} }

17
test/e2e/traffic_split_test.go
View File

@ -131,6 +131,23 @@ func TestTrafficSplit(t *testing.T) {
test.ServiceDelete(r, serviceName) test.ServiceDelete(r, serviceName)
}, },
) )
t.Run("45:55 automatic traffic split", func(t *testing.T) {
t.Log("direct 45% traffic explicitly to 1st rev and remaining automatically to second")
r := test.NewKnRunResultCollector(t, it)
defer r.DumpIfFailed()
serviceName := test.GetNextServiceName(serviceBase)
test.ServiceCreate(r, serviceName)
test.ServiceUpdate(r, serviceName, "--env", "TARGET=v1")
rev1 := fmt.Sprintf("%s-00001", serviceName)
rev2 := fmt.Sprintf("%s-00002", serviceName)
test.ServiceUpdate(r, serviceName, "--traffic", fmt.Sprintf("%s=45", rev1))
expectedTargets := []TargetFields{newTargetFields("", rev1, 45, false), newTargetFields("", rev2, 55, false)}
verifyTargets(r, serviceName, expectedTargets, false)
test.ServiceDelete(r, serviceName)
})
t.Run("TagCandidate", t.Run("TagCandidate",
func(t *testing.T) { func(t *testing.T) {
t.Log("tag a revision as candidate, without otherwise changing any traffic split") t.Log("tag a revision as candidate, without otherwise changing any traffic split")