[UI] Pass namespace to APIs #2676
Conversation
|
@gaoning777 This is ready for review. |
| if (!k8sV1Client) { | ||
| throw new Error('Cannot access kubernetes API'); | ||
| } | ||
| return (k8sV1Client.readNamespacedPodLog(podName, namespace, 'main') as any).then( | ||
| return (k8sV1Client.readNamespacedPodLog(podName, podNamespace || namespace, 'main') as any).then( |
There was a problem hiding this comment.
Do we need to update the global variable: namespace such that the getArgoWorkflow and getK8sSecret also work?
There was a problem hiding this comment.
We need to support those one by one. Global variable is shared and could be modified during processing of a request.
I don't currently know where those methods are used. I guess I should double check and make sure other features don't break.
There was a problem hiding this comment.
e.g. I searched that https://github.com/kubeflow/pipelines/blob/master/frontend/server/workflow-helper.ts#L147 is the usage of the workflow-helper calling the getArgoWorkflow. Could you proof-check if this functions are called in other places.
There was a problem hiding this comment.
Thanks, that was added in #2081 to get pod log using argo workflow log persistence capability. I don't think we need to support this in multi user mode right now
There was a problem hiding this comment.
I verified these methods are only used for the use case I mentioned above. It's fine we defer these support.
There was a problem hiding this comment.
Does it mean if the deployed cluster contains preemptible VMs, the FE can still get the logs regardless of the liveness of the VM that runs the pods?
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Bobgy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test kubeflow-pipeline-sample-test |
|
New changes are detected. LGTM label has been removed. |
* Format other frontend code using prettier * Regenerate frontend api clients * Pass namespace to CreateRun api request * Fetch logs from pod namespace * Fix log handler cannot work with pod name only query bug * Fix and refactor existing tests * Unit test adding namespace to create run api call. * Remove unneeded snapshot * Fix lint errors * Consistently use resource reference id for namespace
Part of multi user support.
Changes:
TODO:
/cc @gaoning777
/area frontend
/kind feature
This change is