From e4bb5e046d0c84f79fbb475d7c215966bbb5697f Mon Sep 17 00:00:00 2001 From: sriharsh Date: Thu, 12 Dec 2024 20:33:12 +0530 Subject: [PATCH 1/6] fix for sporadic porch crash during new package revision --- .../controllers/packagevariant/injection.go | 5 +++- .../packagevariant_controller.go | 26 +++++++++++++------ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/injection.go b/controllers/packagevariants/pkg/controllers/packagevariant/injection.go index 59ef5c40..edad6a32 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/injection.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/injection.go @@ -119,7 +119,10 @@ func ensureConfigInjection(ctx context.Context, return err } - setInjectionPointConditionsAndGates(kptfile, injectionPoints) + err = setInjectionPointConditionsAndGates(kptfile, injectionPoints) + if err != nil { + return err + } prr.Spec.Resources["Kptfile"] = kptfile.String() diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go index 95774ae2..90a087be 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go @@ -596,15 +596,25 @@ func (r *PackageVariantReconciler) deletePackageRevision(ctx context.Context, pr // determine if the downstream PR needs to be updated func (r *PackageVariantReconciler) isUpToDate(pv *api.PackageVariant, downstream *porchapi.PackageRevision) bool { - upstreamLock := downstream.Status.UpstreamLock - lastIndex := strings.LastIndex(upstreamLock.Git.Ref, "/") - if strings.HasPrefix(upstreamLock.Git.Ref, "drafts") { - // The current upstream is a draft, and the target upstream - // will always be a published revision, so we will need to do an update. - return false + if downstream.Status != nil && downstream.Status.UpstreamLock != nil { + upstreamLock := downstream.Status.UpstreamLock + if upstreamLock.Git != nil && upstreamLock.Git.Ref != nil { + lastIndex := strings.LastIndex(upstreamLock.Git.Ref, "/") + if strings.HasPrefix(upstreamLock.Git.Ref, "drafts") { + // The current upstream is a draft, and the target upstream + // will always be a published revision, so we will need to do an update. + return false + } + currentUpstreamRevision := upstreamLock.Git.Ref[lastIndex+1:] + return currentUpstreamRevision == pv.Spec.Upstream.Revision + } else { + klog.Warningf("status.upstreamLock.git or status.upstreamLock.git.ref is nil for PackageRevision %s.", pv.ObjectMeta.Name) + return true + } + } else { + klog.Warningf("status or status.upstreamLock is nil for PackageRevision %s.", pv.ObjectMeta.Name) + return true } - currentUpstreamRevision := upstreamLock.Git.Ref[lastIndex+1:] - return currentUpstreamRevision == pv.Spec.Upstream.Revision } func (r *PackageVariantReconciler) copyPublished(ctx context.Context, From 72b00ee0f2f6ccdb1a2f6fd6d691512413df70eb Mon Sep 17 00:00:00 2001 From: sriharsh Date: Thu, 12 Dec 2024 20:33:12 +0530 Subject: [PATCH 2/6] fix for sporadic porch crash during new package revision --- .../controllers/packagevariant/injection.go | 2 +- .../packagevariant_controller.go | 32 ++++++++----------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/injection.go b/controllers/packagevariants/pkg/controllers/packagevariant/injection.go index edad6a32..d937cdcf 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/injection.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/injection.go @@ -1,4 +1,4 @@ -// Copyright 2023 The kpt and Nephio Authors +// Copyright 2024 The kpt and Nephio Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go index 90a087be..2d4062e0 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go @@ -1,4 +1,4 @@ -// Copyright 2022 The kpt and Nephio Authors +// Copyright 2024 The kpt and Nephio Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -596,25 +596,21 @@ func (r *PackageVariantReconciler) deletePackageRevision(ctx context.Context, pr // determine if the downstream PR needs to be updated func (r *PackageVariantReconciler) isUpToDate(pv *api.PackageVariant, downstream *porchapi.PackageRevision) bool { - if downstream.Status != nil && downstream.Status.UpstreamLock != nil { - upstreamLock := downstream.Status.UpstreamLock - if upstreamLock.Git != nil && upstreamLock.Git.Ref != nil { - lastIndex := strings.LastIndex(upstreamLock.Git.Ref, "/") - if strings.HasPrefix(upstreamLock.Git.Ref, "drafts") { - // The current upstream is a draft, and the target upstream - // will always be a published revision, so we will need to do an update. - return false - } - currentUpstreamRevision := upstreamLock.Git.Ref[lastIndex+1:] - return currentUpstreamRevision == pv.Spec.Upstream.Revision - } else { - klog.Warningf("status.upstreamLock.git or status.upstreamLock.git.ref is nil for PackageRevision %s.", pv.ObjectMeta.Name) - return true - } - } else { - klog.Warningf("status or status.upstreamLock is nil for PackageRevision %s.", pv.ObjectMeta.Name) + if downstream.Status.UpstreamLock == nil { + klog.Warningf("status.upstreamLock is nil for PackageRevision %s.", pv.ObjectMeta.Name) return true } + upstreamLock := downstream.Status.UpstreamLock + if upstreamLock.Git == nil || upstreamLock.Git.Ref == "" { + klog.Warningf("status.upstreamLock.git or status.upstreamLock.git.ref is nil for PackageRevision %s.", pv.ObjectMeta.Name) + return true + } + lastIndex := strings.LastIndex(upstreamLock.Git.Ref, "/") + if strings.HasPrefix(upstreamLock.Git.Ref, "drafts") { + // The current upstream is a draft, and the target upstream + // will always be a published revision, so we will need to do an update. + return false + } } func (r *PackageVariantReconciler) copyPublished(ctx context.Context, From cc610c517d48d1d87ebc87a4c0f952b51f54a817 Mon Sep 17 00:00:00 2001 From: sriharsh Date: Thu, 12 Dec 2024 20:33:12 +0530 Subject: [PATCH 3/6] fix for sporadic porch crash during new package revision --- .../pkg/controllers/packagevariant/packagevariant_controller.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go index 2d4062e0..13725dad 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go @@ -611,6 +611,8 @@ func (r *PackageVariantReconciler) isUpToDate(pv *api.PackageVariant, downstream // will always be a published revision, so we will need to do an update. return false } + currentUpstreamRevision := upstreamLock.Git.Ref[lastIndex+1:] + return currentUpstreamRevision == pv.Spec.Upstream.Revision } func (r *PackageVariantReconciler) copyPublished(ctx context.Context, From 1fc657abb279d041001c9e73af376bfa113c53f1 Mon Sep 17 00:00:00 2001 From: sriharsh Date: Thu, 12 Dec 2024 20:33:12 +0530 Subject: [PATCH 4/6] fix for sporadic porch crash during new package revision --- .../controllers/packagevariant/packagevariant_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go index 13725dad..3fccef8d 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go @@ -597,12 +597,12 @@ func (r *PackageVariantReconciler) deletePackageRevision(ctx context.Context, pr // determine if the downstream PR needs to be updated func (r *PackageVariantReconciler) isUpToDate(pv *api.PackageVariant, downstream *porchapi.PackageRevision) bool { if downstream.Status.UpstreamLock == nil { - klog.Warningf("status.upstreamLock is nil for PackageRevision %s.", pv.ObjectMeta.Name) + klog.Warningf("status.upstreamLock field is empty/missing in downstream PackageRevision: %s", pv.ObjectMeta.Name) return true } upstreamLock := downstream.Status.UpstreamLock if upstreamLock.Git == nil || upstreamLock.Git.Ref == "" { - klog.Warningf("status.upstreamLock.git or status.upstreamLock.git.ref is nil for PackageRevision %s.", pv.ObjectMeta.Name) + klog.Warningf("status.upstreamLock.git or status.upstreamLock.git.ref field is empty/missing in downstream PackageRevision: %s", pv.ObjectMeta.Name) return true } lastIndex := strings.LastIndex(upstreamLock.Git.Ref, "/") From 639a60990c4f69158b5e641aed8eca99af27db99 Mon Sep 17 00:00:00 2001 From: Istvan Kispal Date: Fri, 13 Dec 2024 23:37:26 +0100 Subject: [PATCH 5/6] Update controllers/packagevariants/pkg/controllers/packagevariant/injection.go Co-authored-by: Liam Fallon <35595825+liamfallon@users.noreply.github.com> --- .../packagevariants/pkg/controllers/packagevariant/injection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/injection.go b/controllers/packagevariants/pkg/controllers/packagevariant/injection.go index d937cdcf..3fbf859d 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/injection.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/injection.go @@ -1,4 +1,4 @@ -// Copyright 2024 The kpt and Nephio Authors +// Copyright 2023-2024 The kpt and Nephio Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 93acdfed5e9b12e7ddea34ab6ade51e934297af7 Mon Sep 17 00:00:00 2001 From: Istvan Kispal Date: Fri, 13 Dec 2024 23:37:40 +0100 Subject: [PATCH 6/6] Update controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go Co-authored-by: Liam Fallon <35595825+liamfallon@users.noreply.github.com> --- .../pkg/controllers/packagevariant/packagevariant_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go index 3fccef8d..8b2dda28 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go @@ -1,4 +1,4 @@ -// Copyright 2024 The kpt and Nephio Authors +// Copyright 2023-2024 The kpt and Nephio Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License.