From a202a5824a1b21b472a5c9c2439484d2b5c33482 Mon Sep 17 00:00:00 2001 From: Gunjan Vyas Date: Tue, 30 Nov 2021 14:52:07 +0530 Subject: [PATCH 1/4] Fixed panic when traffic split to unavailable revision --- pkg/serving/config_changes.go | 2 +- pkg/serving/revision_template.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index 771b58810c..5c5cc57e0a 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -145,7 +145,7 @@ func PinImageToDigest(currentRevisionTemplate *servingv1.RevisionTemplateSpec, b } containerStatus := ContainerStatus(baseRevision) - if containerStatus.ImageDigest != "" { + if containerStatus != nil && containerStatus.ImageDigest != "" { return flags.UpdateImage(¤tRevisionTemplate.Spec.PodSpec, containerStatus.ImageDigest) } return nil diff --git a/pkg/serving/revision_template.go b/pkg/serving/revision_template.go index 0339a22fe4..efb6907aa9 100644 --- a/pkg/serving/revision_template.go +++ b/pkg/serving/revision_template.go @@ -65,7 +65,7 @@ func ContainerIndexOfRevisionSpec(revisionSpec *servingv1.RevisionSpec) int { // such status could be found func ContainerStatus(r *servingv1.Revision) *servingv1.ContainerStatus { idx := ContainerIndexOfRevisionSpec(&r.Spec) - if idx == -1 { + if idx < 0 || idx >= len(r.Status.ContainerStatuses) { return nil } return &r.Status.ContainerStatuses[idx] From 76117e692ed17b1a6ffa64fcde81d305ba0cd411 Mon Sep 17 00:00:00 2001 From: Gunjan Vyas Date: Tue, 30 Nov 2021 14:59:26 +0530 Subject: [PATCH 2/4] Added Changelog entry --- CHANGELOG.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 5c5e884456..d5129002e2 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -21,6 +21,10 @@ | Fixed panic in kn service describe | https://github.com/knative/client/pull/1529[#1529] +| 🐛 +| Fixed panic in kn service update +| https://github.com/knative/client/pull/1533[#1533] + |=== ## v1.0.0 (2021-11-02) From 6a8040fb755f7fc458277b0c13831dcb7e440b3f Mon Sep 17 00:00:00 2001 From: Gunjan Vyas Date: Tue, 30 Nov 2021 16:03:07 +0530 Subject: [PATCH 3/4] Added unit tests --- pkg/serving/config_changes_test.go | 10 ++++ pkg/serving/revision_template_test.go | 86 +++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index b8dc7d216e..cb4e51170c 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -141,6 +141,16 @@ func TestPinImageToDigestInvalidImages(t *testing.T) { assert.ErrorContains(t, err, "unexpected image") } +func TestPinImageToDigestNilContainerStatus(t *testing.T) { + template, _ := getRevisionTemplate() + revision := &servingv1.Revision{} + revision.Spec = template.Spec + revision.ObjectMeta = template.ObjectMeta + revision.Status.ContainerStatuses = nil + err := PinImageToDigest(template, revision) + assert.NilError(t, err) +} + func TestUpdateTimestampAnnotation(t *testing.T) { template, _ := getRevisionTemplate() UpdateTimestampAnnotation(template) diff --git a/pkg/serving/revision_template_test.go b/pkg/serving/revision_template_test.go index 69a4ad681b..f6f8ba6f7f 100644 --- a/pkg/serving/revision_template_test.go +++ b/pkg/serving/revision_template_test.go @@ -151,3 +151,89 @@ func TestPort(t *testing.T) { revisionSpec.PodSpec.Containers[0].Ports = []corev1.ContainerPort{port} assert.Equal(t, (int32)(42), *Port(revisionSpec)) } + +func TestContainerStatus(t *testing.T) { + tests := []struct { + name string + rev *servingv1.Revision + want *servingv1.ContainerStatus + }{ + { + "no container", + &servingv1.Revision{ + Spec: servingv1.RevisionSpec{}, + }, + nil, + }, + { + "no container", + &servingv1.Revision{ + Spec: servingv1.RevisionSpec{}, + }, + nil, + }, + { + "1 container", + &servingv1.Revision{ + Spec: servingv1.RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "user-container", + Ports: []corev1.ContainerPort{{ContainerPort: 80}}, + }, + }, + }, + }, + Status: servingv1.RevisionStatus{ContainerStatuses: []servingv1.ContainerStatus{ + { + Name: "user-container", + }, + }}, + }, + &servingv1.ContainerStatus{ + Name: "user-container", + }, + }, + { + "3 containers", + &servingv1.Revision{ + Spec: servingv1.RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "sidecar-container-1", + }, + { + Name: "user-container", + Ports: []corev1.ContainerPort{{ContainerPort: 80}}, + }, + { + Name: "sidecar-container-2", + }, + }}}, + Status: servingv1.RevisionStatus{ + ContainerStatuses: []servingv1.ContainerStatus{ + { + Name: "sidecar-container-1", + }, + { + Name: "user-container", + }, + { + Name: "sidecar-container-2", + }, + }, + }}, + &servingv1.ContainerStatus{ + Name: "user-container", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ContainerStatus(tt.rev) + assert.DeepEqual(t, got, tt.want) + }) + } +} From c80052fc1923497c9f05ee65963ee959261e5129 Mon Sep 17 00:00:00 2001 From: Gunjan Vyas Date: Tue, 30 Nov 2021 19:58:22 +0530 Subject: [PATCH 4/4] Removed duplicate test case --- pkg/serving/revision_template_test.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/serving/revision_template_test.go b/pkg/serving/revision_template_test.go index f6f8ba6f7f..a152719b0e 100644 --- a/pkg/serving/revision_template_test.go +++ b/pkg/serving/revision_template_test.go @@ -162,13 +162,11 @@ func TestContainerStatus(t *testing.T) { "no container", &servingv1.Revision{ Spec: servingv1.RevisionSpec{}, - }, - nil, - }, - { - "no container", - &servingv1.Revision{ - Spec: servingv1.RevisionSpec{}, + Status: servingv1.RevisionStatus{ContainerStatuses: []servingv1.ContainerStatus{ + { + Name: "user-container", + }, + }}, }, nil, },