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

fixes(#606): adds --cluster-local / --no-cluster-local flags #629

Merged
merged 12 commits into from
Mar 10, 2020
9 changes: 9 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@
|===
////

## v0.13.0 (2020-02-14)
maximilien marked this conversation as resolved.
Show resolved Hide resolved

[cols="1,10,3", options="header", width="100%"]
|===
| | Description | PR

| 🎁
| adds --cluster-local / --no-cluster-local flags
navidshaikh marked this conversation as resolved.
Show resolved Hide resolved
| https://github.com/knative/client/pull/629[#629]
maximilien marked this conversation as resolved.
Show resolved Hide resolved

## v0.13.0 (unreleased)

Expand Down
5 changes: 5 additions & 0 deletions docs/cmd/kn_service_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ kn service create NAME --image IMAGE [flags]

# Create a service with annotation
kn service create s1 --image dev.local/ns/image:v3 --annotation sidecar.istio.io/inject=false

# Create a private service (that is a service with no external endpoint)
kn service create s1 --image dev.local/ns/image:v3 --cluster-local
```

### Options
Expand All @@ -45,6 +48,7 @@ kn service create NAME --image IMAGE [flags]
--annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-).
--async Create service and don't wait for it to become ready.
--autoscale-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)
--cluster-local specify that the service be private. (--no-cluster-local will make the service publicly available
--concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica.
--concurrency-target int Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
-e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-).
Expand All @@ -60,6 +64,7 @@ kn service create NAME --image IMAGE [flags]
--min-scale int Minimal number of replicas.
--mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume.
-n, --namespace string Specify the namespace to operate in.
--no-cluster-local do not specify that the service be private. (--no-cluster-local will make the service publicly available (default true)
--no-lock-to-digest do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)
-p, --port int32 The port where application listens on.
--pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
Expand Down
2 changes: 2 additions & 0 deletions docs/cmd/kn_service_update.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ kn service update NAME [flags]
--annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-).
--async Update service and don't wait for it to become ready.
--autoscale-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)
--cluster-local specify that the service be private. (--no-cluster-local will make the service publicly available
--concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica.
--concurrency-target int Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
-e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-).
Expand All @@ -55,6 +56,7 @@ kn service update NAME [flags]
--min-scale int Minimal number of replicas.
--mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume.
-n, --namespace string Specify the namespace to operate in.
--no-cluster-local do not specify that the service be private. (--no-cluster-local will make the service publicly available (default true)
--no-lock-to-digest do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)
-p, --port int32 The port where application listens on.
--pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
Expand Down
11 changes: 11 additions & 0 deletions pkg/kn/commands/service/configuration_edit_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type ConfigurationEditFlags struct {
ServiceAccountName string
ImagePullSecrets string
Annotations []string
ClusterLocal bool

// Preferences about how to do the action.
LockToDigest bool
Expand Down Expand Up @@ -117,6 +118,9 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
p.markFlagMakesRevision("max-scale")
command.Flags().StringVar(&p.AutoscaleWindow, "autoscale-window", "", "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)")
p.markFlagMakesRevision("autoscale-window")
flags.AddBothBoolFlagsUnhidden(command.Flags(), &p.ClusterLocal, "cluster-local", "", false,
"specify that the service be private. (--no-cluster-local will make the service publicly available")
navidshaikh marked this conversation as resolved.
Show resolved Hide resolved
p.markFlagMakesRevision("cluster-local")
command.Flags().IntVar(&p.ConcurrencyTarget, "concurrency-target", 0,
"Recommendation for when to scale up based on the concurrent number of incoming request. "+
"Defaults to --concurrency-limit when given.")
Expand Down Expand Up @@ -309,6 +313,13 @@ func (p *ConfigurationEditFlags) Apply(
}
}

if cmd.Flags().Changed("cluster-local") || cmd.Flags().Changed("no-cluster-local") {
err = servinglib.UpdateClusterLocal(service, template, p.ClusterLocal)
if err != nil {
return err
}
}

if cmd.Flags().Changed("concurrency-target") {
err = servinglib.UpdateConcurrencyTarget(template, p.ConcurrencyTarget)
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion pkg/kn/commands/service/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ var create_example = `
kn service create --force s1 --image dev.local/ns/image:v1

# Create a service with annotation
kn service create s1 --image dev.local/ns/image:v3 --annotation sidecar.istio.io/inject=false`
kn service create s1 --image dev.local/ns/image:v3 --annotation sidecar.istio.io/inject=false

# Create a private service (that is a service with no external endpoint)
kn service create s1 --image dev.local/ns/image:v3 --cluster-local`

func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
var editFlags ConfigurationEditFlags
Expand Down
26 changes: 26 additions & 0 deletions pkg/kn/commands/service/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"strings"
"testing"

"gotest.tools/assert"
api_errors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/watch"
Expand All @@ -34,6 +35,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
client_testing "k8s.io/client-go/testing"
"knative.dev/serving/pkg/apis/serving/v1alpha1"
"knative.dev/serving/pkg/reconciler/route/config"
)

func fakeServiceCreate(args []string, withExistingService bool, sync bool) (
Expand Down Expand Up @@ -493,3 +495,27 @@ func TestServiceCreateWithServiceAccountName(t *testing.T) {
t.Fatalf("wrong service account name:%v", template.Spec.ServiceAccountName)
}
}

func TestServiceCreateWithClusterLocal(t *testing.T) {
action, created, _, err := fakeServiceCreate([]string{
"service", "create", "foo", "--image", "gcr.io/foo/bar:baz",
"--cluster-local", "--async"}, false, false)

if err != nil {
t.Fatal(err)
} else if !action.Matches("create", "services") {
t.Fatalf("Bad action %v", action)
}

template, err := servinglib.RevisionTemplateOfService(created)
if err != nil {
t.Fatal(err)
}

labelValue, present := template.Labels[config.VisibilityLabelKey]
assert.Assert(t, present)

if labelValue != config.VisibilityClusterLocal {
t.Fatalf("Incorrect VisibilityClusterLocal value '%s'", labelValue)
}
}
10 changes: 10 additions & 0 deletions pkg/serving/config_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"knative.dev/serving/pkg/apis/autoscaling"
"knative.dev/serving/pkg/apis/serving"
servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1"
"knative.dev/serving/pkg/reconciler/route/config"
)

// VolumeSourceType is a type standing for enumeration of ConfigMap and Secret
Expand Down Expand Up @@ -193,6 +194,15 @@ func UpdateAutoscaleWindow(template *servingv1alpha1.RevisionTemplateSpec, windo
return UpdateRevisionTemplateAnnotation(template, autoscaling.WindowAnnotationKey, window)
}

// UpdateClusterLocal updates container serving.knative.dev/visibility annotation
func UpdateClusterLocal(service *servingv1alpha1.Service, template *servingv1alpha1.RevisionTemplateSpec, clusterLocal bool) error {
if clusterLocal {
return UpdateLabels(service, template, map[string]string{config.VisibilityLabelKey: config.VisibilityClusterLocal}, []string{})
} else {
return UpdateLabels(service, template, map[string]string{}, []string{config.VisibilityLabelKey})
}
}

// UpdateConcurrencyTarget updates container concurrency annotation
func UpdateConcurrencyTarget(template *servingv1alpha1.RevisionTemplateSpec, target int) error {
return UpdateRevisionTemplateAnnotation(template, autoscaling.TargetAnnotationKey, strconv.Itoa(target))
Expand Down