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.

Sign up for GitHub

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

Jump to bottom

Log main golang runtime env vars #108480

Merged
merged 1 commit into from
Mar 4, 2022
Merged

Log main golang runtime env vars #108480

merged 1 commit into from
Mar 4, 2022

Conversation

wojtek-t
Copy link
Member

@wojtek-t wojtek-t commented Mar 3, 2022

Ref #108357

NONE

/kind cleanup
/priority important-longterm
/sig scalability

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 3, 2022
@wojtek-t
Copy link
Member Author

wojtek-t commented Mar 3, 2022

/hold

Will add to other core components too if we agree that it's what we want.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 3, 2022
@wojtek-t wojtek-t mentioned this pull request Mar 3, 2022
Closed
4 tasks
aojea
aojea reviewed Mar 3, 2022
View reviewed changes
cmd/kube-apiserver/app/server.go Outdated
@@ -151,6 +151,8 @@ cluster's shared state through which all other components interact.`,
func Run(completeOptions completedServerRunOptions, stopCh <-chan struct{}) error {
// To help debugging, immediately log version
klog.Infof("Version: %+v", version.Get())

klog.Infof("Golang settings: GOGC=%s, GOMAXPROCS=%s, GOTRACEBACK=%s", os.GetEnv("GOGC"), os.GetEnv("GOMAXPROCS"), os.GetEnv("GOTRACEBACK"))
Copy link
Member

Choose a reason for hiding this comment

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

cmd/kube-apiserver/app/server.go:155:75: undefined: os.GetEnv

@dims
Copy link
Member

dims commented Mar 3, 2022

looks like this needs a ./hack/update-gofmt.sh :)

@dims
Copy link
Member

dims commented Mar 3, 2022

cc @MadhavJivrajani

@liggitt
Copy link
Member

liggitt commented Mar 3, 2022

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2022
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 3, 2022
@wojtek-t
Copy link
Member Author

wojtek-t commented Mar 3, 2022

@liggitt - I added it to other components too - PTAL

@k8s-ci-robot k8s-ci-robot added area/kubelet sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels Mar 3, 2022
liggitt
liggitt reviewed Mar 3, 2022
View reviewed changes
cmd/kube-scheduler/app/server.go
@@ -140,6 +140,8 @@ func Run(ctx context.Context, cc *schedulerserverconfig.CompletedConfig, sched *
// To help debugging, immediately log version
klog.InfoS("Starting Kubernetes Scheduler", "version", version.Get())

Copy link
Member

Choose a reason for hiding this comment

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

pre-existing, but for consistency, add version info as well?

klog.Infof("Version: %+v", version.Get())

Copy link
Member Author

Choose a reason for hiding this comment

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

It's above - just in a different format. Should we change this?

Copy link
Member

Choose a reason for hiding this comment

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

oh, missed it because the line didn't start with version:. nah, I wouldn't change it here

@liggitt
Copy link
Member

liggitt commented Mar 3, 2022

/lgtm
/approve
/hold in case you want to add the version line to the scheduler in this PR as well

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2022
@liggitt
Copy link
Member

liggitt commented Mar 3, 2022

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2022
@dims
Copy link
Member

dims commented Mar 3, 2022

/lgtm
/approve

/retest

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, liggitt, wojtek-t

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@leilajal
Copy link
Contributor

leilajal commented Mar 3, 2022

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 3, 2022
@fedebongio
Copy link
Contributor

/triage accepted

@dims
Copy link
Member

dims commented Mar 3, 2022 

/home/prow/go/src/k8s.io/kubernetes/cmd/kubelet/app/server.go:396:2: unstructured logging function "Infof" should not be used

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2022
@wojtek-t
Copy link
Member Author

wojtek-t commented Mar 3, 2022

@dims - fixed, PTAL

@dims
Copy link
Member

dims commented Mar 3, 2022

now verify: gofmt tripped up 🤣 :)

@wojtek-t
Copy link
Member Author

wojtek-t commented Mar 4, 2022

now verify: gofmt tripped up rofl :)

Sorry for that... should be fixed now

@aojea
Copy link
Member

aojea commented Mar 4, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2022
@wojtek-t
Copy link
Member Author

wojtek-t commented Mar 4, 2022

/retest

@k8s-ci-robot k8s-ci-robot merged commit d7d1219 into kubernetes:master Mar 4, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@aojea aojea aojea left review comments

@mikedanese mikedanese Awaiting requested review from mikedanese

Assignees

@dims dims

@liggitt liggitt

@aojea aojea

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.24
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@wojtek-t @dims @liggitt @k8s-ci-robot @leilajal @fedebongio @aojea