Add events for container release

[lelenanam]

For ReleaseEventMetadata use spec with both types: ReleaseImageSpec and ReleaseContainersSpec.

Need this for weaveworks/service#2277

[squaremo]

Elena, I reckon this needs a test that makes sure it will decode old serialisations of release metadata -- even if it's just to prevent regressions. I've elaborated in a comment.

[lelenanam]

Right, but if I use embedded structs there is another issue here: json.Marshal skips embedded fields with the same names like Kind and Force (ambiguous selector?):

type ReleaseImageSpec struct {
    ServiceSpecs    []ResourceSpec
    ImageSpec    ImageSpec
    Kind        ReleaseKind
    Excludes    []flux.ResourceID
    Force        bool
}

type ReleaseContainersSpec struct {
    Kind        ReleaseKind
    ContainerSpecs    map[flux.ResourceID][]ContainerUpdate
    SkipMismatches    bool
    Force        bool
}

So now I tend to use a separate type for ReleaseContainersSpec, like:

type ReleaseContainersEventMetadata struct {
	ReleaseEventCommon
	Spec  update.ReleaseContainersSpec `json:"spec"`
	Cause update.Cause                 `json:"cause"`
}

[squaremo]

json.Marshal skips embedded fields with the same names like Kind and Force (ambiguous selector?):

Argh, I knew there was a catch.

How about something like

func (s *ReleaseSpec) UnmarshalJSON(b []byte) error {
  type T ReleaseSpec
  t := (*T)(s)
  if err := json.Unmarshal(b, t); er != nil {
    return err
  }

  switch t.Type {
    case "":
      // it's an old-style inline update.ReleaseSpec
      var r update.ReleaseSpec
      json.Unmarshal(b, &r)
      s.ReleaseImageSpec = &r
      s.Type = ReleaseImageSpecType
    case ReleaseContainersSpecType, ReleaseImageSpecType:
      // great, we got a new style value
    default:
      return errors.New("unknown type")
  }
  return nil
}

EDIT: alias -> type (fingers faster than brain)

[lelenanam]

Not sure what it means:

alias T ReleaseSpec
  t := (*T)(s)

but if I marshal a struct with named fields:

type ReleaseSpec struct {
	Type                  string
	ReleaseImageSpec      *update.ReleaseImageSpec
	ReleaseContainersSpec *update.ReleaseContainersSpec
}

and then try to unmarshal it, I have runtime: goroutine stack exceeds 1000000000-byte limit fatal error: stack overflow during unmarshal because of recursion calls in UnmarshalJSON.

[squaremo]

and then try to unmarshal it, I have runtime: goroutine stack exceeds 1000000000-byte limit fatal error: stack overflow during unmarshal because of recursion calls in UnmarshalJSON.

For posterity: the purpose of the type alias is to create a type that has the same fields (and can therefore be unmarshalled into) but not the UnmarshalJSON method; it's having the latter that leads to stack overflow.

[lelenanam]

@squaremo Added UnmarshalJSON method, PTAL.

[squaremo]

There's a bit of extra decoding which I don't think is necessary (see comments). Also, can you make sure you're using the most recent dep (v0.5.0); I wouldn't expect any changes to Gopkg.lock (and those that are there look like formatting/metadata changes).

[squaremo]

t is already a pointer *T, so you don't need to pass its address.

[lelenanam]

Fixed.

[squaremo]

r is already a pointer; but moreover, this decoding has already been done, in line 264. I think this case is a no-op -- all the fields will have been decoded. (Is there a test that doesn't pass unless this line is present?)

[lelenanam]

Fixed.

[squaremo]

Looks good, thanks Elena!