diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 72f92524d0..d1da34c80e 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -17,6 +17,9 @@ |=== | | Description | PR +| 🎁 +| Add an `client.knative.dev/updateTimestamp` annotation to trigger a new revision when required +| https://github.com/knative/client/pull/1364[#1364] | 🎁 | Adding base64 data handling to ping update command @@ -25,6 +28,7 @@ | ✨ | make --cmd flag as an array instead of string | https://github.com/knative/client/pull/1380[#1380] + |=== ## v0.24.0 (2021-06-29) diff --git a/pkg/kn/commands/flags/listprint_test.go b/pkg/kn/commands/flags/listprint_test.go index c9109485c9..a5e8e0cd46 100644 --- a/pkg/kn/commands/flags/listprint_test.go +++ b/pkg/kn/commands/flags/listprint_test.go @@ -76,10 +76,8 @@ func TestListPrintFlags(t *testing.T) { assert.NilError(t, err) assert.Assert(t, allowMissingTemplateKeys == true) - p, err := flags.ToPrinter() + _, err = flags.ToPrinter() assert.NilError(t, err) - _, ok := p.(hprinters.ResourcePrinter) - assert.Check(t, ok == true) } func TestListPrintFlagsPrint(t *testing.T) { diff --git a/pkg/kn/commands/revision/describe.go b/pkg/kn/commands/revision/describe.go index ff5e5fa4ce..42488ae66e 100644 --- a/pkg/kn/commands/revision/describe.go +++ b/pkg/kn/commands/revision/describe.go @@ -153,8 +153,8 @@ func WriteConcurrencyOptions(dw printers.PrefixWriter, revision *servingv1.Revis // Write the image attribute (with func WriteImage(dw printers.PrefixWriter, revision *servingv1.Revision) { - c, err := clientserving.ContainerOfRevisionSpec(&revision.Spec) - if err != nil { + c := clientserving.ContainerOfRevisionSpec(&revision.Spec) + if c == nil { dw.WriteAttribute("Image", "Unknown") return } @@ -223,8 +223,8 @@ func WriteScale(dw printers.PrefixWriter, revision *servingv1.Revision) { } func WriteResources(dw printers.PrefixWriter, r *servingv1.Revision) { - c, err := clientserving.ContainerOfRevisionSpec(&r.Spec) - if err != nil { + c := clientserving.ContainerOfRevisionSpec(&r.Spec) + if c == nil { return } requests := c.Resources.Requests @@ -280,8 +280,8 @@ func formatScale(minScale *int, maxScale *int) string { } func stringifyEnv(revision *servingv1.Revision) []string { - container, err := clientserving.ContainerOfRevisionSpec(&revision.Spec) - if err != nil { + container := clientserving.ContainerOfRevisionSpec(&revision.Spec) + if container == nil { return nil } @@ -297,8 +297,8 @@ func stringifyEnv(revision *servingv1.Revision) []string { } func stringifyEnvFrom(revision *servingv1.Revision) []string { - container, err := clientserving.ContainerOfRevisionSpec(&revision.Spec) - if err != nil { + container := clientserving.ContainerOfRevisionSpec(&revision.Spec) + if container == nil { return nil } diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index f8100b938d..78aa1f4c1a 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -205,9 +205,9 @@ func (p *ConfigurationEditFlags) Apply( return err } - name := "" + // Client side revision naming requires to set a new revisionname if p.RevisionName != "" { - name, err = servinglib.GenerateRevisionName(p.RevisionName, service) + name, err := servinglib.GenerateRevisionName(p.RevisionName, service) if err != nil { return err } @@ -217,12 +217,20 @@ func (p *ConfigurationEditFlags) Apply( } } - _, userImagePresent := template.Annotations[servinglib.UserImageAnnotationKey] - freezeMode := userImagePresent || cmd.Flags().Changed("lock-to-digest") - if p.LockToDigest && p.AnyMutation(cmd) && freezeMode { - servinglib.SetUserImageAnnot(template) - if !cmd.Flags().Changed("image") { - err = servinglib.FreezeImageToDigest(template, baseRevision) + // If some change happened that can cause a revision, set the update timestamp + // But not for "apply", this would destroy idempotency + if p.AnyMutation(cmd) && cmd.Name() != "apply" { + servinglib.UpdateTimestampAnnotation(template) + } + + if p.shouldPinToImageDigest(template, cmd) { + servinglib.UpdateUserImageAnnotation(template) + // Don't copy over digest of base revision if an image is specified. + // If an --image is given, always use the tagged named to cause a re-resolving + // of the digest by the serving backend (except when you use "apply" where you + // always have to provide an image + if !cmd.Flags().Changed("image") || cmd.Name() == "apply" { + err = servinglib.PinImageToDigest(template, baseRevision) if err != nil { return err } @@ -230,7 +238,7 @@ func (p *ConfigurationEditFlags) Apply( } if !p.LockToDigest { - servinglib.UnsetUserImageAnnot(template) + servinglib.UnsetUserImageAnnotation(template) } if cmd.Flags().Changed("limits-cpu") || cmd.Flags().Changed("limits-memory") { @@ -432,6 +440,23 @@ func (p *ConfigurationEditFlags) Apply( return nil } +// shouldPinToImageDigest checks whether the image digest that has been resolved in the current active +// revision should be used for the image update. This is useful if an update of the image +// should be prevented. +func (p *ConfigurationEditFlags) shouldPinToImageDigest(template *servingv1.RevisionTemplateSpec, cmd *cobra.Command) bool { + // The user-image annotation is an indicator that the service has been + // created with lock-to-digest. If this is not the case and neither --lock-to-digest + // has been given explitly then we won't change the image + _, userImagePresent := template.Annotations[servinglib.UserImageAnnotationKey] + if !userImagePresent && !cmd.Flags().Changed("lock-to-digest") { + return false + } + + // When we want an update and --lock-to-digest is set (either given or the default), then + // the image should be pinned to its digest + return p.LockToDigest && p.AnyMutation(cmd) +} + func (p *ConfigurationEditFlags) updateLabels(obj *metav1.ObjectMeta, flagLabels []string, labelsAllMap map[string]string) error { labelFlagMap, err := util.MapFromArrayAllowingSingles(flagLabels, "=") if err != nil { diff --git a/pkg/kn/commands/service/create_mock_test.go b/pkg/kn/commands/service/create_mock_test.go index d3fdf078cc..9f678373ba 100644 --- a/pkg/kn/commands/service/create_mock_test.go +++ b/pkg/kn/commands/service/create_mock_test.go @@ -78,7 +78,7 @@ func TestServiceCreateEnvMock(t *testing.T) { template.Spec.Containers[0].Env = envVars template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "a=mouse", "--env", "b=cookie", "--env", "empty=", "--no-wait", "--revision-name=") assert.NilError(t, err) @@ -106,7 +106,7 @@ func TestServiceCreateLabel(t *testing.T) { template := &service.Spec.Template template.ObjectMeta.Labels = expected template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "-l", "a=mouse", "--label", "b=cookie", "--label=empty", "--no-wait", "--revision-name=") assert.NilError(t, err) @@ -134,7 +134,7 @@ func TestServiceCreateWithEnvFromConfigMap(t *testing.T) { } template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--env-from", "config-map:config-map-name", "--no-wait", "--revision-name=") assert.NilError(t, err) @@ -154,7 +154,7 @@ func TestServiceCreateWithEnvFromConfigMapRemoval(t *testing.T) { template.Spec.Containers[0].EnvFrom = nil template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--env-from", "config-map:config-map-name-", "--no-wait", "--revision-name=") assert.NilError(t, err) @@ -199,7 +199,7 @@ func TestServiceCreateWithEnvFromSecret(t *testing.T) { } template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--env-from", "secret:secret-name", "--no-wait", "--revision-name=") assert.NilError(t, err) @@ -219,7 +219,7 @@ func TestServiceCreateWithEnvFromSecretRemoval(t *testing.T) { template.Spec.Containers[0].EnvFrom = nil template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--env-from", "secret:secret-name-", "--no-wait", "--revision-name=") assert.NilError(t, err) @@ -259,7 +259,7 @@ func TestServiceCreateWithVolumeAndMountConfigMap(t *testing.T) { template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--mount", "/mount/path=volume-name", "--volume", "volume-name=cm:config-map-name", "--no-wait", "--revision-name=") @@ -300,7 +300,7 @@ func TestServiceCreateWithMountConfigMap(t *testing.T) { template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--mount", "/mount/path=cm:config-map-name", "--no-wait", "--revision-name=") @@ -339,7 +339,7 @@ func TestServiceCreateWithVolumeAndMountSecret(t *testing.T) { template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--mount", "/mount/path=volume-name", "--volume", "volume-name=secret:secret-name", "--no-wait", "--revision-name=") @@ -378,7 +378,7 @@ func TestServiceCreateWithMountSecret(t *testing.T) { template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--mount", "/mount/path=sc:secret-name", "--no-wait", "--revision-name=") @@ -402,7 +402,7 @@ func TestServiceCreateWithUser(t *testing.T) { } template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--user", "1001", "--no-wait", "--revision-name=") assert.NilError(t, err) @@ -433,7 +433,7 @@ func TestServiceCreateWithResources(t *testing.T) { template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--request", "cpu=250m,memory=64Mi", @@ -534,7 +534,7 @@ func TestServiceCreateWithAnnotations(t *testing.T) { servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz", } - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--annotation", "foo=bar", @@ -565,7 +565,7 @@ func TestServiceCreateWithRevisionAndServiceAnnotations(t *testing.T) { servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz", } - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--annotation-service", "foo=bar", @@ -592,7 +592,7 @@ func TestServiceCreateWithRevisionAnnotations(t *testing.T) { servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz", } - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--annotation-revision", autoscaling.InitialScaleAnnotationKey+"=1", @@ -621,7 +621,7 @@ func TestServiceCreateWithServiceAnnotations(t *testing.T) { servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz", } - r.CreateService(service, nil) + r.CreateService(verifyService(service, true), nil) output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--annotation-service", "foo=bar", diff --git a/pkg/kn/commands/service/export.go b/pkg/kn/commands/service/export.go index e54a145309..e556899224 100644 --- a/pkg/kn/commands/service/export.go +++ b/pkg/kn/commands/service/export.go @@ -19,6 +19,8 @@ import ( "errors" "fmt" + clientserving "knative.dev/client/pkg/serving" + "sort" "strconv" @@ -50,6 +52,7 @@ var IgnoredRevisionAnnotations = []string{ "serving.knative.dev/lastPinned", "serving.knative.dev/creator", "serving.knative.dev/routingStateModified", + clientserving.UpdateTimestampAnnotationKey, } // IgnoredServiceLabels defines the label keys which should be removed @@ -178,6 +181,8 @@ func exportLatestService(latestSvc *servingv1.Service, withRoutes bool) *serving stripIgnoredAnnotationsFromService(&exportedSvc) stripIgnoredLabelsFromService(&exportedSvc) + stripIgnoredAnnotationsFromRevisionTemplate(&exportedSvc.Spec.Template) + stripIgnoredLabelsFromRevisionTemplate(&exportedSvc.Spec.Template) return &exportedSvc } @@ -357,6 +362,12 @@ func stripIgnoredAnnotationsFromRevision(revision *servingv1.Revision) { } } +func stripIgnoredAnnotationsFromRevisionTemplate(template *servingv1.RevisionTemplateSpec) { + for _, annotation := range IgnoredRevisionAnnotations { + delete(template.ObjectMeta.Annotations, annotation) + } +} + func stripIgnoredLabelsFromService(svc *servingv1.Service) { for _, label := range IgnoredServiceLabels { delete(svc.ObjectMeta.Labels, label) @@ -368,3 +379,9 @@ func stripIgnoredLabelsFromRevision(rev *servingv1.Revision) { delete(rev.ObjectMeta.Labels, label) } } + +func stripIgnoredLabelsFromRevisionTemplate(template *servingv1.RevisionTemplateSpec) { + for _, label := range IgnoredRevisionLabels { + delete(template.ObjectMeta.Labels, label) + } +} diff --git a/pkg/kn/commands/service/export_test.go b/pkg/kn/commands/service/export_test.go index e2a4ef138e..7b2b2ee83f 100644 --- a/pkg/kn/commands/service/export_test.go +++ b/pkg/kn/commands/service/export_test.go @@ -19,6 +19,8 @@ import ( "encoding/json" "testing" + "knative.dev/client/pkg/serving" + "gotest.tools/v3/assert" v1 "k8s.io/api/core/v1" @@ -127,11 +129,13 @@ func TestServiceExportwithMultipleRevisions(t *testing.T) { servingtest.WithRevisionLabel(apiserving.ConfigurationGenerationLabelKey, "1"), servingtest.WithRevisionAnn("client.knative.dev/user-image", "busybox:v1"), servingtest.WithRevisionAnn("serving.knative.dev/lastPinned", "1111132"), + servingtest.WithRevisionAnn(serving.UpdateTimestampAnnotationKey, "now"), ))), libtest.WithRevision(*(libtest.BuildRevision("foo-rev-2", servingtest.WithRevisionLabel(apiserving.ServiceLabelKey, "foo"), servingtest.WithRevisionLabel(apiserving.ConfigurationGenerationLabelKey, "1"), servingtest.WithRevisionAnn("client.knative.dev/user-image", "busybox:v2"), + servingtest.WithRevisionAnn(serving.UpdateTimestampAnnotationKey, "now"), ))), ), expectedKNExport: libtest.BuildKNExportWithOptions( diff --git a/pkg/kn/commands/service/service_update_mock_test.go b/pkg/kn/commands/service/service_update_mock_test.go index bf7eb45a1c..19db19c5a8 100644 --- a/pkg/kn/commands/service/service_update_mock_test.go +++ b/pkg/kn/commands/service/service_update_mock_test.go @@ -129,9 +129,22 @@ func TestServiceUpdateAnnotationsMock(t *testing.T) { func recordServiceUpdateWithSuccess(r *clientservingv1.ServingRecorder, svcName string, newService *servingv1.Service, updatedService *servingv1.Service) { r.GetService(svcName, nil, errors.NewNotFound(servingv1.Resource("service"), svcName)) - r.CreateService(newService, nil) + r.CreateService(verifyService(newService, true), nil) r.GetService(svcName, newService, nil) - r.UpdateService(updatedService, true, nil) + r.UpdateService(verifyService(updatedService, true), true, nil) +} + +// verifyService checks for a service, and also whether the updateTimestamp is set +func verifyService(srv *servingv1.Service, assertUpdateTimeCheck bool) func(t *testing.T, toCheck *servingv1.Service) { + return func(t *testing.T, toCheck *servingv1.Service) { + updateTimestamp := toCheck.Spec.Template.Annotations[clientserving.UpdateTimestampAnnotationKey] + // the update timestamp should be there + assert.Assert(t, (assertUpdateTimeCheck && updateTimestamp != "") || (!assertUpdateTimeCheck && updateTimestamp == "")) + // Delete the timestamps before doing a deep equal to avoid a conflict on this volatile value + delete(toCheck.Spec.Template.Annotations, clientserving.UpdateTimestampAnnotationKey) + assert.DeepEqual(t, srv, toCheck) + } + } func TestServiceUpdateEnvFromAddingWithConfigMap(t *testing.T) { @@ -283,7 +296,7 @@ func TestServiceUpdateEnvFromRemovalWithConfigMap(t *testing.T) { r.GetService(svcName, updatedService1, nil) //r.UpdateService(updatedService2, nil) // since an error happens, update is not triggered here r.GetService(svcName, updatedService2, nil) - r.UpdateService(updatedService3, true, nil) + r.UpdateService(verifyService(updatedService3, true), true, nil) output, err := executeServiceCommand(client, "create", svcName, "--image", "gcr.io/foo/bar:baz", @@ -369,10 +382,10 @@ func TestServiceUpdateEnvFromRemovalWithEmptyName(t *testing.T) { r := client.Recorder() r.GetService(svcName, nil, errors.NewNotFound(servingv1.Resource("service"), svcName)) - r.CreateService(newService, nil) + r.CreateService(verifyService(newService, true), nil) r.GetService(svcName, newService, nil) r.GetService(svcName, newService, nil) - r.UpdateService(updatedService1, true, nil) + r.UpdateService(verifyService(updatedService1, true), true, nil) output, err := executeServiceCommand(client, "create", svcName, "--image", "gcr.io/foo/bar:baz", @@ -623,13 +636,13 @@ func TestServiceUpdateEnvFromRemovalWithSecret(t *testing.T) { r := client.Recorder() r.GetService(svcName, nil, errors.NewNotFound(servingv1.Resource("service"), svcName)) - r.CreateService(newService, nil) + r.CreateService(verifyService(newService, true), nil) r.GetService(svcName, newService, nil) - r.UpdateService(updatedService1, true, nil) + r.UpdateService(verifyService(updatedService1, true), true, nil) r.GetService(svcName, updatedService1, nil) //r.UpdateService(updatedService2, nil) // since an error happens, update is not triggered here r.GetService(svcName, updatedService2, nil) - r.UpdateService(updatedService3, true, nil) + r.UpdateService(verifyService(updatedService3, true), true, nil) output, err := executeServiceCommand(client, "create", svcName, "--image", "gcr.io/foo/bar:baz", @@ -1381,11 +1394,11 @@ func TestServiceUpdateWithRemovingMount(t *testing.T) { r := client.Recorder() r.GetService(svcName, nil, errors.NewNotFound(servingv1.Resource("service"), svcName)) - r.CreateService(newService, nil) + r.CreateService(verifyService(newService, true), nil) r.GetService(svcName, newService, nil) - r.UpdateService(updatedService1, true, nil) + r.UpdateService(verifyService(updatedService1, true), true, nil) r.GetService(svcName, updatedService1, nil) - r.UpdateService(updatedService2, true, nil) + r.UpdateService(verifyService(updatedService2, true), true, nil) output, err := executeServiceCommand(client, "create", svcName, "--image", "gcr.io/foo/bar:baz", diff --git a/pkg/kn/commands/service/update.go b/pkg/kn/commands/service/update.go index 08d2392693..ebbe22f1ff 100644 --- a/pkg/kn/commands/service/update.go +++ b/pkg/kn/commands/service/update.go @@ -85,7 +85,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { updateFunc := func(service *servingv1.Service) (*servingv1.Service, error) { latestRevisionBeforeUpdate = service.Status.LatestReadyRevisionName var baseRevision *servingv1.Revision - if !cmd.Flags().Changed("image") && editFlags.LockToDigest { + if isImagePinned(cmd, editFlags) { baseRevision, err = client.GetBaseRevision(cmd.Context(), service) var errNoBaseRevision clientservingv1.NoBaseRevisionError if errors.As(err, &errNoBaseRevision) { @@ -151,6 +151,10 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { return serviceUpdateCommand } +func isImagePinned(cmd *cobra.Command, editFlags ConfigurationEditFlags) bool { + return !cmd.Flags().Changed("image") && editFlags.LockToDigest +} + func preCheck(cmd *cobra.Command) error { if cmd.Flags().NFlag() == 0 { return fmt.Errorf("flag(s) not set\nUsage: %s", cmd.Use) diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go index 57bad96f96..dd2037ea85 100644 --- a/pkg/kn/commands/service/update_test.go +++ b/pkg/kn/commands/service/update_test.go @@ -93,6 +93,10 @@ func fakeServiceUpdate(original *servingv1.Service, args []string) ( 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"}, + } + rev.Status.DeprecatedImageDigest = exampleImageByDigest return true, rev, nil }) @@ -183,11 +187,8 @@ func TestServiceUpdateImage(t *testing.T) { } template = &updated.Spec.Template - container, err := servinglib.ContainerOfRevisionTemplate(template) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, container.Image, "gcr.io/foo/quux:xyzzy") + container := servinglib.ContainerOfRevisionSpec(&template.Spec) + assert.Assert(t, container != nil && container.Image == "gcr.io/foo/quux:xyzzy") if !strings.Contains(strings.ToLower(output), "update") || !strings.Contains(output, "foo") || @@ -196,6 +197,14 @@ func TestServiceUpdateImage(t *testing.T) { !strings.Contains(output, "bar") { t.Fatalf("wrong or no success message: %s", output) } + + assert.Assert(t, template.Annotations != nil) + ts := template.Annotations[servinglib.UpdateTimestampAnnotationKey] + assert.Assert(t, ts != "") + updateTs, err := time.Parse(time.RFC3339, ts) + assert.NilError(t, err) + assert.Assert(t, updateTs.Before(time.Now())) + assert.Assert(t, updateTs.After(time.Now().Add(-5*time.Second))) } func TestServiceUpdateWithMultipleImages(t *testing.T) { @@ -353,9 +362,6 @@ func TestServiceUpdateMaxMinScale(t *testing.T) { } template := updated.Spec.Template - if err != nil { - t.Fatal(err) - } actualAnnos := template.Annotations expectedAnnos := []string{ @@ -393,9 +399,6 @@ func TestServiceUpdateScale(t *testing.T) { } template := updated.Spec.Template - if err != nil { - t.Fatal(err) - } actualAnnos := template.Annotations expectedAnnos := []string{ @@ -484,9 +487,6 @@ func TestServiceUpdateScaleWithRange(t *testing.T) { } template := updated.Spec.Template - if err != nil { - t.Fatal(err) - } actualAnnos := template.Annotations expectedAnnos := []string{ @@ -518,9 +518,6 @@ func TestServiceUpdateScaleMinWithRange(t *testing.T) { } template := updated.Spec.Template - if err != nil { - t.Fatal(err) - } actualAnnos := template.Annotations expectedAnnos := []string{ @@ -570,9 +567,6 @@ func TestServiceUpdateScaleMaxWithRange(t *testing.T) { } template := updated.Spec.Template - if err != nil { - t.Fatal(err) - } actualAnnos := template.Annotations expectedAnnos := []string{ @@ -780,7 +774,7 @@ func TestServiceUpdateDoesntPinToDigestWhenPreviouslyDidnt(t *testing.T) { } func TestServiceUpdateRequestsLimitsCPU(t *testing.T) { - service := createMockServiceWithResources(t, "250", "64Mi", "1000m", "1024Mi") + service := createMockServiceWithResources("250", "64Mi", "1000m", "1024Mi") action, updated, _, err := fakeServiceUpdate(service, []string{ "service", "update", "foo", "--request", "cpu=500m", "--limit", "cpu=1000m", "--no-wait"}) @@ -800,25 +794,23 @@ func TestServiceUpdateRequestsLimitsCPU(t *testing.T) { } newTemplate := updated.Spec.Template - if err != nil { - t.Fatal(err) - } else { - if !reflect.DeepEqual( - newTemplate.Spec.Containers[0].Resources.Requests, - expectedRequestsVars) { - t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests) - } - if !reflect.DeepEqual( - newTemplate.Spec.Containers[0].Resources.Limits, - expectedLimitsVars) { - t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits) - } + if !reflect.DeepEqual( + newTemplate.Spec.Containers[0].Resources.Requests, + expectedRequestsVars) { + t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests) + } + + if !reflect.DeepEqual( + newTemplate.Spec.Containers[0].Resources.Limits, + expectedLimitsVars) { + t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits) } + } func TestServiceUpdateRequestsLimitsMemory(t *testing.T) { - service := createMockServiceWithResources(t, "100m", "64Mi", "1000m", "1024Mi") + service := createMockServiceWithResources("100m", "64Mi", "1000m", "1024Mi") action, updated, _, err := fakeServiceUpdate(service, []string{ "service", "update", "foo", "--request", "memory=128Mi", "--limit", "memory=2048Mi", "--no-wait"}) @@ -838,25 +830,22 @@ func TestServiceUpdateRequestsLimitsMemory(t *testing.T) { } newTemplate := updated.Spec.Template - if err != nil { - t.Fatal(err) - } else { - if !reflect.DeepEqual( - newTemplate.Spec.Containers[0].Resources.Requests, - expectedRequestsVars) { - t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests) - } - if !reflect.DeepEqual( - newTemplate.Spec.Containers[0].Resources.Limits, - expectedLimitsVars) { - t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits) - } + if !reflect.DeepEqual( + newTemplate.Spec.Containers[0].Resources.Requests, + expectedRequestsVars) { + t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests) + } + + if !reflect.DeepEqual( + newTemplate.Spec.Containers[0].Resources.Limits, + expectedLimitsVars) { + t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits) } } func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) { - service := createMockServiceWithResources(t, "250m", "64Mi", "1000m", "1024Mi") + service := createMockServiceWithResources("250m", "64Mi", "1000m", "1024Mi") action, updated, _, err := fakeServiceUpdate(service, []string{ "service", "update", "foo", @@ -878,30 +867,23 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) { } newTemplate := updated.Spec.Template - if err != nil { - t.Fatal(err) - } else { - if !reflect.DeepEqual( - newTemplate.Spec.Containers[0].Resources.Requests, - expectedRequestsVars) { - t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests) - } + if !reflect.DeepEqual( + newTemplate.Spec.Containers[0].Resources.Requests, + expectedRequestsVars) { + t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests) + } - if !reflect.DeepEqual( - newTemplate.Spec.Containers[0].Resources.Limits, - expectedLimitsVars) { - t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits) - } + if !reflect.DeepEqual( + newTemplate.Spec.Containers[0].Resources.Limits, + expectedLimitsVars) { + t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits) } } func TestServiceUpdateLabelWhenEmpty(t *testing.T) { original := newEmptyService() origTemplate := original.Spec.Template - origContainer, err := servinglib.ContainerOfRevisionTemplate(&origTemplate) - if err != nil { - t.Fatal(err) - } + origContainer := servinglib.ContainerOfRevisionSpec(&origTemplate.Spec) origContainer.Image = "gcr.io/foo/bar:latest" action, updated, _, err := fakeServiceUpdate(original, []string{ @@ -924,11 +906,8 @@ func TestServiceUpdateLabelWhenEmpty(t *testing.T) { template := updated.Spec.Template actual = template.ObjectMeta.Labels assert.DeepEqual(t, expected, actual) - container, err := servinglib.ContainerOfRevisionTemplate(&template) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, container.Image, exampleImageByDigest) + container := servinglib.ContainerOfRevisionSpec(&template.Spec) + assert.Assert(t, container != nil && container.Image == exampleImageByDigest) } func TestServiceUpdateLabelExisting(t *testing.T) { @@ -1018,9 +997,6 @@ func TestServiceUpdateNoClusterLocalOnPrivateService(t *testing.T) { assert.DeepEqual(t, expected, actual) newTemplate := updated.Spec.Template - if err != nil { - t.Fatal(err) - } actual = newTemplate.ObjectMeta.Labels assert.DeepEqual(t, expected, actual) @@ -1065,7 +1041,7 @@ func newEmptyService() *servingv1.Service { return ret } -func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, limitsCPU, limitsMemory string) *servingv1.Service { +func createMockServiceWithResources(requestCPU, requestMemory, limitsCPU, limitsMemory string) *servingv1.Service { service := newEmptyService() template := service.Spec.Template diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index 0ecb72f1a9..f9b9e28446 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -52,6 +52,26 @@ type Plugin interface { Path() string } +type ContextData map[string]string + +type PluginManifest struct { + // ProducesContextDataKeys is a list of keys for the ContextData that + // a plugin can produce. Nil or an empty list declares that this + // plugin is not ContextDataProducer + ProducesContextDataKeys []string + + // ConsumesContextDataKeys is a list of keys from a ContextData that a + // plugin is interested in to consume. Nil or an empty list declares + // that this plugin is not a ContextDataConsumer + ConsumesContextDataKeys []string +} + +type ContextDataConsumer interface { + // ExecuteWithContextData executes the plugin with the given args much like + // Execute() but with an additional argument that holds the ContextData + ExecuteWithContextData(args []string, data ContextData) error +} + type Manager struct { // Dedicated plugin directory as configured pluginsDir string diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index e8d429ff02..0629b65a75 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -41,8 +41,9 @@ const ( ) var ( - UserImageAnnotationKey = "client.knative.dev/user-image" - ApiTooOldError = errors.New("the service is using too old of an API format for the operation") + UserImageAnnotationKey = "client.knative.dev/user-image" + UpdateTimestampAnnotationKey = "client.knative.dev/updateTimestamp" + APITooOldError = errors.New("the service is using too old of an API format for the operation") ) func (vt VolumeSourceType) String() string { @@ -91,50 +92,80 @@ func UpdateConcurrencyLimit(template *servingv1.RevisionTemplateSpec, limit int6 return nil } -// UnsetUserImageAnnot removes the user image annotation -func UnsetUserImageAnnot(template *servingv1.RevisionTemplateSpec) { +// UnsetUserImageAnnotation removes the user image annotation +func UnsetUserImageAnnotation(template *servingv1.RevisionTemplateSpec) { delete(template.Annotations, UserImageAnnotationKey) } -// SetUserImageAnnot sets the user image annotation if the image isn't by-digest already. -func SetUserImageAnnot(template *servingv1.RevisionTemplateSpec) { +// UpdateUserImageAnnotation sets the user image annotation if the image isn't by-digest already. +func UpdateUserImageAnnotation(template *servingv1.RevisionTemplateSpec) { // If the current image isn't by-digest, set the user-image annotation to it // so we remember what it was. - currentContainer, _ := ContainerOfRevisionTemplate(template) - ui := currentContainer.Image - if strings.Contains(ui, "@") { - prev, ok := template.Annotations[UserImageAnnotationKey] + currentContainer := ContainerOfRevisionSpec(&template.Spec) + if currentContainer == nil { + // No container set in the template, so + return + } + image := currentContainer.Image + if strings.Contains(image, "@") { + // Ensure that the non-digestified image is used + storedImage, ok := template.Annotations[UserImageAnnotationKey] if ok { - ui = prev + image = storedImage } } + ensureAnnotations(template) + template.Annotations[UserImageAnnotationKey] = image +} + +// UpdateTimestampAnnotation update the annotation for the last update with the current timestamp +func UpdateTimestampAnnotation(template *servingv1.RevisionTemplateSpec) { + ensureAnnotations(template) + + template.Annotations[UpdateTimestampAnnotationKey] = time.Now().UTC().Format(time.RFC3339) +} + +func ensureAnnotations(template *servingv1.RevisionTemplateSpec) { if template.Annotations == nil { template.Annotations = make(map[string]string) } - template.Annotations[UserImageAnnotationKey] = ui } -// FreezeImageToDigest sets the image on the template to the image digest of the base revision. -func FreezeImageToDigest(template *servingv1.RevisionTemplateSpec, baseRevision *servingv1.Revision) error { +// PinImageToDigest sets the image on the template to the image digest of the base revision. +func PinImageToDigest(currentRevisionTemplate *servingv1.RevisionTemplateSpec, baseRevision *servingv1.Revision) error { + // If there is no base revision then there is nothing to pin to. It's not an error so let's return + // silently if baseRevision == nil { return nil } - currentContainer, err := ContainerOfRevisionTemplate(template) + err := VerifyThatContainersMatchInCurrentAndBaseRevision(currentRevisionTemplate, baseRevision) if err != nil { - return err + return fmt.Errorf("can not pin image to digest: %w", err) } - baseContainer, err := ContainerOfRevisionSpec(&baseRevision.Spec) - if err != nil { - return err + containerStatus := ContainerStatus(baseRevision) + if containerStatus.ImageDigest != "" { + return flags.UpdateImage(¤tRevisionTemplate.Spec.PodSpec, containerStatus.ImageDigest) } - if currentContainer.Image != baseContainer.Image { - return fmt.Errorf("could not freeze image to digest since current revision contains unexpected image") + return nil +} + +// VerifyThatContainersMatchInCurrentAndBaseRevision checks if the image in the current revision matches +// matches the one in a given base revision +func VerifyThatContainersMatchInCurrentAndBaseRevision(template *servingv1.RevisionTemplateSpec, baseRevision *servingv1.Revision) error { + currentContainer := ContainerOfRevisionSpec(&template.Spec) + if currentContainer == nil { + return fmt.Errorf("no container given in current revision %s", template.Name) } - if baseRevision.Status.DeprecatedImageDigest != "" { - return flags.UpdateImage(&template.Spec.PodSpec, baseRevision.Status.DeprecatedImageDigest) + baseContainer := ContainerOfRevisionSpec(&baseRevision.Spec) + if baseContainer == nil { + return fmt.Errorf("no container found in base revision %s", baseRevision.Name) + } + + if currentContainer.Image != baseContainer.Image { + return fmt.Errorf("current revision %s contains unexpected image (%s) that does not fit to the base revision's %s image (%s)", template.Name, currentContainer.Image, baseRevision.Name, baseContainer.Image) } return nil } diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index f6049f66be..e90e884efe 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -79,7 +79,7 @@ type userImageAnnotCase struct { set bool } -func TestSetUserImageAnnot(t *testing.T) { +func TestSetUserImageAnnotation(t *testing.T) { cases := []userImageAnnotCase{ {"foo/bar", "", "foo/bar", true}, {"foo/bar@sha256:asdfsf", "", "foo/bar@sha256:asdfsf", true}, @@ -99,23 +99,52 @@ func TestSetUserImageAnnot(t *testing.T) { } container.Image = c.image if c.set { - SetUserImageAnnot(template) + UpdateUserImageAnnotation(template) } else { - UnsetUserImageAnnot(template) + UnsetUserImageAnnotation(template) } assert.Equal(t, template.Annotations[UserImageAnnotationKey], c.result) } } -func TestFreezeImageToDigest(t *testing.T) { +func TestPinImageToDigest(t *testing.T) { template, container := getRevisionTemplate() revision := &servingv1.Revision{} revision.Spec = template.Spec revision.ObjectMeta = template.ObjectMeta - revision.Status.DeprecatedImageDigest = "gcr.io/foo/bar@sha256:deadbeef" + revision.Status.ContainerStatuses = []servingv1.ContainerStatus{ + {Name: "user-container", ImageDigest: "gcr.io/foo/bar@sha256:deadbeef"}, + } container.Image = "gcr.io/foo/bar:latest" - FreezeImageToDigest(template, revision) + err := PinImageToDigest(template, revision) + assert.NilError(t, err) assert.Equal(t, container.Image, "gcr.io/foo/bar@sha256:deadbeef") + + // No base revision --> no-op + err = PinImageToDigest(template, nil) + assert.NilError(t, err) +} + +func TestPinImageToDigestInvalidImages(t *testing.T) { + template, container := getRevisionTemplate() + container.Image = "gcr.io/A" + revision := &servingv1.Revision{ + Spec: servingv1.RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Image: "gcr.io/B"}, + }, + }, + }, + } + err := PinImageToDigest(template, revision) + assert.ErrorContains(t, err, "unexpected image") +} + +func TestUpdateTimestampAnnotation(t *testing.T) { + template, _ := getRevisionTemplate() + UpdateTimestampAnnotation(template) + assert.Assert(t, template.Annotations[UpdateTimestampAnnotationKey] != "") } func TestUpdateMinScale(t *testing.T) { diff --git a/pkg/serving/revision_template.go b/pkg/serving/revision_template.go index 553ebc9e95..0339a22fe4 100644 --- a/pkg/serving/revision_template.go +++ b/pkg/serving/revision_template.go @@ -15,7 +15,6 @@ package serving import ( - "fmt" "strconv" corev1 "k8s.io/api/core/v1" @@ -29,15 +28,47 @@ type Scaling struct { Max *int } -func ContainerOfRevisionTemplate(template *servingv1.RevisionTemplateSpec) (*corev1.Container, error) { - return ContainerOfRevisionSpec(&template.Spec) +// ContainerOfRevisionSpec returns the 'main' container of a revision specification and +// use GetServingContainerIndex to identify the container. +// Nil is returned if no such container could be found +func ContainerOfRevisionSpec(revisionSpec *servingv1.RevisionSpec) *corev1.Container { + idx := ContainerIndexOfRevisionSpec(revisionSpec) + if idx == -1 { + return nil + } + return &revisionSpec.Containers[0] +} + +// ContainerIndexOfRevisionSpec returns the index of the "main" container if +// multiple containers are present. The main container is either the single +// container when there is only ony container in the list or the first container +// which has a ports declaration (validation guarantees that there is only one +// such container) +// If no container could be found (list is empty or no container has a port declaration) +// then -1 is returned +// This method's logic is taken from RevisionSpec.GetContainer() +func ContainerIndexOfRevisionSpec(revisionSpec *servingv1.RevisionSpec) int { + switch { + case len(revisionSpec.Containers) == 1: + return 0 + case len(revisionSpec.Containers) > 1: + for i := range revisionSpec.Containers { + if len(revisionSpec.Containers[i].Ports) != 0 { + return i + } + } + } + return -1 } -func ContainerOfRevisionSpec(revisionSpec *servingv1.RevisionSpec) (*corev1.Container, error) { - if len(revisionSpec.Containers) == 0 { - return nil, fmt.Errorf("internal: no container set in spec.template.spec.containers") +// ContainerStatus returns the status of the main container or nil of no +// such status could be found +func ContainerStatus(r *servingv1.Revision) *servingv1.ContainerStatus { + idx := ContainerIndexOfRevisionSpec(&r.Spec) + if idx == -1 { + return nil } - return &revisionSpec.Containers[0], nil + return &r.Status.ContainerStatuses[idx] } func ScalingInfo(m *metav1.ObjectMeta) (*Scaling, error) { @@ -73,8 +104,8 @@ func AutoscaleWindow(m *metav1.ObjectMeta) string { } func Port(revisionSpec *servingv1.RevisionSpec) *int32 { - c, err := ContainerOfRevisionSpec(revisionSpec) - if err != nil { + c := ContainerOfRevisionSpec(revisionSpec) + if c == nil { return nil } if len(c.Ports) > 0 { diff --git a/pkg/serving/revision_template_test.go b/pkg/serving/revision_template_test.go index 2998c2323c..69a4ad681b 100644 --- a/pkg/serving/revision_template_test.go +++ b/pkg/serving/revision_template_test.go @@ -17,12 +17,13 @@ package serving import ( "testing" - "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" + + "gotest.tools/v3/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/serving/pkg/apis/autoscaling" - servingv1 "knative.dev/serving/pkg/apis/serving/v1" ) type scalingInfoTest struct { @@ -74,6 +75,57 @@ func TestScalingInfo(t *testing.T) { } } +func TestContainerIndexOfRevisionSpec(t *testing.T) { + tests := []struct { + name string + revSpec *servingv1.RevisionSpec + want int + }{ + { + "no container", + &servingv1.RevisionSpec{}, + -1, + }, + { + "1 container", + &servingv1.RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "user-container", + Ports: []corev1.ContainerPort{{ContainerPort: 80}}, + }, + }}}, + 0, + }, + { + "2 containers", + &servingv1.RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "sidecar-container-1", + }, + { + Name: "user-container", + Ports: []corev1.ContainerPort{{ContainerPort: 80}}, + }, + { + Name: "sidecar-container-2", + }, + }}}, + 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ContainerIndexOfRevisionSpec(tt.revSpec); got != tt.want { + t.Errorf("ContainerIndexOfRevisionSpec() = %v, want %v", got, tt.want) + } + }) + } +} + func TestAnnotations(t *testing.T) { m := &metav1.ObjectMeta{} m.Annotations = map[string]string{UserImageAnnotationKey: "mockImageVal", diff --git a/pkg/serving/v1/client.go b/pkg/serving/v1/client.go index caf303e78e..e1116f13f0 100644 --- a/pkg/serving/v1/client.go +++ b/pkg/serving/v1/client.go @@ -424,18 +424,11 @@ func getBaseRevision(ctx context.Context, cl KnServingClient, service *servingv1 if err != nil { return nil, err } - latestContainer, err := serving.ContainerOfRevisionSpec(&latestCreated.Spec) - if err != nil { - return nil, err - } - container, err := serving.ContainerOfRevisionTemplate(&template) + + err = serving.VerifyThatContainersMatchInCurrentAndBaseRevision(&template, latestCreated) if err != nil { return nil, err } - if latestContainer.Image != container.Image { - // At this point we know the latestCreatedRevision is out of date. - return nil, noBaseRevisionError - } // There is still some chance the latestCreatedRevision is out of date, // but we can't check the whole thing for equality because of // server-side defaulting. Since what we probably want it for is to diff --git a/pkg/serving/v1/client_test.go b/pkg/serving/v1/client_test.go index 49219a02c5..edaf06ede7 100644 --- a/pkg/serving/v1/client_test.go +++ b/pkg/serving/v1/client_test.go @@ -815,7 +815,7 @@ func TestGetBaseRevision(t *testing.T) { {"foo-asdf", "gcr.io/foo/bar", "foo-asdf", "foo-asdf", "gcr.io/foo/bar", ""}, {"", "gcr.io/foo/bar", "foo-asdf", "foo-asdf", "gcr.io/foo/bar", ""}, {"foo-qwer", "gcr.io/foo/bar", "foo-asdf", "foo-qwer", "gcr.io/foo/bar", ""}, - {"", "gcr.io/foo/bar", "foo-asdf", "foo-asdf", "gcr.io/foo/baz", "base revision not found"}, + {"", "gcr.io/foo/bar", "foo-asdf", "foo-asdf", "gcr.io/foo/baz", "unexpected image"}, } var c baseRevisionCase servingFake.AddReactor("get", "revisions", func(a clienttesting.Action) (bool, runtime.Object, error) { diff --git a/test/e2e/service_apply_test.go b/test/e2e/service_apply_test.go index bbdb5d0e86..f5e74c8b0c 100644 --- a/test/e2e/service_apply_test.go +++ b/test/e2e/service_apply_test.go @@ -41,11 +41,11 @@ func TestServiceApply(t *testing.T) { t.Log("apply hello service (initially)") result := serviceApply(r, "hello-apply") - r.AssertNoError(result) assert.Check(r.T(), util.ContainsAllIgnoreCase(result.Stdout, "creating", "service", "hello-apply", "ready", "http")) t.Log("apply hello service (unchanged)") - result = serviceApply(r, "hello-apply") + result = serviceApply(r, "hello-apply", "--annotation-service", "foo=bar") r.AssertNoError(result) + result = serviceApply(r, "hello-apply", "--annotation-service", "foo=bar") assert.Check(r.T(), util.ContainsAllIgnoreCase(result.Stdout, "no changes", "service", "hello-apply", "http")) t.Log("apply hello service (update env)") diff --git a/test/e2e/service_test.go b/test/e2e/service_test.go index 6b483c2ff2..e4fc0f1b4b 100644 --- a/test/e2e/service_test.go +++ b/test/e2e/service_test.go @@ -93,8 +93,7 @@ func serviceCreatePrivateUpdatePublic(r *test.KnRunResultCollector, serviceName r.AssertNoError(out) assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, network.VisibilityLabelKey, serving.VisibilityClusterLocal)) - out = r.KnTest().Kn().Run("service", "update", serviceName, - "--image", pkgtest.ImagePath("helloworld"), "--no-cluster-local") + out = r.KnTest().Kn().Run("service", "update", serviceName, "--no-cluster-local") r.AssertNoError(out) assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, "service", serviceName, "no new revision", "namespace", r.KnTest().Kn().Namespace()))