(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 <[email protected]>

.gitignore

@@ -2,3 +2,4 @@ build/_output
 *.swp
 *.orig
 .idea/
+coverage.txt

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"`

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
 	}

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)

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"
 )
 

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
-}
+}