Use background context rather than cancelled context

[terryyylim]

Bug

Turing's dependency on Kubernetes API was updated in #183 and most Kubernetes APIs now require context to be passed which caused a regression. When ctx.Done() is called, the context was cancelled and ctx.Err() returns a non-nil error. Thus, when passed to ListPods API, when the ctx.Err() value is first checked, the call exits given a non-nil error and pods that satisfy the provided labels are never returned, causing termination logs to NOT be propagated to the user.

Fix

The fix uses a new empty context via context.Background(), which will not be cancelled and that allows Kubernetes Go Client ListPods API to retrieve the correct pods based on provided labels. Then, the container error logs from Terminated state can be propagated as event logs to the user.

Enhancements

In addition, this PR standardizes how context is being used for the following workflows:

• Creation/Updation: Use ctx with Deployment timeout as specified in config
• Deletion: Use context.Background() with no timeouts (Some functions in api/turing/cluster/controller.go have their timeout parameter removed)
  • GracePeriodSeconds exposed by Kubernetes Go client's metav1.DeleteOptions{} is optional

[krithika369]

1. Could we indicate in the description that this is a regression?
2. Have you also checked the calling code in api/turing/service/router_deployment_service.go as I see we are using context.Background() in some places and original context with deployment timeout in the others.

[terryyylim]

1. Could we indicate in the description that this is a regression?

Done.

2. Have you also checked the calling code in api/turing/service/router_deployment_service.go as I see we are using context.Background() in some places and original context with deployment timeout in the others.

In functions where original context are being passed to functions, a defer statement has been specified for the context cancel call. In cases where ctx.Done() is called, the context is no longer used and errors are being returned. Unless you mean something else here like standardising how context is being initialised and passed? (i.e along the lines of if original context is not required, we use context.Background() instead of original context)

[krithika369]

We have 2 deployment-related timeout values here:

• Timeout is for the deployment of the new version (so the original ctx can be used for all creation / update operations; this can help cancel the deployment when we hit timeout)
• DeletionTimeout is for the cleaning up of old resources (such as from the previous version; so this can be applied to all deletion actions). At the same time, it's probably ok to also use context.Background here and use the timeout value only when the API specifically needs it (example).

Lastly, I think context.Background() is also appropriate when we have operations that are mandatory (like getting the pod description).

[krithika369]

Code changes LGTM. Please feel free to merge once the test cases are addressed.