Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch Argo workflow logs #1332

Merged

Conversation

tonyyang-svail
Copy link
Collaborator

@tonyyang-svail tonyyang-svail commented Dec 4, 2019

Part of #1066.

Next step:

  1. Support multiple steps.
  2. Support long-running jobs.

@tonyyang-svail tonyyang-svail changed the title [wip] Fetch Argo workflow logs Fetch Argo workflow logs Dec 6, 2019
return string(output), nil
}

func getPodLogs(podName string) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kubectl logs would fetch all the logs, please add a TODO comment here to implement fetch logs by an incremental way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I will add this in the next PR.

@@ -163,6 +164,16 @@ func writeArgoFile(coulerFileName string) (string, error) {
return argoYaml.Name(), nil
}

func getWorkflowID(output string) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated with workflow.go#getWorkflowID ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will move workflow related code to package workflow in the next PR.

}

func getWorkflowStatusPhase(job pb.Job) (string, error) {
cmd := exec.Command("kubectl", "get", "wf", job.Id, "-o", "jsonpath={.status.phase}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use https://github.com/kubernetes/client-go instead of calling the command line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the advantage of using the client-go? I am not sure if the client-go supports parsing the Argo workflow descriptions, so I may need to spend some time to test it out.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an issue: #1362 to discuss go-client or kubectl, maybe we can move to make some disccusion.

Copy link
Collaborator

@Yancey1989 Yancey1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tonyyang-svail tonyyang-svail merged commit 8575425 into sql-machine-learning:develop Dec 9, 2019
@tonyyang-svail tonyyang-svail deleted the fetch_wf_logs branch December 9, 2019 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants