Redesign DB Draft and Proposed Pushing

api/sql/porch-db-1.6.0-1.6.4.sql

@@ -0,0 +1,18 @@
+/*
+Copyright 2026 The kpt Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+ALTER TABLE package_revisions
+    ADD COLUMN IF NOT EXISTS last_pushed_db_updated TIMESTAMP;

api/sql/porch-db-1.6.4-1.6.0.sql

@@ -0,0 +1,18 @@
+/*
+Copyright 2026 The kpt Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+ALTER TABLE package_revisions
+    DROP COLUMN IF EXISTS last_pushed_db_updated;

api/sql/porch-db.sql

@@ -88,6 +88,7 @@ CREATE TABLE IF NOT EXISTS package_revisions (
     kptfile_status   TEXT NOT NULL DEFAULT '{}',
     resources_size   BIGINT NOT NULL DEFAULT 0,
     upstream_ref_name TEXT NOT NULL DEFAULT '',
+    last_pushed_db_updated       TIMESTAMP,
     PRIMARY KEY (k8s_name_space, k8s_name),
     CONSTRAINT fk_package
         FOREIGN KEY (k8s_name_space, package_k8s_name)

deployments/porch/3-porch-postgres-bundle.yaml

@@ -356,6 +356,7 @@ data:
         kptfile_status   TEXT NOT NULL DEFAULT '{}',
         resources_size   BIGINT NOT NULL DEFAULT 0,
         upstream_ref_name TEXT NOT NULL DEFAULT '',
+        last_pushed_db_updated       TIMESTAMP,
         PRIMARY KEY (k8s_name_space, k8s_name),
         CONSTRAINT fk_package
             FOREIGN KEY (k8s_name_space, package_k8s_name)

deployments/porch/9-controllers.yaml

@@ -51,6 +51,7 @@ spec:
         imagePullPolicy: IfNotPresent
         args:
         - --repositories.cache-type=DB
+        - --repositories.push-drafts-to-git=false
         - --repositories.create-v1alpha2-rpkg=false
         securityContext:
           runAsNonRoot: true

make/deploy.mk

@@ -29,6 +29,9 @@ export FN_RUNNER_WARM_UP_POD_CACHE ?= true
 # Enable v1alpha2 PackageRevision support (CRD install + controller flag + reconciler)
 export CREATE_V1ALPHA2_RPKG ?= false
 
+# Push draft & proposed PR's to git rather than DB only
+export DB_PUSH_DRAFTS_TO_GIT ?= false
+
 # Reconciler configuration
 ALL_RECONCILERS=packagevariants,packagevariantsets,repositories
 ifndef RECONCILERS
@@ -52,6 +55,7 @@ run-in-kind: load-images-to-kind deployment-config deploy-current-config
 run-in-kind-v1alpha2: IMAGE_REPO=porch-kind## Build and deploy porch into a kind cluster with DB cache and v1alpha2 PackageRevision CRD creation
 run-in-kind-v1alpha2: PORCH_CACHE_TYPE=DB
 run-in-kind-v1alpha2: CREATE_V1ALPHA2_RPKG=true
+run-in-kind-v1alpha2: DB_PUSH_DRAFTS_TO_GIT=true
 run-in-kind-v1alpha2: load-images-to-kind deployment-config deploy-current-config
 
 .PHONY: run-in-kind-v1alpha2-no-controller

pkg/cache/dbcache/dbpackagerevision.go

@@ -26,7 +26,6 @@ import (
 	"github.com/kptdev/kpt/pkg/kptfile/kptfileutil"
 	porchapi "github.com/kptdev/porch/api/porch/v1alpha1"
 	cachetypes "github.com/kptdev/porch/pkg/cache/types"
-	"github.com/kptdev/porch/pkg/engine"
 	"github.com/kptdev/porch/pkg/repository"
 	"github.com/kptdev/porch/pkg/util"
 	pctx "github.com/kptdev/porch/pkg/util/context"
@@ -76,27 +75,22 @@ func extractFromKptfile(resources map[string]string) (kptfileStatus, []porchapi.
 }
 
 type dbPackageRevision struct {
-	repo               *dbRepository
-	pkgRevKey          repository.PackageRevisionKey
-	meta               metav1.ObjectMeta
-	spec               *porchapi.PackageRevisionSpec
-	updated            time.Time
-	updatedBy          string
-	lifecycle          porchapi.PackageRevisionLifecycle
-	extPRID            kptfile.Locator
-	latest             bool
-	deployment         bool
-	tasks              []porchapi.Task
-	resources          map[string]string
-	resourcesDirty     bool
-	kptfileStatus      kptfileStatus
-	resourcesSizeBytes int64
-
-	// gitPRDraft maintains the draft in the external git repository during editing (when pushDraftsToGit is true)
-	gitPRDraft repository.PackageRevisionDraft
-
-	// gitPR is the closed package revision in git (when pushDraftsToGit is true)
-	gitPR repository.PackageRevision
+	repo                *dbRepository
+	pkgRevKey           repository.PackageRevisionKey
+	meta                metav1.ObjectMeta
+	spec                *porchapi.PackageRevisionSpec
+	updated             time.Time
+	updatedBy           string
+	lifecycle           porchapi.PackageRevisionLifecycle
+	extPRID             kptfile.Locator
+	latest              bool
+	deployment          bool
+	tasks               []porchapi.Task
+	resources           map[string]string
+	resourcesDirty      bool
+	kptfileStatus       kptfileStatus
+	resourcesSizeBytes  int64
+	lastPushedDbUpdated *time.Time
 }
 
 // ensureRepo resolves the repository from the cache if pr.repo is nil.
@@ -158,8 +152,9 @@ func (pr *dbPackageRevision) savePackageRevision(ctx context.Context, saveResour
 		pr.updatedBy = getCurrentUser()
 	}
 
-	_, err := pkgRevReadFromDB(ctx, pr.Key(), false)
+	existing, err := pkgRevReadFromDB(ctx, pr.Key(), false)
 	if err == nil {
+		preservePushMarkersIfUnset(pr, existing)
 		updErr := pkgRevUpdateDB(ctx, pr, saveResources)
 		if updErr == nil && saveResources {
 			sent := pr.repo.repoPRChangeNotifier.NotifyPackageRevisionChange(watch.Modified, pr)
@@ -202,6 +197,9 @@ func (pr *dbPackageRevision) UpdateLifecycle(ctx context.Context, newLifecycle p
 	_, span := tracer.Start(ctx, "dbPackageRevision::UpdateLifecycle", trace.WithAttributes())
 	defer span.End()
 
+	lockPkgKey(pr.pkgRevKey.PkgKey)
+	defer unlockPkgKey(pr.pkgRevKey.PkgKey)
+
 	if err := pr.ensureRepo(); err != nil {
 		return fmt.Errorf("cannot update lifecycle for package revision %s: %w", pr.KubeObjectName(), err)
 	}
@@ -214,11 +212,6 @@ func (pr *dbPackageRevision) UpdateLifecycle(ctx context.Context, newLifecycle p
 			klog.V(3).InfoS("[DB Cache] Lifecycle updated in database and pushed to external repo for PackageRevision",
 				pctx.LogMetadataFrom(ctx)...)
 		}()
-	} else if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
-		klog.InfoS("[DB Cache] Updating lifecycle in database and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
-		defer func() {
-			klog.V(3).InfoS("[DB Cache] Lifecycle updated in database and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
-		}()
 	} else {
 		klog.InfoS("[DB Cache] Updating lifecycle in database for PackageRevision", pctx.LogMetadataFrom(ctx)...)
 		defer func() {
@@ -231,20 +224,12 @@ func (pr *dbPackageRevision) UpdateLifecycle(ctx context.Context, newLifecycle p
 			pr.pkgRevKey.Revision = 0
 			return pkgerrors.Wrapf(err, "dbPackageRevision:UpdateLifecycle: could not publish package revision %+v", pr.Key())
 		}
-		// drops cached stale draft so it doesnt trigger closure
-		pr.gitPRDraft = nil
 	} else if porchapi.LifecycleIsPublished(pr.lifecycle) {
 		return pr.updateLifecycleOnPublishedPR(ctx, newLifecycle)
 	}
 
 	pr.lifecycle = newLifecycle
 
-	if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
-		if err := pr.gitPRDraft.UpdateLifecycle(ctx, newLifecycle); err != nil {
-			klog.Warningf("failed to update git draft lifecycle for %+v: %v", pr.Key(), err)
-		}
-	}
-
 	return nil
 }
 
@@ -408,7 +393,17 @@ func (pr *dbPackageRevision) SetMeta(ctx context.Context, meta metav1.ObjectMeta
 	_, span := tracer.Start(ctx, "dbPackageRevision::SetMeta", trace.WithAttributes())
 	defer span.End()
 
+	lockPkgKey(pr.pkgRevKey.PkgKey)
+	defer unlockPkgKey(pr.pkgRevKey.PkgKey)
+
 	pr.meta = meta
+
+	if existing, err := pkgRevReadFromDB(ctx, pr.Key(), false); err == nil {
+		preservePushMarkersIfUnset(pr, existing)
+	} else if err != sql.ErrNoRows {
+		return err
+	}
+
 	return pkgRevUpdateDB(ctx, pr, false)
 }
 
@@ -482,6 +477,17 @@ func (pr *dbPackageRevision) copyToThis(otherPr *dbPackageRevision) {
 	pr.tasks = otherPr.tasks
 	pr.resources = otherPr.resources
 	pr.resourcesSizeBytes = otherPr.resourcesSizeBytes
+	pr.lastPushedDbUpdated = otherPr.lastPushedDbUpdated
+}
+
+func preservePushMarkersIfUnset(pr, existing *dbPackageRevision) {
+	if pr.lastPushedDbUpdated != nil || existing.lastPushedDbUpdated == nil {
+		return
+	}
+	pr.lastPushedDbUpdated = existing.lastPushedDbUpdated
+	if extPRCommit(pr) == unpushedGitCommit {
+		pr.extPRID = existing.extPRID
+	}
 }
 
 func (pr *dbPackageRevision) UpdateResources(ctx context.Context, new *porchapi.PackageRevisionResources, change *porchapi.Task) error {
@@ -492,18 +498,6 @@ func (pr *dbPackageRevision) UpdateResources(ctx context.Context, new *porchapi.
 		return fmt.Errorf("cannot update resources for package revision %s: %w", pr.KubeObjectName(), err)
 	}
 
-	if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
-		klog.InfoS("[DB Cache] Updating resources in memory and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
-		defer func() {
-			klog.V(3).InfoS("[DB Cache] Resources updated in memory and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
-		}()
-	} else {
-		klog.InfoS("[DB Cache] Updating resources in memory for PackageRevision", pctx.LogMetadataFrom(ctx)...)
-		defer func() {
-			klog.V(3).InfoS("[DB Cache] Resources updated in memory for PackageRevision", pctx.LogMetadataFrom(ctx)...)
-		}()
-	}
-
 	pr.resources = new.Spec.Resources
 	pr.resourcesDirty = true
 	status, gates, pkgMeta := extractFromKptfile(pr.resources)
@@ -523,12 +517,6 @@ func (pr *dbPackageRevision) UpdateResources(ctx context.Context, new *porchapi.
 		pr.tasks = []porchapi.Task{*change}
 	}
 
-	if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
-		if err := pr.gitPRDraft.UpdateResources(ctx, new, change); err != nil {
-			klog.Warningf("failed to update git draft resources for %+v: %v", pr.Key(), err)
-		}
-	}
-
 	return nil
 }
 
@@ -544,16 +532,7 @@ func (pr *dbPackageRevision) publishPR(ctx context.Context, newLifecycle porchap
 	pr.pkgRevKey.Revision = latestRev + 1
 	pr.lifecycle = newLifecycle
 
-	var gitPR repository.PackageRevision
-	if pr.repo.pushDraftsToGit {
-		if pr.gitPR != nil {
-			gitPR = pr.gitPR
-		} else {
-			gitPR = pr.repo.getCachedGitPR(pr.Key().PkgKey, pr.Key().WorkspaceName)
-		}
-	}
-
-	pushedPRExtID, err := engine.PushPackageRevision(ctx, pr.repo.externalRepo, pr, pr.repo.pushDraftsToGit, gitPR)
+	pushedPRExtID, err := PushPublishedPackageRevision(ctx, pr.repo.externalRepo, pr, pr.repo.pushDraftsToGit, hasBeenPushedToGit(pr))
 	if err != nil {
 		klog.Warningf("push of package revision %+v to external repo failed, %q", pr.Key(), err)
 		pr.pkgRevKey.Revision = 0
@@ -562,6 +541,8 @@ func (pr *dbPackageRevision) publishPR(ctx context.Context, newLifecycle porchap
 	}
 
 	pr.extPRID = pushedPRExtID
+	dbUpdated := pr.updated
+	pr.lastPushedDbUpdated = &dbUpdated
 
 	if err = pkgRevUpdateDB(ctx, pr, false); err != nil {
 		return pkgerrors.Wrapf(err, "dbPackageRevision:publishPR: failed to save package revision %+v to database after push to external repo", pr.Key())

pkg/cache/dbcache/dbpackagerevision_test.go

@@ -770,13 +770,6 @@ func (t *DbTestSuite) TestDBPackageRevisionPublishWithPushDraftsToGit() {
 
 	testRepo.externalRepo = extRepo
 	testRepo.pushDraftsToGit = true
-	testRepo.gitPRCache = make(map[string]repository.PackageRevision)
-
-	// Create: a git draft is opened and then closed, yielding the cached git PR.
-	initialGitDraft := mockrepo.NewMockPackageRevisionDraft(t.T())
-	cachedGitPR := mockrepo.NewMockPackageRevision(t.T())
-	extRepo.EXPECT().CreatePackageRevisionDraft(mock.Anything, mock.Anything).Return(initialGitDraft, nil).Once()
-	extRepo.EXPECT().ClosePackageRevisionDraft(mock.Anything, initialGitDraft, 0).Return(cachedGitPR, nil).Once()
 
 	newPRDef := porchapi.PackageRevision{
 		Spec: porchapi.PackageRevisionSpec{
@@ -792,7 +785,6 @@ func (t *DbTestSuite) TestDBPackageRevisionPublishWithPushDraftsToGit() {
 
 	dbPR, err := testRepo.ClosePackageRevisionDraft(ctx, prDraft, 0)
 	t.Require().NoError(err)
-	t.Require().Nil(dbPR.(*dbPackageRevision).gitPRDraft, "closing a draft must release the git draft handle")
 
 	// Propose.
 	err = dbPR.UpdateLifecycle(ctx, porchapi.PackageRevisionLifecycleProposed)
@@ -801,30 +793,20 @@ func (t *DbTestSuite) TestDBPackageRevisionPublishWithPushDraftsToGit() {
 	dbPR, err = testRepo.ClosePackageRevisionDraft(ctx, dbPR.(repository.PackageRevisionDraft), 0)
 	t.Require().NoError(err)
 
-	// Approve. dbRepository.UpdatePackageRevision reopens the cached git PR as a draft, and
-	// publishPR opens and closes a second one with the real revision number.
-	staleGitDraft := mockrepo.NewMockPackageRevisionDraft(t.T())
+	// Publish pushes to git via PushPublishedPackageRevision (no prior draft push, so a new git draft is created).
 	publishGitDraft := mockrepo.NewMockPackageRevisionDraft(t.T())
 	publishedGitPR := mockrepo.NewMockPackageRevision(t.T())
 
-	extRepo.EXPECT().UpdatePackageRevision(mock.Anything, cachedGitPR).Return(staleGitDraft, nil).Once()
-	extRepo.EXPECT().UpdatePackageRevision(mock.Anything, cachedGitPR).Return(publishGitDraft, nil).Once()
+	extRepo.EXPECT().CreatePackageRevisionDraft(mock.Anything, mock.Anything).Return(publishGitDraft, nil).Once()
+	publishGitDraft.EXPECT().UpdateResources(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
 	publishGitDraft.EXPECT().UpdateLifecycle(mock.Anything, porchapi.PackageRevisionLifecyclePublished).Return(nil).Once()
-	// The revision number, never 0, is what reaches git for a Published draft.
 	extRepo.EXPECT().ClosePackageRevisionDraft(mock.Anything, publishGitDraft, 1).Return(publishedGitPR, nil).Once()
 	publishedGitPR.EXPECT().GetLock(mock.Anything).Return(kptfilev1.Upstream{}, kptfilev1.Locator{}, nil).Once()
 
-	approveDraft, err := testRepo.UpdatePackageRevision(ctx, dbPR)
-	t.Require().NoError(err)
-	t.Require().Equal(staleGitDraft, approveDraft.(*dbPackageRevision).gitPRDraft)
-
-	err = approveDraft.UpdateLifecycle(ctx, porchapi.PackageRevisionLifecyclePublished)
+	err = dbPR.UpdateLifecycle(ctx, porchapi.PackageRevisionLifecyclePublished)
 	t.Require().NoError(err)
-	t.Require().Nil(approveDraft.(*dbPackageRevision).gitPRDraft,
-		"publishing must release the stale git draft handle so it is not closed with version 0")
 
-	// No further git calls are expected here: staleGitDraft is never closed.
-	publishedPR, err := testRepo.ClosePackageRevisionDraft(ctx, approveDraft, 0)
+	publishedPR, err := testRepo.ClosePackageRevisionDraft(ctx, dbPR.(repository.PackageRevisionDraft), 0)
 	t.Require().NoError(err)
 	t.Require().Equal(1, publishedPR.Key().Revision)
 	t.Require().Equal(porchapi.PackageRevisionLifecyclePublished, publishedPR.Lifecycle(ctx))