spec icon indicating copy to clipboard operation
spec copied to clipboard

May runtimes support fields that are not defined by the spec?

Open negz opened this issue 4 years ago • 10 comments

Does the OAM spec require runtimes support ONLY the fields defined by the spec, or may they choose to support additional fields?

Currently the OAM spec does not touch on the status field that is common in Kubernetes resources - it describes only apiVersion, kind, metadata, and spec. The nascent Crossplane implementation of the v1alpha2 OAM spec therefore assumes it may store whatever it needs in the status of the various CRs (e.g. ApplicationConfiguration, ContainerizedWorkload, etc).

In addition to resource status, there's a few cases in which a runtime might want to set fields under metadata or spec that are not covered by the OAM specification. Some examples include:

  1. Both Rudr and Crossplane set metadata.ownerReferences.
  2. Per https://github.com/oam-dev/spec/issues/323 it would be useful to maintain a reference from a trait to a workload at the trait instance's spec.workloadRef.
  3. It would be useful for a container in the spec.containers array of a ContainerizedWorkload to be able to use existing Kubernetes valueFrom functionality to create an environment variable from a configmap or a secret - concepts that do not exist in OAM.

Presumably if an OAM runtime were allowed to support spec fields that were not described by the OAM specification these fields would either be:

  1. Expected to be only set automatically by some OAM controller.
  2. Optional fields expected to be set by an OAM persona (e.g. infra op, app op, app dev).

Note that setting an optional spec field that was not supported by the OAM specification (e.g. to set a container's env vars from a Kubernetes secret) would mean sacrificing cross-runtime portability. An AppConfig that leveraged the optional fields of runtime A could not be used with runtime B. I believe cross-runtime portability is not a goal of OAM - it's already not guaranteed that an AppConfig written for runtime A will work with runtime B if the AppConfig uses standard or extended workload or trait kinds.

negz avatar Mar 03 '20 21:03 negz

I had the same thoughts. The concern was that we may ended up with multiple runtimes that looks too similar to their underlying service providers

zhxu2 avatar Mar 04 '20 02:03 zhxu2

  • Expected to be only set automatically by some OAM controller.
  • Optional fields expected to be set by an OAM persona (e.g. infra op, app op, app dev

IIRC, the first case doesn't seem to affect interoperability. The second case seems to violate the spec in the sense that if it requires its user to fill in a field not in the OAM spec.

ryanzhang-oss avatar Mar 04 '20 06:03 ryanzhang-oss

The second case seems to violate the spec in the sense that if it requires its user to fill in a field not in the OAM spec.

I'm not sure it's that cut and dry. Wouldn't whether it violated the spec depend on whether we defined the spec as the minimum set of fields that must be supported vs the only set of fields that should be supported?

negz avatar Mar 04 '20 19:03 negz

Just to note, the spec already allows some fields to be optionally supported, so OAM objects are not strictly portable today. I assume the reasoning there is that we want to acknowledge that some runtimes may differ, but want to make sure they don't differ too much. I am not sure this model is sustainable long-term though.

hasheddan avatar Mar 04 '20 19:03 hasheddan

That is a great question. We purposely put explicit extension points in the spec with workload types, traits, and scopes so that it's obvious where the differences in implementations would be. We haven't made a strong statement about the case of adding fields in places not designed to be extension points. I suspect it may have been implied that implementations aren't supposed to make changes to the schema of core types, but maybe it should be explicitly stated.

I suppose an argument could be made that if a runtime adds its own fields in random places and expects a user (app dev or app ops) to set that field, then it's not really OAM anymore, but something OAM-like.

A good example here would be the "core" parts of OAM. Anything that is "core" is meant to be portable, so adding new fields would seem to violate that principle.

Implementations can technically work around this by defining their own workload types, traits, and scopes that have all the fields that they need, but I think it would be problematic if every implementation defined its own slight variation of core workload types, traits, and scopes.

Those are good examples you brought up, @negz. I wonder if it's good enough to address these as they come up with solutions directly in the spec, and what do we do if there is a long tail of these things?

vturecek avatar Mar 11 '20 21:03 vturecek

@negz @hasheddan Though open for discussion, I personally think the quick answer for core workload/trait/scope is: NO - newly added fields should be introduced into the spec, and I'd suggest they should go thru formal review.

The spec.workloadRef is really a special case because another runtime could choose to use annotation instead, i.e. this "internal impl detail only" field doesn't have to be part of spec and it won't affect portability. Not sure how we can clarify it clearly though.

On the other hand, I proposed to use k8s metadata directly in the spec. It's definitely non-goal to reinvent this field and this idea applies to status field - we will need to define it clearly for core types.

And if we want strong enforcement, we will have to enforce all "OAM runtimes" install our core workload/trait/scope controllers (the idea of OAM core lib) to ensure 100% portability.

Not sure if I missed other cases may break the portability?

resouer avatar Mar 21 '20 01:03 resouer

I agree with @resouer but I want to add some of my points:

  1. Runtime could add new fields if end users will never use it directly. ownerReferences and workloadRef are both good examples as end users will never need to specify these two fields in AppConfig. They are automatically added by implementations. So, this means runtime could add some fields for implement convenience, but these fields won't be perceived by end users.
  2. We don't need to enforce all "OAM runtimes" install our core workload/trait/scope controllers, and we can't. We only need to clarify in our SPEC that one runtime can be called as OAM runtime only when all core OAM things are supported by that runtime. Which means runtime itself should be responsible for core OAM spec.
  3. We could use MUST, SHOULD in our notational_convention doc for clarify things. We could setup a folder called best practices, which will contain all implementation suggestions.

wonderflow avatar Mar 22 '20 03:03 wonderflow

For early stage, I agree we may want to avoid "strong enforcement". Instead, a verification/conformance tests for core workload/trait/scope would make more sense.

/cc @HaishiBai, I recalled you were investigating OAM conformance thing.

resouer avatar Mar 22 '20 04:03 resouer

@negz @hasheddan Though open for discussion, I personally think the quick answer for core workload/trait/scope is: NO - newly added fields should be introduced into the spec, and I'd suggest they should go thru formal review.

The spec.workloadRef is really a special case because another runtime could choose to use annotation instead, i.e. this "internal impl detail only" field doesn't have to be part of spec and it won't affect portability. Not sure how we can clarify it clearly though.

On the other hand, I proposed to use k8s metadata directly in the spec. It's definitely non-goal to reinvent this field and this idea applies to status field - we will need to define it clearly for core types.

And if we want strong enforcement, we will have to enforce all "OAM runtimes" install our core workload/trait/scope controllers (the idea of OAM core lib) to ensure 100% portability.

Not sure if I missed other cases may break the portability?

I think what @negz was suggesting is that should one runtime choose to support its own runtime-friendly optional fields. Portability is not a goal of OAM while I believe those same configs strictly following OAM spec should be ready to deploy onto any runtimes without twisting.

Curious of what "OAM conformance" are we referring to here? I am also wondering if infra op, app op, app dev has an opinion towards the metadata fields, would there be a way to support those?

zhxu2 avatar Mar 23 '20 21:03 zhxu2

Portability is not a goal of OAM

I don't have opinion on this but from my previous understanding with other maintainers, it seems to be a goal. /cc @vturecek

Curious of what "OAM conformance" are we referring to here?

It's not well defined for now. It could be similar to Kubernetes e2e conformance tests, or strict API object level verification. In either case, the expected result will be similar to CNCF conformance certification in my mind.

I am also wondering if infra op, app op, app dev has an opinion towards the metadata fields, would there be a way to support those?

My 2c is, in most cases, they can live peacefully as annotations instead of metadata fields, unless they are common enough to be promoted and passed formal review process.

resouer avatar Mar 23 '20 22:03 resouer