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

workflow task node #1866

Merged
merged 36 commits into from
Jun 16, 2021
Merged

workflow task node #1866

merged 36 commits into from
Jun 16, 2021

Conversation

STRRL
Copy link
Member

@STRRL STRRL commented May 20, 2021

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

What problem does this PR solve?

This PR implements the task node for workflow.

What is changed and how does it work?

  • this PR adds two new modules in /pkg/expr and /pkg/workflow/task
  • expr use github.com/antonmedv/expr for evaluating expression
  • /pkg/workflow/task will create Pod by given spec, and collect information during execution, like stdout and exitcode
  • the rest of task reconcile is very similar to parallel node reconcile, but with conditions

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 added 2 commits May 18, 2021 20:06
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
@codecov-commenter
Copy link

codecov-commenter commented May 20, 2021

Codecov Report

Merging #1866 (2ebfcff) into master (18ad845) will decrease coverage by 5.83%.
The diff coverage is 45.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1866      +/-   ##
==========================================
- Coverage   39.47%   33.63%   -5.84%     
==========================================
  Files          17      121     +104     
  Lines         608     7283    +6675     
==========================================
+ Hits          240     2450    +2210     
- Misses        337     4624    +4287     
- Partials       31      209     +178     
Impacted Files Coverage Δ
api/v1alpha1/awschaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_type.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/gcpchaos_types.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% <0.00%> (-32.56%) ⬇️
api/v1alpha1/jvmchaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/kernelchaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/networkchaos_types.go 0.00% <0.00%> (-14.44%) ⬇️
... and 185 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 ae11cf7...2ebfcff. Read the comment docs.

STRRL added 5 commits May 21, 2021 00:39
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
 compliable issue with kubernetes 1.18, detail:
kubernetes-sigs/controller-tools#480

Signed-off-by: STRRL <str_ruiling@outlook.com>
STRRL added 12 commits May 24, 2021 16:37
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
This reverts commit 8e05196.

Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
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
Copy link
Member Author

STRRL commented Jun 11, 2021

Weird, the integration test fails. I am looking on that.

@AsterNighT
Copy link
Collaborator

Weird, the integration test fails. I am looking on that.

The CustomResourceDefinition "workflownodes.chaos-mesh.org" is invalid: metadata.annotations: Too long: must have at most 262144 bytes

@STRRL
Copy link
Member Author

STRRL commented Jun 11, 2021

Weird, the integration test fails. I am looking on that.

The CustomResourceDefinition "workflownodes.chaos-mesh.org" is invalid: metadata.annotations: Too long: must have at most 262144 bytes

I could not reproduce on my local cluster, both k8s v1.15.x and v1.20.x, and both crd v1beta1 and crd v1. 😢Still finding why.

I think I found the reason, kubectl wants to create an annotation called last-applied-configuration which contains the full content of the CRD. We should use kubectl create instead of kubectl apply

I am going to fix it.

apply will create a huge annotaion which exceed the limit of size of
annotation

Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
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.

Except the name, LGTM.

The fields will be renamed in another PR, right?

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • AsterNighT
  • YangKeao

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 submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@STRRL
Copy link
Member Author

STRRL commented Jun 16, 2021

The fields will be renamed in another PR, right?

YES!

@STRRL
Copy link
Member Author

STRRL commented Jun 16, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 06af0af

@STRRL
Copy link
Member Author

STRRL commented Jun 16, 2021

/run-e2e-tests

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

cwen0 commented Jun 16, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: b0d2c5f

@ti-chi-bot ti-chi-bot merged commit c8be4ad into chaos-mesh:master Jun 16, 2021
aido123 pushed a commit to aido123/chaos-mesh that referenced this pull request Jun 16, 2021
* feat: conditional branches in workflownode

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

* feat: reconciler of task node

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

* feat: setup one container for task node

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

* fix: address the comments by muse-dev

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

* chore: cleanup duplicated codes

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

* fix: bump controller-gen version

 compliable issue with kubernetes 1.18, detail:
kubernetes-sigs/controller-tools#480

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

* chore: bump the version of controller-gen to v0.4.1

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

* fix: restrict kubernetes version with replace

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

* fix: also restrict version for e2e-test

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

* fix: also bump the verison of controller-runtime

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

* fix: fallback to v1beta1

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

* Revert "fix: also bump the verison of controller-runtime"

This reverts commit 8e05196.

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

* fix: fill the conditional branches if nil

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

* fix: permission of custom task about creating pod

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

* feat: use v1 instead of v1beta1 with CRD

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

* fix: NOOP when pods exactly same

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

* refactor: refactor the collectors

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

* feat: finish the rest of task nodes

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

* fix: check before sync nodes in task

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

* chore: make linters happier

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

* chore: make linters happier

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

* Update pkg/workflow/controllers/task_reconciler.go

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

Co-authored-by: AsterNighT <klxjt99@outlook.com>

* fix: use create instead of apply

apply will create a huge annotaion which exceed the limit of size of
annotation

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

* chore: update the comments for workflow template

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

* fix: crd installation in e2e test

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

Co-authored-by: AsterNighT <klxjt99@outlook.com>
Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: root <root@vm3.jisiqhvhuzaupp2x5y5vff1eff.fx.internal.cloudapp.net>
aido123 pushed a commit to aido123/chaos-mesh that referenced this pull request Jun 16, 2021
* feat: conditional branches in workflownode

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

* feat: reconciler of task node

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

* feat: setup one container for task node

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

* fix: address the comments by muse-dev

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

* chore: cleanup duplicated codes

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

* fix: bump controller-gen version

 compliable issue with kubernetes 1.18, detail:
kubernetes-sigs/controller-tools#480

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

* chore: bump the version of controller-gen to v0.4.1

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

* fix: restrict kubernetes version with replace

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

* fix: also restrict version for e2e-test

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

* fix: also bump the verison of controller-runtime

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

* fix: fallback to v1beta1

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

* Revert "fix: also bump the verison of controller-runtime"

This reverts commit 8e05196.

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

* fix: fill the conditional branches if nil

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

* fix: permission of custom task about creating pod

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

* feat: use v1 instead of v1beta1 with CRD

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

* fix: NOOP when pods exactly same

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

* refactor: refactor the collectors

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

* feat: finish the rest of task nodes

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

* fix: check before sync nodes in task

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

* chore: make linters happier

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

* chore: make linters happier

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

* Update pkg/workflow/controllers/task_reconciler.go

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

Co-authored-by: AsterNighT <klxjt99@outlook.com>

* fix: use create instead of apply

apply will create a huge annotaion which exceed the limit of size of
annotation

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

* chore: update the comments for workflow template

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

* fix: crd installation in e2e test

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

Co-authored-by: AsterNighT <klxjt99@outlook.com>
Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: root <root@vm3.jisiqhvhuzaupp2x5y5vff1eff.fx.internal.cloudapp.net>
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

6 participants