spec icon indicating copy to clipboard operation
spec copied to clipboard

service.upgraded can be merged to service.deployed

Open xibz opened this issue 8 months ago • 2 comments

I think we should just have service.deployed and drop service.upgraded. They’re the same event in practice, and the only difference is contextual, like whether a previous artifact existed. That can easily be captured in an optional field like previousArtifact or previousState.

Having two separate events for this adds complexity without adding any real benefit to interoperability. It makes consumer logic messier and bloats the spec with redundant types. Most systems like Kubernetes or GitHub Actions just treat this as a deploy with metadata — not a totally different event type.

Unless there’s a strong reason this distinction needs to happen at the type level, this feels like unnecessary overhead. Keeping it as a single event with metadata is simpler, more flexible, and easier to work with across the board.

xibz avatar Apr 18 '25 16:04 xibz

I agree,

  • In my dashboards, I merge deployed, upgraded and rolledback as a deployment
  • In the event generator, the distinction introduces a bit more complexity

eg https://github.com/cdviz-dev/cdviz-collector/blob/829b8f6f3ab6124d72d9508e9e7d899c0f47399c/config/transformers/kubewatch_cloudevents.vrl#L83

    # removed = container_name no longer used (ignore the image)
    # upgraded = container_name exist in both old and new, but image is different
    # created = container_name is new but not in old
    # same => no event/output

    for_each(new_containers) -> |_index, container| {
      previous = filter(old_containers) -> |_index, old_container| {
        old_container.container_name == container.container_name
      }
      type = if is_empty(previous) {
        type = "dev.cdevents.service.deployed.0.2.0"
      } else if (previous[0].repository_url != container.repository_url) || (previous[0].tag != container.tag) || (previous[0].version != container.version) {
        type = "dev.cdevents.service.upgraded.0.2.0"
      } else {
        null
      }

But if the merge implies moving (and not removing) the distinction via metadata or fields, I don't see the gain. And if it implies having a different "structure", then I prefer to keep them separated.

davidB avatar May 12 '25 12:05 davidB

@davidB

But if the merge implies moving (and not removing) the distinction via metadata or fields, I don't see the gain. And if it implies having a different "structure", then I prefer to keep them separated.

I dont think there should be ANY additional metadata between these events. They are all deployments at the end of the day, and the difference is merely contextual.

Now, we could have a single enum that may help distinguish the types, but at the end of the day that would be optional and the fields would be the same.

xibz avatar May 12 '25 17:05 xibz