diff --git a/pkg/kn/commands/configuration_edit_flags.go b/pkg/kn/commands/configuration_edit_flags.go index 184e046844..f6747b704f 100644 --- a/pkg/kn/commands/configuration_edit_flags.go +++ b/pkg/kn/commands/configuration_edit_flags.go @@ -30,6 +30,8 @@ type ConfigurationEditFlags struct { Env []string RequestsFlags, LimitsFlags ResourceFlags ForceCreate bool + Async bool + WaitTimeout int } type ResourceFlags struct { @@ -51,6 +53,8 @@ func (p *ConfigurationEditFlags) AddUpdateFlags(command *cobra.Command) { func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) { p.AddUpdateFlags(command) command.Flags().BoolVar(&p.ForceCreate, "force", false, "Create service forcefully, replaces existing service if any.") + command.Flags().BoolVar(&p.Async, "async", false, "Don't wait for the service to become Ready.") + command.Flags().IntVar(&p.WaitTimeout, "wait-timeout", 60, "Wait for that many seconds before giving up when creating a service.") command.MarkFlagRequired("image") } diff --git a/pkg/kn/commands/service_create.go b/pkg/kn/commands/service_create.go index dcda990958..554cef5f20 100644 --- a/pkg/kn/commands/service_create.go +++ b/pkg/kn/commands/service_create.go @@ -17,7 +17,16 @@ package commands import ( "errors" "fmt" + "github.com/knative/pkg/apis" + "io" + api_errors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/watch" + "time" + serving_lib "github.com/knative/client/pkg/serving" + serving_v1alpha1_api "github.com/knative/serving/pkg/apis/serving/v1alpha1" + serving_v1alpha1_client "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1" servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" @@ -25,6 +34,7 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) + func NewServiceCreateCommand(p *KnParams) *cobra.Command { var editFlags ConfigurationEditFlags @@ -52,10 +62,11 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) (err error) { if len(args) != 1 { - return errors.New("requires the service name.") + return errors.New("'service create' requires the service name given as single argument") } + name := args[0] if editFlags.Image == "" { - return errors.New("requires the image name to run.") + return errors.New("'service create' requires the image name to run provided with the --image option") } namespace, err := GetNamespace(cmd) @@ -63,51 +74,37 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { return err } - service := servingv1alpha1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: args[0], - Namespace: namespace, - }, + service, err := constructService(cmd, editFlags, name, namespace) + if err != nil { + return err } - service.Spec.DeprecatedRunLatest = &servingv1alpha1.RunLatestType{ - Configuration: servingv1alpha1.ConfigurationSpec{ - DeprecatedRevisionTemplate: &servingv1alpha1.RevisionTemplateSpec{ - Spec: servingv1alpha1.RevisionSpec{ - DeprecatedContainer: &corev1.Container{}, - }, - }, - }, + client, err := p.ServingFactory() + if err != nil { + return err } - err = editFlags.Apply(&service, cmd) + serviceExists, err := serviceExists(client, service.Name, namespace) if err != nil { return err } - client, err := p.ServingFactory() + + if editFlags.ForceCreate && serviceExists { + err = replaceService(client, service, namespace, cmd.OutOrStdout()) + } else { + err = createService(client, service, namespace, cmd.OutOrStdout()) + } if err != nil { return err } - var serviceExists bool = false - if editFlags.ForceCreate { - existingService, err := client.Services(namespace).Get(args[0], v1.GetOptions{}) - if err == nil { - serviceExists = true - service.ResourceVersion = existingService.ResourceVersion - _, err = client.Services(namespace).Update(&service) - if err != nil { - return err - } - fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully replaced in namespace '%s'.\n", args[0], namespace) - } - } - if !serviceExists { - _, err = client.Services(namespace).Create(&service) - if err != nil { + + if !editFlags.Async { + if err := waitForService(client, name, namespace, cmd.OutOrStdout(), editFlags.WaitTimeout); err != nil { return err } - fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully created in namespace '%s'.\n", args[0], namespace) + return showUrl(client, name, namespace, cmd.OutOrStdout()) } + return nil }, } @@ -115,3 +112,140 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command { editFlags.AddCreateFlags(serviceCreateCommand) return serviceCreateCommand } + +func createService(client serving_v1alpha1_client.ServingV1alpha1Interface, service *serving_v1alpha1_api.Service, namespace string, out io.Writer) error { + _, err := client.Services(namespace).Create(service) + if err != nil { + return err + } + fmt.Fprintf(out, "Service '%s' successfully created in namespace '%s'.\n", service.Name, namespace) + return nil +} + +func replaceService(client serving_v1alpha1_client.ServingV1alpha1Interface, service *serving_v1alpha1_api.Service, namespace string, out io.Writer) error { + existingService, err := client.Services(namespace).Get(service.Name, v1.GetOptions{}) + if err != nil { + return err + } + service.ResourceVersion = existingService.ResourceVersion + _, err = client.Services(namespace).Update(service) + if err != nil { + return err + } + fmt.Fprintf(out, "Service '%s' successfully replaced in namespace '%s'.\n", service.Name, namespace) + return nil +} + +func serviceExists(client serving_v1alpha1_client.ServingV1alpha1Interface, name string, namespace string) (bool, error) { + _, err := client.Services(namespace).Get(name, v1.GetOptions{}) + if api_errors.IsNotFound(err) { + return false, nil + } + if err != nil { + return false, err + } + return true, nil +} + +// Create service struct from provided options +func constructService(cmd *cobra.Command, editFlags ConfigurationEditFlags, name string, namespace string) (*serving_v1alpha1_api.Service, + error) { + + service := serving_v1alpha1_api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + + // TODO: Should it always be `runLatest` ? + service.Spec.DeprecatedRunLatest = &serving_v1alpha1_api.RunLatestType{ + Configuration: serving_v1alpha1_api.ConfigurationSpec{ + DeprecatedRevisionTemplate: &serving_v1alpha1_api.RevisionTemplateSpec{ + Spec: serving_v1alpha1_api.RevisionSpec{ + DeprecatedContainer: &corev1.Container{}, + }, + }, + }, + } + + config, err := serving_lib.GetConfiguration(&service) + if err != nil { + return nil, err + } + err = editFlags.Apply(config, cmd) + if err != nil { + return nil, err + } + return &service, nil +} + +func showUrl(client serving_v1alpha1_client.ServingV1alpha1Interface, serviceName string, namespace string, out io.Writer) error { + service, err := client.Services(namespace).Get(serviceName,v1.GetOptions{}) + if err != nil { + return fmt.Errorf("cannot fetch service %s in namespace %s for extracting the URL", serviceName, namespace) + } + url := service.Status.URL.String() + if url == "" { + url = service.Status.DeprecatedDomain + } + fmt.Fprintln(out, "\nService URL:") + fmt.Fprintf(out, "%s\n", url) + return nil +} + +// Duck type for writers having a flush +type flusher interface { + Flush() error +} + +func waitForService(client serving_v1alpha1_client.ServingV1alpha1Interface, serviceName string, namespace string, out io.Writer, timeout int) error { + // Wait for service to enter 'Ready' state, with a timeout of which is slightly larger than the provided + // timeout. We have our own timeout which fire after "timeout" seconds and stop the watch + var timeoutWatch int64 = int64(timeout + 30) + opts := v1.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("metadata.name", serviceName).String(), + TimeoutSeconds: &timeoutWatch, + } + watcher, err := client.Services(namespace).Watch(opts) + if err != nil { + return err + } + fmt.Fprintf(out, "Waiting for service %s to become ready ... ", serviceName) + if flusher, ok := out.(flusher); ok { + flusher.Flush() + } + + if err := waitForReadyCondition(watcher, timeout); err != nil { + fmt.Fprintln(out) + return err + } + fmt.Fprintln(out, "OK") + return nil +} + +func waitForReadyCondition(watcher watch.Interface, timeout int) error { + defer watcher.Stop() + for { + select { + case <- time.After(time.Duration(timeout) * time.Second): + return fmt.Errorf("timeout: Service not ready after %d seconds", timeout) + case event, ok := <- watcher.ResultChan(): + if !ok || event.Object == nil { + return errors.New("timeout while waiting for service to become ready") + } + service := event.Object.(*serving_v1alpha1_api.Service) + for _, cond := range service.Status.Conditions { + if cond.Type == apis.ConditionReady { + switch cond.Status { + case corev1.ConditionTrue: + return nil + case corev1.ConditionFalse: + return fmt.Errorf("%s: %s",cond.Reason,cond.Message) + } + } + } + } + } +} + diff --git a/pkg/kn/commands/service_create_test.go b/pkg/kn/commands/service_create_test.go index 7cfc014dd6..06594fc9fc 100644 --- a/pkg/kn/commands/service_create_test.go +++ b/pkg/kn/commands/service_create_test.go @@ -18,6 +18,8 @@ import ( "bytes" "errors" "fmt" + api_errors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" "reflect" "strings" "testing" @@ -32,7 +34,7 @@ import ( client_testing "k8s.io/client-go/testing" ) -func fakeServiceCreate(args []string) ( +func fakeServiceCreate(args []string, withExistingService bool) ( action client_testing.Action, created *v1alpha1.Service, output string, @@ -44,12 +46,19 @@ func fakeServiceCreate(args []string) ( Output: buf, ServingFactory: func() (serving.ServingV1alpha1Interface, error) { return fakeServing, nil }, }) - fakeServing.AddReactor("*", "*", + fakeServing.AddReactor("get", "services", + func(a client_testing.Action) (bool, runtime.Object, error) { + if withExistingService { + return true, &v1alpha1.Service{}, nil + } + return true, nil, api_errors.NewNotFound(schema.GroupResource{}, "") + }) + fakeServing.AddReactor("create", "services", func(a client_testing.Action) (bool, runtime.Object, error) { createAction, ok := a.(client_testing.CreateAction) action = createAction if !ok { - return true, nil, fmt.Errorf("wrong kind of action %v", action) + return true, nil, fmt.Errorf("wrong kind of action %v", a) } created, ok = createAction.GetObject().(*v1alpha1.Service) if !ok { @@ -68,7 +77,7 @@ func fakeServiceCreate(args []string) ( func TestServiceCreateImage(t *testing.T) { action, created, output, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:baz"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--async"}, false) if err != nil { t.Fatal(err) } else if !action.Matches("create", "services") { @@ -87,7 +96,7 @@ func TestServiceCreateImage(t *testing.T) { func TestServiceCreateEnv(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "A=DOGS", "--env", "B=WOLVES"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "A=DOGS", "--env", "B=WOLVES", "--async"}, false) if err != nil { t.Fatal(err) @@ -118,7 +127,7 @@ func TestServiceCreateEnv(t *testing.T) { func TestServiceCreateWithRequests(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--requests-memory", "64Mi"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--requests-memory", "64Mi","--async"}, false) if err != nil { t.Fatal(err) @@ -144,7 +153,7 @@ func TestServiceCreateWithRequests(t *testing.T) { func TestServiceCreateWithLimits(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--limits-cpu", "1000m", "--limits-memory", "1024Mi"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--limits-cpu", "1000m", "--limits-memory", "1024Mi","--async"}, false) if err != nil { t.Fatal(err) @@ -170,7 +179,7 @@ func TestServiceCreateWithLimits(t *testing.T) { func TestServiceCreateRequestsLimitsCPU(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--limits-cpu", "1000m"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--limits-cpu", "1000m","--async"}, false) if err != nil { t.Fatal(err) @@ -207,7 +216,7 @@ func TestServiceCreateRequestsLimitsCPU(t *testing.T) { func TestServiceCreateRequestsLimitsMemory(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-memory", "64Mi", "--limits-memory", "1024Mi"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-memory", "64Mi", "--limits-memory", "1024Mi","--async"}, false) if err != nil { t.Fatal(err) @@ -246,7 +255,7 @@ func TestServiceCreateRequestsLimitsCPUMemory(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--requests-cpu", "250m", "--limits-cpu", "1000m", - "--requests-memory", "64Mi", "--limits-memory", "1024Mi"}) + "--requests-memory", "64Mi", "--limits-memory", "1024Mi","--async"}, false) if err != nil { t.Fatal(err) @@ -293,12 +302,12 @@ func parseQuantity(t *testing.T, quantityString string) resource.Quantity { func TestServiceCreateImageForce(t *testing.T) { _, _, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:v1"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:v1","--async"}, false) if err != nil { t.Fatal(err) } action, created, output, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2"}) + "service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2","--async"}, false) if err != nil { t.Fatal(err) } else if !action.Matches("create", "services") { @@ -316,12 +325,12 @@ func TestServiceCreateImageForce(t *testing.T) { func TestServiceCreateEnvForce(t *testing.T) { _, _, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:v1", "-e", "A=DOGS", "--env", "B=WOLVES"}) + "service", "create", "foo", "--image", "gcr.io/foo/bar:v1", "-e", "A=DOGS", "--env", "B=WOLVES","--async"}, false) if err != nil { t.Fatal(err) } action, created, output, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2", "-e", "A=CATS", "--env", "B=LIONS"}) + "service", "create", "foo", "--force", "--image", "gcr.io/foo/bar:v2", "-e", "A=CATS", "--env", "B=LIONS","--async"}, false) if err != nil { t.Fatal(err)