feat(pod-delete): Adding pod-delete experiment in go #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Logs of pod-delete job |
Signed-off-by: shubhamchaudhary <shubham.chaudhary@mayadata.io>
chandankumar4
suggested changes
May 12, 2020
There was a problem hiding this comment.
Some structuring need to change a bit, Please try to go through this: https://github.com/litmuschaos/litmus/blob/cfd91c26ca1ca668a40d2bc85fdbc23f576de8df/contribute/developer_guide/STYLE_GUIDE.md and change it accordingly
chandankumar4
suggested changes
May 13, 2020
pkg/utils/types.go
Outdated
Comment on lines
40
to
93
| func (clientSets *ClientSets) GenerateClientSetFromKubeConfig() error { | ||
|
|
||
| config, err := getKubeConfig() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| k8sClientSet, err := GenerateK8sClientSet(config) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| litmusClientSet, err := GenerateLitmusClientSet(config) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| clientSets.KubeClient = k8sClientSet | ||
| clientSets.LitmusClient = litmusClientSet | ||
| return nil | ||
| } | ||
|
|
||
| // getKubeConfig setup the config for access cluster resource | ||
| func getKubeConfig() (*rest.Config, error) { | ||
| kubeconfig := flag.String("kubeconfig", "", "absolute path to the kubeconfig file") | ||
| flag.Parse() | ||
| // Use in-cluster config if kubeconfig path is specified | ||
| if *kubeconfig == "" { | ||
| config, err := rest.InClusterConfig() | ||
| if err != nil { | ||
| return config, err | ||
| } | ||
| } | ||
| config, err := clientcmd.BuildConfigFromFlags("", *kubeconfig) | ||
| if err != nil { | ||
| return config, err | ||
| } | ||
| return config, err | ||
| } | ||
|
|
||
| // GenerateK8sClientSet will generation k8s client | ||
| func GenerateK8sClientSet(config *rest.Config) (*kubernetes.Clientset, error) { | ||
| k8sClientSet, err := kubernetes.NewForConfig(config) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "Unable to generate kubernetes clientSet %s: ", err) | ||
| } | ||
| return k8sClientSet, nil | ||
| } | ||
|
|
||
| // GenerateLitmusClientSet will generate a LitmusClient | ||
| func GenerateLitmusClientSet(config *rest.Config) (*chaosClient.LitmuschaosV1alpha1Client, error) { | ||
| litmusClientSet, err := chaosClient.NewForConfig(config) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "Unable to create LitmusClientSet: %v", err) | ||
| } | ||
| return litmusClientSet, nil | ||
| } |
There was a problem hiding this comment.
Move this function in a new file called: pkg/clientset.go
chandankumar4
suggested changes
May 13, 2020
ksatchit
reviewed
May 13, 2020
ksatchit
reviewed
May 13, 2020
ksatchit
reviewed
May 13, 2020
ksatchit
reviewed
May 13, 2020
ksatchit
reviewed
May 13, 2020
| @@ -0,0 +1,22 @@ | |||
| FROM ubuntu:bionic | |||
There was a problem hiding this comment.
Can use a lighter image as well (post 1.4)
ksatchit
reviewed
May 13, 2020
ksatchit
reviewed
May 13, 2020
ksatchit
reviewed
May 13, 2020
ksatchit
reviewed
May 13, 2020
| ) | ||
|
|
||
| func main() { | ||
| var err error |
There was a problem hiding this comment.
##########################
vars: --
##########################
ksatchit
reviewed
May 13, 2020
| experimentsDetails := utils.ExperimentDetails{} | ||
| clients := utils.ClientSets{} | ||
|
|
||
| //Getting kubeConfig and Generate ClientSets |
There was a problem hiding this comment.
########################
environment: -
########################
ksatchit
reviewed
May 13, 2020
| if err != nil { | ||
| klog.V(0).Infof("Unable to Create the Chaos Result due to %v\n", err) | ||
| failStep = "Updating the chaos result of pod-delete experiment (SOT)" | ||
| utils.UpdateChaosResultMiddle(&experimentsDetails, clients, flag,failStep); return |
ksatchit
reviewed
May 13, 2020
| } | ||
|
|
||
| //PRE-CHAOS AUXILIARY APPLICATION LIVENESS CHECK | ||
| if experimentsDetails.AuxiliaryAppInfo == "no" { |
ksatchit
reviewed
May 13, 2020
| err = litmus.PreparePodDelete(&experimentsDetails, clients,&failStep) | ||
| if err != nil { | ||
| klog.V(0).Infof("Chaos injection failed due to %v\n", err) | ||
| utils.UpdateChaosResultMiddle(&experimentsDetails, clients, flag,failStep); return |
ksatchit
reviewed
May 13, 2020
|
|
||
| //Updating the chaosResult in the end of experiment | ||
| klog.V(0).Infof("[The End]: Updating the chaos result of %v experiment (EOT)",experimentsDetails.ExperimentName) | ||
| err = utils.UpdateChaosResultEnd(&experimentsDetails, clients, flag,failStep) |
ksatchit
reviewed
May 13, 2020
pkg/utils/chaosResult.go
Outdated
| @@ -0,0 +1,93 @@ | |||
| package utils | |||
|
|
|||
There was a problem hiding this comment.
can have single fn taking action and values (post 1.4.0)
ksatchit
reviewed
May 13, 2020
pkg/utils/chaosResult.go
Outdated
| // UpdateChaosResultMiddle Update the chaos result (Middle of Test) | ||
| func UpdateChaosResultMiddle(experimentsDetails *ExperimentDetails, clients ClientSets, flag string, failStep string) { | ||
|
|
||
| klog.V(0).Infof("[The End]: Updating the chaos result of %v experiment (EOT)",experimentsDetails.ExperimentName) |
There was a problem hiding this comment.
Log optimization -- we might not lead to log every action that we are performing -- only user relevant info is sufficient.
This was referenced May 13, 2020
Signed-off-by: shubhamchaudhary <shubham.chaudhary@mayadata.io>
chandankumar4
approved these changes
May 14, 2020
There was a problem hiding this comment.
Some more improvement is required based on the import path and function rearrangement, We can improve in the upcoming PRs
/LGTM
Thanks @ispeakc0de
rahulchheda
approved these changes
May 14, 2020
There was a problem hiding this comment.
/LGTM
Added some small comments, nothing major, we can do post 1.4.x
Thanks @ispeakc0de , great PR!
Makefile
Outdated
| @echo "------------------" | ||
| @echo "--> Build litmus-go image" | ||
| @echo "------------------" | ||
| sudo docker build . -f build/litmus-go/Dockerfile -t shubh214/litmus-go:ci2 |
There was a problem hiding this comment.
docker repo name change needed
| if err != nil { | ||
| return err | ||
| } | ||
| klog.V(0).Infof("Killing %v pods", targetPodList) |
There was a problem hiding this comment.
Suggested change
| klog.V(0).Infof("Killing %v pods", targetPodList) | |
| klog.Infof("Killing %v pods", targetPodList) |
There was a problem hiding this comment.
Seems like you are not using other Verbosity Levels, so you might not need to set it also.
| labels: | ||
| experiment: pod-delete | ||
| spec: | ||
| # Placeholder that is updated by the executor for automated runs |
There was a problem hiding this comment.
Unable to get the context of this comment
| serviceAccountName: litmus | ||
| restartPolicy: Never | ||
| containers: | ||
| - name: gotest |
There was a problem hiding this comment.
can we use appropriate container name? something like go-exp
| golang.org/x/sys v0.0.0-20200509044756-6aff5f38e54f // indirect | ||
| golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1 // indirect | ||
| google.golang.org/appengine v1.6.6 // indirect | ||
| k8s.io/apimachinery v0.0.0-20190817020851-f2f3a405f61d |
There was a problem hiding this comment.
add a pin comment here to help others understand this timestamp
| // CheckPodStatus checks the status of the application pod | ||
| func CheckPodStatus(appNs string, appLabel string, clients environment.ClientSets) error { | ||
| err := retry. | ||
| Times(90). |
There was a problem hiding this comment.
What if got running before 180sec ? are we handling that in this retry func?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: shubhamchaudhary shubham.chaudhary@mayadata.io
Issue reference: litmuschaos/litmus#1466