Alpha fields

[Tim Allclair]

In last weeks sig-architecture review of RuntimeClass, there was an inconclusive discussion of how to reference alpha-APIs from stable objects. In this specific case, the question was how to reference the RuntimeClass instance from a Pod, but the problem applies generally to all stable API objects.

I want to kick off the follow-up discussion, since I believe a lot of projects are currently blocked on this. The solutions discussed so far include:

- Allow annotations, with better guidelines around best practices

- Add support for feature-gated fields

- Add an "extensions" list to the ObjectMeta, which can reference arbitrary resources

- Parallel "sidecar" resources that hold the additional fields - either with names matching the extended resource, or a back reference. Either way, this runs into lifecycle issues (how does it work with replication? label-selector?).

We are planning to discuss this in this weeks sig-architecture meeting, but to help us reach a conclusion & unblock these projects, let's start the discussion here.

[Clayton]

On Aug 13, 2018, at 8:07 PM, 'Tim Allclair' via kubernetes-sig-architecture <kubernetes-si...@googlegroups.com> wrote:

In last weeks sig-architecture review of RuntimeClass, there was an inconclusive discussion of how to reference alpha-APIs from stable objects. In this specific case, the question was how to reference the RuntimeClass instance from a Pod, but the problem applies generally to all stable API objects.

I want to kick off the follow-up discussion, since I believe a lot of projects are currently blocked on this. The solutions discussed so far include:

- Allow annotations, with better guidelines around best practices

- Add support for feature-gated fields

We already support and recommend this for most things - I’m not sure why this was not the preferred choice.  Is it about discoverability of the field?  Were there other options?

- Add an "extensions" list to the ObjectMeta, which can reference arbitrary resources

Do we have any other example use cases where a pattern like this would have been useful?

- Parallel "sidecar" resources that hold the additional fields - either with names matching the extended resource, or a back reference. Either way, this runs into lifecycle issues (how does it work with replication? label-selector?).

For services and endpoints this already works, but the objects were designed with that parallelism in mind.

Agree replication for pods makes this questionable.

We are planning to discuss this in this weeks sig-architecture meeting, but to help us reach a conclusion & unblock these projects, let's start the discussion here.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
To post to this group, send email to kubernetes-si...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CADtktAUnY2TF8i9_tXGGJcpC_qgVWO-zvWnq0O37G%2BKYvvBQgQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Jordan Liggitt]

On Aug 13, 2018, at 8:15 PM, Clayton <smarter...@gmail.com> wrote:

On Aug 13, 2018, at 8:07 PM, 'Tim Allclair' via kubernetes-sig-architecture <kubernetes-si...@googlegroups.com> wrote:

In last weeks sig-architecture review of RuntimeClass, there was an inconclusive discussion of how to reference alpha-APIs from stable objects. In this specific case, the question was how to reference the RuntimeClass instance from a Pod, but the problem applies generally to all stable API objects.

I want to kick off the follow-up discussion, since I believe a lot of projects are currently blocked on this. The solutions discussed so far include:

- Allow annotations, with better guidelines around best practices

- Add support for feature-gated fields

We already support and recommend this for most things - I’m not sure why this was not the preferred choice.  Is it about discoverability of the field?  Were there other options?

The gating support is incomplete. The field still shows up in schemas, documentation, etc. 

- Add an "extensions" list to the ObjectMeta, which can reference arbitrary resources

Do we have any other example use cases where a pattern like this would have been useful?

- Parallel "sidecar" resources that hold the additional fields - either with names matching the extended resource, or a back reference. Either way, this runs into lifecycle issues (how does it work with replication? label-selector?).

For services and endpoints this already works, but the objects were designed with that parallelism in mind.

Agree replication for pods makes this questionable.

We are planning to discuss this in this weeks sig-architecture meeting, but to help us reach a conclusion & unblock these projects, let's start the discussion here.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
To post to this group, send email to kubernetes-si...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CADtktAUnY2TF8i9_tXGGJcpC_qgVWO-zvWnq0O37G%2BKYvvBQgQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
To post to this group, send email to kubernetes-si...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/672E5EC7-2646-44A3-A198-D5FC64C781A0%40gmail.com.

[Clayton]

Can we do better and kill two birds with one stone?  Ie Mark fields with @alpha Gate and propagate?

[David Oppenheimer]

Clarification question: is "reference alpha-APIs from stable objects" the same as adding an alpha field to a stable object (i.e. the use case described here)?

(I assume it is, since I would assume that the field that references the alpha API object must itself be an alpha field, but just wanted to make sure.)

On Mon, Aug 13, 2018 at 5:29 PM, Clayton <smarter...@gmail.com> wrote:

Can we do better and kill two birds with one stone?  Ie Mark fields with @alpha Gate and propagate?

On Aug 13, 2018, at 8:27 PM, Jordan Liggitt <jlig...@redhat.com> wrote:

On Aug 13, 2018, at 8:15 PM, Clayton <smarter...@gmail.com> wrote:

On Aug 13, 2018, at 8:07 PM, 'Tim Allclair' via kubernetes-sig-architecture <kubernetes-sig-architecture@googlegroups.com> wrote:

In last weeks sig-architecture review of RuntimeClass, there was an inconclusive discussion of how to reference alpha-APIs from stable objects. In this specific case, the question was how to reference the RuntimeClass instance from a Pod, but the problem applies generally to all stable API objects.

I want to kick off the follow-up discussion, since I believe a lot of projects are currently blocked on this. The solutions discussed so far include:

- Allow annotations, with better guidelines around best practices

- Add support for feature-gated fields

We already support and recommend this for most things - I’m not sure why this was not the preferred choice.  Is it about discoverability of the field?  Were there other options?

The gating support is incomplete. The field still shows up in schemas, documentation, etc. 

- Add an "extensions" list to the ObjectMeta, which can reference arbitrary resources

Do we have any other example use cases where a pattern like this would have been useful?

- Parallel "sidecar" resources that hold the additional fields - either with names matching the extended resource, or a back reference. Either way, this runs into lifecycle issues (how does it work with replication? label-selector?).

For services and endpoints this already works, but the objects were designed with that parallelism in mind.

Agree replication for pods makes this questionable.

We are planning to discuss this in this weeks sig-architecture meeting, but to help us reach a conclusion & unblock these projects, let's start the discussion here.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-architecture+unsub...@googlegroups.com.
To post to this group, send email to kubernetes-sig-architecture@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CADtktAUnY2TF8i9_tXGGJcpC_qgVWO-zvWnq0O37G%2BKYvvBQgQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-architecture+unsub...@googlegroups.com.
To post to this group, send email to kubernetes-sig-architecture@googlegroups.com.

[Jordan Liggitt]

On Aug 13, 2018, at 9:14 PM, David Oppenheimer <davi...@google.com> wrote:

Clarification question: is "reference alpha-APIs from stable objects" the same as adding an alpha field to a stable object (i.e. the use case described here)?

Yes

(I assume it is, since I would assume that the field that references the alpha API object must itself be an alpha field, but just wanted to make sure.)

On Mon, Aug 13, 2018 at 5:29 PM, Clayton <smarter...@gmail.com> wrote:

Can we do better and kill two birds with one stone?  Ie Mark fields with @alpha Gate and propagate?

Yes. Something like that was proposed but not yet built (or fully understood w.r.t. the implications to the way we decode into structs). The "clear before validate/persist" approach we have now was a minimal stop-gap to unblock work held up on the annotations-vs-fields decision. 

[Clayton]

On Aug 13, 2018, at 9:22 PM, Jordan Liggitt <jlig...@redhat.com> wrote:

On Aug 13, 2018, at 9:14 PM, David Oppenheimer <davi...@google.com> wrote:

Clarification question: is "reference alpha-APIs from stable objects" the same as adding an alpha field to a stable object (i.e. the use case described here)?

Yes

(I assume it is, since I would assume that the field that references the alpha API object must itself be an alpha field, but just wanted to make sure.)

On Mon, Aug 13, 2018 at 5:29 PM, Clayton <smarter...@gmail.com> wrote:

Can we do better and kill two birds with one stone?  Ie Mark fields with @alpha Gate and propagate?

Yes. Something like that was proposed but not yet built (or fully understood w.r.t. the implications to the way we decode into structs). The "clear before validate/persist" approach we have now was a minimal stop-gap to unblock work held up on the annotations-vs-fields decision. 

It hasn’t worked out that badly in that we haven’t yet regretted it.

The best benefit of the field is that it shows up in docs, has clear semantics, and can be validated.  A generic metadata field can’t do that.  A sidecar policy resource isn’t necessarily discoverable (although we could also improve that, and I like sidecars like pod preset).  An argument against would be that we can’t grow the scope of pod forever, but runtime class seems pretty fundamental to the pod.

[Tim Hockin]

I intended to send a very similar note, Tim. Thanks.

I took away 3 main topics from last week's call and the subsequent
chatter in hallways.

1) "Tag-along" resources. AKA external annotations. AKA Decorator
resources. We've seen this pattern in a bunch of places. It's
interesting. Define a CRD that is a sort of "subordinate" resource to
some primary resource. Generally this is a 1:1 relationship (by
name), but maybe we need more than that (e.g. by selector) E.g. a
CSINode is 1:1 with Node. I have seen people extend Service this way.
It allows someone to provide a structured, validated, (etc) block of
config that is not part of the main resource's API. Better than
annotations because of structure. I don't think this is the main
topic of the RuntimeClass proposal, but I want to pull this pattern to
the front of people's minds anyway. I like it. I think we should
endorse it and maybe formalize it (e.g. formalize a reference from
primary object to subordinate, so clients can auto-find them, and so
delete handline or GC can deal with them).

2) How to link a primary object to a new alpha resource. This pattern
has also come up many times. See volume snapshots for another
example. We have a new (alpha) resource that extends functionality.
We have a stable object that we want to add a small number of fields
(usually 1) that reference that new resource. If we add a field, it
shows up in docs even though it is alpha. If we use an annotation,
well, we've been dos that road in general. Maybe annotations are not
SO bad for alpha ONLY, but the structure issue still exists (e.g. new
thing per URL path in ingress == JSON blobs in a string annotation).
Field gating should help make the docs problem go away, but seems
complex.

3) Lenses. This was not discussed in call, but after. It's related
to, but not the same as flag gating fields. With a flag gated field,
a user can query the API endpoint and either see or not see the field,
based ONLY on flag in the master. The field is not obviously alpha or
beta, unless you read the docs. The idea of a lens is that you're
ASKING to see alpha or beta fields (or resources). It maybe *also*
wants per-field gating or maybe not. This is also sort of API
versioning, but somewhat lower overhead. It's not simply asking for
v1alpha1, since that implies being before v1. Think of it as asking
for Pod v1 PLUS alpha fields. If we versioned the API on every field
addition, it might come close.

All of these are related, somewhat, in my mind. They all speak to how
we extend and evolve the API without exposing the details to people
who don't want/need to see it.

>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
>> To post to this group, send email to kubernetes-si...@googlegroups.com.

>> To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CADtktAUnY2TF8i9_tXGGJcpC_qgVWO-zvWnq0O37G%2BKYvvBQgQ%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.

>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
>> To post to this group, send email to kubernetes-si...@googlegroups.com.

[Daniel Smith]

On Tue, Aug 14, 2018 at 12:04 PM Tim Hockin <tho...@google.com> wrote:

I intended to send a very similar note, Tim.  Thanks.

I took away 3 main topics from last week's call and the subsequent
chatter in hallways.

1) "Tag-along" resources.  AKA external annotations.  AKA Decorator
resources.  We've seen this pattern in a bunch of places.  It's
interesting.  Define a CRD that is a sort of "subordinate" resource to
some primary resource.  Generally this is a 1:1 relationship (by
name), but maybe we need more than that (e.g. by selector) E.g. a
CSINode is 1:1 with Node.  I have seen people extend Service this way.
It allows someone to provide a structured, validated, (etc) block of
config that is not part of the main resource's API.  Better than
annotations because of structure.  I don't think this is the main
topic of the RuntimeClass proposal, but I want to pull this pattern to
the front of people's minds anyway.  I like it.  I think we should
endorse it and maybe formalize it (e.g. formalize a reference from
primary object to subordinate, so clients can auto-find them, and so
delete handline or GC can deal with them).

What's wrong with a regular OwnerReference? It would go on the CRD and reference the beta/stable object. This avoids polluting the beta/stable object, which seems good.

But has anyone thought about how we'd promote such a thing back into the main object?

[Tim Allclair]

There are lots of challenges with owner references on the CRD. How do you deal with replication? How do you enforce a 1:1 mapping? How do you validate deletion? I also worry about scalability issues for pods, where the kubelet would need to search over every CRD instance to find the one referencing the pod.

[Daniel Smith]

I think you have none of those problems if you require the name to match.

[Tim Allclair]

You still need to deal with replication, or generated names more generally

[Daniel Smith]

Can you give a concrete scenario?

[Tim Allclair]

I'll give a RuntimeClass example, but I think this pattern is a common one for features that need to be added to the pod.

The field I want to add to the pod for RuntimeClass is just a reference to a RuntimeClass that the pod should be run with. IIUC, what is being proposed is another CRD, let's call it "RuntimeClassRef", that has the same name/namespace as the pod it is adding the RuntimeClass for, and a reference to the desired RuntimeClass. More concretely:

I have a pod "wordpress" that should reference the RuntimeClass "sandboxed", so I create a RuntimeClassRef named "wordpress", with a reference to "sandboxed".

Now what happens when rather than a wordpress pod, I have a wordpress deployment? I suppose I could add a custom controller that creates RuntimeClassRefs for each replica, but now I need another CRD for ReplicaSets, DaemonSets, JobSets, and any other pod generator (still missing custom ones).

We could encode the type into the name, and have a generic controller that watches for pods with an owner ref to an object with the same name as a RuntimeClassRef, and then generate additional Refs for those pods, but that sounds pretty messy and gets back to the issues of watching all pods. It also feels like it doesn't actually solve the problems with annotations (multiple ways to specify something), and just has a worse user & developer experience.

[Daniel Smith]

On Tue, Aug 14, 2018 at 2:18 PM Tim Allclair <tall...@google.com> wrote:

I'll give a RuntimeClass example, but I think this pattern is a common one for features that need to be added to the pod.

The field I want to add to the pod for RuntimeClass is just a reference to a RuntimeClass that the pod should be run with. IIUC, what is being proposed is another CRD, let's call it "RuntimeClassRef", that has the same name/namespace as the pod it is adding the RuntimeClass for, and a reference to the desired RuntimeClass. More concretely:

I have a pod "wordpress" that should reference the RuntimeClass "sandboxed", so I create a RuntimeClassRef named "wordpress", with a reference to "sandboxed".

Now what happens when rather than a wordpress pod, I have a wordpress deployment? I suppose I could add a custom controller that creates RuntimeClassRefs for each replica, but now I need another CRD for ReplicaSets, DaemonSets, JobSets, and any other pod generator (still missing custom ones).

It's much easier than that, fortunately. You just need to use an admission webhook or an initializer (which is on the chopping block but what you just described might make a legitimate use case) to hold off construction of the pod while you make the ancillary resource.

[Tim Allclair]

Hmm, I don't follow. Is the initializer on the pod or the ReplicaSet? Maybe we can chat offline.

I'd like to revisit the annotations. I think we can solve a lot of the problems with a few restrictions:

1. Value must represent a single value, or a reference to another resource. I.e. don't embed json structs in annotations.

2. Annotation must be immutable, enforced in validation (when the feature gate is enabled). This eliminates the problem of annotation & field values becoming out of sync on update (say field always takes precedence). Another way of thinking about this is that the annotation provides the "default" value of the field.

3. Annotation must be pod level. I.e. no field paths in the key, or container names. Instead, encode that information in the referenced object.

This is a good fit for the 1:many binding model (e.g. RuntimeClass), but we still need to address the 1:1 case of per-pod extensions. I think we can accomplish that by splitting the spec & status into separate resources. The spec would be a 1:many binding (i.e. all replicas use the same spec), and then the initializer or webhook approach could create the status instance (which doesn't need prior knowledge). I need to think more about how the transition from alpha -> GA would work for this model though.

Optionally, we could formalize all this in some sort of "extensions" field in the ObjectMeta. Maybe `map[FeatureGateName][]ObjectReference`?

[Daniel Smith]

On Tue, Aug 14, 2018 at 2:58 PM Tim Allclair <tall...@google.com> wrote:

Hmm, I don't follow. Is the initializer on the pod or the ReplicaSet? Maybe we can chat offline.

No, on the pod: pod is created, but uninitialized so nothing acts on it. Your controller creates the ancillary object and removes its initializer, the pod becomes visible to the rest of the system.

Alternatively, your validating admission webhook synchronously makes the ancillary object before returning. (Now that I write it down the admission controller sounds better. back on the chopping block for initializers.)

In either case, the ancillary object is created before the rest of the system observes the pod.

Setting OwnerReference will cause the GC to clean up after you so no special reconciliation process is required.

Come find me if that doesn't make sense.

[Daniel Smith]

Tim explained to me that the field is user-defined (not computable from the rest of the pod) and ideally would go in the pod template. With this information, I initially thought that a webhook/initializer is not a good way to handle initializing an ancillary object. Certainly it's not good to make a CRD for everything that embeds a pod template.

The way of implementing this would have to be by defining (in the runtime class case) a RuntimeClassAssigner CRD which selects pods (via label is most obvious, but anything can work since an admission controller is going to read this and consider pods one-by-one) and provides the desired runtime class. Then the webhook admission controller I mentioned would need a process watching for assignment CRs.

That seems a little rube-goldberg-ish for this feature: in practice people would probably make one assigner object per class which looks at a runtime-class label, and people would set the label in their templates to get the class they wanted. (this is OK security wise since there's no chance for the label to be mutated before it gets to the admission controller webhook.)

In fact, you could get the same security properties by just looking at the label (or annotation) in the first place. E.g., could make a mutating webhook that observes a runtime-class annotation, initialize the ancillary object from the annotation, and then **DELETES THE ANNOTATION** so it's never actually stored to cause problems or confusion.

If the field in the CR is ever promoted to a field on the pod object, you can modify the webhook to set it instead of making the ancillary object, continuing to delete the annotation.

This does seem meaningfully better than just using the annotation, because there's no old clients depending on *reading* the annotation, making the task of upgrading much easier.

[Clayton]

If the field is ever something that would be a natural property of a pod, inventing a whole new pattern and process seems a bit byzantine.  I would agree that separate “policy” objects are nice for some behaviors like podpreset, but do we really want to break new ground and innovate on something this simple like “the type of runtime for this pod”?

The point of the gates is to locate the data to where it belongs, but still control the process whereby it appears and is persisted, in the way closest to how an end user actually wants to use the field.  All the extra problems from extra objects seems like a lot of work to preserve the small chance that we might not be 100% happy with the api when it goes beta.

[Daniel Smith]

I agree that the thing I described sounds byzantine. Just because we can doesn't mean we should. It offers marginal improvements for users with high cost to devs.

But we do *have* to pick a poison:

1) inaccurate docs

2) byzantine mechanism

3) block & wait for API Machinery to have bandwidth to implement field gating.

4) block & debate endlessly

3 presumably isn't an option, 2 is a worse version of 3 that puts the pain on people with more bandwidth, 1 puts the pain onto users. 4 is what happens by default if we can't pick a different poison.

[Jordan Liggitt]

Regarding docs, we have tried to make the documentation for alpha fields explicitly call out that they are alpha-level and not enabled by default. That (can) help users that read the field docs (surely such users exist). It doesn't help clients auto-generated from the schemas.

>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-architecture+unsub...@googlegroups.com.
>> To post to this group, send email to kubernetes-sig-architecture@googlegroups.com.

>> To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CADtktAUnY2TF8i9_tXGGJcpC_qgVWO-zvWnq0O37G%2BKYvvBQgQ%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.

>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-architecture+unsub...@googlegroups.com.
>> To post to this group, send email to kubernetes-sig-architecture@googlegroups.com.

[Clayton]

Wait, why is the current 1 state unacceptable?  We already do it for multiple fields.  Let’s just make it better...

I’d rather incrementally improve what we’re already using (flag gated fields) with solutions where we add better support (which benefits fields as well) over investing in new effort for new patterns.

I don’t think the api doc part is hard, and if we can tie that to some sort of minimal validation that’s even better.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CAB_J3bbeeZW%2BNnxKPJUW%2B6kWQoQK%2BnjAKCW6aU7Ru-D5FX8xkQ%40mail.gmail.com.

[Tim Allclair]

So we didn't get to this in today's sig-arch, can we make a decision to unblock the alpha features that need this?

To focus the discussion, let's consider this common use case:

- A single alpha field added to the pod spec

- User defined value, not computable

- Must be useable from objects that embed a PodTemplate

So far the options I've heard stated are:

1. Proceed with the documented approach for alpha fields, and prioritize incremental improvements (1.13?)

2. Return to alpha annotations, with additional constraints to address the original problems with alpha annotations. Optionally formalize in a dedicated ObjectMeta field.

3. "Assigner" CRD that links a spec CRD to a pod, probably based on labels. This probably ends up reimplementing annotation APIs as labels, with less control.

4. New mechanism, lenses: large restructuring of API & clients, unlikely to happen without significant effort in the next few releases

5. Block alpha features until alpha-gated fields can be implemented in the API & client code generation. Unlikely to happen before 1.13.

Anything else I missed?

As I'm invested in unblocking RuntimeClass I have a strong preference for options (1) or (2), but I'm not confident that I understand all the issues with those approaches.

Who is the decider(s) on this issue?

[Clayton Coleman]

I am strongly against 2 in general because we moved to fields for very clear reasons that I haven't heard anyone offer a counter argument to.  I consider alpha annotations dead at this point.

I don't see why 5 would be harshly imposed against RuntimeClass when other fields are not treated so harshly.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CADtktAU0nGq_2ZsJ9WgVHUJouLp4Rmk3v7dzpnP9YNeVad21Fg%40mail.gmail.com.

[Brian Grant]

Catching up

On Mon, Aug 13, 2018 at 5:27 PM Jordan Liggitt <jlig...@redhat.com> wrote:

On Aug 13, 2018, at 8:15 PM, Clayton <smarter...@gmail.com> wrote:

On Aug 13, 2018, at 8:07 PM, 'Tim Allclair' via kubernetes-sig-architecture <kubernetes-si...@googlegroups.com> wrote:

In last weeks sig-architecture review of RuntimeClass, there was an inconclusive discussion of how to reference alpha-APIs from stable objects. In this specific case, the question was how to reference the RuntimeClass instance from a Pod, but the problem applies generally to all stable API objects.

I want to kick off the follow-up discussion, since I believe a lot of projects are currently blocked on this. The solutions discussed so far include:

- Allow annotations, with better guidelines around best practices

- Add support for feature-gated fields

We already support and recommend this for most things - I’m not sure why this was not the preferred choice.  Is it about discoverability of the field?  Were there other options?

The gating support is incomplete. The field still shows up in schemas, documentation, etc. 

Right. The field-level versioning issue is:

https://github.com/kubernetes/kubernetes/issues/34508

More and more I'm thinking we need fine-grained API versions that act like full API versions but can be "generated" without copy/pasting types.go. That would be a little more convenient if the API version weren't in the path, which should be unnecessary with it in the metadata.

[Brian Grant]

On Mon, Aug 13, 2018 at 5:08 PM Tim Allclair <tall...@google.com> wrote:

In last weeks sig-architecture review of RuntimeClass, there was an inconclusive discussion of how to reference alpha-APIs from stable objects. In this specific case, the question was how to reference the RuntimeClass instance from a Pod, but the problem applies generally to all stable API objects.

I want to kick off the follow-up discussion, since I believe a lot of projects are currently blocked on this. The solutions discussed so far include:

- Allow annotations, with better guidelines around best practices

- Add support for feature-gated fields

- Add an "extensions" list to the ObjectMeta, which can reference arbitrary resources

- Parallel "sidecar" resources that hold the additional fields - either with names matching the extended resource, or a back reference. Either way, this runs into lifecycle issues (how does it work with replication? label-selector?).

Why do we need to replicate them if they are all identical? Why couldn't they just share 1 instance?

In the effort to improve garbage collection, I figured out a robust way to do reference counting.

[Brian Grant]

On Mon, Aug 13, 2018 at 6:52 PM Clayton <smarter...@gmail.com> wrote:

On Aug 13, 2018, at 9:22 PM, Jordan Liggitt <jlig...@redhat.com> wrote:

On Aug 13, 2018, at 9:14 PM, David Oppenheimer <davi...@google.com> wrote:

Clarification question: is "reference alpha-APIs from stable objects" the same as adding an alpha field to a stable object (i.e. the use case described here)?

Yes

(I assume it is, since I would assume that the field that references the alpha API object must itself be an alpha field, but just wanted to make sure.)

On Mon, Aug 13, 2018 at 5:29 PM, Clayton <smarter...@gmail.com> wrote:

Can we do better and kill two birds with one stone?  Ie Mark fields with @alpha Gate and propagate?

Yes. Something like that was proposed but not yet built (or fully understood w.r.t. the implications to the way we decode into structs). The "clear before validate/persist" approach we have now was a minimal stop-gap to unblock work held up on the annotations-vs-fields decision. 

It hasn’t worked out that badly in that we haven’t yet regretted it.

The best benefit of the field is that it shows up in docs, has clear semantics, and can be validated.  A generic metadata field can’t do that.

That's more important for beta, but yes.

Annotations could be validated (I'm pretty sure they have been in the past).

Annotations do have the unfortunate property (for field-like use cases) that they persist by default rather than being dropped by default prior to support being added for them.

 

 A sidecar policy resource isn’t necessarily discoverable (although we could also improve that, and I like sidecars like pod preset).  An argument against would be that we can’t grow the scope of pod forever, but runtime class seems pretty fundamental to the pod. 

As we are trying to push earlier-stage work to extensions, we do need a mechanism for experimentation that doesn't require code changes to kubernetes/kubernetes. In this case, it would need to flow through CRI. We had discussed passing through opaque parameters to docker in the past.

How confident are we that this is the way we want to express this?

 

On Aug 13, 2018, at 8:27 PM, Jordan Liggitt <jlig...@redhat.com> wrote:

On Aug 13, 2018, at 8:15 PM, Clayton <smarter...@gmail.com> wrote:

On Aug 13, 2018, at 8:07 PM, 'Tim Allclair' via kubernetes-sig-architecture <kubernetes-si...@googlegroups.com> wrote:

In last weeks sig-architecture review of RuntimeClass, there was an inconclusive discussion of how to reference alpha-APIs from stable objects. In this specific case, the question was how to reference the RuntimeClass instance from a Pod, but the problem applies generally to all stable API objects.

I want to kick off the follow-up discussion, since I believe a lot of projects are currently blocked on this. The solutions discussed so far include:

- Allow annotations, with better guidelines around best practices

- Add support for feature-gated fields

We already support and recommend this for most things - I’m not sure why this was not the preferred choice.  Is it about discoverability of the field?  Were there other options?

The gating support is incomplete. The field still shows up in schemas, documentation, etc. 

- Add an "extensions" list to the ObjectMeta, which can reference arbitrary resources

Do we have any other example use cases where a pattern like this would have been useful?

- Parallel "sidecar" resources that hold the additional fields - either with names matching the extended resource, or a back reference. Either way, this runs into lifecycle issues (how does it work with replication? label-selector?).

For services and endpoints this already works, but the objects were designed with that parallelism in mind.

Agree replication for pods makes this questionable.

We are planning to discuss this in this weeks sig-architecture meeting, but to help us reach a conclusion & unblock these projects, let's start the discussion here.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
To post to this group, send email to kubernetes-si...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CADtktAUnY2TF8i9_tXGGJcpC_qgVWO-zvWnq0O37G%2BKYvvBQgQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
To post to this group, send email to kubernetes-si...@googlegroups.com.

[Tim Allclair]

> Why do we need to replicate them if they are all identical? Why couldn't they just share 1 instance?

If we wanted to associate the "sidecar" resources by name, then they need to match the names of the replicas. If we have a single resource, we would do something like label selection, and then you just end up with specifying the api through labels, which isn't that different from annotations, except we offload the work to the user. (e.g. sandboxed runtime class get's bound to pods with the label "runtime-class.k8s.io = sandboxed".

> We had discussed passing through opaque parameters to docker in the past.

We have that today with annotations. If the parameters are opaque, you end up with fragmentation across CRI implementations (see untrusted annotations). If we want to standardize it, then it's an API as an annotation, and we're in the same boat.

> How confident are we that this is the way we want to express this?

I'm reasonably confident in the approach of pod -> RuntimeClass -> implementation. One question about the pod -> RuntimeClass that has been discussed is the idea of "fitting" a RuntimeClass to a pod, for instance through a label selector, with properties labeled on the RuntimeClass. In this case, I think we should treat it like scheduling, and then we still want the RuntimeClassName field to determine the actual RuntimeClass that was matched to the label selector. So in summary, I think that this approach is sufficiently general to inspire confidence.

That said, isn't the point of making a field alpha that we can change it later, if need be?

[Clayton]

On Aug 16, 2018, at 4:39 PM, Tim Allclair <tall...@google.com> wrote:

> Why do we need to replicate them if they are all identical? Why couldn't they just share 1 instance?

If we wanted to associate the "sidecar" resources by name, then they need to match the names of the replicas. If we have a single resource, we would do something like label selection, and then you just end up with specifying the api through labels, which isn't that different from annotations, except we offload the work to the user. (e.g. sandboxed runtime class get's bound to pods with the label "runtime-class.k8s.io = sandboxed".

> We had discussed passing through opaque parameters to docker in the past.

We have that today with annotations. If the parameters are opaque, you end up with fragmentation across CRI implementations (see untrusted annotations). If we want to standardize it, then it's an API as an annotation, and we're in the same boat.

> How confident are we that this is the way we want to express this?

I'm reasonably confident in the approach of pod -> RuntimeClass -> implementation. One question about the pod -> RuntimeClass that has been discussed is the idea of "fitting" a RuntimeClass to a pod, for instance through a label selector, with properties labeled on the RuntimeClass. In this case, I think we should treat it like scheduling, and then we still want the RuntimeClassName field to determine the actual RuntimeClass that was matched to the label selector. So in summary, I think that this approach is sufficiently general to inspire confidence.

That said, isn't the point of making a field alpha that we can change it later, if need be?

Yes

[Tim Hockin]

Catching up all in one message

I wrote:

>> think we should endorse it and maybe formalize it (e.g. formalize a reference from primary object to subordinate, so clients can auto-find them, and so delete handline or GC can deal with them).

Daniel wrote:

> What's wrong with a regular OwnerReference? It would go on the CRD and reference the beta/stable object.

This doesn't enable generic clients to do forward references. If I
know about Pod and I want to list all of the tagalongs, how do I find
them?

Tim Allclair wrote:

> IIUC, what is being proposed is another CRD, let's call it "RuntimeClassRef", that has the same name/namespace as the pod it is adding the RuntimeClass for, and a reference to the desired RuntimeClass.

It seems really heavyweight to add a whole CRD and instance for a
single field that just points to another instance of another type. In
my note I thought of that as case 2 (How to link a primary object to a
new alpha resource), which I guess COULD be implemented as case 1
(Tag-along) if we really need to.

> I'd like to revisit the annotations. I think we can solve a lot of the problems with a few restrictions:
>1. Value must represent a single value, or a reference to another resource. I.e. don't embed json structs in annotations.

The problem is that I can easily imagine a link like this that we want
per-container. Now my annotation says
`kubernetes.io/per-container-thing: "{ \"containers\": [ {\"name\":
\"c1\", \"new-thing\": 123}, {\"name\": \"c2\", \"new-thing\":
456}]}"`. This is maybe good enough for alpha features, but it ain't
pretty.

> 3. Annotation must be pod level. I.e. no field paths in the key, or container names. Instead, encode that information in the referenced object.

That seems unrealistic and overly imposing.

Clayton wrote:

> I’d rather incrementally improve what we’re already using (flag gated fields) with solutions where we add better support

The idea of "lenses" actually helps here. I can look at the API docs
for v1+alphas or v1+betas or just plain v1 and choose what to see as a
client or end-user, rather than as a cluster admin (which is what flag
gates do).

Tim Allclair wrote:

> can we make a decision to unblock the alpha features that need this?

ACK. What we have is imperfect, but I think a little more-of-the-same
isn't going to kill us.

Brian wrote:

> More and more I'm thinking we need fine-grained API versions that act like full API versions but can be "generated" without copy/pasting types.go

My memory is crap, but I remember VERY CLEARLY sitting on the patio of
our previous->previous building agreeing on the same thing with you,
Daniel, myself, maybe Eric and others.

Tim Allclair wrote:

> If we wanted to associate the "sidecar" resources by name, then they need to match the names of the replicas.

You could also have an annotation that names which tagalong to use,
facilitating tagalong reuse (as in replica sets) but not requiring it
(e.g. some other type of set).

> That said, isn't the point of making a field alpha that we can change it later, if need be?

Once you define a field of a given type, reusing the name for a
different type is very dangerous - any component that is out of date
can break the whole thing. So we generally say that a field name is
burned once released. If you decide it's not a String but an
ObjectRef. you get to pick a different field name.

[Daniel Smith]

On Mon, Aug 20, 2018 at 1:14 PM Tim Hockin <tho...@google.com> wrote:

Catching up all in one message


I wrote:

>>  think we should endorse it and maybe formalize it (e.g. formalize a reference from primary object to subordinate, so clients can auto-find them, and so delete handline or GC can deal with them).

Daniel wrote:

> What's wrong with a regular OwnerReference? It would go on the CRD and reference the beta/stable object.

This doesn't enable generic clients to do forward references.  If I
know about Pod and I want to list all of the tagalongs, how do I find
them?

You never have that problem, as the name matches the pod name, and you must already know the type anyway if you want to do something useful with the value.

(disclaimer: I don't think this solution is right for this use)

Yep, the field gating doc came shortly after that and is really that old.

 

Tim Allclair wrote:

> If we wanted to associate the "sidecar" resources by name, then they need to match the names of the replicas.

You could also have an annotation that names which tagalong to use,
facilitating tagalong reuse (as in replica sets) but not requiring it
(e.g. some other type of set).

Hot take: it'd be total insanity to not use the same name.

[Tim Allclair]

OK, I'm hearing weak consensus on continuing to allow use of alpha fields (for now). To reiterate, who is the decider on this? Should I open a PR to add the desired field?

[Clayton Coleman]

The point of an alpha field (behind an alpha gate) is that we specifically don't support that.  Creating an alpha field and then changing the type while alpha is *not* burning the name, because any component that is using that name is tolerating alpha fields which we don't support on upgrade.  So no, I don't think we say that the field name is burned.

 

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CAO_RewbLkTCR%2BCr7_aea7R_28J9KqvqSR_5ZptG5o8cxycVM_g%40mail.gmail.com.

[Jordan Liggitt]

> On Aug 20, 2018, at 6:57 PM, Clayton Coleman <ccol...@redhat.com> wrote:
>
> Creating an alpha field and then changing the type while alpha is *not* burning the name, because any component that is using that name is tolerating alpha fields which we don't support on upgrade. So no, I don't think we say that the field name is burned.

If you change the type, you cause n-1 client to fail to decode the new
field type coming from a new server. Releasing clients with knowledge
of an alpha field of a given type locks that field name to that type.

[Clayton Coleman]

Which is maybe an argument that our generated clients shouldn't include alpha fields by default, or we should split them, or that we should have an "alpha fields" gate on deserialization and do a better job of guarding those types.  We're creating the problem for ourselves in our generated clients, but unstructured clients don't have that problem.

Summarizing the various things in this thread re: fields

1. add godoc descriptors for alpha fields (linking to gates)

2. improve documentation output to list alpha fields

3. api minor versions / serialization control with the ability to selectively control alpha fields in responses and requests (either changes to server and client, or just server)

4. potentially having generated clients not include alpha fields to prevent type lock errors

[Tim Hockin]

On Mon, Aug 20, 2018 at 1:44 PM Daniel Smith <dbs...@google.com> wrote:
>
>
>
> On Mon, Aug 20, 2018 at 1:14 PM Tim Hockin <tho...@google.com> wrote:
>>
>> Catching up all in one message
>>
>>
>> I wrote:
>>
>> >> think we should endorse it and maybe formalize it (e.g. formalize a reference from primary object to subordinate, so clients can auto-find them, and so delete handline or GC can deal with them).
>>
>> Daniel wrote:
>>
>> > What's wrong with a regular OwnerReference? It would go on the CRD and reference the beta/stable object.
>>
>> This doesn't enable generic clients to do forward references. If I
>> know about Pod and I want to list all of the tagalongs, how do I find
>> them?
>
>
> You never have that problem, as the name matches the pod name, and you must already know the type anyway if you want to do something useful with the value.

I can list that it exists without knowing anything about it. That's useful.

> (disclaimer: I don't think this solution is right for this use)

Agree

This is a case of policy management, in some sense. Some policies
want to select targets, is it so bad that some targets want to select
policies?