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

feat: apply multi type of chaos nodes with code generation #1621

Merged
merged 15 commits into from
Apr 7, 2021

Conversation

STRRL
Copy link
Member

@STRRL STRRL commented Mar 30, 2021

Signed-off-by: STRRL str_ruiling@outlook.com

What problem does this PR solve?

Make workflow supports all the type of chaos

What is changed and how does it work?

  • new method which mapping templateType and the CRD Kind, reuse the chaosKindMap
  • spawn the chaos object from EmbedChaos by templateType
  • tests on dependency of chaosKindMap
  • appending the example workflow with stress-chaos

Checklist

Tests

  • Unit test
  • E2E test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Breaking backward compatibility

Related changes

  • Need to update the documentation

Does this PR introduce a user-facing change?

NONE

@STRRL
Copy link
Member Author

STRRL commented Mar 30, 2021

/hold

Signed-off-by: STRRL <str_ruiling@outlook.com>
@STRRL STRRL force-pushed the workflow-more-type-of-chaos-nodes branch from c05f549 to b5a8ad9 Compare March 30, 2021 09:50
Signed-off-by: STRRL <str_ruiling@outlook.com>
@codecov-io
Copy link

codecov-io commented Mar 31, 2021

Codecov Report

Merging #1621 (8af5b58) into master (7e9ff3f) will decrease coverage by 5.34%.
The diff coverage is 55.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1621      +/-   ##
==========================================
- Coverage   55.78%   50.43%   -5.35%     
==========================================
  Files          68       90      +22     
  Lines        4383     5601    +1218     
==========================================
+ Hits         2445     2825     +380     
- Misses       1768     2483     +715     
- Partials      170      293     +123     
Impacted Files Coverage Δ
api/v1alpha1/common_types.go 0.00% <0.00%> (ø)
api/v1alpha1/common_webhook.go 100.00% <ø> (ø)
api/v1alpha1/dnschaos_type.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/httpchaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/httpchaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/iochaos_types.go 0.00% <ø> (-40.00%) ⬇️
api/v1alpha1/jvmchaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/kernelchaos_types.go 0.00% <ø> (-20.00%) ⬇️
api/v1alpha1/kernelchaos_webhook.go 94.11% <0.00%> (+8.93%) ⬆️
... and 146 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8968d9...8af5b58. Read the comment docs.

Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
@STRRL STRRL force-pushed the workflow-more-type-of-chaos-nodes branch from 3055934 to 6fe8a43 Compare March 31, 2021 09:03
@STRRL
Copy link
Member Author

STRRL commented Mar 31, 2021

/run-e2e-tests

@STRRL STRRL marked this pull request as ready for review March 31, 2021 09:07
@STRRL
Copy link
Member Author

STRRL commented Mar 31, 2021

/hold cancel

@STRRL
Copy link
Member Author

STRRL commented Mar 31, 2021

I think this PR is ready for review. 😁
/cc @chaos-mesh/committers
/cc @chaos-mesh/maintainers

@ti-chi-bot ti-chi-bot requested review from a team March 31, 2021 09:09
Copy link
Contributor

@Colstuwjx Colstuwjx left a comment

Choose a reason for hiding this comment

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

rest LGTM.

helm/chaos-mesh/values.yaml Outdated Show resolved Hide resolved
@@ -51,51 +49,32 @@ func (it *ChaosNodeReconciler) Reconcile(request reconcile.Request) (reconcile.R
return reconcile.Result{}, client.IgnoreNotFound(err)
}

if !v1alpha1.IsChoasTemplateType(node.Spec.Type) {
return reconcile.Result{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we return an error here?

Copy link
Member Author

@STRRL STRRL Mar 31, 2021

Choose a reason for hiding this comment

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

No, this reconciler would receive events about all workflownode, even type is not Type<Some>Chaos, and the v1alpha1.IsChoasTemplateType() plays as a filter.

Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Copy link
Contributor

@Colstuwjx Colstuwjx left a comment

Choose a reason for hiding this comment

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

LGTM

@AsterNighT
Copy link
Collaborator

/lgtm

@STRRL
Copy link
Member Author

STRRL commented Apr 1, 2021

Hi, as the GcpChaos has been merged #1526, this PR updates the definition of gcp chaos, so it need review again.

/cc @chaos-mesh/committers

@ti-chi-bot ti-chi-bot requested a review from a team April 1, 2021 09:37
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
@STRRL
Copy link
Member Author

STRRL commented Apr 2, 2021

/run-e2e-tests

@STRRL
Copy link
Member Author

STRRL commented Apr 2, 2021

/lgtm cancel

@STRRL
Copy link
Member Author

STRRL commented Apr 2, 2021

/run-e2e-tests

@STRRL
Copy link
Member Author

STRRL commented Apr 5, 2021

/run-e2e-tests

Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@cwen0 cwen0 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • YangKeao
  • cwen0

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@STRRL
Copy link
Member Author

STRRL commented Apr 7, 2021

PTAL @AsterNighT @Colstuwjx

@cwen0
Copy link
Member

cwen0 commented Apr 7, 2021

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 8af5b58

@Colstuwjx
Copy link
Contributor

/lgtm

@STRRL
Copy link
Member Author

STRRL commented Apr 7, 2021

/run-e2e-tests

@ti-chi-bot
Copy link
Member

@STRRL: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@ti-chi-bot ti-chi-bot merged commit 936d764 into chaos-mesh:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants