-
Notifications
You must be signed in to change notification settings - Fork 44
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
[WIP] Fix tekton results storage configurations #686
base: main
Are you sure you want to change the base?
[WIP] Fix tekton results storage configurations #686
Conversation
sayan-biswas
commented
Jun 16, 2023
•
edited
Loading
edited
- Add minio user creation through tenant, and disable TLS
- Add minio and postgres as multiple sources in pipeline storage argocd application
- Increase timeouts
- Fix reset script
- Add pipeline service monitoring application to reset script
350f50a
to
43e7680
Compare
@sayan-biswas the upgrade test failure and at first blush does not seem like a flake:
as it is attempting to patch something in the tekton-results namespace. That said, I don't think it is one of the directory subtrees you directly impacted. Perhaps there is a relationship between what you changed and this path that failed? If you analyzed this already and reported, apologies, I missed the analysis. |
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.
/lgtm
@sayan-biswas PR needs a rebase. |
@sayan-biswas What's the status on this PR. It seems to be stale. |
@Roming22 I will rebase with the latest changes. |
PR is stale and not being actively worked on. Adding a |
43e7680
to
a83c777
Compare
8205baa
to
66bccc6
Compare
Updated PR adjusting the new changes |
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.
The changes look OK best to my understanding @sayan-biswas
Given the complexity / amount of change, attempting to bootstrap from infra-deployments where you point pipeline-service to your branch/commit for feels worthwhile, so that we "pre-validate" how this will look there before we couple this commit with our other commits.
Have you happened to do that already? If not, could you do it either before your EOB today or tomorrow, and then I'll merge once you say the infra-deployments bootstrap is setting up everything correctly as well. Just want to make sure nothing minio/postgresql related has leaked into infra-deployments that would conflict with this.
thanks
Sounds good. I'll do that today. |
66bccc6
to
68805d0
Compare
Were you able to verify @sayan-biswas ? If so, the PR is green and I can hit the merge button if everything checked out in infra-deployments. |
bump @sayan-biswas - were you able to do that infra-deployments based verification ? |
Yes, I created a PR in infra-deployment and argoCD is failing to sync. I am debugging that. |
@sayan-biswas Since you are still working on making sure this PR can be safely merged, I'll add Remove it when the PR can be merged. |
@sayan-biswas Not a priority, but make sure this PR either go somewhere or is closed. |