From ecbcef916caede7205eaf7ec88ebab090daf7e0b Mon Sep 17 00:00:00 2001 From: Rahul M Chheda <53308066+rahulchheda@users.noreply.github.com> Date: Fri, 8 May 2020 10:04:30 +0530 Subject: [PATCH] (refactor) Changed scope of EngineInfo structure from Global to Local (#179) * Replaced global engine Varible with local variable * - Fixed the local variable issues - Added Chaos-operator Logs in BDD test * Introduce Patch for deletion of chaosengine Signed-off-by: Rahul M Chheda --- .gitignore | 1 + .../litmuschaos/v1alpha1/chaosengine_types.go | 19 ++-- .../chaosengine/chaosengine_controller.go | 42 ++++---- pkg/controller/resource/deploymentconfig.go | 3 +- pkg/controller/resource/resource.go | 2 +- pkg/dynamic/dynamic.go | 5 +- tests/bdd/bdd_test.go | 96 ++++++++++++++++--- 7 files changed, 122 insertions(+), 46 deletions(-) diff --git a/.gitignore b/.gitignore index 7450adc3d..676d58a7c 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ build/_output *.swp *.orig .idea/ +coverage.txt diff --git a/pkg/apis/litmuschaos/v1alpha1/chaosengine_types.go b/pkg/apis/litmuschaos/v1alpha1/chaosengine_types.go index 22f48b321..9b2c99a9a 100644 --- a/pkg/apis/litmuschaos/v1alpha1/chaosengine_types.go +++ b/pkg/apis/litmuschaos/v1alpha1/chaosengine_types.go @@ -56,15 +56,22 @@ const ( EngineStateStop EngineState = "stop" ) +// ExperimentStatus is typecasted to string for supporting the values below. type ExperimentStatus string const ( - ExperimentStatusRunning ExperimentStatus = "Running" - ExperimentStatusCompleted ExperimentStatus = "Completed" - ExperimentStatusWaiting ExperimentStatus = "Waiting for Job Creation" - ExperimentStatusNotFound ExperimentStatus = "ChaosExperiment Not Found" + // ExperimentStatusRunning is status of Experiment which is currently running + ExperimentStatusRunning ExperimentStatus = "Running" + // ExperimentStatusCompleted is status of Experiment which has been completed + ExperimentStatusCompleted ExperimentStatus = "Completed" + // ExperimentStatusWaiting is status of Experiment which will be executed via a Job + ExperimentStatusWaiting ExperimentStatus = "Waiting for Job Creation" + // ExperimentStatusNotFound is status of Experiment which is not found inside ChaosNamespace + ExperimentStatusNotFound ExperimentStatus = "ChaosExperiment Not Found" + // ExperimentStatusSuccessful is status of a Successful experiment execution ExperimentStatusSuccessful ExperimentStatus = "Execution Successful" - ExperimentStatusAborted ExperimentStatus = "Forcefully Aborted" + // ExperimentStatusAborted is status of a Experiment is forcefully aborted + ExperimentStatusAborted ExperimentStatus = "Forcefully Aborted" ) // EngineStatus provides interface for all supported strings in status.EngineStatus @@ -95,7 +102,7 @@ const ( // ChaosEngineStatus derives information about status of individual experiments type ChaosEngineStatus struct { - // + //EngineStatus is a typed string to support limited values for ChaosEngine Status EngineStatus EngineStatus `json:"engineStatus"` //Detailed status of individual experiments Experiments []ExperimentStatuses `json:"experiments"` diff --git a/pkg/controller/chaosengine/chaosengine_controller.go b/pkg/controller/chaosengine/chaosengine_controller.go index 88b914621..62b190627 100644 --- a/pkg/controller/chaosengine/chaosengine_controller.go +++ b/pkg/controller/chaosengine/chaosengine_controller.go @@ -100,8 +100,6 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return nil } -var engine *chaosTypes.EngineInfo - // watchSecondaryResources watch's for changes in chaos resources func watchChaosResources(clientSet client.Client, c controller.Controller) error { // Watch for Primary Chaos Resource @@ -125,8 +123,8 @@ func watchChaosResources(clientSet client.Client, c controller.Controller) error // Result.Requeue is true, otherwise upon completion it will remove the work from the queue. func (r *ReconcileChaosEngine) Reconcile(request reconcile.Request) (reconcile.Result, error) { reqLogger := startReqLogger(request) - - err := r.getChaosEngineInstance(request) + engine := &chaosTypes.EngineInfo{} + err := r.getChaosEngineInstance(engine, request) if err != nil { if k8serrors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. @@ -139,7 +137,7 @@ func (r *ReconcileChaosEngine) Reconcile(request reconcile.Request) (reconcile.R //Handle deletion of Chaos Engine if engine.Instance.ObjectMeta.GetDeletionTimestamp() != nil { - return r.reconcileForDelete(request) + return r.reconcileForDelete(engine, request) } // Start the reconcile by setting default values into Chaos Engine @@ -154,17 +152,17 @@ func (r *ReconcileChaosEngine) Reconcile(request reconcile.Request) (reconcile.R // Handling Graceful completion of Chaos Engine if engine.Instance.Spec.EngineState == litmuschaosv1alpha1.EngineStateStop && engine.Instance.Status.EngineStatus == litmuschaosv1alpha1.EngineStatusCompleted { - return r.reconcileForComplete(request) + return r.reconcileForComplete(engine, request) } // Handling forceful Abort of Chaos Engine if engine.Instance.Spec.EngineState == litmuschaosv1alpha1.EngineStateStop && engine.Instance.Status.EngineStatus != litmuschaosv1alpha1.EngineStatusCompleted { - return r.reconcileForDelete(request) + return r.reconcileForDelete(engine, request) } // Handling restarting of Chaos Engine if engine.Instance.Spec.EngineState == litmuschaosv1alpha1.EngineStateActive && (engine.Instance.Status.EngineStatus == litmuschaosv1alpha1.EngineStatusCompleted || engine.Instance.Status.EngineStatus == litmuschaosv1alpha1.EngineStatusStopped) { - return r.reconcileForRestart(request) + return r.reconcileForRestart(engine, request) } return reconcile.Result{}, nil @@ -319,16 +317,14 @@ func engineRunnerPod(runnerPod *podEngineRunner) error { } // Fetch the ChaosEngine instance -func (r *ReconcileChaosEngine) getChaosEngineInstance(request reconcile.Request) error { +func (r *ReconcileChaosEngine) getChaosEngineInstance(engine *chaosTypes.EngineInfo, request reconcile.Request) error { instance := &litmuschaosv1alpha1.ChaosEngine{} err := r.client.Get(context.TODO(), request.NamespacedName, instance) if err != nil { // Error reading the object - requeue the request. return err } - engine = &chaosTypes.EngineInfo{ - Instance: instance, - } + engine.Instance = instance return nil } @@ -396,7 +392,7 @@ func (r *ReconcileChaosEngine) checkEngineRunnerPod(engine *chaosTypes.EngineInf //setChaosResourceImage take the runner image from engine spec //if it is not there then it will take from chaos-operator env //at last if it is not able to find image in engine spec and operator env then it will take default images -func setChaosResourceImage() { +func setChaosResourceImage(engine *chaosTypes.EngineInfo) { ChaosRunnerImage := os.Getenv("CHAOS_RUNNER_IMAGE") @@ -408,7 +404,7 @@ func setChaosResourceImage() { } // getAnnotationCheck() checks for annotation on the application -func getAnnotationCheck() error { +func getAnnotationCheck(engine *chaosTypes.EngineInfo) error { if engine.Instance.Spec.AnnotationCheck == "" { engine.Instance.Spec.AnnotationCheck = chaosTypes.DefaultAnnotationCheck @@ -421,13 +417,13 @@ func getAnnotationCheck() error { } // reconcileForDelete reconciles for deletion/force deletion of Chaos Engine -func (r *ReconcileChaosEngine) reconcileForDelete(request reconcile.Request) (reconcile.Result, error) { +func (r *ReconcileChaosEngine) reconcileForDelete(engine *chaosTypes.EngineInfo, request reconcile.Request) (reconcile.Result, error) { err := r.forceRemoveChaosResources(engine, request) if err != nil { r.recorder.Eventf(engine.Instance, corev1.EventTypeWarning, "ChaosResourcesOperationFailed", "Unable to delete chaos resources") return reconcile.Result{}, err } - opts := client.UpdateOptions{} + patch := client.MergeFrom(engine.Instance.DeepCopy()) if engine.Instance.ObjectMeta.Finalizers != nil { engine.Instance.ObjectMeta.Finalizers = utils.RemoveString(engine.Instance.ObjectMeta.Finalizers, "chaosengine.litmuschaos.io/finalizer") @@ -437,7 +433,7 @@ func (r *ReconcileChaosEngine) reconcileForDelete(request reconcile.Request) (re updateExperimentStatusesForStop(engine) engine.Instance.Status.EngineStatus = litmuschaosv1alpha1.EngineStatusStopped - if err := r.client.Update(context.TODO(), engine.Instance, &opts); err != nil { + if err := r.client.Patch(context.TODO(), engine.Instance, patch); err != nil { r.recorder.Eventf(engine.Instance, corev1.EventTypeWarning, "ChaosResourcesOperationFailed", "Unable to update chaosengine") return reconcile.Result{}, fmt.Errorf("Unable to remove Finalizer from chaosEngine Resource, due to error: %v", err) } @@ -492,7 +488,7 @@ func (r *ReconcileChaosEngine) checkRunnerPodCompletedStatus(engine *chaosTypes. } // gracefullyRemoveDefaultChaosResources removes all chaos-resources gracefully -func (r *ReconcileChaosEngine) gracefullyRemoveDefaultChaosResources(request reconcile.Request) (reconcile.Result, error) { +func (r *ReconcileChaosEngine) gracefullyRemoveDefaultChaosResources(engine *chaosTypes.EngineInfo, request reconcile.Request) (reconcile.Result, error) { if engine.Instance.Spec.JobCleanUpPolicy == litmuschaosv1alpha1.CleanUpPolicyDelete { if err := r.gracefullyRemoveChaosPods(engine, request); err != nil { @@ -521,9 +517,9 @@ func (r *ReconcileChaosEngine) gracefullyRemoveChaosPods(engine *chaosTypes.Engi } // reconcileForComplete reconciles for graceful completion of Chaos Engine -func (r *ReconcileChaosEngine) reconcileForComplete(request reconcile.Request) (reconcile.Result, error) { +func (r *ReconcileChaosEngine) reconcileForComplete(engine *chaosTypes.EngineInfo, request reconcile.Request) (reconcile.Result, error) { - _, err := r.gracefullyRemoveDefaultChaosResources(request) + _, err := r.gracefullyRemoveDefaultChaosResources(engine, request) if err != nil { r.recorder.Eventf(engine.Instance, corev1.EventTypeWarning, "ChaosResourcesOperationFailed", "Unable to delete chaos resources") return reconcile.Result{}, err @@ -537,7 +533,7 @@ func (r *ReconcileChaosEngine) reconcileForComplete(request reconcile.Request) ( } // reconcileForRestart reconciles for restart of Chaos Engine -func (r *ReconcileChaosEngine) reconcileForRestart(request reconcile.Request) (reconcile.Result, error) { +func (r *ReconcileChaosEngine) reconcileForRestart(engine *chaosTypes.EngineInfo, request reconcile.Request) (reconcile.Result, error) { err := r.forceRemoveChaosResources(engine, request) if err != nil { return reconcile.Result{}, err @@ -614,10 +610,10 @@ func startReqLogger(request reconcile.Request) logr.Logger { func (r *ReconcileChaosEngine) validateAnnontatedApplication(engine *chaosTypes.EngineInfo) error { // Get the image for runner pod from chaosengine spec,operator env or default values. - setChaosResourceImage() + setChaosResourceImage(engine) //getAnnotationCheck fetch the annotationCheck from engine spec - err := getAnnotationCheck() + err := getAnnotationCheck(engine) if err != nil { return err } diff --git a/pkg/controller/resource/deploymentconfig.go b/pkg/controller/resource/deploymentconfig.go index 83e4d390e..977c143f2 100644 --- a/pkg/controller/resource/deploymentconfig.go +++ b/pkg/controller/resource/deploymentconfig.go @@ -19,7 +19,8 @@ var ( Resource: "deploymentconfigs", } ) -// CheckDeploymentAnnotation will check the annotation of deployment + +// CheckDeploymentConfigAnnotation will check the annotation of deployment func CheckDeploymentConfigAnnotation(clientSet dynamic.Interface, engine *chaosTypes.EngineInfo) (*chaosTypes.EngineInfo, error) { deploymentConfigList, err := getDeploymentConfigList(clientSet, engine) diff --git a/pkg/controller/resource/resource.go b/pkg/controller/resource/resource.go index abab55ed8..2be000550 100644 --- a/pkg/controller/resource/resource.go +++ b/pkg/controller/resource/resource.go @@ -21,8 +21,8 @@ import ( "os" "strings" - dynamic "github.com/litmuschaos/chaos-operator/pkg/dynamic" chaosTypes "github.com/litmuschaos/chaos-operator/pkg/controller/types" + dynamic "github.com/litmuschaos/chaos-operator/pkg/dynamic" k8s "github.com/litmuschaos/chaos-operator/pkg/kubernetes" ) diff --git a/pkg/dynamic/dynamic.go b/pkg/dynamic/dynamic.go index ff98313cb..2ae73a17a 100644 --- a/pkg/dynamic/dynamic.go +++ b/pkg/dynamic/dynamic.go @@ -5,7 +5,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/config" ) -func CreateClientSet() (*dynamic.Interface, error){ +// CreateClientSet returns a Dynamic Kubernetes ClientSet +func CreateClientSet() (*dynamic.Interface, error) { restConfig, err := config.GetConfig() if err != nil { return nil, err @@ -15,4 +16,4 @@ func CreateClientSet() (*dynamic.Interface, error){ return nil, err } return &clientSet, nil -} \ No newline at end of file +} diff --git a/tests/bdd/bdd_test.go b/tests/bdd/bdd_test.go index e56e4c30a..2ca7a5e86 100644 --- a/tests/bdd/bdd_test.go +++ b/tests/bdd/bdd_test.go @@ -17,7 +17,9 @@ limitations under the License. package bdd import ( + "bytes" "fmt" + "io" "os" "os/exec" "testing" @@ -34,6 +36,7 @@ import ( scheme "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "k8s.io/client-go/tools/clientcmd" + "k8s.io/klog" "github.com/litmuschaos/chaos-operator/pkg/apis/litmuschaos/v1alpha1" chaosClient "github.com/litmuschaos/chaos-operator/pkg/client/clientset/versioned/typed/litmuschaos/v1alpha1" @@ -83,7 +86,7 @@ var _ = BeforeSuite(func() { err = exec.Command("kubectl", "apply", "-f", "../../deploy/operator.yaml").Run() Expect(err).To(BeNil()) - fmt.Println("chaos-operator created successfully") + klog.Infoln("chaos-operator created successfully") //Wait for the creation of chaos-operator time.Sleep(50 * time.Second) @@ -151,7 +154,7 @@ var _ = Describe("BDD on chaos-operator", func() { _, err := client.AppsV1().Deployments("litmus").Create(deployment) if err != nil { - fmt.Println("Deployment is not created and error is ", err) + klog.Infoln("Deployment is not created and error is ", err) } //creating chaos-experiment for pod-delete @@ -204,7 +207,7 @@ var _ = Describe("BDD on chaos-operator", func() { _, err = clientSet.ChaosExperiments("litmus").Create(ChaosExperiment) Expect(err).To(BeNil()) - fmt.Println("ChaosExperiment created successfully...") + klog.Infoln("ChaosExperiment created successfully...") //Creating chaosEngine By("Creating ChaosEngine") @@ -240,7 +243,7 @@ var _ = Describe("BDD on chaos-operator", func() { _, err = clientSet.ChaosEngines("litmus").Create(chaosEngine) Expect(err).To(BeNil()) - fmt.Println("Chaosengine created successfully...") + klog.Infoln("Chaosengine created successfully...") //Wait till the creation of runner pod time.Sleep(50 * time.Second) @@ -275,7 +278,7 @@ var _ = Describe("BDD on chaos-operator", func() { _, err = clientSet.ChaosEngines("litmus").Update(engine) Expect(err).To(BeNil()) - fmt.Println("Chaosengine updated successfully...") + klog.Infoln("Chaosengine updated successfully...") //Wait till the creation of runner pod time.Sleep(50 * time.Second) @@ -290,10 +293,10 @@ var _ = Describe("BDD on chaos-operator", func() { //Fetching engine-nginx-runner pod _, err := client.CoreV1().Pods("litmus").Get("engine-nginx-runner", metav1.GetOptions{}) - fmt.Printf("%v", err) + klog.Infof("%v\n", err) isNotFound := errors.IsNotFound(err) Expect(isNotFound).To(BeTrue()) - fmt.Println("chaos-runner pod deletion verified") + klog.Infoln("chaos-runner pod deletion verified") }) @@ -315,7 +318,7 @@ var _ = Describe("BDD on chaos-operator", func() { err := clientSet.ChaosEngines("litmus").Delete("engine-nginx", &metav1.DeleteOptions{}) Expect(err).To(BeNil()) - fmt.Println("chaos engine deleted successfully") + klog.Infoln("chaos engine deleted successfully") }) @@ -370,10 +373,10 @@ var _ = Describe("BDD on chaos-operator", func() { //Fetching engine-nginx-runner pod _, err := client.CoreV1().Pods("litmus").Get("engine-nginx-runner", metav1.GetOptions{}) - fmt.Printf("%v", err) + klog.Infof("%v\n", err) isNotFound := errors.IsNotFound(err) Expect(isNotFound).To(BeTrue()) - fmt.Println("chaos-runner pod deletion verified") + klog.Infoln("chaos-runner pod deletion verified") }) @@ -389,12 +392,79 @@ var _ = Describe("BDD on chaos-operator", func() { }) }) + Context("Validate via Chaos-Operator Logs", func() { + + It("Should Generate Operator logs", func() { + pods, err := client.CoreV1().Pods("litmus").List(metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%v=%v", "name", "chaos-operator"), + }) + Expect(err).To(BeNil()) + + if len(pods.Items) > 1 { + klog.Infof("Multiple Chaos-Operator Pods found") + return + } + if len(pods.Items) < 1 { + klog.Infof("Unable to find Chaos-Operator Pod") + return + } + + podName := pods.Items[0].Name + Expect(podName).To( + Not(BeEmpty()), + "Unable to get the operator pod name", + ) + + klog.Infof("Got Pod Name: %v\n", podName) + + podLogOpts := v1.PodLogOptions{} + + req := client.CoreV1().Pods("litmus").GetLogs(podName, &podLogOpts) + + podLogs, err := req.Stream() + Expect(err).To(BeNil()) + + defer podLogs.Close() + + buf := new(bytes.Buffer) + _, err = io.Copy(buf, podLogs) + Expect(err).To(BeNil()) + + str := buf.String() + + klog.Infof("Chaos Operator Logs:\n%v\n", str) + + }) + }) + }) //Deleting all unused resources var _ = AfterSuite(func() { - By("Deleting all CRDs") - crdDeletion := exec.Command("kubectl", "delete", "-f", "../../deploy/chaos_crds.yaml").Run() - Expect(crdDeletion).To(BeNil()) + //Deleting ChaosExperiments + By("Deleting ChaosExperiments") + err := exec.Command("kubectl", "delete", "chaosexperiments", "--all", "-n", "litmus").Run() + Expect(err).To(BeNil()) + + //Deleting ChaosEngines + By("Deleting ChaosEngines") + err = exec.Command("kubectl", "delete", "chaosengine", "--all", "-n", "litmus").Run() + Expect(err).To(BeNil()) + + //Deleting Chaos-Operator + By("Deleting operator") + err = exec.Command("kubectl", "delete", "-f", "../../deploy/operator.yaml").Run() + Expect(err).To(BeNil()) + + //Deleting rbacs + By("Deleting RBAC's") + err = exec.Command("kubectl", "delete", "-f", "../../deploy/rbac.yaml").Run() + Expect(err).To(BeNil()) + + //Deleting CRD's + By("Deleting chaosengine crd") + err = exec.Command("kubectl", "delete", "-f", "../../deploy/chaos_crds.yaml").Run() + Expect(err).To(BeNil()) + })