-
Notifications
You must be signed in to change notification settings - Fork 260
Fixed #108 CoreOS osInfo #150
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
Conversation
|
Another changes related to the build issues #148 went to my fork, sorry. I can prepare another PR if you want. Let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IvanovOleg Please create PR for /etc/os-release issue alone. We are already fixing the get cmd part.
telemetry/telemetry_linux.go
Outdated
| osInfoArr := make(map[string]string) | ||
|
|
||
| for i := range linesArr { | ||
| s := strings.Split(linesArr[i], "=") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we check if the Split is successful here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works, but you can check it yourself if you want. I've checked this split by isolating the function and printing the result, it worked. Then I built the artifact from my fork and use it with my kubernetes 1.10.3. That issue disappeared.
|
I am going to prepare two PRs instead of this one. |
|
@IvanovOleg closing this PR as you opened a new PR #154 |
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #108Special notes for your reviewer:
Could you please at least build it and upload artifacts somewhere? Because this problem blocks me and I can't build it myself because of #148
Release note: