reimplement the twophase controller in chaos mesh

[YangKeao]

It has been one year since Chaos Mesh got open sourced. The controller-manager of Chaos Mesh keeps growing and turning into a huge and hard to manage project 😢 . The twophase controller of it epitomises the difficulty, which I have submitted a lot of PRs to try to make it simpler.

In this discussion, I will do a self-criticism about the bad design —— duration and scheduler of current controller and CRDs and will also explain the historical reason of the bad design. Several choices to overcome these issues will be provided as references (with or without breaking the compatibility).

Under this discussion, **I hope we can decide one acceptable plan to make this thing better. And the implementation of the choice should arrive as soon as possible. **

The duration and scheduler fields in every CRDs is really a bad design. It combines the thing of scheduler with "chaos implementation". With these fields in the CRD, we have to manage much more status in one CRD. What's more, a lot of "high-level" features are brought to them later: like pause and dynamic scheduler.

Now, nearly every new features on the controller (e.g. "injecting chaos to newly created chaos", "dynamically change configuration") are blocked by the complexity of twophase controller. We (at least I) didn't have enough confidence to write a bug-free implementation without breaking twophase controller.

Though I have tried to make twophase controller easier, it has been proved this attempt failed finally 😿. It doesn't make things better enough.

historical reason

The first CRD of Chaos Mesh is PodChaos, and the first chaos of Chaos Mesh is pod-kill, which is a "job" but not a "status",. Only with duration and scheduler, which means the pods are killed periodically, it becomes a status. Then all of the Chaos followed it, without thinking whether it's really a good design.

Solution

All these solutions will make the CRD more complicate. Just like Deployment -> ReplicaSet -> Pod, spliting the functions into multiple standalone resource shouldn't be feared, and cloud be a good practice. The only concern is about how to describe the phenomenon to the users.

1. Wait for CronWorkflow and remove these two fields from the CRD
   Pros:
   • The design could be much simpler, and no extra work need to do.
     Cons:
   • Break compatibility
   • It will take several months before it arrives 😢 (Though I'm willing to help and make it arrive earlier 😋 )
2. Treat CRD with duration and scheduler like a standalone resource, which will create common chaos to fulfill its implementation. Cronjob (twophase scheduler) through standalone resource #1481
   Pros:
   • The code base is relatively small, and easy to understand
   • It could be a practice to manager multi-level custom resources, which us developers should be able to do.
     Cons:
   • Bring confusing user experience: the users will find a lot of XXXChaos created by the machine.
   • Didn't eliminate the multi-meaning of XXXChaos
3. Create a new unstructured CRD RawChaos, and the twophase controller implements through creating and monitoring the RawChaos.
   Pros:
   • A one-layer RawChaos CRD will bring us a unified interface of chaos, which could be really helpful for plugin
   • A newly created CRD will not break the compatibility 😋
     Cons:
   • The function of RawChaos and common chaos are nearly the same.

@Yiyiyimu @YangKeao @STRRL @Hexilee You have all contributed to the twophase controller or talked about the design of it with me before. Do you have any suggestions on this problem?

[Colstuwjx]

I have participated in the discussion of "update scheduler", see discussion 1491 #comment, and I'd like to give some suggestions from my view:

1. What's the difference between CronWorkflow and Workflow with Cron scheduler? If they are the same thing, I suggest that we make a separate component called Chaos Scheduler, and it cares about the one-time job or cron-like job scheduling, similar to your plan 2 ;

2. In general, our chaos didn't equal a job, if we said ran a job, the job should have a status that indicates it succeeds or failed. But Chaos didn't. We preset each chaos would keep running if it's not paused, and it doesn't have an exit gate. We may need to think that how to define a Chaos Job, its primitive and when to exit, maybe it could combine with your plan 3 RawChaos, or sth called MetaChaos;

3. For the dev roadmap, I suggest that we prepare to land this feature in version 2.0, and deprecate the old version at a scheduled date ( Prometheus project refactored its storage engine from 1.0 to 2.0, and it finally merged into the mainstream in this PR, I think it should be a good example for a breaking change ).

Thanks.

[YangKeao]

For the first question:

@STRRL I don't think there will be a Workflow with Cron scheduler. Every Workflow will only be executed once. The CronWorkflow is used to create Workflow periodically. The argo also split these two features into Workflow and CronWorkflow. I think it's a good practice (as combining scheduler with other CRD has been proved to be a bad idea 👿 )

For the second question:

👍 Yes. This is an essential problem 😢. Some chaos are more like a "status", which infers that the cluster is / (should be) wrong, e.g. the network connection is blocked, the disk is degraded... And I believe these "status" matches the philosophy of Kubernetes, just like ReplicaSet describes the status that there is / (should be) 5 pods in the cluster.

However, there is another type of Chaos, like pod-kill, which is not a "status" but more like a job 😿

[STRRL]

CronWorkflow is much simpler than Workflow with Cron; CronWorkflow only creates each new Workflow and leaves it runs alone.

Now we always use cron and duration as one bound bundle, so the controller should keep tracking the status of the workload, it's what the two-phase controller does.

[STRRL]

I want to break the compatibility and make our definition simple. I think we should not fear breaking backward compatibility.

Create new CronIoChaos, CronPodChaos, etc. Move the fields: duration and scheduler to the new CronXXXChaos(remove them from current CRD), then we could apply the two-phase controller on it and reuse the common controller.

[STRRL]

So each state chaos like NetworkChaos could use duration, and the others like AwsChaos CAN NOT, right?

Yes!

If chaosPeriod is required while running a NetworkChaos such state chaos, it seems a little complicated to the newbies.

ChaosPeriod is not required for running a NetworkChaos. Use could apply one NetworkChaos for ad-hoc usage.

If a user only applies a NetworkChaos, Chaos Mesh will affect the network chaos right now, and recover until set Phase to Done or remove it.

If a user applies a NetworkChaos wrapped by ChaosPeriod, Chaos Mesh will affect chaos right now, and recover after duration. It's also suitable for ad-hoc usage.

If a user applies a NetworkChaos wrapped by Schedule, its behavior is mostly like the current two-phase controller.

[Colstuwjx]

@STRRL Awesome! LGTM!

[YangKeao]

@STRRL This configuration is intuitive for the Chaos Mesh developers, but it's hard to understand and use for users.

If a user applies a NetworkChaos wrapped by Schedule, its behavior is mostly like the current two-phase controller.

? Do you mean a NetworkChaos wrapped by ChaosPeriod wrapped by Schedule?

[STRRL]

? Do you mean a NetworkChaos wrapped by ChaosPeriod wrapped by Schedule?

Yes

[STRRL]

This configuration is intuitive for the Chaos Mesh developers, but it's hard to understand and use for users.

Maybe we could keep the current Kubernetes CRDs. Then migrate it to Schedule/ChaosPeriod/MetaChaos, and apply the common controller on MetaChaos.

[YangKeao]

@STRRL

I have another proposal: Keep the duration field in the XXXChaos, and setup another controller to track this field. I have came up an idea (actually, learn from kubernetes) to solve both this problem and some problems of selector.
As we have agreed that XXXChaos should be able to live even after Recover, we should unbind the Recover and delete:

Unbind Recover and Delete

The Chaos should have two desired phase: one is Running and the other is Finished, they should be recorded in the Status. The Running doesn't mean all chaos injection applied. Instead, it could be partially applied (and the rest of them failed). The status of injected Pod or Container should be recorded in the Status to manage the error report and retry (but it's another issue to detail the implementation). After created, the desired phase is set to Running by default.

Use complicated Status (which will record every injected Pod or Container) but not a single field to represent the "current phase" would also be helpful to extend the selector later.

duration controller and delete controller

We can setup another two controller to handle the duration field and the delete behavior from user. The implementation would be really simple:

The duration controller will set desired phase to Finished when it arrives createTime + duration.
The delete controller will set the desired status into Finished and check the Status to see whether all Pod/ containers have been recovered. If not, do nothing, if they were, remove the finalizer and let it be deleted.

Schedule CRD

Then the Schedule could be a really simple CRD like CronJob. We can also add a historyLimit field to keep some XXXChaos even after the desired of status of them are finished and the Status of them seem to be clean enough.

pause

A Schedule and the XXXChaos could all be paused. After paused, the Schedule will not start new XXXChaos and all started XXXChaos will also be paused. When a XXXChaos is paused, the desired status of it will be set to Finished. We can reference the implementation of spec.suspend field in CronJob

[YangKeao]

The idea of seperated controller for duration and delete came from the TTL Controller of kubernetes, which will delete a finished job after a period of time.

The idea of tracking every pod/container injection status in the Status came from ReplicaSet. The only difference is that the ReplicaSet will create Pod as an entity, but XXXChaos doesn't have any entity. The "every" should be defined carefully especially considering the "target" selector in NetworkChaos. We could discuss about it in another issue.

[YangKeao]

Here is a road map to implement this feature:

1. (1-2 week) Modify current two phase controller to fit the "desired phase" and status design. Actually two phase can also be abstracted into a single controller with modifying "desired phase". It could also simplify nearly every chaos implementation by abstracting selector (Fix Encapsulate multi-pod chaos #961 in a better way, for not only Recover, but also Apply)
2. (1 week) Add a duration only controller with a standalone controller mentioned in the design.
3. (1 week) Add a Schedule CRD and deprecate scheduler field.

Only the third step will break compatibility. I believe it will take more time to review than coding 😋 , so I would continue to develop in different branch and submit multiple PRs to avoid blocking my development schedule.

[YangKeao]

Any modification and more problem will be filed in this discussion.

[AsterNighT]

That is intuitive to me. In fact I'm about to come to a similar idea when I ran into this reply 🍻.
I've got another question.

A Schedule and the XXXChaos could all be paused. After paused, the Schedule will not start new XXXChaos and all started XXXChaos will also be paused. When a XXXChaos is paused, the desired status of it will be set to Finished. We can reference the implementation of spec.suspend field in CronJob

I can see it is necessary to pause a Schedule, but do we really need pause for a regular XXXChaos created by some Schedule ? I mean, what if we simply delete the job and create a new one when we resume it? Would that simplify the designing further?