Skip to content

Commit 6afc5e9

Browse files
Preliminary draft pushing refactor
Signed-off-by: Rendre Greyling <rendre.greyling@nokia.com>
1 parent ed230dc commit 6afc5e9

22 files changed

Lines changed: 2610 additions & 398 deletions

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
Copyright 2026 The kpt Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
ALTER TABLE package_revisions
18+
ADD COLUMN IF NOT EXISTS last_pushed_commit TEXT,
19+
ADD COLUMN IF NOT EXISTS last_pushed_commit_timestamp TIMESTAMP,
20+
ADD COLUMN IF NOT EXISTS last_pushed_db_updated TIMESTAMP;

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
Copyright 2026 The kpt Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
ALTER TABLE package_revisions
18+
DROP COLUMN IF EXISTS last_pushed_commit,
19+
DROP COLUMN IF EXISTS last_pushed_commit_timestamp,
20+
DROP COLUMN IF EXISTS last_pushed_db_updated;

api/sql/porch-db.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ CREATE TABLE IF NOT EXISTS package_revisions (
8888
kptfile_status TEXT NOT NULL DEFAULT '{}',
8989
resources_size BIGINT NOT NULL DEFAULT 0,
9090
upstream_ref_name TEXT NOT NULL DEFAULT '',
91+
last_pushed_commit TEXT,
92+
last_pushed_commit_timestamp TIMESTAMP,
93+
last_pushed_db_updated TIMESTAMP,
9194
PRIMARY KEY (k8s_name_space, k8s_name),
9295
CONSTRAINT fk_package
9396
FOREIGN KEY (k8s_name_space, package_k8s_name)

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,9 @@ data:
356356
kptfile_status TEXT NOT NULL DEFAULT '{}',
357357
resources_size BIGINT NOT NULL DEFAULT 0,
358358
upstream_ref_name TEXT NOT NULL DEFAULT '',
359+
last_pushed_commit TEXT,
360+
last_pushed_commit_timestamp TIMESTAMP,
361+
last_pushed_db_updated TIMESTAMP,
359362
PRIMARY KEY (k8s_name_space, k8s_name),
360363
CONSTRAINT fk_package
361364
FOREIGN KEY (k8s_name_space, package_k8s_name)

pkg/cache/dbcache/dbpackagerevision.go

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"github.com/kptdev/kpt/pkg/kptfile/kptfileutil"
2727
porchapi "github.com/kptdev/porch/api/porch/v1alpha1"
2828
cachetypes "github.com/kptdev/porch/pkg/cache/types"
29-
"github.com/kptdev/porch/pkg/engine"
3029
"github.com/kptdev/porch/pkg/repository"
3130
"github.com/kptdev/porch/pkg/util"
3231
pctx "github.com/kptdev/porch/pkg/util/context"
@@ -92,11 +91,18 @@ type dbPackageRevision struct {
9291
kptfileStatus kptfileStatus
9392
resourcesSizeBytes int64
9493

95-
// gitPRDraft maintains the draft in the external git repository during editing (when pushDraftsToGit is true)
96-
gitPRDraft repository.PackageRevisionDraft
94+
// lastPushedCommit is the git commit hash of the last successful push of this revision to git.
95+
// A nil value means the revision has never been (successfully) pushed to git.
96+
lastPushedCommit *string
9797

98-
// gitPR is the closed package revision in git (when pushDraftsToGit is true)
99-
gitPR repository.PackageRevision
98+
// lastPushedCommitTimestamp is the timestamp associated with the last commit pushed to git.
99+
// It is used by conflict resolution to reason about the git side of the last push.
100+
lastPushedCommitTimestamp *time.Time
101+
102+
// lastPushedDbUpdated is the value of the DB `updated` column at the time of the last successful
103+
// push to git. It is used to detect whether the revision content has changed since it was last
104+
// pushed, and by conflict resolution to reason about the DB side of the last push.
105+
lastPushedDbUpdated *time.Time
100106
}
101107

102108
// ensureRepo resolves the repository from the cache if pr.repo is nil.
@@ -158,8 +164,9 @@ func (pr *dbPackageRevision) savePackageRevision(ctx context.Context, saveResour
158164
pr.updatedBy = getCurrentUser()
159165
}
160166

161-
_, err := pkgRevReadFromDB(ctx, pr.Key(), false)
167+
existing, err := pkgRevReadFromDB(ctx, pr.Key(), false)
162168
if err == nil {
169+
preservePushMarkersIfUnset(pr, existing)
163170
updErr := pkgRevUpdateDB(ctx, pr, saveResources)
164171
if updErr == nil && saveResources {
165172
sent := pr.repo.repoPRChangeNotifier.NotifyPackageRevisionChange(watch.Modified, pr)
@@ -202,6 +209,10 @@ func (pr *dbPackageRevision) UpdateLifecycle(ctx context.Context, newLifecycle p
202209
_, span := tracer.Start(ctx, "dbPackageRevision::UpdateLifecycle", trace.WithAttributes())
203210
defer span.End()
204211

212+
pkgMutex := getOrInsertPkgLock(pr.pkgRevKey.PkgKey)
213+
pkgMutex.Lock()
214+
defer pkgMutex.Unlock()
215+
205216
if err := pr.ensureRepo(); err != nil {
206217
return fmt.Errorf("cannot update lifecycle for package revision %s: %w", pr.KubeObjectName(), err)
207218
}
@@ -214,11 +225,6 @@ func (pr *dbPackageRevision) UpdateLifecycle(ctx context.Context, newLifecycle p
214225
klog.V(3).InfoS("[DB Cache] Lifecycle updated in database and pushed to external repo for PackageRevision",
215226
pctx.LogMetadataFrom(ctx)...)
216227
}()
217-
} else if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
218-
klog.InfoS("[DB Cache] Updating lifecycle in database and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
219-
defer func() {
220-
klog.V(3).InfoS("[DB Cache] Lifecycle updated in database and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
221-
}()
222228
} else {
223229
klog.InfoS("[DB Cache] Updating lifecycle in database for PackageRevision", pctx.LogMetadataFrom(ctx)...)
224230
defer func() {
@@ -231,20 +237,12 @@ func (pr *dbPackageRevision) UpdateLifecycle(ctx context.Context, newLifecycle p
231237
pr.pkgRevKey.Revision = 0
232238
return pkgerrors.Wrapf(err, "dbPackageRevision:UpdateLifecycle: could not publish package revision %+v", pr.Key())
233239
}
234-
// drops cached stale draft so it doesnt trigger closure
235-
pr.gitPRDraft = nil
236240
} else if porchapi.LifecycleIsPublished(pr.lifecycle) {
237241
return pr.updateLifecycleOnPublishedPR(ctx, newLifecycle)
238242
}
239243

240244
pr.lifecycle = newLifecycle
241245

242-
if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
243-
if err := pr.gitPRDraft.UpdateLifecycle(ctx, newLifecycle); err != nil {
244-
klog.Warningf("failed to update git draft lifecycle for %+v: %v", pr.Key(), err)
245-
}
246-
}
247-
248246
return nil
249247
}
250248

@@ -408,7 +406,18 @@ func (pr *dbPackageRevision) SetMeta(ctx context.Context, meta metav1.ObjectMeta
408406
_, span := tracer.Start(ctx, "dbPackageRevision::SetMeta", trace.WithAttributes())
409407
defer span.End()
410408

409+
pkgMutex := getOrInsertPkgLock(pr.pkgRevKey.PkgKey)
410+
pkgMutex.Lock()
411+
defer pkgMutex.Unlock()
412+
411413
pr.meta = meta
414+
415+
if existing, err := pkgRevReadFromDB(ctx, pr.Key(), false); err == nil {
416+
preservePushMarkersIfUnset(pr, existing)
417+
} else if err != sql.ErrNoRows {
418+
return err
419+
}
420+
412421
return pkgRevUpdateDB(ctx, pr, false)
413422
}
414423

@@ -482,6 +491,18 @@ func (pr *dbPackageRevision) copyToThis(otherPr *dbPackageRevision) {
482491
pr.tasks = otherPr.tasks
483492
pr.resources = otherPr.resources
484493
pr.resourcesSizeBytes = otherPr.resourcesSizeBytes
494+
pr.lastPushedCommit = otherPr.lastPushedCommit
495+
pr.lastPushedCommitTimestamp = otherPr.lastPushedCommitTimestamp
496+
pr.lastPushedDbUpdated = otherPr.lastPushedDbUpdated
497+
}
498+
499+
func preservePushMarkersIfUnset(pr, existing *dbPackageRevision) {
500+
if pr.lastPushedCommit != nil || existing.lastPushedCommit == nil {
501+
return
502+
}
503+
pr.lastPushedCommit = existing.lastPushedCommit
504+
pr.lastPushedCommitTimestamp = existing.lastPushedCommitTimestamp
505+
pr.lastPushedDbUpdated = existing.lastPushedDbUpdated
485506
}
486507

487508
func (pr *dbPackageRevision) UpdateResources(ctx context.Context, new *porchapi.PackageRevisionResources, change *porchapi.Task) error {
@@ -492,18 +513,6 @@ func (pr *dbPackageRevision) UpdateResources(ctx context.Context, new *porchapi.
492513
return fmt.Errorf("cannot update resources for package revision %s: %w", pr.KubeObjectName(), err)
493514
}
494515

495-
if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
496-
klog.InfoS("[DB Cache] Updating resources in memory and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
497-
defer func() {
498-
klog.V(3).InfoS("[DB Cache] Resources updated in memory and in Git draft for PackageRevision", pctx.LogMetadataFrom(ctx)...)
499-
}()
500-
} else {
501-
klog.InfoS("[DB Cache] Updating resources in memory for PackageRevision", pctx.LogMetadataFrom(ctx)...)
502-
defer func() {
503-
klog.V(3).InfoS("[DB Cache] Resources updated in memory for PackageRevision", pctx.LogMetadataFrom(ctx)...)
504-
}()
505-
}
506-
507516
pr.resources = new.Spec.Resources
508517
pr.resourcesDirty = true
509518
status, gates, pkgMeta := extractFromKptfile(pr.resources)
@@ -523,12 +532,6 @@ func (pr *dbPackageRevision) UpdateResources(ctx context.Context, new *porchapi.
523532
pr.tasks = []porchapi.Task{*change}
524533
}
525534

526-
if pr.repo.pushDraftsToGit && pr.gitPRDraft != nil {
527-
if err := pr.gitPRDraft.UpdateResources(ctx, new, change); err != nil {
528-
klog.Warningf("failed to update git draft resources for %+v: %v", pr.Key(), err)
529-
}
530-
}
531-
532535
return nil
533536
}
534537

@@ -544,16 +547,7 @@ func (pr *dbPackageRevision) publishPR(ctx context.Context, newLifecycle porchap
544547
pr.pkgRevKey.Revision = latestRev + 1
545548
pr.lifecycle = newLifecycle
546549

547-
var gitPR repository.PackageRevision
548-
if pr.repo.pushDraftsToGit {
549-
if pr.gitPR != nil {
550-
gitPR = pr.gitPR
551-
} else {
552-
gitPR = pr.repo.getCachedGitPR(pr.Key().PkgKey, pr.Key().WorkspaceName)
553-
}
554-
}
555-
556-
pushedPRExtID, err := engine.PushPackageRevision(ctx, pr.repo.externalRepo, pr, pr.repo.pushDraftsToGit, gitPR)
550+
pushedPRExtID, commitTimestamp, err := PushPublishedPackageRevision(ctx, pr.repo.externalRepo, pr, pr.repo.pushDraftsToGit, pr.lastPushedCommit != nil)
557551
if err != nil {
558552
klog.Warningf("push of package revision %+v to external repo failed, %q", pr.Key(), err)
559553
pr.pkgRevKey.Revision = 0
@@ -562,6 +556,13 @@ func (pr *dbPackageRevision) publishPR(ctx context.Context, newLifecycle porchap
562556
}
563557

564558
pr.extPRID = pushedPRExtID
559+
if pushedPRExtID.Git != nil && pushedPRExtID.Git.Commit != "" {
560+
pr.lastPushedCommit = new(pushedPRExtID.Git.Commit)
561+
if commitTimestamp.IsZero() {
562+
commitTimestamp = time.Now()
563+
}
564+
pr.lastPushedCommitTimestamp = &commitTimestamp
565+
}
565566

566567
if err = pkgRevUpdateDB(ctx, pr, false); err != nil {
567568
return pkgerrors.Wrapf(err, "dbPackageRevision:publishPR: failed to save package revision %+v to database after push to external repo", pr.Key())

0 commit comments

Comments
 (0)