(feat)(cr) define the chaos custom resource specifications #3

ksatchit · 2019-04-23T16:08:25Z

Consists of custom resource spec updates into the placeholders provided by operator-sdk

Signed-off-by: ksatchit karthik.s@openebs.io

Signed-off-by: ksatchit <karthik.s@openebs.io>

…eed upon Signed-off-by: ksatchit <karthik.s@openebs.io>

AmitKumarDas

@ksatchit I have provided some comments here.

AmitKumarDas · 2019-04-24T03:41:29Z

deploy/crds/litmuschaos_v1alpha1_chaosengine_cr.yaml

+---
+## This is the chaos engine profile requested by dev for the nginx 
+## app, i.e., the user facing custom resource. Mapped to a dedicated
+## contrller which triggers the actual chaos experiments as per 


Will correct

AmitKumarDas · 2019-04-24T03:43:37Z

deploy/crds/litmuschaos_v1alpha1_chaosengine_cr.yaml

@@ -1,7 +1,101 @@
+---


What are your thoughts on below folder structure?

// deploy/nginx/chaosengine.yaml

Agree that chaos engine is the app-mapping chaos resource. However, the general spec of the engine doesn't contain any app-specific elements. One potential reference to nginx/specific-app may come if we include experiments belonging to an "nginx" chart - like below - but that is an optional inclusion. By having this folder structure - the repo will contain several "similar" yamls w/ minimum changes.

Instead should we leave it to developer to just take the "reference" engine we have in deploy/crds/litmuschaos_v1alpha1_chaosengine_cr.yaml and make his changes.

Would like to change name to chaosengine.yaml over the current name, though.

ok... we can avoid the org & version value in the name at-least

AmitKumarDas · 2019-04-24T03:45:07Z

deploy/crds/litmuschaos_v1alpha1_chaosexperiment_cr.yaml

@@ -1,7 +1,43 @@
-apiVersion: litmuschaos.io/v1alpha1
+---
+## An experiment is the definition of a chaos test and is listed as an item 


We might want to explain via charts. In other words can the folder structure be as follows?

// deploy/nginx/charts/0.1.0/chaosexperiment.yaml

Based on this above discussion (on path/naming conventions) - thinking more of // deploy/charts/nginx/0.1.0/chaosexperiment.yaml (so that there can be deploy/charts/kubernetes/0.2.0/chaosexperiment.yaml and others as well) ... However, if we are thinking of a separate chart hub etc,. maybe this structure should go there and this repo should contain the standard template only?

ok..
In future we have to think of validating the yamls pushed into hub via some means, e.g travis or ci etc.

We shall keep this structuring in the chart hub - when it is defined.

AmitKumarDas · 2019-04-24T03:50:34Z

deploy/crds/litmuschaos_v1alpha1_chaostemplate_cr.yaml

+## resource. Consists of default values for chaos-specific params
+## which are expected to be overridden from the chaosExperiment  
+
+apiVersion: litmus.io/v1alpha1


Do we want to call this as ChaosGraph or ChaosType ?

Ok with both @AmitKumarDas .. I thought graph conveys some low-level descriptor (actual action). But it was also chosen as nothing better was striking my head :(

Also have a reference there to litmus.io over litmuschaos.io - will change

AmitKumarDas · 2019-04-24T03:59:11Z

deploy/crds/litmuschaos_v1alpha1_chaosexperiment_cr.yaml

-  size: 3
+
+  chart: 
+    - name: kubernetes


I am a bit confused here.
What are we trying to convey by putting chart as kubernetes & its chart version?
Does this relate to ChaosTemplate/ChaosType/ChaosGraph?

Do we want to convey below?

kind: ChaosExperiment metadata: name: disappearing-pods namespace: nginx labels: chart/type: nginx chart/version: 0.1.0 spec: chaosGraph: type: kubernetes name: pod-delete

kind: ChaosGraph metadata: name: pod-delete namespace: nginx labels: chart/type: kubernetes chart/version: 2.0 spec:

Yes, above fits my thought process as well. Experiments, if belonging/pulled by a chart -> the chaosGraph/Type also gets pulled as part of it in an implict way - because of the 1-1 mapping between them.

It should be OK for the experiment to reuse an "older" or "newer" template, i.e., w/ a different chart version - as a backward/forward compatibility feature. This is feasible as the executor is embedded into the chaosTemplate itself!

AmitKumarDas · 2019-04-24T04:00:07Z

deploy/crds/litmuschaos_v1alpha1_chaostemplate_cr.yaml

-  size: 3
+  definition:
+    labels: 
+      name: simple-pod-failure


IMO name has to be very specific.
Pod failure can be done via various ways, e.g.:

pod delete

pod oom

pod's image not found

etc.

Ok. will update

AmitKumarDas · 2019-04-24T04:02:18Z

deploy/crds/litmuschaos_v1alpha1_chaostemplate_cr.yaml

+    env:
+      - name: ANSIBLE_STDOUT_CALLBACK
+        value: default
+


We need not have extra spaces unless & until its needed

AmitKumarDas · 2019-04-24T04:04:43Z

deploy/crds/litmuschaos_v1alpha1_chaosexperiment_cr.yaml

-  name: example-chaosexperiment
+
+  ## Eventually launched chaos litmusbook/job will bear <name>-<hash>
+  name: disappearing-pods


IMO this should convey what it will try to do. In other words, it should convey the exact chaos experiment that will be done.

Can ChaosExperiment & ChaosTemplate/ChaosGraph have same name?
If above is a good thing to do, then we can remove the entire mapping to ChaosTemplate/ChaosGraph from within ChaosExperiment entirely.

AFAIK ChaosExperiment will do only one specific task. ChaosGraph has a direct relation with underlying infrastructure to execute this specific task. e.g. spawn a k8s job that deletes a pod.

We might need to rethink if infrastructure should be abstracted as well. e.g. killing a pod via a k8s job might be different from killing a pod via some api service or other chaos tooling.

Agree on name changes, will do that.

Very nice! Will think on addition to chaosGraph spec to package-in the executor nature (via traditional k8s litmus job, some other api - maybe directly reuse chaostoolkit api for example etc.,)

The experiment-graph/template was originally conceived in order to keep the spec of a "test"/"chaos experiment" homogeneous and common irrespective of what it does underneath. This interface would then reference low-level specs which can change greatly on a case-by-case basis. Now there are two developments which make us rethink this approach:

a/ Emergence on chaosEngine which is now the app/user-facing resource - which will be homogeneous.

b/ Introduction of spec.definition in the chaosGraph which packages all heterogeneous chaos parameters as ENV - which satisfies in a way the strict-definition/uniformity requirements of chaosGraph.

Now, even w/ (a) & (b), are there are some situations which necessitates two separate CRs ? I can think of this future case: An experiment might be the result of a (chaosjob-1) + (chaosjob-2) + (chaosjob-3) executed in that order to simulate a "multi-component" failure or an "issue-build-up" ? It can be a developer discretion also, to define it that way (This requirement varies slightly from the engine listing <exp1, exp2, exp3 by rank> - this is more of a batch run of chaos like diagnostics... the case we are discussing is the experiment-itself which is a case of more than 1 chaos event/action.) This thinking violates the current approach of 1-1 mapping, and may need a corresponding change to the spec - but is an important consideration nonetheless.

Current decision on the 3rd point based on discussions with @AmitKumarDas :

Multi-chaos experiments is definitely a possibility - but we currently feel these are best handled by the executors. Exposing programming constructs into the YAMLs may not be wise at this point.

However, we will still go ahead with both the CRs to cover for cases where a user-defined chaosGraph is added and experiment can refer to it while maintaining the components abstraction. Future ones might come up. Moreover, there is no controller mapped against the resource anyway - it is only to hold data.

We will have same names for chaosExperiment and chaosGraph to enable simple identification. User can specify different on a need basis.

It will be of great value to add a "description" attribute to the experiment spec to hold information about what it does. This will be very useful for developers and help abstract-away lower-level chaos details (also another reason to maintain a separate CR).

Signed-off-by: ksatchit <karthik.s@openebs.io>

AmitKumarDas

/lgtm
/approve
Minor comments @ksatchit on naming conventions

AmitKumarDas · 2019-04-24T06:58:35Z

deploy/crds/litmuschaos_v1alpha1_chaosengine_cr.yaml

@@ -1,7 +1,101 @@
+---


ok... we can avoid the org & version value in the name at-least

AmitKumarDas · 2019-04-24T07:00:32Z

deploy/crds/litmuschaos_v1alpha1_chaosexperiment_cr.yaml

@@ -1,7 +1,43 @@
-apiVersion: litmuschaos.io/v1alpha1
+---
+## An experiment is the definition of a chaos test and is listed as an item 


ok..
In future we have to think of validating the yamls pushed into hub via some means, e.g travis or ci etc.

AmitKumarDas · 2019-04-24T10:18:42Z

deploy/crds/chaosengine.yaml

+
+        schedule: 
+          interval: ""
+          excluded-times: ""


should all the property names follow a certain naming standard?
e.g. why not excludedTimes & excludedDays

AmitKumarDas · 2019-04-24T10:19:12Z

deploy/crds/chaosengine.yaml

+  schedule:
+    # quarter-hourly, half-hourly, hourly, bi-hourly, trihoral, daily
+    interval: "half-hourly"
+    excluded-times: ""


same comment

AmitKumarDas · 2019-04-24T10:19:53Z

deploy/crds/chaosexperiment.yaml

+    chart/version: 0.9
+
+description:
+  data: |


can we rename data to message

Signed-off-by: ksatchit <karthik.s@openebs.io>

(refactor)exporter: support non-default ns & add engine name label to metrics

(feat)(cr) define the chaos custom resource specifications

fc45f80

Signed-off-by: ksatchit <karthik.s@openebs.io>

ksatchit requested review from umamukkara and chandankumar4 April 23, 2019 16:08

(refactor)(cr) fall back to 'chaosTemplate' until other names are agr…

1926f32

…eed upon Signed-off-by: ksatchit <karthik.s@openebs.io>

ksatchit requested a review from AmitKumarDas April 23, 2019 18:11

AmitKumarDas added the area/custom-resource label Apr 24, 2019

AmitKumarDas reviewed Apr 24, 2019

View reviewed changes

(refactor)(cr): enhance the chaos experiment and graph (template) specs

f23ea38

Signed-off-by: ksatchit <karthik.s@openebs.io>

AmitKumarDas approved these changes Apr 24, 2019

View reviewed changes

(refactor)(cr): update spec attribute names to camelCase

7cf54a7

Signed-off-by: ksatchit <karthik.s@openebs.io>

AmitKumarDas merged commit 4a401fe into litmuschaos:master Apr 24, 2019

ksatchit deleted the chaos_spec_updates branch April 24, 2019 12:57

ksatchit self-assigned this Apr 24, 2019

sureshpathipati pushed a commit to sureshpathipati/chaos-operator that referenced this pull request Oct 6, 2019

Merge pull request litmuschaos#3 from ksatchit/master

ed0f021

(refactor)exporter: support non-default ns & add engine name label to metrics

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat)(cr) define the chaos custom resource specifications #3

(feat)(cr) define the chaos custom resource specifications #3

ksatchit commented Apr 23, 2019 •

edited

Loading

AmitKumarDas left a comment

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019 •

edited

Loading

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019

ksatchit Apr 24, 2019

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019

ksatchit Apr 24, 2019

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

ksatchit Apr 24, 2019

ksatchit Apr 24, 2019

AmitKumarDas left a comment

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

AmitKumarDas Apr 24, 2019

(feat)(cr) define the chaos custom resource specifications #3

(feat)(cr) define the chaos custom resource specifications #3

Conversation

ksatchit commented Apr 23, 2019 • edited Loading

AmitKumarDas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksatchit Apr 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmitKumarDas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksatchit commented Apr 23, 2019 •

edited

Loading

ksatchit Apr 24, 2019 •

edited

Loading