[BUG] Fix undeploy router error

[deadlycoconuts]

Context

This PR addresses a bug that occurs when after a failed router deployment attempt, Turing API performs a cleanup of any k8s resources created in the process using the deployment service method UndeployRouterVersion. Instead of cleaning up any resource created during the deployment process, UndeployRouterVersion instead attempts to clean up all resources associated with a generic router deployment (as if the router in concern had been created successfully).

In other words, should the router deployment fail during the secret deployment stage, which is the first k8s resource to be created in DeployRouterVersion (see this), Turing API will not only attempt to remove the secrets but also other components like ensemblers, enrichers, routers, fluentd services, experiment engine plugins servers, etc. using UndeployRouterVersion, which assumes that these components have already been created successfully.

Unsurprisingly, any attempt to delete these inexistent components will result in an error being thrown back (since these components cannot be found), hence making it seem as if the undeployment process has also failed.

Fix

To address the bug, an isCleanUp flag has been created as part of the UndeployRouterVersion signature, which is set as true when the method is called as part of a cleanup process (i.e. remove as many components as can be found), and false when the method is called as part of a regular undeployment process (i.e. remove all components as one would expect is normally associated with a working Turing Router deployment).

This flag is passed to the individual component removal helper functions such as deleteSecret, deleteK8sService, deletePVC and deleteKnServices. These helper functions then call controller methods to perform the corresponding actions with an additional ignoreNotFound flag. Essentially, in a clean up operation (isCleanUp is true), any attempt to delete a resources that does not exist will return a nil value instead of an error, if ignoreNotFound flag is true.

E.g.

// deleteSecret deletes a secret.
func deleteSecret(controller cluster.Controller, secret *cluster.Secret, isCleanUp bool) error {
	if isCleanUp {
		return controller.DeleteSecret(secret.Name, secret.Namespace, true)
	}
	return controller.DeleteSecret(secret.Name, secret.Namespace, false)
}

However, this change also necessitated some minor refactoring of these existing helper functions (and their dependent functions) as we now need to perform the resource existence check at each deletion step (each resource creation step is a potential point of failure in the router deployment). In particular, DeleteKubernetesService of the cluster controller had to be separated into two new methods, DeleteKubernetesService (as before), and DeleteKubernetesDeployment, each of which concerns itself with only deleting the k8s service OR k8s deployment resource.

Unrelated Minor Bug Fix

A method to verify that a default route specified indeed exists when setting up a router using the SDK, was introduced in PR #192. This method throws an error when a default route specified is found to not exist amongst all the routes that were defined for a router. The SDK sample script has been modified accordingly to now specify a default route that exists in the router configuration. In addition, the image of the Enricher and Ensembler has had docker.io/ prefixed to the existing image so as to allow the DockerRegistryPopover of the UI to render correctly.

Modifications

• api/turing/api/deployment_controller.go - Addition of isCleanUp flag argument to UndeployRouterVersion calls
• api/turing/cluster/controller.go - Addition of helper methods to determine whether a k8s resource exists and separation of DeleteKubernetesService into DeleteKubernetesService and DeleteKubernetesDeployment
• api/turing/service/router_deployment_service.go - Addition of logical checks for each helper function that deletes a k8s component
• sdk/samples/router/general.py - Minor bug fixes to the sample script

[krithika369]

Thanks for this fix! I agree with the overall approach but left some suggestions on the implementation. Please feel free to merge when they have been reviewed / addressed.

[deadlycoconuts]

Thanks for the comments! I've refactored the changes with the ignoreNotFound flag and removed/modified the corresponding tests. I'll be merging this now! 🚀