-
Notifications
You must be signed in to change notification settings - Fork 805
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
Schedule workflow #1861
Schedule workflow #1861
Conversation
* feat: apis for endtime and status Signed-off-by: STRRL <str_ruiling@outlook.com> * feat: refactor workflow entry reconciler, update conditions in stastus Signed-off-by: STRRL <str_ruiling@outlook.com> * feat: update the end time of workflow Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: fix comments and typos Signed-off-by: STRRL <str_ruiling@outlook.com> * test: fix test cases Signed-off-by: STRRL <str_ruiling@outlook.com> * fix: fix the condition on updating end time of workflow Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: AsterNighT <klxjt99@outlook.com>
Signed-off-by: AsterNighT <klxjt99@outlook.com>
Signed-off-by: AsterNighT <klxjt99@outlook.com>
Signed-off-by: AsterNighT <klxjt99@outlook.com>
2b6d8b3
to
f2a0e19
Compare
@@ -183,7 +181,7 @@ func InitClientSet() (*ClientSet, error) { | |||
// GetPods returns pod list and corresponding chaos daemon | |||
func GetPods(ctx context.Context, chaosName string, status v1alpha1.ChaosStatus, selectorSpec v1alpha1.PodSelectorSpec, c client.Client) ([]v1.Pod, []v1.Pod, error) { | |||
// get podName | |||
failedMessage := status.FailedMessage | |||
failedMessage := "" | |||
if failedMessage != "" { |
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.
opt.semgrep.eqeq-is-bad: useless comparison operation failedMessage == failedMessage
or failedMessage != failedMessage
(at-me in a reply with help
or ignore
)
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.
@Muse-Dev ignore
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.
I've recorded this as ignored for this pull request. If you change your mind, just comment @muse-dev unignore
.
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.
Hi @AsterNighT , I try to apply that CRD into my kubernetes clusters (tried on v1.15.12 and v1.20.2), both of them told me:
CustomResourceDefinition.apiextensions.k8s.io "schedules.chaos-mesh.org" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[workflow].type: Required value: must not be empty for specified object fields
also happends on
workflownodes.chaos-mesh.org
,workflows.chaos-mesh.org
Seems the controller-gen does NOT make the correct definition of Workflow in EmbededChaos.
FYI, then I tried to regenerate CRD YAML with controller-gen@v0.4.1 and controller-gen@v0.5.0, only controller-gen@v0.5.0 works.
@@ -1898,6 +1898,7 @@ spec: | |||
description: 'TODO: use a custom type, as `TemplateType` contains other | |||
possible values' | |||
type: string | |||
workflow: {} |
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.
I think this is the reason that makes the CRD invalid.
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.
I separate schedule totally from workflow. It no longer use embeded chaos any more, but generate its own types. Does it work now?
Signed-off-by: AsterNighT <klxjt99@outlook.com>
Oh my, 7162+! What the hell is that? |
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
I am sure we will refactor the definition of EmbedChaos or ScheduleItem in the future.
/lgtm |
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by writing |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 47132da
|
* Simplify implementation Signed-off-by: AsterNighT <klxjt99@outlook.com> * provide jvmchaos Signed-off-by: AsterNighT <klxjt99@outlook.com> * ignore not found Signed-off-by: AsterNighT <klxjt99@outlook.com> * PodIOChaos through reconciler (#1836) * podnetworkchaos through reconciler Signed-off-by: YangKeao <keao.yang@yahoo.com> * manage status machine in common controller Signed-off-by: YangKeao <keao.yang@yahoo.com> * fix subresource observation Signed-off-by: YangKeao <keao.yang@yahoo.com> * make check and remove comments Signed-off-by: YangKeao <keao.yang@yahoo.com> * fix according to review Signed-off-by: YangKeao <keao.yang@yahoo.com> * bootstrap podiochaos Signed-off-by: YangKeao <keao.yang@yahoo.com> * podiochaos implementation through reconciler Signed-off-by: YangKeao <keao.yang@yahoo.com> * fix iochaos owns watch Signed-off-by: YangKeao <keao.yang@yahoo.com> * use simple list to map podxxxchaos to xxxchaos in controller Signed-off-by: YangKeao <keao.yang@yahoo.com> * fix a typo in the comment Signed-off-by: YangKeao <keao.yang@yahoo.com> * Schedule workflow (#1861) * tmp Signed-off-by: AsterNighT <klxjt99@outlook.com> * feat: apis for workflow endtime and status (#1828) * feat: apis for endtime and status Signed-off-by: STRRL <str_ruiling@outlook.com> * feat: refactor workflow entry reconciler, update conditions in stastus Signed-off-by: STRRL <str_ruiling@outlook.com> * feat: update the end time of workflow Signed-off-by: STRRL <str_ruiling@outlook.com> * chore: fix comments and typos Signed-off-by: STRRL <str_ruiling@outlook.com> * test: fix test cases Signed-off-by: STRRL <str_ruiling@outlook.com> * fix: fix the condition on updating end time of workflow Signed-off-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * implement gc for workflow Signed-off-by: AsterNighT <klxjt99@outlook.com> * Apply generate Signed-off-by: AsterNighT <klxjt99@outlook.com> * Update test and yam; Signed-off-by: AsterNighT <klxjt99@outlook.com> * Fix type assertion issues Signed-off-by: AsterNighT <klxjt99@outlook.com> * Add todo Signed-off-by: AsterNighT <klxjt99@outlook.com> * Separate schedule from workflow Signed-off-by: AsterNighT <klxjt99@outlook.com> Co-authored-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * Simplify implementation Signed-off-by: AsterNighT <klxjt99@outlook.com> * provide jvmchaos Signed-off-by: AsterNighT <klxjt99@outlook.com> * ignore not found Signed-off-by: AsterNighT <klxjt99@outlook.com> * fix selector in jvmchaos Signed-off-by: YangKeao <keao.yang@yahoo.com> * update example Signed-off-by: AsterNighT <klxjt99@outlook.com> Co-authored-by: YangKeao <keao.yang@yahoo.com> Co-authored-by: STRRL <str_ruiling@outlook.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
What problem does this PR solve?
What is changed and how does it work?
Checklist
Tests
Side effects
Related changes
Does this PR introduce a user-facing change?