Skip to content
This repository has been archived by the owner on Jul 1, 2023. It is now read-only.

Properly parse Gravity 5.0.36 status #262

Merged
merged 4 commits into from
Aug 20, 2020

Conversation

wadells
Copy link
Contributor

@wadells wadells commented Aug 19, 2020

Description

This PR fixes a Gravity 5.0.x compatiblity regression introduced by #233. Specifically, when trying to parse the output of gravity status --output=json on 5.0.36 robotest would fail with the following error: json: cannot unmarshal string into Go struct field.

Risk Profile

  • Bug fix (patch release)

Related Issues

Fixes #247.

Testing Done

See the unit tests added. Here is what it looks like to run them:

go test ./infra/gravity before the second commit
walt@work:~/git/robotest$ go test ./infra/gravity/
--- FAIL: TestGravity5036ActiveStatusValidation (0.00s)
    gravity_test.go:127: 
                Error Trace:    gravity_test.go:127
                Error:          Received unexpected error:
                                json: cannot unmarshal string into Go struct field ClusterStatus.cluster.system_status of type int
                Test:           TestGravity5036ActiveStatusValidation
    gravity_test.go:130: 
                Error Trace:    gravity_test.go:130
                Error:          Received unexpected error:
                                expected system_status 1, found 0
                Test:           TestGravity5036ActiveStatusValidation
--- FAIL: TestGravity5036DegradedStatusValidation (0.00s)
    gravity_test.go:147: 
                Error Trace:    gravity_test.go:147
                Error:          Received unexpected error:
                                json: cannot unmarshal string into Go struct field ClusterStatus.cluster.system_status of type int
                Test:           TestGravity5036DegradedStatusValidation
FAIL
FAIL    github.com/gravitational/robotest/infra/gravity 0.013s
FAIL
go test ./infra/gravity after
walt@work:~/git/robotest$ go test ./infra/gravity/
ok      github.com/gravitational/robotest/infra/gravity (cached)

Here is a complete 5.0.36 install: logs

Additional Information

I considered splitting the datamodel, such that GravityStatus became an interface fufilled by two different implementations: Gravity50ClusterStatus and Gravity52ClusterStatus (or the like). I chose not to go with this approach because:

  • It is more invasive. It required rewriting every signature and piece of code that consumes GravityStatus or any of its member fields.
  • The life expectancy of this code is not that long. 5.0.x has been out of support for over a year (EoS April 13, 2019), and at some point we'll stop testing --upgrade-via tests from this release whereupon all this code can be removed wholesale.

These two tests ensure that Robotest continues to work correctly when
interacting with old versions of gravity (5.0.x).

Contributes to gravitational#247.
For backwards compatibility the `system_status` json field needs to be
able to accept `running` or `degraded` in addition to the integer
values.

Fixes gravitational#247.
infra/gravity/node_commands.go Outdated Show resolved Hide resolved
infra/gravity/node_commands.go Outdated Show resolved Hide resolved
infra/gravity/node_commands.go Outdated Show resolved Hide resolved
This is a bit easier to read, because the short branch of the if
statement is first.
We always want to use the custom umarshalling.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot unmarshal string into Go struct field ClusterStatus.system_status of type int
2 participants