Implementing promotion algorithm defined in level-triggered architecture rfc

[luizbafilho]

Implements the algorithm defined in level-triggered architecture rfc.

Notes:

• It does not handle any of the promotion strategies. Those are not idempotent right now, so running them as is will cause unexpected issues.
• replaces github.com/golang/mock with go.uber.org/mock since the former is now unmaintained.

Closes #195

[yitsushi]

A small simplification suggestion, but overall looks good to me. For the simplifications, obviously one of them as they conflict with each other.

I don't know if it's important to check first env separately (like you have other plans in the future that would be easier to have it as a separate check.

[yitsushi]

I think we can use a simple range here, I don't see i is use anyone where.

for _, env := range pipeline.Spec.Environments[1:] {
  // ...
}

[yitsushi]

If latestRevision is set to empty string, we can merge this into the loop below and the check would be something like:

		ok, revision := checkAllTargetsHaveSameRevision(pipeline.Status.Environments[env.Name])
		if !ok {
			// not all targets have the same revision, so we can't proceed
			return ctrl.Result{}, nil
		}

		if latestRevision == "" {
			latestRevision = revision
			continue
		}

		if revision != latestRevision {
			// the same as you have now.
		}

[luizbafilho]

yep, that would work, thanks

[squaremo]

👍 for keeping this low impact (it doesn't splat across lots of files) and pretty accurate. I think the definition for when to trigger a promotion needs another iteration -- see my comment on controller.go#L199. If you can demonstrate that working as described, with tests (see comment on controller_test.go), we're there.
It's worth putting in a guard against nil pointer deference, too. The other comment, on checkAllTargets....(), is just commentary -- do whatever works :-)

[squaremo]

• It does not handle any of the promotion strategies. Those are not idempotent right now, so running them as is will cause unexpected issues.

That's fine, making promotion actions work reliably is the subject of #197.

[squaremo]

Thanks for following up on those previous comments. The way you've structured the algorithm, with the checkAll, checkAny, etc., makes it clear what's happening. Should it break if it successfully calls promote?

[squaremo]

All of my changes / comments have been addressed, thanks @luizbafilho. I think it's worth spending no more than an hour tidying, squashing fixups and making sure the git commits have good messages, before merging. This is a bit big to be one squashed merge.

[yitsushi]

lgtm