knative
/
client
mirror of https://github.com/knative/client.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

By default, set `Image` to the image of the prev. revision by digest (#373) 

* Option to freeze revision to digest

* Tests pass. Checkpoint.

* Tests for env now check pinning to digest

* Bool flag using convention. Tweak usage.

* Describing the image carefully using the annotation and digest

* lint

* Test matrix of locking to digest behaviors

* Expose both flags, and rewrite help text again

* Unit tests.

* Removed unsed method

* Add tests for getting base revision

* Make tests actually test stuff better

* Make tests actually test stuff better

* A mergeout killed a returning of error. Restore it

* Help text again
This commit is contained in:
Naomi Seyfer 2019-08-27 11:42:40 -07:00 committed by Knative Prow Robot
parent 10f981ccaa
commit 0ff537ad98
17 changed files with 589 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/cmd/kn_service_create.md
View File

@ -49,9 +49,11 @@ kn service create NAME --image IMAGE [flags]
-l, --label stringArray Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-).
--limits-cpu string The limits on the requested CPU (e.g., 1000m).
--limits-memory string The limits on the requested memory (e.g., 1024Mi).
--lock-to-digest 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) (default true)
--max-scale int Maximal number of replicas.
--min-scale int Minimal number of replicas.
-n, --namespace string List the requested object(s) in given namespace.
--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.
--requests-cpu string The requested CPU (e.g., 250m).
--requests-memory string The requested memory (e.g., 64Mi).

2
docs/cmd/kn_service_update.md
View File

@ -47,9 +47,11 @@ kn service update NAME [flags]
-l, --label stringArray Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-).
--limits-cpu string The limits on the requested CPU (e.g., 1000m).
--limits-memory string The limits on the requested memory (e.g., 1024Mi).
--lock-to-digest 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) (default true)
--max-scale int Maximal number of replicas.
--min-scale int Minimal number of replicas.
-n, --namespace string List the requested object(s) in given namespace.
--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.
--requests-cpu string The requested CPU (e.g., 250m).
--requests-memory string The requested memory (e.g., 64Mi).

27
pkg/kn/commands/service/configuration_edit_flags.go
View File

@ -21,6 +21,7 @@ import (
"github.com/spf13/cobra"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"knative.dev/client/pkg/kn/flags"
servinglib "knative.dev/client/pkg/serving"
util "knative.dev/client/pkg/util"
servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1"
@ -41,7 +42,9 @@ type ConfigurationEditFlags struct {
RevisionName string
// Preferences about how to do the action.
ForceCreate bool
LockToDigest bool
GenerateRevisionName bool
ForceCreate bool
// Bookkeeping
flags []string
@ -100,6 +103,11 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
"Accepts golang templates, allowing {{.Service}} for the service name, "+
"{{.Generation}} for the generation, and {{.Random [n]}} for n random consonants.")
p.markFlagMakesRevision("revision-name")
flags.AddBothBoolFlagsUnhidden(command.Flags(), &p.LockToDigest, "lock-to-digest", "", true,
"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)")
// Don't mark as changing the revision.
}
// AddUpdateFlags adds the flags specific to update.
@ -118,6 +126,7 @@ func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) {
// Apply mutates the given service according to the flags in the command.
func (p *ConfigurationEditFlags) Apply(
service *servingv1alpha1.Service,
baseRevision *servingv1alpha1.Revision,
cmd *cobra.Command) error {
template, err := servinglib.RevisionTemplateOfService(service)
@ -157,12 +166,28 @@ func (p *ConfigurationEditFlags) Apply(
return err
}
}
imageSet := false
if cmd.Flags().Changed("image") {
err = servinglib.UpdateImage(template, p.Image)
if err != nil {
return err
}
imageSet = true
}
_, userImagePresent := template.Annotations[servinglib.UserImageAnnotationKey]
freezeMode := userImagePresent || cmd.Flags().Changed("lock-to-digest")
if p.LockToDigest && p.AnyMutation(cmd) && freezeMode {
servinglib.SetUserImageAnnot(template)
if !imageSet {
err = servinglib.FreezeImageToDigest(template, baseRevision)
if err != nil {
return err
}
}
} else if !p.LockToDigest {
servinglib.UnsetUserImageAnnot(template)
}
limitsResources, err := p.computeResources(p.LimitsFlags)
if err != nil {
return err

9
pkg/kn/commands/service/service_create.go
View File

@ -20,7 +20,9 @@ import (
"io"
"knative.dev/client/pkg/kn/commands"
servinglib "knative.dev/client/pkg/serving"
"knative.dev/client/pkg/serving/v1alpha1"
"knative.dev/serving/pkg/apis/serving"
"github.com/spf13/cobra"
@ -208,10 +210,15 @@ func constructService(cmd *cobra.Command, editFlags ConfigurationEditFlags, name
service.Spec.Template = &serving_v1alpha1_api.RevisionTemplateSpec{
Spec: serving_v1alpha1_api.RevisionSpec{},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
servinglib.UserImageAnnotationKey: "", // Placeholder. Will be replaced or deleted as we apply mutations.
},
},
}
service.Spec.Template.Spec.Containers = []corev1.Container{{}}
err := editFlags.Apply(&service, cmd)
err := editFlags.Apply(&service, nil, cmd)
if err != nil {
return nil, err
}

5
pkg/kn/commands/service/service_create_mock_test.go
View File

@ -68,7 +68,10 @@ func TestServiceCreateLabel(t *testing.T) {
"b": "cookie",
"empty": "",
}
service.ObjectMeta.Labels = expected
service.Labels = expected
service.Spec.Template.Annotations = map[string]string{
servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz",
}
template, err := servinglib.RevisionTemplateOfService(service)
assert.NilError(t, err)
template.ObjectMeta.Labels = expected

17
pkg/kn/commands/service/service_describe.go
View File

@ -29,6 +29,7 @@ import (
"knative.dev/serving/pkg/apis/serving"
"knative.dev/client/pkg/printers"
client_serving "knative.dev/client/pkg/serving"
serving_kn_v1alpha1 "knative.dev/client/pkg/serving/v1alpha1"
"knative.dev/pkg/apis"
@ -72,6 +73,7 @@ type revisionDesc struct {
timeoutSeconds *int64
image string
userImage string
imageDigest string
env []string
port *int32
@ -321,8 +323,18 @@ func formatStatus(status corev1.ConditionStatus) string {
// Return either image name with tag or together with its resolved digest
func getImageDesc(desc *revisionDesc) string {
image := desc.image
if printDetails && desc.imageDigest != "" {
return fmt.Sprintf("%s (%s)", image, shortenDigest(desc.imageDigest))
// Check if the user image is likely a more user-friendly description
pinnedDesc := "at"
if desc.userImage != "" && strings.Contains(image, "@") && desc.imageDigest != "" {
parts := strings.Split(image, "@")
// Check if the user image refers to the same thing.
if strings.HasPrefix(desc.userImage, parts[0]) {
pinnedDesc = "pinned to"
image = desc.userImage
}
}
if desc.imageDigest != "" {
return fmt.Sprintf("%s (%s %s)", image, pinnedDesc, shortenDigest(desc.imageDigest))
}
return image
}
@ -503,6 +515,7 @@ func newRevisionDesc(revision *v1alpha1.Revision, target *v1alpha1.TrafficTarget
name: revision.Name,
logURL: revision.Status.LogURL,
timeoutSeconds: revision.Spec.TimeoutSeconds,
userImage: revision.Annotations[client_serving.UserImageAnnotationKey],
imageDigest: revision.Status.ImageDigest,
creationTimestamp: revision.CreationTimestamp.Time,

56
pkg/kn/commands/service/service_describe_test.go
View File

@ -34,6 +34,7 @@ import (
"knative.dev/serving/pkg/apis/serving/v1alpha1"
"knative.dev/serving/pkg/apis/serving/v1beta1"
client_serving "knative.dev/client/pkg/serving"
knclient "knative.dev/client/pkg/serving/v1alpha1"
"knative.dev/client/pkg/util"
)
@ -63,6 +64,7 @@ func TestServiceDescribeBasic(t *testing.T) {
validateServiceOutput(t, "foo", output)
assert.Assert(t, util.ContainsAll(output, "Env:", "label1=lval1, label2=lval2\n"))
assert.Assert(t, util.ContainsAll(output, "1234567"))
assert.Assert(t, util.ContainsAll(output, "Annotations:", "anno1=aval1, anno2=aval2, anno3="))
assert.Assert(t, cmp.Regexp(`(?m)\s*Annotations:.*\.\.\.$`, output))
assert.Assert(t, util.ContainsAll(output, "Labels:", "label1=lval1, label2=lval2\n"))
@ -196,6 +198,55 @@ func TestServiceDescribeResources(t *testing.T) {
}
}
func TestServiceDescribeUserImageVsImage(t *testing.T) {
// New mock client
client := knclient.NewMockKnClient(t)
// Recording:
r := client.Recorder()
// Prepare service
expectedService := createTestService("foo", []string{"rev1", "rev2", "rev3", "rev4"}, goodConditions())
r.GetService("foo", &expectedService, nil)
rev1 := createTestRevision("rev1", 1)
rev2 := createTestRevision("rev2", 2)
rev3 := createTestRevision("rev3", 3)
rev4 := createTestRevision("rev4", 4)
// Different combinations of image annotations and not.
rev1.Spec.Containers[0].Image = "gcr.io/test/image:latest"
rev1.Annotations[client_serving.UserImageAnnotationKey] = "gcr.io/test/image:latest"
rev2.Spec.Containers[0].Image = "gcr.io/test/image@" + imageDigest
rev2.Annotations[client_serving.UserImageAnnotationKey] = "gcr.io/test/image:latest"
// rev3 is as if we changed the image but didn't change the annotation
rev3.Annotations[client_serving.UserImageAnnotationKey] = "gcr.io/test/image:latest"
rev3.Spec.Containers[0].Image = "gcr.io/a/b"
// rev4 is without the annotation at all and no hash
rev4.Status.ImageDigest = ""
rev4.Spec.Containers[0].Image = "gcr.io/x/y"
// Fetch the revisions
r.GetRevision("rev1", &rev1, nil)
r.GetRevision("rev2", &rev2, nil)
r.GetRevision("rev3", &rev3, nil)
r.GetRevision("rev4", &rev4, nil)
// Testing:
output, err := executeServiceCommand(client, "describe", "foo")
assert.NilError(t, err)
validateServiceOutput(t, "foo", output)
assert.Assert(t, util.ContainsAll(output, "Image", "Name", "gcr.io/test/image:latest (at 123456789012)",
"gcr.io/test/image:latest (pinned to 123456789012)", "gcr.io/a/b (at 123456789012)", "gcr.io/x/y"))
assert.Assert(t, util.ContainsAll(output, "[1]", "[2]"))
// Validate that all recorded API methods have been called
r.Validate()
}
func TestServiceDescribeVerbose(t *testing.T) {
// New mock client
@ -234,7 +285,7 @@ func TestServiceDescribeVerbose(t *testing.T) {
validateServiceOutput(t, "foo", output)
assert.Assert(t, util.ContainsAll(output, "Image", "Name", "(123456789012)", "50%", "gcr.io/test/image", "(0s)"))
assert.Assert(t, util.ContainsAll(output, "Image", "Name", "gcr.io/test/image (at 123456789012)", "50%", "(0s)"))
assert.Assert(t, util.ContainsAll(output, "Env:", "label1=lval1\n", "label2=lval2\n"))
assert.Assert(t, util.ContainsAll(output, "Annotations:", "anno1=aval1\n", "anno2=aval2\n"))
assert.Assert(t, util.ContainsAll(output, "Labels:", "label1=lval1\n", "label2=lval2\n"))
@ -381,6 +432,7 @@ func createTestRevision(revision string, gen int64) v1alpha1.Revision {
Namespace: "default",
Generation: 1,
Labels: labels,
Annotations: make(map[string]string),
CreationTimestamp: metav1.Time{Time: time.Now()},
},
Spec: v1alpha1.RevisionSpec{
@ -402,7 +454,7 @@ func createTestRevision(revision string, gen int64) v1alpha1.Revision {
},
},
Status: v1alpha1.RevisionStatus{
ImageDigest: imageDigest,
ImageDigest: "gcr.io/test/image@" + imageDigest,
},
}
}

11
pkg/kn/commands/service/service_update.go
View File

@ -24,6 +24,8 @@ import (
"knative.dev/client/pkg/kn/traffic"
"knative.dev/client/pkg/kn/commands"
serving "knative.dev/client/pkg/serving/v1alpha1"
"knative.dev/serving/pkg/apis/serving/v1alpha1"
)
func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
@ -77,7 +79,14 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
}
service = service.DeepCopy()
err = editFlags.Apply(service, cmd)
var baseRevision *v1alpha1.Revision
if !cmd.Flags().Changed("image") && editFlags.LockToDigest {
baseRevision, err = client.GetBaseRevision(service)
if _, ok := err.(*serving.NoBaseRevisionError); ok {
fmt.Fprintf(cmd.OutOrStdout(), "Warning: No reivision found to update image digest")
}
}
err = editFlags.Apply(service, baseRevision, cmd)
if err != nil {
return err
}

363
pkg/kn/commands/service/service_update_test.go
View File

@ -38,11 +38,16 @@ import (
"knative.dev/serving/pkg/apis/serving/v1alpha1"
)
var exampleImageByDigest = "gcr.io/foo/bar@sha256:deadbeefdeadbeef"
var exampleRevisionName = "foo-asdf"
var exampleRevisionName2 = "foo-xyzzy"
func fakeServiceUpdate(original *v1alpha1.Service, args []string, sync bool) (
action client_testing.Action,
updated *v1alpha1.Service,
output string,
err error) {
var reconciled v1alpha1.Service
knParams := &commands.KnParams{}
cmd, fakeServing, buf := commands.CreateTestKnCommand(NewServiceCommand(knParams), knParams)
fakeServing.AddReactor("update", "*",
@ -58,9 +63,30 @@ func fakeServiceUpdate(original *v1alpha1.Service, args []string, sync bool) (
}
return true, updated, nil
})
fakeServing.AddReactor("get", "*",
fakeServing.AddReactor("get", "services",
func(a client_testing.Action) (bool, runtime.Object, error) {
return true, original, nil
if updated == nil {
original.Status.LatestCreatedRevisionName = exampleRevisionName
return true, original, nil
}
reconciled = *updated
if updated.Spec.Template.Name == "" {
reconciled.Status.LatestCreatedRevisionName = exampleRevisionName2
} else {
reconciled.Status.LatestCreatedRevisionName = updated.Spec.Template.Name
}
return true, &reconciled, nil
})
fakeServing.AddReactor("get", "revisions",
// This is important for the way we set images to their image digest
func(a client_testing.Action) (bool, runtime.Object, error) {
rev := &v1alpha1.Revision{}
rev.Spec = original.Spec.Template.Spec
rev.ObjectMeta = original.Spec.Template.ObjectMeta
rev.Name = original.Status.LatestCreatedRevisionName
rev.Status.ImageDigest = exampleImageByDigest
return true, rev, nil
})
if sync {
fakeServing.AddWatchReactor("services",
@ -128,42 +154,46 @@ func TestServiceUpdateImageSync(t *testing.T) {
template, err = servinglib.RevisionTemplateOfService(updated)
assert.NilError(t, err)
assert.Equal(t, template.Spec.DeprecatedContainer.Image, "gcr.io/foo/quux:xyzzy")
assert.Equal(t, template.Spec.Containers[0].Image, "gcr.io/foo/quux:xyzzy")
assert.Assert(t, util.ContainsAll(strings.ToLower(output), "update", "foo", "service", "namespace", "bar", "ok", "waiting"))
}
func TestServiceUpdateImage(t *testing.T) {
orig := newEmptyService()
for _, orig := range []*v1alpha1.Service{newEmptyServiceBetaAPIStyle(), newEmptyServiceAlphaAPIStyle()} {
template, err := servinglib.RevisionTemplateOfService(orig)
if err != nil {
t.Fatal(err)
}
template, err := servinglib.RevisionTemplateOfService(orig)
if err != nil {
t.Fatal(err)
}
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
action, updated, output, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "--image", "gcr.io/foo/quux:xyzzy", "--namespace", "bar", "--async"}, false)
action, updated, output, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "--image", "gcr.io/foo/quux:xyzzy", "--namespace", "bar", "--async"}, false)
if err != nil {
t.Fatal(err)
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
if err != nil {
t.Fatal(err)
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
template, err = servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/quux:xyzzy" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
}
template, err = servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
}
container, err := servinglib.ContainerOfRevisionTemplate(template)
if err != nil {
t.Fatal(err)
}
assert.Equal(t, container.Image, "gcr.io/foo/quux:xyzzy")
if !strings.Contains(strings.ToLower(output), "update") ||
!strings.Contains(output, "foo") ||
!strings.Contains(strings.ToLower(output), "service") ||
!strings.Contains(strings.ToLower(output), "namespace") ||
!strings.Contains(output, "bar") {
t.Fatalf("wrong or no success message: %s", output)
if !strings.Contains(strings.ToLower(output), "update") ||
!strings.Contains(output, "foo") ||
!strings.Contains(strings.ToLower(output), "service") ||
!strings.Contains(strings.ToLower(output), "namespace") ||
!strings.Contains(output, "bar") {
t.Fatalf("wrong or no success message: %s", output)
}
}
}
@ -299,33 +329,13 @@ func TestServiceUpdateMaxMinScale(t *testing.T) {
}
func TestServiceUpdateEnv(t *testing.T) {
orig := &v1alpha1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "knative.dev/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "default",
},
Spec: v1alpha1.ServiceSpec{
DeprecatedRunLatest: &v1alpha1.RunLatestType{
Configuration: v1alpha1.ConfigurationSpec{
DeprecatedRevisionTemplate: &v1alpha1.RevisionTemplateSpec{
Spec: v1alpha1.RevisionSpec{
DeprecatedContainer: &corev1.Container{},
},
},
},
},
},
}
orig := newEmptyService()
template, err := servinglib.RevisionTemplateOfService(orig)
if err != nil {
t.Fatal(err)
}
template.Spec.DeprecatedContainer.Env = []corev1.EnvVar{
template.Spec.Containers[0].Env = []corev1.EnvVar{
{Name: "EXISTING", Value: "thing"},
{Name: "OTHEREXISTING"},
}
@ -346,13 +356,115 @@ func TestServiceUpdateEnv(t *testing.T) {
}
template, err = servinglib.RevisionTemplateOfService(updated)
assert.NilError(t, err)
// Test that we pinned to digest
assert.Equal(t, template.Spec.Containers[0].Image, exampleImageByDigest)
assert.Equal(t, template.Spec.Containers[0].Env[0], expectedEnvVar)
}
func TestServiceUpdatePinsToDigestWhenAsked(t *testing.T) {
orig := newEmptyService()
template, err := servinglib.RevisionTemplateOfService(orig)
delete(template.Annotations, servinglib.UserImageAnnotationKey)
if err != nil {
t.Fatal(err)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
} else if template.Spec.DeprecatedContainer.Env[0] != expectedEnvVar {
t.Fatalf("wrong env set: %v", template.Spec.DeprecatedContainer.Env)
}
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
action, updated, _, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "-e", "TARGET=Awesome", "--lock-to-digest", "--async"}, false)
if err != nil {
t.Fatal(err)
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
template, err = servinglib.RevisionTemplateOfService(updated)
assert.NilError(t, err)
// Test that we pinned to digest
assert.Equal(t, template.Spec.Containers[0].Image, exampleImageByDigest)
}
func TestServiceUpdatePinsToDigestWhenPreviouslyDidSo(t *testing.T) {
orig := newEmptyService()
template, err := servinglib.RevisionTemplateOfService(orig)
if err != nil {
t.Fatal(err)
}
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
action, updated, _, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "-e", "TARGET=Awesome", "--async"}, false)
if err != nil {
t.Fatal(err)
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
template, err = servinglib.RevisionTemplateOfService(updated)
assert.NilError(t, err)
// Test that we pinned to digest
assert.Equal(t, template.Spec.Containers[0].Image, exampleImageByDigest)
}
func TestServiceUpdateDoesntPinToDigestWhenUnAsked(t *testing.T) {
orig := newEmptyService()
template, err := servinglib.RevisionTemplateOfService(orig)
if err != nil {
t.Fatal(err)
}
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
action, updated, _, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "-e", "TARGET=Awesome", "--no-lock-to-digest", "--async"}, false)
if err != nil {
t.Fatal(err)
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
template, err = servinglib.RevisionTemplateOfService(updated)
assert.NilError(t, err)
// Test that we pinned to digest
assert.Equal(t, template.Spec.Containers[0].Image, "gcr.io/foo/bar:baz")
_, present := template.Annotations[servinglib.UserImageAnnotationKey]
assert.Assert(t, !present)
}
func TestServiceUpdateDoesntPinToDigestWhenPreviouslyDidnt(t *testing.T) {
orig := newEmptyService()
template, err := servinglib.RevisionTemplateOfService(orig)
delete(template.Annotations, servinglib.UserImageAnnotationKey)
if err != nil {
t.Fatal(err)
}
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
action, updated, _, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "-e", "TARGET=Awesome", "--async"}, false)
if err != nil {
t.Fatal(err)
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
template, err = servinglib.RevisionTemplateOfService(updated)
assert.NilError(t, err)
// Test that we pinned to digest
assert.Equal(t, template.Spec.Containers[0].Image, "gcr.io/foo/bar:baz")
_, present := template.Annotations[servinglib.UserImageAnnotationKey]
assert.Assert(t, !present)
}
func TestServiceUpdateRequestsLimitsCPU(t *testing.T) {
@ -380,15 +492,15 @@ func TestServiceUpdateRequestsLimitsCPU(t *testing.T) {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
newTemplate.Spec.DeprecatedContainer.Resources.Requests,
newTemplate.Spec.Containers[0].Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests)
}
if !reflect.DeepEqual(
newTemplate.Spec.DeprecatedContainer.Resources.Limits,
newTemplate.Spec.Containers[0].Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits)
}
}
}
@ -418,15 +530,15 @@ func TestServiceUpdateRequestsLimitsMemory(t *testing.T) {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
newTemplate.Spec.DeprecatedContainer.Resources.Requests,
newTemplate.Spec.Containers[0].Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests)
}
if !reflect.DeepEqual(
newTemplate.Spec.DeprecatedContainer.Resources.Limits,
newTemplate.Spec.Containers[0].Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits)
}
}
}
@ -458,21 +570,30 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
newTemplate.Spec.DeprecatedContainer.Resources.Requests,
newTemplate.Spec.Containers[0].Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", newTemplate.Spec.Containers[0].Resources.Requests)
}
if !reflect.DeepEqual(
newTemplate.Spec.DeprecatedContainer.Resources.Limits,
newTemplate.Spec.Containers[0].Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", newTemplate.Spec.Containers[0].Resources.Limits)
}
}
}
func TestServiceUpdateLabelWhenEmpty(t *testing.T) {
original := newEmptyService()
origTemplate, err := servinglib.RevisionTemplateOfService(original)
if err != nil {
t.Fatal(err)
}
origContainer, err := servinglib.ContainerOfRevisionTemplate(origTemplate)
if err != nil {
t.Fatal(err)
}
origContainer.Image = "gcr.io/foo/bar:latest"
action, updated, _, err := fakeServiceUpdate(original, []string{
"service", "update", "foo", "-l", "a=mouse", "--label", "b=cookie", "-l=single", "--async"}, false)
@ -495,6 +616,11 @@ func TestServiceUpdateLabelWhenEmpty(t *testing.T) {
assert.NilError(t, err)
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)
}
func TestServiceUpdateLabelExisting(t *testing.T) {
@ -526,6 +652,52 @@ func TestServiceUpdateLabelExisting(t *testing.T) {
}
func newEmptyService() *v1alpha1.Service {
return newEmptyServiceBetaAPIStyle()
}
func newEmptyServiceBetaAPIStyle() *v1alpha1.Service {
ret := &v1alpha1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "knative.dev/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "default",
},
Spec: v1alpha1.ServiceSpec{},
}
ret.Spec.Template = &v1alpha1.RevisionTemplateSpec{}
ret.Spec.Template.Annotations = map[string]string{
servinglib.UserImageAnnotationKey: "",
}
ret.Spec.Template.Spec.Containers = []corev1.Container{{}}
return ret
}
func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, limitsCPU, limitsMemory string) *v1alpha1.Service {
service := newEmptyService()
template, err := servinglib.RevisionTemplateOfService(service)
if err != nil {
t.Fatal(err)
}
template.Spec.Containers[0].Resources = corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(requestCPU),
corev1.ResourceMemory: resource.MustParse(requestMemory),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(limitsCPU),
corev1.ResourceMemory: resource.MustParse(limitsMemory),
},
}
return service
}
func newEmptyServiceAlphaAPIStyle() *v1alpha1.Service {
return &v1alpha1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
@ -548,62 +720,3 @@ func newEmptyService() *v1alpha1.Service {
},
}
}
func newEmptyServiceBetaAPIStyle() *v1alpha1.Service {
ret := &v1alpha1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "knative.dev/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "default",
},
Spec: v1alpha1.ServiceSpec{},
}
ret.Spec.Template = &v1alpha1.RevisionTemplateSpec{}
ret.Spec.Template.Spec.Containers = []corev1.Container{{}}
return ret
}
func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, limitsCPU, limitsMemory string) *v1alpha1.Service {
service := &v1alpha1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "knative.dev/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "default",
},
Spec: v1alpha1.ServiceSpec{
DeprecatedRunLatest: &v1alpha1.RunLatestType{
Configuration: v1alpha1.ConfigurationSpec{
DeprecatedRevisionTemplate: &v1alpha1.RevisionTemplateSpec{
Spec: v1alpha1.RevisionSpec{
DeprecatedContainer: &corev1.Container{},
},
},
},
},
},
}
template, err := servinglib.RevisionTemplateOfService(service)
if err != nil {
t.Fatal(err)
}
template.Spec.DeprecatedContainer.Resources = corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(requestCPU),
corev1.ResourceMemory: resource.MustParse(requestMemory),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(limitsCPU),
corev1.ResourceMemory: resource.MustParse(limitsMemory),
},
}
return service
}

6
pkg/kn/commands/testing_helper.go
View File

@ -25,6 +25,8 @@ import (
"github.com/spf13/viper"
"gotest.tools/assert"
client_testing "k8s.io/client-go/testing"
"knative.dev/client/pkg/kn/flags"
"knative.dev/client/pkg/serving/v1alpha1"
"knative.dev/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1/fake"
)
@ -107,6 +109,10 @@ Eventing: Manage event subscriptions and channels. Connect up event sources.`,
// Prevents Cobra from dealing with errors as we deal with them in main.go
SilenceErrors: true,
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
return flags.ReconcileBoolFlags(cmd.Flags())
},
}
if params.Output != nil {
rootCmd.SetOutput(params.Output)

15
pkg/kn/flags/bool.go
View File

@ -24,16 +24,23 @@ import (
var negPrefix = "no-"
// AddBothBoolFlags adds the given flag in both `--foo` and `--no-foo` variants.
// If you do this, make sure you call ReconcileBoolFlags later to catch errors and
// set the relationship between the flag values.
func AddBothBoolFlags(f *pflag.FlagSet, p *bool, name, short string, value bool, usage string) {
// AddBothBoolFlagsUnhidden is just like AddBothBoolFlags but shows both flags.
func AddBothBoolFlagsUnhidden(f *pflag.FlagSet, p *bool, name, short string, value bool, usage string) {
negativeName := negPrefix + name
f.BoolVarP(p, name, short, value, usage)
f.Bool(negativeName, !value, "do not "+usage)
}
// AddBothBoolFlags adds the given flag in both `--foo` and `--no-foo` variants.
// If you do this, make sure you call ReconcileBoolFlags later to catch errors
// and set the relationship between the flag values. Only the flag that does the
// non-default behavior is visible; the other is hidden.
func AddBothBoolFlags(f *pflag.FlagSet, p *bool, name, short string, value bool, usage string) {
AddBothBoolFlagsUnhidden(f, p, name, short, value, usage)
negativeName := negPrefix + name
if value {
err := f.MarkHidden(name)
if err != nil {

52
pkg/serving/config_changes.go
View File

@ -19,6 +19,7 @@ import (
"errors"
"fmt"
"strconv"
"strings"
corev1 "k8s.io/api/core/v1"
"knative.dev/serving/pkg/apis/autoscaling"
@ -26,6 +27,8 @@ import (
servingv1beta1 "knative.dev/serving/pkg/apis/serving/v1beta1"
)
var UserImageAnnotationKey = "client.knative.dev/user-image"
// UpdateEnvVars gives the configuration all the env var values listed in the given map of
// vars. Does not touch any environment variables not mentioned, but it can add
// new env vars and change the values of existing ones.
@ -54,7 +57,7 @@ func UpdateConcurrencyTarget(template *servingv1alpha1.RevisionTemplateSpec, tar
// TODO(toVersus): Remove the following validation once serving library is updated to v0.8.0
// and just rely on ValidateAnnotations method.
if target < autoscaling.TargetMin {
return fmt.Errorf("Invalid 'concurrency-target' value: must be an integer greater than 0: %s",
return fmt.Errorf("invalid 'concurrency-target' value: must be an integer greater than 0: %s",
autoscaling.TargetAnnotationKey)
}
@ -67,7 +70,7 @@ func UpdateConcurrencyLimit(template *servingv1alpha1.RevisionTemplateSpec, limi
// Validate input limit
ctx := context.Background()
if err := cc.Validate(ctx).ViaField("spec.containerConcurrency"); err != nil {
return fmt.Errorf("Invalid 'concurrency-limit' value: %s", err)
return fmt.Errorf("invalid 'concurrency-limit' value: %s", err)
}
template.Spec.ContainerConcurrency = cc
return nil
@ -120,6 +123,7 @@ func EnvToMap(vars []corev1.EnvVar) (map[string]string, error) {
// UpdateImage a given image
func UpdateImage(template *servingv1alpha1.RevisionTemplateSpec, image string) error {
// When not setting the image to a digest, add the user image annotation.
container, err := ContainerOfRevisionTemplate(template)
if err != nil {
return err
@ -128,6 +132,50 @@ func UpdateImage(template *servingv1alpha1.RevisionTemplateSpec, image string) e
return nil
}
// UnsetUserImageAnnot removes the user image annotation
func UnsetUserImageAnnot(template *servingv1alpha1.RevisionTemplateSpec) {
delete(template.Annotations, UserImageAnnotationKey)
}
// SetUserImageAnnot sets the user image annotation if the image isn't by-digest already.
func SetUserImageAnnot(template *servingv1alpha1.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]
if ok {
ui = prev
}
}
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 *servingv1alpha1.RevisionTemplateSpec, baseRevision *servingv1alpha1.Revision) error {
currentContainer, err := ContainerOfRevisionTemplate(template)
if baseRevision == nil {
return nil
}
baseContainer, err := ContainerOfRevisionSpec(&baseRevision.Spec)
if err != nil {
return err
}
if currentContainer.Image != baseContainer.Image {
return fmt.Errorf("could not freeze image to digest since current revision contains unexpected image.")
}
if baseRevision.Status.ImageDigest != "" {
return UpdateImage(template, baseRevision.Status.ImageDigest)
}
return nil
}
// UpdateContainerPort updates container with a give port
func UpdateContainerPort(template *servingv1alpha1.RevisionTemplateSpec, port int32) error {
container, err := ContainerOfRevisionTemplate(template)

50
pkg/serving/config_changes_test.go
View File

@ -99,6 +99,52 @@ func TestUpdateEnvVarsAppendOld(t *testing.T) {
assertNoV1alpha1Old(t, template)
}
type userImageAnnotCase struct {
image string
annot string
result string
set bool
}
func TestSetUserImageAnnot(t *testing.T) {
cases := []userImageAnnotCase{
{"foo/bar", "", "foo/bar", true},
{"foo/bar@sha256:asdfsf", "", "foo/bar@sha256:asdfsf", true},
{"foo/bar@sha256:asdf", "foo/bar", "foo/bar", true},
{"foo/bar", "baz/quux", "foo/bar", true},
{"foo/bar", "", "", false},
{"foo/bar", "baz/quux", "", false},
}
for _, c := range cases {
template, container := getV1alpha1Config()
if c.annot == "" {
template.Annotations = nil
} else {
template.Annotations = map[string]string{
UserImageAnnotationKey: c.annot,
}
}
container.Image = c.image
if c.set {
SetUserImageAnnot(template)
} else {
UnsetUserImageAnnot(template)
}
assert.Equal(t, template.Annotations[UserImageAnnotationKey], c.result)
}
}
func TestFreezeImageToDigest(t *testing.T) {
template, container := getV1alpha1Config()
revision := &servingv1alpha1.Revision{}
revision.Spec = template.Spec
revision.ObjectMeta = template.ObjectMeta
revision.Status.ImageDigest = "gcr.io/foo/bar@sha256:deadbeef"
container.Image = "gcr.io/foo/bar:latest"
FreezeImageToDigest(template, revision)
assert.Equal(t, container.Image, "gcr.io/foo/bar@sha256:deadbeef")
}
func testUpdateEnvVarsAppendOld(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) {
container.Env = []corev1.EnvVar{
{Name: "a", Value: "foo"},
@ -205,7 +251,7 @@ func TestUpdateConcurrencyTarget(t *testing.T) {
checkAnnotationValue(t, template, autoscaling.TargetAnnotationKey, 10)
// Update with invalid value
err = UpdateConcurrencyTarget(template, -1)
assert.ErrorContains(t, err, "Invalid")
assert.ErrorContains(t, err, "invalid")
}
func TestUpdateConcurrencyLimit(t *testing.T) {
@ -216,7 +262,7 @@ func TestUpdateConcurrencyLimit(t *testing.T) {
checkContainerConcurrency(t, template, 10)
// Update with invalid value
err = UpdateConcurrencyLimit(template, -1)
assert.ErrorContains(t, err, "Invalid")
assert.ErrorContains(t, err, "invalid")
}
func TestUpdateContainerImage(t *testing.T) {

60
pkg/serving/v1alpha1/client.go
View File

@ -61,6 +61,10 @@ type KnClient interface {
// Get a revision by name
GetRevision(name string) (*v1alpha1.Revision, error)
// Get the "base" revision for a Service; the one that corresponds to the
// current template.
GetBaseRevision(service *v1alpha1.Service) (*v1alpha1.Revision, error)
// List revisions
ListRevisions(opts ...ListConfig) (*v1alpha1.RevisionList, error)
@ -230,6 +234,62 @@ func (cl *knClient) GetRevision(name string) (*v1alpha1.Revision, error) {
return revision, nil
}
type NoBaseRevisionError struct {
msg string
}
func (e NoBaseRevisionError) Error() string {
return e.msg
}
var noBaseRevisionError = &NoBaseRevisionError{"base revision not found"}
// Get a "base" revision. This is the revision corresponding to the template of
// a Service. It may not be findable with our heuristics, in which case this
// method returns Errors()["no-base-revision"]. If it simply doesn't exist (like
// it wasn't yet created or was deleted), return the usual not found error.
func (cl *knClient) GetBaseRevision(service *v1alpha1.Service) (*v1alpha1.Revision, error) {
return getBaseRevision(cl, service)
}
func getBaseRevision(cl KnClient, service *v1alpha1.Service) (*v1alpha1.Revision, error) {
template, err := serving.RevisionTemplateOfService(service)
if err != nil {
return nil, err
}
// First, try to get it by name. If the template has a particular name, the
// base revision is the one created with that name.
if template.Name != "" {
return cl.GetRevision(template.Name)
}
// Next, let's try the LatestCreatedRevision, and see if that matches the
// template, at least in terms of the image (which is what we care about here).
if service.Status.LatestCreatedRevisionName != "" {
latestCreated, err := cl.GetRevision(service.Status.LatestCreatedRevisionName)
if err != nil {
return nil, err
}
latestContainer, err := serving.ContainerOfRevisionSpec(&latestCreated.Spec)
if err != nil {
return nil, err
}
container, err := serving.ContainerOfRevisionTemplate(template)
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
// check the image digest anyway, keep it as good enough.
return latestCreated, nil
}
return nil, noBaseRevisionError
}
// Delete a revision by name
func (cl *knClient) DeleteRevision(name string) error {
err := cl.client.Revisions(cl.namespace).Delete(name, &v1.DeleteOptions{})

4
pkg/serving/v1alpha1/client_mock.go
View File

@ -194,6 +194,10 @@ func (r *Recorder) GetConfiguration(name string, config *v1alpha1.Configuration,
}
func (c *MockKnClient) GetBaseRevision(service *v1alpha1.Service) (*v1alpha1.Revision, error) {
return getBaseRevision(c, service)
}
// GetConfiguration returns a configuration looked up by name
func (c *MockKnClient) GetConfiguration(name string) (*v1alpha1.Configuration, error) {
call := c.getCall("GetConfiguration")

50
pkg/serving/v1alpha1/client_test.go
View File

@ -394,6 +394,56 @@ func TestWaitForService(t *testing.T) {
})
}
type baseRevisionCase struct {
templateName string
templateImage string
latestCreated string
requestedRevisionName string
foundRevisionImage string
errText string
}
func TestGetBaseRevision(t *testing.T) {
serving, client := setup()
cases := []baseRevisionCase{
{"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"},
}
var c baseRevisionCase
serving.AddReactor("get", "revisions", func(a client_testing.Action) (bool, runtime.Object, error) {
revision := &v1alpha1.Revision{ObjectMeta: metav1.ObjectMeta{Name: c.requestedRevisionName, Namespace: testNamespace}}
revision.Spec.Containers = []corev1.Container{{}}
name := a.(client_testing.GetAction).GetName()
assert.Equal(t, name, c.requestedRevisionName)
if c.foundRevisionImage != "" {
revision.Spec.Containers[0].Image = c.foundRevisionImage
return true, revision, nil
} else {
return true, nil, errors.NewNotFound(v1alpha1.Resource("revision"), name)
}
})
for _, c = range cases {
service := v1alpha1.Service{}
service.Spec.Template = &v1alpha1.RevisionTemplateSpec{}
service.Spec.Template.Name = c.templateName
service.Status.LatestCreatedRevisionName = c.latestCreated
service.Spec.Template.Spec.Containers = []corev1.Container{{}}
service.Spec.Template.Spec.Containers[0].Image = c.templateImage
r, err := client.GetBaseRevision(&service)
if err == nil {
assert.Equal(t, r.Spec.Containers[0].Image, c.foundRevisionImage)
} else {
assert.Assert(t, c.errText != "")
assert.ErrorContains(t, err, c.errText)
}
}
}
func TestGetConfiguration(t *testing.T) {
serving, client := setup()

2
test/e2e/service_options_test.go
View File

@ -43,7 +43,7 @@ func TestServiceOptions(t *testing.T) {
t.Run("update concurrency options with invalid values for service", func(t *testing.T) {
command := []string{"service", "update", "svc1", "--concurrency-limit", "-1", "--concurrency-target", "0"}
_, err := test.kn.RunWithOpts(command, runOpts{NoNamespace: false, AllowError: true})
assert.ErrorContains(t, err, "Invalid")
assert.ErrorContains(t, err, "invalid")
})
t.Run("returns steady concurrency options for service", func(t *testing.T) {