generic-specializer: skip condition walk when Kptfile.Status is nil#1105
generic-specializer: skip condition walk when Kptfile.Status is nil#1105SAY-5 wants to merge 1 commit into
Conversation
kptv1.KptFile.Status is an optional *Status, so newly cloned repos, raw drafts, and hand-authored packages reach the reconciler with Status == nil. The reconciler ranged over kptfile.Status.Conditions and then called porchcondition.GetPorchConditions(kptfile.Status.Conditions) without checking, which panicked and stalled the PackageRevision lifecycle (#1099). Guard the whole block with `if kptfile.Status != nil`. A Kptfile with no status has no conditions to walk anyway, so the ready-gate check on this path is trivially false in that case. Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com> Fixes #1099
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
Hi @SAY-5. Thanks for your PR. I'm waiting for a nephio-project member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
| strings.HasPrefix(c.Type, kptfilelibv1.GetConditionType(&ipamFor)+".") || | ||
| strings.HasPrefix(c.Type, kptfilelibv1.GetConditionType(&configInjectFor)+".") { | ||
| log.Info("generic specializer conditions", "packageName", pr.Spec.PackageName, "repository", pr.Spec.RepositoryName, "status", c.Status, "condition", c.Type, "message", c.Message) | ||
| // kptv1.KptFile.Status is an optional *Status; newly |
There was a problem hiding this comment.
Can KptFile.Status ever be nil?
2 participants
Summary
kptv1.KptFile.Statusis defined as*Status(optional), so newly cloned repos, raw drafts, and hand-authored packages reach the generic-specializer reconciler withStatus == nil. The reconciler ranged overkptfile.Status.Conditionsand then calledporchcondition.GetPorchConditions(kptfile.Status.Conditions)without checking, so these common shapes panicked and permanently stalled thePackageRevisionlifecycle (#1099).Fixes kptdev/porch#909.
Change
controllers/pkg/reconcilers/generic-specializer/reconciler.go, wrap the condition walk + readiness-gate check inif kptfile.Status != nil { ... }. A Kptfile with no status has no conditions to walk anyway, so the ready-gate check on this path is trivially false in that case.Testing
go build ./controllers/...passes.PackageRevisionwhose Kptfile has nostatusblock. Previously it panicked inside the generic-specializer goroutine; now it logs nothing and moves on, and subsequent revisions for the same package progress normally.