mirror of https://github.com/knative/client.git
Add --tag flag to service create and allow traffic split <100 when @latest is specified (#1514)
* Add --tag flag to service create * added comments * Added tests * handled @latest tag in traffic split * added unit tests * added e2e tests * added comments * simplified code * add e2e tests for error cases * Add handling for non latest revisions with mutation bool
This commit is contained in:
parent
2b1e77de20
commit
8fd19e6bea
|
|
@ -92,6 +92,7 @@ kn service create NAME --image IMAGE
|
||||||
--scale-utilization int Percentage of concurrent requests utilization before scaling up. (default 70)
|
--scale-utilization int Percentage of concurrent requests utilization before scaling up. (default 70)
|
||||||
--scale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
|
--scale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
|
||||||
--service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
|
--service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
|
||||||
|
--tag strings Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times.
|
||||||
--target string Work on local directory instead of a remote cluster (experimental)
|
--target string Work on local directory instead of a remote cluster (experimental)
|
||||||
--user int The user ID to run the container (e.g., 1001).
|
--user int The user ID to run the container (e.g., 1001).
|
||||||
--volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-.
|
--volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-.
|
||||||
|
|
|
||||||
|
|
@ -25,22 +25,37 @@ type Traffic struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
func (t *Traffic) Add(cmd *cobra.Command) {
|
func (t *Traffic) Add(cmd *cobra.Command) {
|
||||||
cmd.Flags().StringSliceVar(&t.RevisionsPercentages,
|
t.AddTrafficFlag(cmd)
|
||||||
"traffic",
|
|
||||||
nil,
|
|
||||||
"Set traffic distribution (format: --traffic revisionRef=percent) where revisionRef can be a revision or a tag or '@latest' string "+
|
|
||||||
"representing latest ready revision. This flag can be given multiple times with percent summing up to 100%.")
|
|
||||||
|
|
||||||
|
t.AddTagFlag(cmd)
|
||||||
|
|
||||||
|
t.AddUntagFlag(cmd)
|
||||||
|
}
|
||||||
|
|
||||||
|
// AddUntagFlag adds the flag --untag to the command
|
||||||
|
func (t *Traffic) AddUntagFlag(cmd *cobra.Command) {
|
||||||
|
cmd.Flags().StringSliceVar(&t.UntagRevisions,
|
||||||
|
"untag",
|
||||||
|
nil,
|
||||||
|
"Untag revision (format: --untag tagName). This flag can be specified multiple times.")
|
||||||
|
}
|
||||||
|
|
||||||
|
// AddTagFlag adds the flag --tag to the command
|
||||||
|
func (t *Traffic) AddTagFlag(cmd *cobra.Command) {
|
||||||
cmd.Flags().StringSliceVar(&t.RevisionsTags,
|
cmd.Flags().StringSliceVar(&t.RevisionsTags,
|
||||||
"tag",
|
"tag",
|
||||||
nil,
|
nil,
|
||||||
"Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. "+
|
"Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. "+
|
||||||
"This flag can be specified multiple times.")
|
"This flag can be specified multiple times.")
|
||||||
|
}
|
||||||
|
|
||||||
cmd.Flags().StringSliceVar(&t.UntagRevisions,
|
// AddTrafficFlag adds the flag --traffic to the command
|
||||||
"untag",
|
func (t *Traffic) AddTrafficFlag(cmd *cobra.Command) {
|
||||||
|
cmd.Flags().StringSliceVar(&t.RevisionsPercentages,
|
||||||
|
"traffic",
|
||||||
nil,
|
nil,
|
||||||
"Untag revision (format: --untag tagName). This flag can be specified multiple times.")
|
"Set traffic distribution (format: --traffic revisionRef=percent) where revisionRef can be a revision or a tag or '@latest' string "+
|
||||||
|
"representing latest ready revision. This flag can be given multiple times with percent summing up to 100%.")
|
||||||
}
|
}
|
||||||
|
|
||||||
func (t *Traffic) PercentagesChanged(cmd *cobra.Command) bool {
|
func (t *Traffic) PercentagesChanged(cmd *cobra.Command) bool {
|
||||||
|
|
|
||||||
|
|
@ -23,6 +23,8 @@ import (
|
||||||
|
|
||||||
"knative.dev/client/pkg/config"
|
"knative.dev/client/pkg/config"
|
||||||
"knative.dev/client/pkg/kn/commands"
|
"knative.dev/client/pkg/kn/commands"
|
||||||
|
"knative.dev/client/pkg/kn/commands/flags"
|
||||||
|
"knative.dev/client/pkg/kn/traffic"
|
||||||
servinglib "knative.dev/client/pkg/serving"
|
servinglib "knative.dev/client/pkg/serving"
|
||||||
|
|
||||||
"knative.dev/serving/pkg/apis/serving"
|
"knative.dev/serving/pkg/apis/serving"
|
||||||
|
|
@ -82,6 +84,7 @@ var create_example = `
|
||||||
func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
|
func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
|
||||||
var editFlags ConfigurationEditFlags
|
var editFlags ConfigurationEditFlags
|
||||||
var waitFlags commands.WaitFlags
|
var waitFlags commands.WaitFlags
|
||||||
|
var trafficFlags flags.Traffic
|
||||||
|
|
||||||
serviceCreateCommand := &cobra.Command{
|
serviceCreateCommand := &cobra.Command{
|
||||||
Use: "create NAME --image IMAGE",
|
Use: "create NAME --image IMAGE",
|
||||||
|
|
@ -118,6 +121,15 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
if trafficFlags.TagsChanged(cmd) {
|
||||||
|
trafficFlags.RevisionsPercentages = []string{"@latest=100"}
|
||||||
|
|
||||||
|
traffic, err := traffic.Compute(cmd, service, &trafficFlags, nil, editFlags.AnyMutation(cmd))
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
service.Spec.Traffic = traffic
|
||||||
|
}
|
||||||
serviceExists, err := serviceExists(cmd.Context(), client, service.Name)
|
serviceExists, err := serviceExists(cmd.Context(), client, service.Name)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
|
@ -143,6 +155,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
|
||||||
commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false)
|
commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false)
|
||||||
commands.AddGitOpsFlags(serviceCreateCommand.Flags())
|
commands.AddGitOpsFlags(serviceCreateCommand.Flags())
|
||||||
editFlags.AddCreateFlags(serviceCreateCommand)
|
editFlags.AddCreateFlags(serviceCreateCommand)
|
||||||
|
trafficFlags.AddTagFlag(serviceCreateCommand)
|
||||||
waitFlags.AddConditionWaitFlags(serviceCreateCommand, commands.WaitDefaultTimeout, "create", "service", "ready")
|
waitFlags.AddConditionWaitFlags(serviceCreateCommand, commands.WaitDefaultTimeout, "create", "service", "ready")
|
||||||
return serviceCreateCommand
|
return serviceCreateCommand
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1084,3 +1084,18 @@ func TestServiceCreateFromYAMLWithOverrideError(t *testing.T) {
|
||||||
assert.Assert(t, err != nil)
|
assert.Assert(t, err != nil)
|
||||||
assert.Assert(t, util.ContainsAll(err.Error(), "expected", "0", "<=", "2147483647", autoscaling.MaxScaleAnnotationKey))
|
assert.Assert(t, util.ContainsAll(err.Error(), "expected", "0", "<=", "2147483647", autoscaling.MaxScaleAnnotationKey))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestServiceCreateTag(t *testing.T) {
|
||||||
|
action, created, _, err := fakeServiceCreate([]string{
|
||||||
|
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--tag", "foo"}, false)
|
||||||
|
assert.NilError(t, err)
|
||||||
|
assert.Assert(t, action.Matches("create", "services"))
|
||||||
|
assert.Equal(t, len(created.Spec.Traffic), 1)
|
||||||
|
assert.Equal(t, created.Spec.Traffic[0].Tag, "foo")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestServiceCreateTagWithError(t *testing.T) {
|
||||||
|
_, _, _, err := fakeServiceCreate([]string{
|
||||||
|
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--tag", "foo,bar"}, false)
|
||||||
|
assert.Error(t, err, "repetition of identifier @latest is not allowed, use only once with --tag flag")
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -107,7 +107,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
traffic, err := traffic.Compute(cmd, service.Spec.Traffic, &trafficFlags, service.Name, revisions.Items)
|
traffic, err := traffic.Compute(cmd, service, &trafficFlags, revisions.Items, editFlags.AnyMutation(cmd))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -31,7 +31,6 @@ var latestRevisionRef = "@latest"
|
||||||
|
|
||||||
const (
|
const (
|
||||||
errorDistributionRevisionCount = iota
|
errorDistributionRevisionCount = iota
|
||||||
errorDistributionLatestTag
|
|
||||||
errorDistributionRevisionNotFound
|
errorDistributionRevisionNotFound
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
@ -44,6 +43,9 @@ func newServiceTraffic(traffic []servingv1.TrafficTarget) ServiceTraffic {
|
||||||
|
|
||||||
func splitByEqualSign(pair string) (string, string, error) {
|
func splitByEqualSign(pair string) (string, string, error) {
|
||||||
parts := strings.Split(pair, "=")
|
parts := strings.Split(pair, "=")
|
||||||
|
if len(parts) == 1 {
|
||||||
|
return latestRevisionRef, parts[0], nil
|
||||||
|
}
|
||||||
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
|
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
|
||||||
return "", "", fmt.Errorf("expecting the value format in value1=value2, given %s", pair)
|
return "", "", fmt.Errorf("expecting the value format in value1=value2, given %s", pair)
|
||||||
}
|
}
|
||||||
|
|
@ -224,12 +226,10 @@ func errorTrafficDistribution(sum int, reason int) error {
|
||||||
switch reason {
|
switch reason {
|
||||||
case errorDistributionRevisionCount:
|
case errorDistributionRevisionCount:
|
||||||
errMsg = "incorrect number of revisions specified. Only 1 revision should be missing"
|
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:
|
case errorDistributionRevisionNotFound:
|
||||||
errMsg = "cannot determine the missing revision"
|
errMsg = "cannot determine the missing revision"
|
||||||
}
|
}
|
||||||
return fmt.Errorf("unable to allocate the remaining traffic %d. %s", 100-sum, errMsg)
|
return fmt.Errorf("unable to allocate the remaining traffic: %d%%. Reason: %s", 100-sum, errMsg)
|
||||||
}
|
}
|
||||||
|
|
||||||
func errorSumGreaterThan100(sum int) error {
|
func errorSumGreaterThan100(sum int) error {
|
||||||
|
|
@ -240,12 +240,14 @@ func errorSumGreaterThan100(sum int) error {
|
||||||
// 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, revisions []servingv1.Revision) error {
|
// - if traffic as per flags sums to 100
|
||||||
|
// - if traffic as per flags < 100, can remaining traffic be automatically directed
|
||||||
|
func verifyInput(trafficFlags *flags.Traffic, svc *servingv1.Service, revisions []servingv1.Revision, mutation bool) error {
|
||||||
// check if traffic is being sent to @latest tag
|
// check if traffic is being sent to @latest tag
|
||||||
var latestRefTraffic bool
|
var latestRefTraffic bool
|
||||||
|
|
||||||
// number of revisions
|
// number of existing revisions
|
||||||
var revisionCount = len(revisions)
|
var existingRevisionCount = len(revisions)
|
||||||
|
|
||||||
err := verifyLatestTag(trafficFlags)
|
err := verifyLatestTag(trafficFlags)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -257,6 +259,14 @@ func verifyInput(trafficFlags *flags.Traffic, revisions []servingv1.Revision) er
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// further verification is not required if sum >= 100
|
||||||
|
if sum == 100 {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
if sum > 100 {
|
||||||
|
return errorSumGreaterThan100(sum)
|
||||||
|
}
|
||||||
|
|
||||||
if _, ok := revisionRefMap[latestRevisionRef]; ok {
|
if _, ok := revisionRefMap[latestRevisionRef]; ok {
|
||||||
// traffic has been routed to @latest tag
|
// traffic has been routed to @latest tag
|
||||||
latestRefTraffic = true
|
latestRefTraffic = true
|
||||||
|
|
@ -264,29 +274,47 @@ func verifyInput(trafficFlags *flags.Traffic, revisions []servingv1.Revision) er
|
||||||
|
|
||||||
// number of revisions specified in traffic flags
|
// number of revisions specified in traffic flags
|
||||||
specRevPercentCount := len(trafficFlags.RevisionsPercentages)
|
specRevPercentCount := len(trafficFlags.RevisionsPercentages)
|
||||||
// equivalent check for `cmd.Flags().Changed("traffic")` as we don't have `cmd` in this function
|
|
||||||
if specRevPercentCount > 0 {
|
// no traffic to route
|
||||||
if sum > 100 {
|
if specRevPercentCount == 0 {
|
||||||
return errorSumGreaterThan100(sum)
|
return nil
|
||||||
}
|
}
|
||||||
if sum < 100 && specRevPercentCount != revisionCount-1 {
|
|
||||||
|
// cannot determine remaining revision. Only 1 should be left out
|
||||||
|
if specRevPercentCount < existingRevisionCount-1 {
|
||||||
|
return errorTrafficDistribution(sum, errorDistributionRevisionCount)
|
||||||
|
}
|
||||||
|
|
||||||
|
// if mutation is set, a new revision will be created after service update.
|
||||||
|
// specRevPercentCount should be equal to existingRevisionCount
|
||||||
|
if mutation {
|
||||||
|
if specRevPercentCount != existingRevisionCount {
|
||||||
return errorTrafficDistribution(sum, errorDistributionRevisionCount)
|
return errorTrafficDistribution(sum, errorDistributionRevisionCount)
|
||||||
}
|
}
|
||||||
if sum < 100 && specRevPercentCount == revisionCount-1 {
|
if _, ok := revisionRefMap[latestRevisionRef]; !ok {
|
||||||
if latestRefTraffic {
|
// remaining % can only go to the @latest tag
|
||||||
return errorTrafficDistribution(sum, errorDistributionLatestTag)
|
trafficFlags.RevisionsPercentages = append(trafficFlags.RevisionsPercentages, fmt.Sprintf("%s=%d", latestRevisionRef, 100-sum))
|
||||||
}
|
return nil
|
||||||
for _, rev := range revisions {
|
}
|
||||||
if !checkRevisionPresent(revisionRefMap, rev) {
|
} else {
|
||||||
trafficFlags.RevisionsPercentages = append(trafficFlags.RevisionsPercentages, fmt.Sprintf("%s=%d", rev.Name, 100-sum))
|
// if mutation is not set, specRevPercentCount should be equal to existingRevisionCount
|
||||||
return nil
|
if specRevPercentCount != existingRevisionCount-1 {
|
||||||
}
|
return errorTrafficDistribution(sum, errorDistributionRevisionCount)
|
||||||
}
|
}
|
||||||
return errorTrafficDistribution(sum, errorDistributionRevisionNotFound)
|
if latestRefTraffic {
|
||||||
|
revisionRefMap[svc.Status.LatestReadyRevisionName] = 0
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil
|
// remaining % to 100
|
||||||
|
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)
|
||||||
}
|
}
|
||||||
|
|
||||||
func verifyRevisionSumAndReferences(trafficFlags *flags.Traffic) (revisionRefMap map[string]int, sum int, err error) {
|
func verifyRevisionSumAndReferences(trafficFlags *flags.Traffic) (revisionRefMap map[string]int, sum int, err error) {
|
||||||
|
|
@ -352,10 +380,14 @@ func checkRevisionPresent(refMap map[string]int, rev servingv1.Revision) bool {
|
||||||
return tagExists || nameExists
|
return tagExists || nameExists
|
||||||
}
|
}
|
||||||
|
|
||||||
// Compute takes service traffic targets and updates per given traffic flags
|
// Compute takes service object, computes traffic per given traffic flags and returns the new traffic. If
|
||||||
func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget,
|
// total traffic per flags < 100, the params 'revisions' and 'mutation' are used to direct remaining
|
||||||
trafficFlags *flags.Traffic, serviceName string, revisions []servingv1.Revision) ([]servingv1.TrafficTarget, error) {
|
// traffic. Param 'mutation' is set to true if a new revision will be created on service update
|
||||||
err := verifyInput(trafficFlags, revisions)
|
func Compute(cmd *cobra.Command, svc *servingv1.Service,
|
||||||
|
trafficFlags *flags.Traffic, revisions []servingv1.Revision, mutation bool) ([]servingv1.TrafficTarget, error) {
|
||||||
|
targets := svc.Spec.Traffic
|
||||||
|
serviceName := svc.Name
|
||||||
|
err := verifyInput(trafficFlags, svc, revisions, mutation)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
@ -416,7 +448,7 @@ func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget,
|
||||||
traffic = traffic.TagRevision(tag, revision)
|
traffic = traffic.TagRevision(tag, revision)
|
||||||
}
|
}
|
||||||
|
|
||||||
if cmd.Flags().Changed("traffic") {
|
if cmd.Flags().Changed("traffic") || (cmd.Name() == "create" && len(trafficFlags.RevisionsTags) > 0) {
|
||||||
// reset existing traffic portions as what's on CLI is desired state of traffic split portions
|
// reset existing traffic portions as what's on CLI is desired state of traffic split portions
|
||||||
traffic.ResetAllTargetPercent()
|
traffic.ResetAllTargetPercent()
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,7 @@ package traffic
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"gotest.tools/v3/assert"
|
"gotest.tools/v3/assert"
|
||||||
|
corev1 "k8s.io/api/core/v1"
|
||||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||||
"knative.dev/client/pkg/kn/commands/revision"
|
"knative.dev/client/pkg/kn/commands/revision"
|
||||||
servingv1 "knative.dev/serving/pkg/apis/serving/v1"
|
servingv1 "knative.dev/serving/pkg/apis/serving/v1"
|
||||||
|
|
@ -35,6 +36,8 @@ type trafficTestCase struct {
|
||||||
desiredTags []string
|
desiredTags []string
|
||||||
desiredPercents []int64
|
desiredPercents []int64
|
||||||
existingRevisions []servingv1.Revision
|
existingRevisions []servingv1.Revision
|
||||||
|
mutation bool
|
||||||
|
latestRevision string
|
||||||
}
|
}
|
||||||
|
|
||||||
type trafficErrorTestCase struct {
|
type trafficErrorTestCase struct {
|
||||||
|
|
@ -43,6 +46,8 @@ type trafficErrorTestCase struct {
|
||||||
inputFlags []string
|
inputFlags []string
|
||||||
errMsg string
|
errMsg string
|
||||||
existingRevisions []servingv1.Revision
|
existingRevisions []servingv1.Revision
|
||||||
|
mutation bool
|
||||||
|
latestRevision string
|
||||||
}
|
}
|
||||||
|
|
||||||
func newTestTrafficCommand() (*cobra.Command, *flags.Traffic) {
|
func newTestTrafficCommand() (*cobra.Command, *flags.Traffic) {
|
||||||
|
|
@ -56,6 +61,30 @@ func newTestTrafficCommand() (*cobra.Command, *flags.Traffic) {
|
||||||
return trafficCmd, &trafficFlags
|
return trafficCmd, &trafficFlags
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func getService(name, latestRevision string, existingTraffic ServiceTraffic) *servingv1.Service {
|
||||||
|
service := &servingv1.Service{
|
||||||
|
ObjectMeta: metav1.ObjectMeta{
|
||||||
|
Name: name,
|
||||||
|
Namespace: "default",
|
||||||
|
},
|
||||||
|
Spec: servingv1.ServiceSpec{},
|
||||||
|
}
|
||||||
|
|
||||||
|
service.Spec.Template = servingv1.RevisionTemplateSpec{
|
||||||
|
Spec: servingv1.RevisionSpec{},
|
||||||
|
}
|
||||||
|
|
||||||
|
service.Spec.Traffic = existingTraffic
|
||||||
|
service.Spec.Template.Spec.Containers = []corev1.Container{{
|
||||||
|
Resources: corev1.ResourceRequirements{
|
||||||
|
Limits: corev1.ResourceList{},
|
||||||
|
Requests: corev1.ResourceList{},
|
||||||
|
},
|
||||||
|
}}
|
||||||
|
service.Status.LatestReadyRevisionName = latestRevision
|
||||||
|
return service
|
||||||
|
}
|
||||||
|
|
||||||
func TestCompute(t *testing.T) {
|
func TestCompute(t *testing.T) {
|
||||||
for _, testCase := range []trafficTestCase{
|
for _, testCase := range []trafficTestCase{
|
||||||
{
|
{
|
||||||
|
|
@ -66,6 +95,14 @@ func TestCompute(t *testing.T) {
|
||||||
desiredTags: []string{"latest"},
|
desiredTags: []string{"latest"},
|
||||||
desiredPercents: []int64{100},
|
desiredPercents: []int64{100},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "assign 'latest' tag to @latest revision without specifying key",
|
||||||
|
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
|
||||||
|
inputFlags: []string{"--tag", "latest"},
|
||||||
|
desiredRevisions: []string{"@latest"},
|
||||||
|
desiredTags: []string{"latest"},
|
||||||
|
desiredPercents: []int64{100},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "assign tag to revision",
|
name: "assign tag to revision",
|
||||||
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "echo-v1", 100, false)),
|
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "echo-v1", 100, false)),
|
||||||
|
|
@ -217,6 +254,102 @@ func TestCompute(t *testing.T) {
|
||||||
},
|
},
|
||||||
}},
|
}},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "traffic split sum < 100 with @latest which mutates service should specify N revs",
|
||||||
|
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,rev-00003=40,@latest=20"},
|
||||||
|
desiredRevisions: []string{"rev-00001", "rev-00002", "", "rev-00003"},
|
||||||
|
desiredPercents: []int64{10, 30, 20, 40},
|
||||||
|
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",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}},
|
||||||
|
mutation: true,
|
||||||
|
latestRevision: "rev-00003",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "traffic split sum < 100 with @latest and does not mutate the service should specify N-1 revs",
|
||||||
|
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"},
|
||||||
|
desiredRevisions: []string{"rev-00001", "rev-00002", ""},
|
||||||
|
desiredPercents: []int64{10, 70, 20},
|
||||||
|
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",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}},
|
||||||
|
mutation: false,
|
||||||
|
latestRevision: "rev-00003",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "traffic split sum < 100 without which mutates service should specify N revs. Remaining should go to @latest",
|
||||||
|
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,rev-00003=40,rev-00002=30"},
|
||||||
|
desiredRevisions: []string{"rev-00001", "rev-00002", "", "rev-00003"},
|
||||||
|
desiredPercents: []int64{10, 30, 20, 40},
|
||||||
|
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",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}},
|
||||||
|
mutation: true,
|
||||||
|
latestRevision: "rev-00003",
|
||||||
|
},
|
||||||
} {
|
} {
|
||||||
t.Run(testCase.name, func(t *testing.T) {
|
t.Run(testCase.name, func(t *testing.T) {
|
||||||
if lper, lrev, ltag := len(testCase.desiredPercents), len(testCase.desiredRevisions), len(testCase.desiredTags); lper != lrev || lper != ltag {
|
if lper, lrev, ltag := len(testCase.desiredPercents), len(testCase.desiredRevisions), len(testCase.desiredTags); lper != lrev || lper != ltag {
|
||||||
|
|
@ -227,7 +360,8 @@ 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", testCase.existingRevisions)
|
svc := getService("serviceName", testCase.latestRevision, testCase.existingTraffic)
|
||||||
|
targets, err := Compute(testCmd, svc, tFlags, testCase.existingRevisions, testCase.mutation)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
|
@ -270,6 +404,12 @@ func TestComputeErrMsg(t *testing.T) {
|
||||||
inputFlags: []string{"--tag", "@latest=latest,@latest=2"},
|
inputFlags: []string{"--tag", "@latest=latest,@latest=2"},
|
||||||
errMsg: "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",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "repeatedly tagging to @latest revision not allowed, without key",
|
||||||
|
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "", 100, true)),
|
||||||
|
inputFlags: []string{"--tag", "latest,current"},
|
||||||
|
errMsg: "repetition of identifier @latest is not allowed, use only once with --tag flag",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "overwriting tag not allowed, to @latest from other revision",
|
name: "overwriting tag not allowed, to @latest from other revision",
|
||||||
existingTraffic: 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)),
|
||||||
|
|
@ -353,10 +493,10 @@ func TestComputeErrMsg(t *testing.T) {
|
||||||
}},
|
}},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "traffic split sum < 100 should not have @latest specified",
|
name: "traffic split sum < 100 with @latest which mutates service should specify N revs",
|
||||||
existingTraffic: append(newServiceTraffic([]servingv1.TrafficTarget{}), newTarget("", "rev-00001", 0, false), newTarget("", "rev-00002", 0, false), newTarget("", "rev-00003", 100, true)),
|
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"},
|
inputFlags: []string{"--traffic", "rev-00001=10,@latest=20"},
|
||||||
errMsg: errorTrafficDistribution(30, errorDistributionLatestTag).Error(),
|
errMsg: errorTrafficDistribution(30, errorDistributionRevisionCount).Error(),
|
||||||
existingRevisions: []servingv1.Revision{{
|
existingRevisions: []servingv1.Revision{{
|
||||||
ObjectMeta: metav1.ObjectMeta{
|
ObjectMeta: metav1.ObjectMeta{
|
||||||
Name: "rev-00001",
|
Name: "rev-00001",
|
||||||
|
|
@ -379,6 +519,36 @@ func TestComputeErrMsg(t *testing.T) {
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
}},
|
}},
|
||||||
|
mutation: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "traffic split sum < 100 with @latest and mutates the service should specify N revs",
|
||||||
|
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,rev-00002=10,@latest=20"},
|
||||||
|
errMsg: errorTrafficDistribution(40, 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",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}},
|
||||||
|
mutation: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "traffic split sum < 100 error when remaining revision not found",
|
name: "traffic split sum < 100 error when remaining revision not found",
|
||||||
|
|
@ -416,7 +586,8 @@ func TestComputeErrMsg(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", testCase.existingRevisions)
|
svc := getService("serviceName", testCase.latestRevision, testCase.existingTraffic)
|
||||||
|
_, err := Compute(testCmd, svc, tFlags, testCase.existingRevisions, testCase.mutation)
|
||||||
assert.Error(t, err, testCase.errMsg)
|
assert.Error(t, err, testCase.errMsg)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -148,6 +148,72 @@ func TestTrafficSplit(t *testing.T) {
|
||||||
verifyTargets(r, serviceName, expectedTargets, false)
|
verifyTargets(r, serviceName, expectedTargets, false)
|
||||||
test.ServiceDelete(r, serviceName)
|
test.ServiceDelete(r, serviceName)
|
||||||
})
|
})
|
||||||
|
t.Run("45:55 automatic traffic split to new @latest and previous revision after mutation", func(t *testing.T) {
|
||||||
|
t.Log("direct 45% traffic explicitly to newly created revision (@latest) and remaining 55 will automatically be directed to previous revision")
|
||||||
|
r := test.NewKnRunResultCollector(t, it)
|
||||||
|
defer r.DumpIfFailed()
|
||||||
|
|
||||||
|
serviceName := test.GetNextServiceName(serviceBase)
|
||||||
|
|
||||||
|
test.ServiceCreate(r, serviceName)
|
||||||
|
|
||||||
|
test.ServiceUpdate(r, serviceName, "--env", "TARGET=v1", "--traffic", "@latest=45")
|
||||||
|
|
||||||
|
rev1 := fmt.Sprintf("%s-00001", serviceName)
|
||||||
|
rev2 := fmt.Sprintf("%s-00002", serviceName)
|
||||||
|
|
||||||
|
expectedTargets := []TargetFields{newTargetFields("", rev2, 45, true), newTargetFields("", rev1, 55, false)}
|
||||||
|
verifyTargets(r, serviceName, expectedTargets, false)
|
||||||
|
test.ServiceDelete(r, serviceName)
|
||||||
|
})
|
||||||
|
t.Run("45:55 automatic traffic split to @latest after mutation", func(t *testing.T) {
|
||||||
|
t.Log("direct 45% traffic explicitly to previous revision and remaining 55 will automatically be directed to @latest")
|
||||||
|
r := test.NewKnRunResultCollector(t, it)
|
||||||
|
defer r.DumpIfFailed()
|
||||||
|
|
||||||
|
serviceName := test.GetNextServiceName(serviceBase)
|
||||||
|
|
||||||
|
test.ServiceCreate(r, serviceName)
|
||||||
|
|
||||||
|
rev1 := fmt.Sprintf("%s-00001", serviceName)
|
||||||
|
rev2 := fmt.Sprintf("%s-00002", serviceName)
|
||||||
|
test.ServiceUpdate(r, serviceName, "--env", "TARGET=v1", "--traffic", fmt.Sprintf("%s=%d", rev1, 45))
|
||||||
|
|
||||||
|
expectedTargets := []TargetFields{newTargetFields("", rev2, 55, true), newTargetFields("", rev1, 45, false)}
|
||||||
|
verifyTargets(r, serviceName, expectedTargets, false)
|
||||||
|
test.ServiceDelete(r, serviceName)
|
||||||
|
})
|
||||||
|
t.Run("automatic traffic split failure", func(t *testing.T) {
|
||||||
|
t.Log("direct 50% traffic to one of the three revisions. Remaining will not be automatically redirected as only one revision should be missing from spec")
|
||||||
|
r := test.NewKnRunResultCollector(t, it)
|
||||||
|
defer r.DumpIfFailed()
|
||||||
|
|
||||||
|
serviceName := test.GetNextServiceName(serviceBase)
|
||||||
|
|
||||||
|
rev1 := fmt.Sprintf("%s-00001", serviceName)
|
||||||
|
|
||||||
|
test.ServiceCreate(r, serviceName)
|
||||||
|
|
||||||
|
test.ServiceUpdate(r, serviceName, "--env", "TARGET=v1")
|
||||||
|
test.ServiceUpdate(r, serviceName, "--env", "TARGET=v2")
|
||||||
|
test.ServiceUpdateWithError(r, serviceName, "--traffic", fmt.Sprintf("%s=%d", rev1, 50))
|
||||||
|
|
||||||
|
test.ServiceDelete(r, serviceName)
|
||||||
|
})
|
||||||
|
t.Run("automatic traffic split failure with @latest", func(t *testing.T) {
|
||||||
|
t.Log("direct 50% traffic to @latest of the three revisions. Remaining will not be automatically redirected as only one revision should be missing from spec")
|
||||||
|
r := test.NewKnRunResultCollector(t, it)
|
||||||
|
defer r.DumpIfFailed()
|
||||||
|
|
||||||
|
serviceName := test.GetNextServiceName(serviceBase)
|
||||||
|
|
||||||
|
test.ServiceCreate(r, serviceName)
|
||||||
|
|
||||||
|
test.ServiceUpdate(r, serviceName, "--env", "TARGET=v1")
|
||||||
|
test.ServiceUpdateWithError(r, serviceName, "--env", "TARGET=v2", "--traffic", "@latest=50")
|
||||||
|
|
||||||
|
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")
|
||||||
|
|
@ -364,6 +430,44 @@ func TestTrafficSplit(t *testing.T) {
|
||||||
test.ServiceDelete(r, serviceName)
|
test.ServiceDelete(r, serviceName)
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
t.Run("TagLatestAsCurrentWithoutKey",
|
||||||
|
func(t *testing.T) {
|
||||||
|
t.Log("tag latest ready revision of service as current")
|
||||||
|
r := test.NewKnRunResultCollector(t, it)
|
||||||
|
defer r.DumpIfFailed()
|
||||||
|
|
||||||
|
serviceName := test.GetNextServiceName(serviceBase)
|
||||||
|
// existing state: latest revision has no tag
|
||||||
|
rev1 := fmt.Sprintf("%s-rev-1", serviceName)
|
||||||
|
serviceCreateWithOptions(r, serviceName, "--revision-name", rev1)
|
||||||
|
|
||||||
|
// desired state: tag latest ready revision as 'current'
|
||||||
|
test.ServiceUpdate(r, serviceName, "--tag", "current")
|
||||||
|
|
||||||
|
expectedTargets := []TargetFields{newTargetFields("current", rev1, 100, true)}
|
||||||
|
verifyTargets(r, serviceName, expectedTargets, false)
|
||||||
|
test.ServiceDelete(r, serviceName)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
t.Run("TagMisspelledLatestAsCurrent",
|
||||||
|
func(t *testing.T) {
|
||||||
|
t.Log("tag misspelled @latest tag of service")
|
||||||
|
r := test.NewKnRunResultCollector(t, it)
|
||||||
|
defer r.DumpIfFailed()
|
||||||
|
|
||||||
|
serviceName := test.GetNextServiceName(serviceBase)
|
||||||
|
// existing state: latest revision has no tag
|
||||||
|
rev1 := fmt.Sprintf("%s-rev-1", serviceName)
|
||||||
|
serviceCreateWithOptions(r, serviceName, "--revision-name", rev1)
|
||||||
|
|
||||||
|
// desired state: tag latest ready revision as 'current'
|
||||||
|
test.ServiceUpdateWithError(r, serviceName, "--tag", "@ltest=current")
|
||||||
|
|
||||||
|
expectedTargets := []TargetFields{newTargetFields("", rev1, 100, true)}
|
||||||
|
verifyTargets(r, serviceName, expectedTargets, true)
|
||||||
|
test.ServiceDelete(r, serviceName)
|
||||||
|
},
|
||||||
|
)
|
||||||
t.Run("UpdateTag:100:0",
|
t.Run("UpdateTag:100:0",
|
||||||
func(t *testing.T) {
|
func(t *testing.T) {
|
||||||
t.Log("update tag for a revision as testing and assign all the traffic to it")
|
t.Log("update tag for a revision as testing and assign all the traffic to it")
|
||||||
|
|
@ -426,6 +530,24 @@ func TestTrafficSplit(t *testing.T) {
|
||||||
test.ServiceDelete(r, serviceName)
|
test.ServiceDelete(r, serviceName)
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
t.Run("CreateWithTag",
|
||||||
|
func(t *testing.T) {
|
||||||
|
t.Log("use --tag with create to create a tagged revision")
|
||||||
|
r := test.NewKnRunResultCollector(t, it)
|
||||||
|
defer r.DumpIfFailed()
|
||||||
|
|
||||||
|
serviceName := test.GetNextServiceName(serviceBase)
|
||||||
|
rev1 := fmt.Sprintf("%s-rev-1", serviceName)
|
||||||
|
serviceCreateWithOptions(r, serviceName, "--tag", "foo", "--revision-name", rev1)
|
||||||
|
|
||||||
|
expectedTargets := []TargetFields{
|
||||||
|
newTargetFields("foo", rev1, 100, true),
|
||||||
|
}
|
||||||
|
|
||||||
|
verifyTargets(r, serviceName, expectedTargets, true)
|
||||||
|
test.ServiceDelete(r, serviceName)
|
||||||
|
},
|
||||||
|
)
|
||||||
t.Run("UntagNonExistentTag",
|
t.Run("UntagNonExistentTag",
|
||||||
func(t *testing.T) {
|
func(t *testing.T) {
|
||||||
t.Log("use --untag on a tag that does not exist")
|
t.Log("use --untag on a tag that does not exist")
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue