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

gRPC - App logs #205

Merged
merged 16 commits into from
Jun 9, 2017
Merged

gRPC - App logs #205

merged 16 commits into from
Jun 9, 2017

Conversation

drgarcia1986
Copy link
Contributor

@drgarcia1986 drgarcia1986 commented Jun 6, 2017

  • Implement app logs
  • add pod name in front of log msg
  • add gRPC Stream Interceptor
  • add K8S Client based on client-go
  • Remove go-swagger implementation
  • Clean up

(related #203 )


This change is Reviewable

@drgarcia1986 drgarcia1986 mentioned this pull request Jun 6, 2017
26 tasks
Copy link
Contributor

@aguerra aguerra left a comment

Choose a reason for hiding this comment

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

LGTM, maybe the interface can be simplified:

PodsLogs(namespace string, lines int64, follow bool) (io.ReadCloser, error)

This way the app pkg doesn't need to know about pod status

}
if follow {
rand.Seed(42) // The Anwser
for i := 0; i <= rand.Intn(5); i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: Answer

a := new(App)
if err := json.Unmarshal([]byte(annotation), a); err != nil {
return "", nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess return "", err, right?

"Comment": "v2.0.0",
"Rev": "e121606b0d09b2e1c467183ee46217fa85a6b672"
},
{
"ImportPath": "k8s.io/kubernetes/pkg/api",
"Comment": "v1.4.0-alpha.2-550-gda53a24",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if these k8s.io/apimachinery shoud be here.

Copy link
Contributor Author

@drgarcia1986 drgarcia1986 Jun 7, 2017

Choose a reason for hiding this comment

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

I have tried to remove from Godeps.json and run godeps save ./..., but it's back again 😞

@drgarcia1986
Copy link
Contributor Author

@aguerra I don't understand, are you suggest to remove arg podName for app.K8sOperations.PodLogs() interface? I don't follow, because, we need to get logs for a single pod (e.g. deploy). About pod status, can you explain your idea ?

@aguerra
Copy link
Contributor

aguerra commented Jun 7, 2017

As we talked in person this way is more granular so ok4me.

now we log all pods with any status
found = true
break
}
}
return found
}

func (ops *AppOperations) getAppTeam(appName string) (string, error) {
annotation, err := ops.kops.NamespaceAnnotation(appName, TeresaAnnotation)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change to Label instead of Annotation

Copy link
Contributor

@aguerra aguerra left a comment

Choose a reason for hiding this comment

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

🔁

@drgarcia1986 drgarcia1986 merged commit a12ce78 into master Jun 9, 2017
@drgarcia1986 drgarcia1986 deleted the dg-app_logs branch June 9, 2017 14:03
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.

2 participants