Skip to content
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

introduce pipeline #17

Merged
merged 31 commits into from
Sep 9, 2021
Merged

introduce pipeline #17

merged 31 commits into from
Sep 9, 2021

Conversation

squeedee
Copy link
Member

@squeedee squeedee commented Sep 1, 2021

Introduce pipeline.

closes #6

Currently blocked on: #16

@squeedee squeedee force-pushed the 6-introduce-pipeline branch 3 times, most recently from 158f2f8 to be1542f Compare September 3, 2021 16:35
Rasheed Abdul-Aziz added 3 commits September 3, 2021 13:25
@squeedee squeedee force-pushed the 6-introduce-pipeline branch from a621a39 to 7332bf8 Compare September 3, 2021 17:31
Rasheed Abdul-Aziz and others added 3 commits September 7, 2021 11:06
[#6]

Co-authored-by: Emily Johnson <emjohnson@vmware.com>
[#6]

Co-authored-by: Emily Johnson <emjohnson@vmware.com>
- must use a namespace
- cannot create-or-update, must 'create'

[#6]

Co-authored-by: Emily Johnson <emjohnson@vmware.com>
@squeedee squeedee force-pushed the 6-introduce-pipeline branch from 1886c35 to b915a98 Compare September 7, 2021 15:08
@emmjohnson emmjohnson marked this pull request as ready for review September 8, 2021 18:52
Signed-off-by: Ciro S. Costa <ciroscosta@vmware.com>
@cirocosta
Copy link
Contributor

have you been able to consistently run the integration tests? I quite often hit this snag

Summarizing 1 Failure:

[Fail] Stamping a resource on Pipeline Creation a RunTemplate that produces a Resource a Pipeline that matches the RunTemplateRef [It] Stamps a new Resource
/home/cirocosta/dev/vmware-tanzu/cartographer/tests/integration/pipeline_service/pipeline_service_test.go:107

Ran 2 of 2 Specs in 10.880 seconds
FAIL! -- 1 Passed | 1 Failed | 0 Pending | 0 Skipped
--- FAIL: TestWebhookIntegration (10.88s)
FAIL

seems like there's some big flakiness there

Copy link
Contributor

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few comments, mostly nitpicking, but if there's one thing I'd take a deeper look into is the requirement of kind and namespace in the runTemplateRef - I think kind should not exist and namespace should be optional defaulting to default

let me know what you think 😁 good stuff! thx!

waciumawanjohi and others added 3 commits September 9, 2021 11:34
[#6]

Co-authored-by: Waciuma Wanjohi <lwanjohi@pivotal.io>
Co-authored-by: Emily Johnson <emjohnson@vmware.com>
[#6]

Co-authored-by: Waciuma Wanjohi <lwanjohi@pivotal.io>
Co-authored-by: Emily Johnson <emjohnson@vmware.com>
[#6]

Co-authored-by: Waciuma Wanjohi <lwanjohi@pivotal.io>
Co-authored-by: Emily Johnson <emjohnson@vmware.com>
@emmjohnson
Copy link
Contributor

emmjohnson commented Sep 9, 2021

have you been able to consistently run the integration tests? I quite often hit this snag

Summarizing 1 Failure:

[Fail] Stamping a resource on Pipeline Creation a RunTemplate that produces a Resource a Pipeline that matches the RunTemplateRef [It] Stamps a new Resource
/home/cirocosta/dev/vmware-tanzu/cartographer/tests/integration/pipeline_service/pipeline_service_test.go:107

Ran 2 of 2 Specs in 10.880 seconds
FAIL! -- 1 Passed | 1 Failed | 0 Pending | 0 Skipped
--- FAIL: TestWebhookIntegration (10.88s)
FAIL

seems like there's some big flakiness there

So this is indeed not flakey and just a real failure. The update status call is requeueing the pipeline for reconcillation. This is in turn creating another stamped resource for the runtemplate. This is probably an infinite cycle. We will address this in very next PR. 😔

Making the test "pass" for now.

@cirocosta
Copy link
Contributor

cirocosta commented Sep 9, 2021

thx for taking the comments into consideration!

current check failure:

Screen Shot 2021-09-09 at 12 05 24 PM

blaaaah 😅


emmjohnson [...] This is probably an infinite cycle. We will address this in very next PR. 😔

no worries, sgtm 😁

@emmjohnson emmjohnson merged commit 6bf6d3f into main Sep 9, 2021
@emmjohnson emmjohnson deleted the 6-introduce-pipeline branch September 9, 2021 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce Pipeline
4 participants