dock icon indicating copy to clipboard operation
dock copied to clipboard

Published 20 hours ago • verified publisher icon sodafoundation

Bump up Golang version

Open vharsh opened this issue 2 years ago • 9 comments

What type of PR is this? /kind cleanup #SODACODE2022

What this PR does / why we need it:

  • Bumps up Golang version

Which issue(s) this PR fixes:

Fixes #83

Test Report Added?:

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line: /kind TESTED

/kind NOT-TESTED

Special notes for your reviewer:

Breaking changes

  1. Post Golang 1.16, calls to os.Exit(0) will now fail the unit tests.
  2. [WIP] Some Metrics tests are failing for Ceph, I'm taking a look at them.

vharsh avatar Mar 07 '22 16:03 vharsh

Could you please provide the testing done after the version upgrade? Build and test updates can be provided. I see the CI failed with the upgrade!

skdwriting avatar Mar 15 '22 16:03 skdwriting

Please mark the PR for #SODACODE2022 in the description

skdwriting avatar Mar 17 '22 05:03 skdwriting

@vharsh Can you please look at the failing CI jobs

asifdxtreme avatar Mar 17 '22 07:03 asifdxtreme

@vharsh Can you please look at the failing CI jobs

I'm on it, it has broken from Golang 1.14 and it's lacking some metric Component name from the 15th metric, I'm trying to understand how the tests were passing all this while.

vharsh avatar Mar 17 '22 12:03 vharsh

@vharsh thanks...

skdwriting avatar Mar 18 '22 18:03 skdwriting

I did some debugging again this long weekend, sharing all what I've found.

What factors are causing the Ceph metrics to fail?

  • The upgraded Golang(from 1.14) version has some changes to json.Unmarshall(....), which is not allowing stringified values like "osd.0"(which clearly isn't a floating point Number) into the json.Number type (which is an alias for the string type) into the Name field( from metrics_cli.go -> cephOSDDF.OSDNodes[*].Name ), which should likely have been a string, a JSON unmarshall operation is failing.
  • I've verified the expected values changes across go 1.13.x & v1.18.x, there have been some change in behaviour and this cannot be fixed by just fixing the expectedValues in the unit-test.

Some errors only visible in the newer Golang v1.17 or v1.18

E0419 12:02:09.894663    8531 metrics_cli.go:52] file ReadDefaultConfigFile can't readrados: No such file or directory
E0419 12:02:09.895347    8531 metrics_cli.go:461] unmarshal failed

Things I need to look further:

  • Go-Ceph bindings have are pinned to a very old revision from 2017, some changes might be required to change the expected internal types to string.
  • Should I bump up the Go-Ceph bindings to a release post 2020, so that dock users can get newer Ceph versions and likely the more supported & stable versions?

vharsh avatar Apr 19 '22 06:04 vharsh

@vharsh Thanks for debugging this, feel free to bump the go-ceph to newer versions.

asifdxtreme avatar Apr 19 '22 07:04 asifdxtreme

@vharsh Can you please look at @asifdxtreme suggestion and check further, thanks

sushanthakumar avatar May 05 '22 06:05 sushanthakumar

@sushanthakumar Yeah, I was investigating this last weekend, I'll take a look at it soon-ish.

vharsh avatar May 05 '22 12:05 vharsh