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.

Sign up for GitHub

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

Jump to bottom

ADR to enable chaos testing of apps #171

Merged
merged 1 commit into from
Jun 10, 2024
Merged

ADR to enable chaos testing of apps #171

merged 1 commit into from
Jun 10, 2024

Conversation

yogananth-subramanian
Copy link
Contributor

The objective of this ADR is to propose a way to enable the end user to run chaos tests for his app as part of integration testing.

@yogananth-subramanian yogananth-subramanian requested a review from a team as a code owner March 5, 2024 17:41
ralphbean
ralphbean requested changes Mar 9, 2024
View reviewed changes
Copy link
Member

@ralphbean ralphbean left a comment

Choose a reason for hiding this comment

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

The context around the usefulness of chaos testing here is good, but kind of general. I'm not sure how this applies to konflux concretely. Can you refactor the ADR to be more specific about the decision and the impacts you think that it will have on konflux and the konflux development process?

@yogananth-subramanian
Copy link
Contributor Author

cc @psuriset @chaitanyaenr

@yogananth-subramanian
Copy link
Contributor Author

The context around the usefulness of chaos testing here is good, but kind of general. I'm not sure how this applies to konflux concretely. Can you refactor the ADR to be more specific about the decision and the impacts you think that it will have on konflux and the konflux development process?

Hello @ralphbean
I apologize for being unclear. The purpose of the ADR is to allow Konflux users to run chaos tests as part of IntegrationTestScenarios.. As part of the ADR, we would like to propose that the service account created as part of integration-service be used to access thanos-querier-openshift-monitoring or prometheus-k8s-openshift-monitoring endpoints. The change it would cause would be that the service account would now be able to run promQL queries.
Let me change the ADR and update the proposal.

@ralphbean
Copy link
Member

ralphbean commented Mar 28, 2024 •
edited
Loading

@amisstea, @dirgim can I request your review on this? It is related to both integration service and #172

@ralphbean ralphbean requested review from amisstea and dirgim March 28, 2024 18:56
@ralphbean ralphbean added the kind/feat Something new label Apr 30, 2024
@yogananth-subramanian yogananth-subramanian force-pushed the chaos-test branch 2 times, most recently from 0ef410b to 4b86e87 Compare May 16, 2024 20:07
@yogananth-subramanian
Copy link
Contributor Author

Hello @ralphbean @amisstea @dirgim I have revised the ADR based on the discussion during the arch call to incorporate the utilization of ephemeral clusters.
It would be wonderful if you could provide your input on the updated ADR.

amisstea
amisstea reviewed May 21, 2024
View reviewed changes
ADR/0035-apps-continuous-chaos-testing.md Outdated Show resolved Hide resolved
ADR/0035-apps-continuous-chaos-testing.md Outdated Show resolved Hide resolved
@yogananth-subramanian
Copy link
Contributor Author

Hello @amisstea , I have revised the ADR based on your suggestion and have also taken out some technical implementation that might not be relevant for the decision-making.
It would be wonderful if you could provide your input on the updated ADR.

amisstea
amisstea approved these changes Jun 3, 2024
View reviewed changes
Copy link
Contributor

@amisstea amisstea left a comment

Choose a reason for hiding this comment

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

/lgtm

@yogananth-subramanian you may want to update the date and status at the top before merging.

dirgim
dirgim approved these changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@dirgim dirgim left a comment

Choose a reason for hiding this comment

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

LGTM aside the tiny nit

@yogananth-subramanian
ADR to enable chaos testing of apps
d21c561 
The objective of this ADR is to propose a way to enable the end user to run chaos tests for his app as part of integration testing.
@yogananth-subramanian
Copy link
Contributor Author

Hello @amisstea @dirgim @ralphbean I would like to thank you for your patience review and helpful comments.
Have updated the dat to today's date and set the status to Accepted.

@ralphbean ralphbean merged commit e657ff7 into konflux-ci:main Jun 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dirgim dirgim dirgim approved these changes

@chaitanyaenr chaitanyaenr chaitanyaenr left review comments

@yftacherzog yftacherzog yftacherzog left review comments

@amisstea amisstea amisstea approved these changes

@ralphbean ralphbean ralphbean approved these changes

Assignees
No one assigned
Labels
kind/feat Something new
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yogananth-subramanian @ralphbean @yftacherzog @chaitanyaenr @amisstea @dirgim