Add rollback #209

josephsawaya · 2022-07-18T16:02:17Z

This PR adds rollback so fetchit won't
have deployed only part of a set of changes
so the state will only correspond to one commit
at a time.

This PR adds simple rollback by trying to
undo the changes made during a failed Apply by
Applying in the opposite direction.

Signed-off-by: Joseph Sawaya [email protected]

jeffvance · 2022-07-18T18:34:45Z

pkg/engine/common.go

+type empty struct {
+}
+
+var badCommitList map[plumbing.Hash]empty = make(map[plumbing.Hash]empty)


super nit: it may be more idiomatic to see:
var badCommitList map[plumbing.Hash]struct{} = make(map[plumbing.Hash]struct{})
rather than defining empty

pkg/engine/types.go

pkg/engine/raw.go

pkg/engine/common.go

sallyom

I commented on the single-line errors nit that keeps me up at night.
If it's possible, add a workflow for this b4 merging.
Other than that, lgtm and rollback ftw!

This commit adds simple rollback by trying to undo the changes made during a failed Apply by Applying in the opposite direction. Signed-off-by: Joseph Sawaya <[email protected]>

sallyom · 2022-09-01T19:37:13Z

pkg/engine/common.go

+		}
+
+		if _, ok := BadCommitList[directory][m.GetKind()][latest]; ok {
+			klog.Infof("No changes applied to target %s this run, %s currently at %s", directory, m.GetKind(), current)


update klog.Infof to logger.Infof - we transitioned to using zap for logging #225

sallyom · 2022-09-01T19:37:42Z

pkg/engine/common.go

+		if err = m.Apply(ctx, conn, current, latest, tag); err != nil {
+			if target.rollback {
+				// Roll back automatically
+				klog.Errorf("Failed to apply changes, rolling back to %v: %v", current, err)


klog.Errorf -> logger.Errorf

sallyom

update klog to logger & then run go mod tidy; go mod vendor (might need to run go mod tidy -compat=1.17, since it will try to be compatible with go 1.16 and fail)
I'm wondering if we should include the opt-in & the boolean type for Rollback - we're only doing that for now bc it's new, right? Maybe we should remove the option and make it the default - but still opt-in for trackBadCommits. wdyt? @cooktheryan ?

sallyom · 2022-09-02T12:54:13Z

#242 discussion on whether fetchit should/should not track bad commits

jeffvance reviewed Jul 18, 2022

View reviewed changes

josephsawaya force-pushed the rollback branch from 8aaec49 to 5d42f4c Compare July 19, 2022 14:12

josephsawaya changed the title ~~[wip] fix raw and add rollback~~ Fix raw and add rollback Jul 20, 2022

josephsawaya force-pushed the rollback branch from 5d42f4c to 5e53503 Compare July 21, 2022 15:06

sallyom reviewed Jul 22, 2022

View reviewed changes

pkg/engine/types.go Outdated Show resolved Hide resolved