Skip to content
This repository was archived by the owner on Dec 1, 2022. It is now read-only.

[release-v0.23.2] TestServiceExport works with images specified by digest (#1435) #794

Merged
openshift-merge-robot merged 1 commit into from
Aug 31, 2021
Merged

[release-v0.23.2] TestServiceExport works with images specified by digest (#1435) #794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions test/e2e/service_export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,16 @@ func TestServiceExport(t *testing.T) {
t.Log("create service with byo revision")
serviceCreateWithOptions(r, "hello", "--revision-name", "rev1")

userImage := pkgtest.ImagePath("helloworld")
if strings.Contains(userImage, "@") {
// Images specified by digest are not set in user-image annotation.
userImage = ""
}
t.Log("export service-revision1 and compare")
serviceExport(r, "hello", test.BuildServiceWithOptions("hello",
servingtest.WithConfigSpec(test.BuildConfigurationSpec()),
servingtest.WithBYORevisionName("hello-rev1"),
test.WithRevisionAnnotations(map[string]string{"client.knative.dev/user-image": pkgtest.ImagePath("helloworld")}),
test.WithRevisionAnnotations(map[string]string{"client.knative.dev/user-image": userImage}),
Copy link
Copy Markdown

@rhuss rhuss Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the whole annotation not be set if the userImage is "" (instead of setting the annotation with an empty string ?)

Copy link
Copy Markdown
Author

@mgencur mgencur Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've chosen this approach to keep the tests simple. In the end the annotation is set to an empty value by the kn client anyway: https://github.com/knative/client/blob/release-0.23/pkg/kn/commands/service/create.go#L261

Copy link
Copy Markdown

@rhuss rhuss Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but as it seems to work upstream also with an empty value, looks fine to me. Please note though that this fix might not work with --lock-to-digest option as this option relies on this annotation to contain the value with digest (and if I think twice, I think this annotation is supposed to always hold the digest, but let me check)

Copy link
Copy Markdown

@rhuss rhuss Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, no. it's the other way round, actually the annotation shouldn't contain any digest, so the fix looks good to me.

/approve
/lgtm

Copy link
Copy Markdown
Author

@mgencur mgencur Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking!

), "--mode", "replay", "-o", "json")

t.Log("update service - add env variable")
Expand Down Expand Up @@ -97,7 +102,7 @@ func TestServiceExport(t *testing.T) {
servingtest.WithConfigSpec(test.BuildConfigurationSpec()),
servingtest.WithBYORevisionName("hello-rev1"),
test.WithRevisionAnnotations(map[string]string{
"client.knative.dev/user-image": pkgtest.ImagePath("helloworld"),
"client.knative.dev/user-image": userImage,
"serving.knative.dev/routes": "hello",
}),
)),
Expand All @@ -117,7 +122,7 @@ func TestServiceExport(t *testing.T) {
servingtest.WithEnv(corev1.EnvVar{Name: "a", Value: "mouse"}),
), test.BuildKNExportWithOptions(
test.WithKNRevision(*(test.BuildRevision("hello-rev1",
servingtest.WithRevisionAnn("client.knative.dev/user-image", pkgtest.ImagePath("helloworld")),
servingtest.WithRevisionAnn("client.knative.dev/user-image", userImage),
servingtest.WithRevisionAnn("serving.knative.dev/routes", "hello"),
servingtest.WithRevisionLabel("serving.knative.dev/configuration", "hello"),
servingtest.WithRevisionLabel("serving.knative.dev/configurationGeneration", "1"),
Expand All @@ -136,7 +141,7 @@ func TestServiceExport(t *testing.T) {
test.WithService(test.BuildServiceWithOptions("hello",
servingtest.WithConfigSpec(test.BuildConfigurationSpec()),
test.WithRevisionAnnotations(map[string]string{
"client.knative.dev/user-image": pkgtest.ImagePath("helloworld"),
"client.knative.dev/user-image": userImage,
"serving.knative.dev/routes": "hello",
}),
servingtest.WithBYORevisionName("hello-rev1"),
Expand Down Expand Up @@ -167,7 +172,7 @@ func TestServiceExport(t *testing.T) {
servingtest.WithEnv(corev1.EnvVar{Name: "a", Value: "mouse"}, corev1.EnvVar{Name: "b", Value: "cat"}),
), test.BuildKNExportWithOptions(
test.WithKNRevision(*(test.BuildRevision("hello-rev1",
servingtest.WithRevisionAnn("client.knative.dev/user-image", pkgtest.ImagePath("helloworld")),
servingtest.WithRevisionAnn("client.knative.dev/user-image", userImage),
servingtest.WithRevisionAnn("serving.knative.dev/routes", "hello"),
servingtest.WithRevisionLabel("serving.knative.dev/configuration", "hello"),
servingtest.WithRevisionLabel("serving.knative.dev/configurationGeneration", "1"),
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/source_container_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func containerSourceList(r *test.KnRunResultCollector, containerSources ...strin
r.AssertNoError(out)
assert.Check(r.T(), util.ContainsAll(out.Stdout, "NAME", "IMAGE", "SINK", "READY"))
assert.Check(r.T(), util.ContainsAll(out.Stdout, containerSources...))
assert.Check(r.T(), util.ContainsAll(out.Stdout, "grpc-ping", "ksvc:testsvc0"))
assert.Check(r.T(), util.ContainsAll(out.Stdout, "ksvc:testsvc0"))
}

func containerSourceCreateMissingSink(r *test.KnRunResultCollector, sourceName string, sink string) {
Expand Down