Fixed panic when traffic split to unavailable revision #1533
Conversation
There was a problem hiding this comment.
@vyasgun: 0 warnings.
In response to this:
Description
Panic during updating traffic to a revision that is not ready
Before:
$ kn service update hello-example-3 --env TARGET="pqr" --traffic abcd=50,hello-example-3-00002=50 E1130 00:13:13.786017 76154 runtime.go:78] Observed a panic: runtime.boundsError{x:0, y:0, signed:true, code:0x0} (runtime error: index out of range [0] with length 0) goroutine 1 [running]: k8s.io/apimachinery/pkg/util/runtime.logPanic({0x246a6a0, 0xc0006ac570}) /home/prow/go/src/knative.dev/client/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:74 +0x85 k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0x5a0}) /home/prow/go/src/knative.dev/client/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:48 +0x75 panic({0x246a6a0, 0xc0006ac570}) /root/.gvm/gos/go1.17.2/src/runtime/panic.go:1038 +0x215 knative.dev/client/pkg/serving.ContainerStatus(...) /home/prow/go/src/knative.dev/client/pkg/serving/revision_template.go:71 knative.dev/client/pkg/serving.PinImageToDigest(0xc00011a118, 0xc000192700) /home/prow/go/src/knative.dev/client/pkg/serving/config_changes.go:147 +0x293 knative.dev/client/pkg/kn/commands/service.(*ConfigurationEditFlags).Apply(0xc00012e780, 0xc00011a000, 0x23376a0, 0xc00069cb00) /home/prow/go/src/knative.dev/client/pkg/kn/commands/service/configuration_edit_flags.go:242 +0x252 knative.dev/client/pkg/kn/commands/service.NewServiceUpdateCommand.func1.1(0xc00011a000) /home/prow/go/src/knative.dev/client/pkg/kn/commands/service/update.go:100 +0x1d9Now:
$ ./kn service update hello-example-3 --env TARGET="pqr" --traffic abcd=50,hello-example-3-00002=50 Updating Service 'hello-example-3' in namespace 'default': 0.033s Revision "hello-example-3-00002" failed to become ready. 0.093s ... Error: RevisionMissing: Revision "hello-example-3-00002" failed to become ready.Changes
- Added nil pointed check
- Added array length check
Reference
Fixes #
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
knative-prow-robot
added
the
size/XS
Codecov Report
@@ Coverage Diff @@
## main #1533 +/- ##
==========================================
+ Coverage 79.52% 79.56% +0.03%
==========================================
Files 162 162
Lines 8553 8553
==========================================
+ Hits 6802 6805 +3
+ Misses 1073 1071 -2
+ Partials 678 677 -1
Continue to review full report at Codecov.
|
knative-prow-robot
added
size/L
| { | ||
| "no container", | ||
| &servingv1.Revision{ | ||
| Spec: servingv1.RevisionSpec{}, | ||
| }, | ||
| nil, | ||
| }, |
There was a problem hiding this comment.
Is it a duplicate? Maybe that should be No ImageDigest?
There was a problem hiding this comment.
Anyway lets add not empty status without image digest. That'd be in config_change_test.go I suppose. That was my initial thought. :)
There was a problem hiding this comment.
oops, yes. Removed the duplicate test case and modified the no container case with a non-empty Status field.
|
The following is the coverage report on the affected files.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dsimansk, vyasgun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
Description
Panic during updating traffic to a revision that is not ready
Before:
Now:
Changes
Reference
Fixes #