-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(RELEASE-1053): add support for final pipeline run #572
Conversation
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks quite good for a draft. I didn't review too closely as it is too hard to tell what your changes are since mine are also being shown until my PR is merged. One thing I will say though is that the PostActions thing is the finalPipeline. So anywhere you see PostActionsExecuted in like release_types.go or something, you can remove it
d96f9d7
to
2dc3602
Compare
2dc3602
to
b871db5
Compare
b871db5
to
69600dd
Compare
69600dd
to
cfbbbd4
Compare
b332950
to
8e977a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't look at the release_types_test.go or adapter_test.go files until we get the code nailed down
937ef87
to
aa74b51
Compare
1a653e7
to
175eb87
Compare
3126228
to
bf71b0d
Compare
It all working now |
/retest |
cb7ff97
to
1cf9ba4
Compare
30db648
to
4d02b66
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #572 +/- ##
==========================================
+ Coverage 84.09% 84.13% +0.04%
==========================================
Files 26 26
Lines 1635 1740 +105
==========================================
+ Hits 1375 1464 +89
- Misses 182 192 +10
- Partials 78 84 +6 ☔ View full report in Codecov by Sentry. |
I am only missing a Pipeline specific test, which is that the final Pipeline should not be passed alone. In case it is, it should fail. Besides that, I see that the Tenant/Managed/Final pipeline testing might need improvement but this can be done later. |
a214d5c
to
bf7a787
Compare
Ok, so I have added the test in : It("should return false if only the Final Pipeline is set", func() {
adapter.ctx = toolkit.GetMockedContext(ctx, []toolkit.MockData{
{
ContextKey: loader.ReleasePlanContextKey,
Resource: &v1alpha1.ReleasePlan{
ObjectMeta: metav1.ObjectMeta{
Name: "release-plan",
Namespace: "default",
},
Spec: v1alpha1.ReleasePlanSpec{
Application: application.Name,
ReleaseGracePeriodDays: 6,
FinalPipeline: parameterizedPipeline,
},
},
},
})
result := adapter.validatePipelineDefined()
Expect(result.Valid).To(BeFalse())
Expect(result.Err).NotTo(HaveOccurred())
}) |
bf7a787
to
87337e0
Compare
@seanconroy2021 can you rebase this so we can merge it? |
Introduce a new final pipeline that runs immediately after all other pipelines. The final pipeline receives the same resources as the tenant pipeline. Signed-off-by: Sean Conroy <[email protected]>
87337e0
to
9fd591c
Compare
New changes are detected. LGTM label has been removed. |
@johnbieren Are You happy to merge after the rebase, then ... |
Yes, we have David's approval, and the only things that seem to have changed since mine are little nit picks. So let's merge it once it passes e2e. We'd like to have it in staging by a meeting tomorrow with UI |
/retest |
Added support for the final pipeline to run immediately after all other pipelines in the tenant workspace.
The final pipeline now receives references to the same resources as the tenant pipeline, including the Release resource.
A release is marked as failed if the final pipeline fails.
Jira: Release-1053