Add built image to image status #39
Conversation
- Add IsSuccess, IsFailure, and BuiltImage to Build type closes #9 Signed-off-by: Joao Pereira <jdealmeidapereira@pivotal.io>
djoyahoy
requested review from
thisisnotashwin and
matthewmcnew
and removed request for
thisisnotashwin
| @@ -55,6 +55,7 @@ type ImageBuild struct { | |||
| type ImageStatus struct { | |||
| duckv1alpha1.Status `json:",inline"` | |||
| LastBuildRef string `json:"lastBuildRef"` | |||
| LastBuiltImage string `json:"lastBuiltImage"` | |||
There was a problem hiding this comment.
Could if we call this latestImage to ease integration with riff's types? https://github.com/projectriff/system/blob/7c6ea07de861e60c1dd02492ea6e53437add7aa9/pkg/apis/build/v1alpha1/shared_types.go#L66
| @@ -21,9 +20,9 @@ func newBuildList(builds []*v1alpha1.Build) (buildList, error) { | |||
| buildList := buildList{} | |||
|
|
|||
| for _, build := range builds { | |||
| if build.Status.GetCondition(duckv1alpha1.ConditionSucceeded).IsTrue() { | |||
| if build.IsSuccess() { | |||
| @@ -148,6 +148,9 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { | |||
|
|
|||
| image.Status.LastBuildRef = reconciledBuild.Build.BuildRef() | |||
| image.Status.BuildCounter = reconciledBuild.BuildCounter | |||
| if lastBuild.IsSuccess() { | |||
There was a problem hiding this comment.
Could this be moved to the reconciledBuild?
Perhaps a reconciledBuild.LatestImage?
That way the logic for an image.Status is recalculated on each reconcile even if it still utilizes the previous latestImage.
| ObservedGeneration: originalGeneration, | ||
| }, | ||
| LastBuildRef: "image-name-build-1", | ||
| LastBuiltImage: "some/image@some-sha", |
There was a problem hiding this comment.
Could we provide slightly more realistic example by including the "sha256"?
- Move LastImage reconciliation into reconcileBuild
Signed-off-by: Ashwin Venkatesh <avenkatesh@pivotal.io>
|
Updated the field names to LatestBuildRef and LatestImage |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| latestImage := im.Status.LatestImage | ||
| if latestBuild.IsSuccess() { |
There was a problem hiding this comment.
Do you have a strong opinion on this?
Super subtle but, I find the imperative style a bit less reducible/more noisy.
thing = val
if
thing= something-else
end
Perhaps delegatable to?
image.currentLatestForImage(lastBuild)
Signed-off-by: Ashwin Venkatesh <avenkatesh@pivotal.io>
closes #9
Signed-off-by: Joao Pereira jdealmeidapereira@pivotal.io