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

Add pod and container name to log context #291

Merged
merged 6 commits into from
Sep 17, 2019

Conversation

SupriyaKasten
Copy link
Contributor

Change Overview

Add pod name and container name to context in addition to actionset name and phase name

Pull request type

Please check the type of change your PR introduces:

  • Work in Progress
  • Refactoring (no functional changes, no api changes)
  • Trivial/Minor
  • Bugfix
  • Feature
  • Documentation

Copy link
Contributor

@tdmanv tdmanv left a comment

Choose a reason for hiding this comment

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

For the Functions that use PodRunner, can we just set these values there?

@SupriyaKasten SupriyaKasten marked this pull request as ready for review September 16, 2019 22:50
@@ -176,6 +177,10 @@ func (*restoreDataFunc) Exec(ctx context.Context, tp param.TemplateParams, args
}
// Validate and get optional arguments
restorePath, encryptionKey, pod, vols, backupTag, backupID, err := validateAndGetOptArgs(args)

if pod != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't this Function also use pod runner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, but the pod name set here is different than the one that is generated by pod runner.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would either drop this or rename the key since it's getting overwritten anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

Use a different key in order to tell apart the 2 pod names?

@@ -128,6 +129,7 @@ func (*restoreDataAllFunc) Exec(ctx context.Context, tp param.TemplateParams, ar
output := make(map[string]interface{})
for _, pod := range pods {
go func(pod string) {
ctx = field.Context(ctx, field.PodNameKey, pod)
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@SupriyaKasten SupriyaKasten changed the base branch from master to fields-const September 17, 2019 18:46
@SupriyaKasten SupriyaKasten changed the base branch from fields-const to master September 17, 2019 20:30
@SupriyaKasten SupriyaKasten merged commit d9fcee5 into master Sep 17, 2019
@SupriyaKasten SupriyaKasten deleted the log-podname-containername branch September 17, 2019 23:01
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.

None yet

3 participants