Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --tag flag to service create and allow traffic split <100 when @latest is specified #1514

Merged
merged 10 commits into from
Nov 29, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/cmd/kn_service_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ kn service create NAME --image IMAGE
--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)
--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)
--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-.
Expand Down
29 changes: 22 additions & 7 deletions pkg/kn/commands/flags/traffic.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,37 @@ type Traffic struct {
}

func (t *Traffic) Add(cmd *cobra.Command) {
cmd.Flags().StringSliceVar(&t.RevisionsPercentages,
"traffic",
t.AddTrafficFlag(cmd)

t.AddTagFlag(cmd)

t.AddUntagFlag(cmd)
}

// AddUntagFlag adds the flag --untag to the command
func (t *Traffic) AddUntagFlag(cmd *cobra.Command) {
vyasgun marked this conversation as resolved.
Show resolved Hide resolved
cmd.Flags().StringSliceVar(&t.UntagRevisions,
"untag",
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%.")
"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) {
vyasgun marked this conversation as resolved.
Show resolved Hide resolved
cmd.Flags().StringSliceVar(&t.RevisionsTags,
"tag",
nil,
"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.")
}

cmd.Flags().StringSliceVar(&t.UntagRevisions,
"untag",
// AddTrafficFlag adds the flag --traffic to the command
func (t *Traffic) AddTrafficFlag(cmd *cobra.Command) {
vyasgun marked this conversation as resolved.
Show resolved Hide resolved
cmd.Flags().StringSliceVar(&t.RevisionsPercentages,
"traffic",
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 {
Expand Down
13 changes: 13 additions & 0 deletions pkg/kn/commands/service/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (

"knative.dev/client/pkg/config"
"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"

"knative.dev/serving/pkg/apis/serving"
Expand Down Expand Up @@ -82,6 +84,7 @@ var create_example = `
func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
var editFlags ConfigurationEditFlags
var waitFlags commands.WaitFlags
var trafficFlags flags.Traffic

serviceCreateCommand := &cobra.Command{
Use: "create NAME --image IMAGE",
Expand Down Expand Up @@ -118,6 +121,15 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
if err != nil {
return err
}
if trafficFlags.TagsChanged(cmd) {
trafficFlags.RevisionsPercentages = []string{"@latest=100"}

traffic, err := traffic.Compute(cmd, service.Spec.Traffic, &trafficFlags, service.Name, nil)
if err != nil {
return err
}
service.Spec.Traffic = traffic
}
serviceExists, err := serviceExists(cmd.Context(), client, service.Name)
if err != nil {
return err
Expand All @@ -143,6 +155,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false)
commands.AddGitOpsFlags(serviceCreateCommand.Flags())
editFlags.AddCreateFlags(serviceCreateCommand)
trafficFlags.AddTagFlag(serviceCreateCommand)
waitFlags.AddConditionWaitFlags(serviceCreateCommand, commands.WaitDefaultTimeout, "create", "service", "ready")
return serviceCreateCommand
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/kn/commands/service/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1082,3 +1082,12 @@ func TestServiceCreateFromYAMLWithOverrideError(t *testing.T) {
assert.Assert(t, err != nil)
assert.Assert(t, util.ContainsAll(err.Error(), "expected", "0", "<=", "2147483647", "autoscaling.knative.dev/maxScale"))
}

vyasgun marked this conversation as resolved.
Show resolved Hide resolved
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")
}
5 changes: 4 additions & 1 deletion pkg/kn/traffic/compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ func newServiceTraffic(traffic []servingv1.TrafficTarget) ServiceTraffic {

func splitByEqualSign(pair string) (string, string, error) {
parts := strings.Split(pair, "=")
if len(parts) == 1 {
return latestRevisionRef, parts[0], nil
vyasgun marked this conversation as resolved.
Show resolved Hide resolved
}
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
return "", "", fmt.Errorf("expecting the value format in value1=value2, given %s", pair)
}
Expand Down Expand Up @@ -416,7 +419,7 @@ func Compute(cmd *cobra.Command, targets []servingv1.TrafficTarget,
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
traffic.ResetAllTargetPercent()

Expand Down
19 changes: 19 additions & 0 deletions test/e2e/traffic_split_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,24 @@ func TestTrafficSplit(t *testing.T) {
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",
func(t *testing.T) {
t.Log("use --untag on a tag that does not exist")
Expand Down Expand Up @@ -485,6 +503,7 @@ func TestTrafficSplit(t *testing.T) {

func verifyTargets(r *test.KnRunResultCollector, serviceName string, expectedTargets []TargetFields, expectErr bool) {
out := test.ServiceDescribeWithJSONPath(r, serviceName, targetsJsonPath)
r.T().Log(out)
vyasgun marked this conversation as resolved.
Show resolved Hide resolved
assert.Check(r.T(), out != "")
actualTargets, err := splitTargets(out, targetsSeparator, len(expectedTargets))
if !expectErr {
Expand Down