Skip to content

Commit

Permalink
reconciler: Fix Status JSON marshalling
Browse files Browse the repository at this point in the history
43587d4 broke JSON marshalling of Status by introducing a separate statusJSON
type and Unmarshal{JSON,YAML} methods, but didn't introduce the Marshal{JSON,YAML}
methods, which caused dictionary key mismatches as Status struct was used to marshal
instead of statusJSON.

Fix this by removing the whole statusJSON construct and add json and yaml tags to Status.
There is no need for filling in the `id` field when unmarshalling Status as this is completely
internal to the reconciler.

Fixes: 43587d4 ("reconciler: Implement tests using scripttest")

Signed-off-by: Jussi Maki <[email protected]>
  • Loading branch information
joamaki committed Oct 31, 2024
1 parent e0d473d commit 132edd6
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 37 deletions.
51 changes: 51 additions & 0 deletions reconciler/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,57 @@ func TestStatusString(t *testing.T) {
assert.Regexp(t, `Error: hey I'm an error \([0-9]+\.[0-9]+.+s ago\)`, s.String())
}

func TestStatusJSON(t *testing.T) {
testCases := []struct {
s Status
expected string
}{
{
Status{
Kind: StatusKindDone,
UpdatedAt: time.Unix(1, 0).UTC(),
Error: "",
},
`{"kind":"Done","updated-at":"1970-01-01T00:00:01Z"}`,
},
{
Status{
Kind: StatusKindPending,
UpdatedAt: time.Unix(2, 0).UTC(),
Error: "",
},
`{"kind":"Pending","updated-at":"1970-01-01T00:00:02Z"}`,
},
{
Status{
Kind: StatusKindError,
UpdatedAt: time.Unix(3, 0).UTC(),
Error: "some-error",
},
`{"kind":"Error","updated-at":"1970-01-01T00:00:03Z","error":"some-error"}`,
},
{
Status{
Kind: StatusKindRefreshing,
UpdatedAt: time.Unix(4, 0).UTC(),
Error: "",
},
`{"kind":"Refreshing","updated-at":"1970-01-01T00:00:04Z"}`,
},
}

for _, tc := range testCases {
b, err := json.Marshal(tc.s)
assert.NoError(t, err, "Marshal")
assert.Equal(t, tc.expected, string(b))

var s Status
assert.NoError(t, json.Unmarshal(b, &s), "Unmarshal")
assert.Equal(t, tc.s, s)
}

}

func sanitizeAgo(s string) string {
r := regexp.MustCompile(`\(.* ago\)`)
return string(r.ReplaceAll([]byte(s), []byte("(??? ago)")))
Expand Down
40 changes: 3 additions & 37 deletions reconciler/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/cilium/statedb"
"github.com/cilium/statedb/index"
"github.com/cilium/statedb/internal"
"gopkg.in/yaml.v3"
)

type Reconciler[Obj any] interface {
Expand Down Expand Up @@ -132,9 +131,9 @@ func (s StatusKind) Key() index.Key {
// the reconciler. Object may have multiple reconcilers and
// multiple reconciliation statuses.
type Status struct {
Kind StatusKind
UpdatedAt time.Time
Error string
Kind StatusKind `json:"kind" yaml:"kind"`
UpdatedAt time.Time `json:"updated-at" yaml:"updated-at"`
Error string `json:"error,omitempty" yaml:"error,omitempty"`

// id is a unique identifier for a pending object.
// The reconciler uses this to compare whether the object
Expand All @@ -144,39 +143,6 @@ type Status struct {
id uint64
}

// statusJSON defines the JSON/YAML format for [Status]. Separate to
// [Status] to allow custom unmarshalling that fills in [id].
type statusJSON struct {
Kind string `json:"kind" yaml:"kind"`
UpdatedAt time.Time `json:"updated-at" yaml:"updated-at"`
Error string `json:"error,omitempty" yaml:"error,omitempty"`
}

func (sj *statusJSON) fill(s *Status) {
s.Kind = StatusKind(sj.Kind)
s.UpdatedAt = sj.UpdatedAt
s.Error = sj.Error
s.id = nextID()
}

func (s *Status) UnmarshalYAML(value *yaml.Node) error {
var sj statusJSON
if err := value.Decode(&sj); err != nil {
return err
}
sj.fill(s)
return nil
}

func (s *Status) UnmarshalJSON(data []byte) error {
var sj statusJSON
if err := json.Unmarshal(data, &sj); err != nil {
return err
}
sj.fill(s)
return nil
}

func (s Status) IsPendingOrRefreshing() bool {
return s.Kind == StatusKindPending || s.Kind == StatusKindRefreshing
}
Expand Down

0 comments on commit 132edd6

Please sign in to comment.