Overview
We have a number of cases in the API where only one of a set of fields is allowed to be specified, aka undiscriminated union / oneof.
VolumeSource
is the canonical example: it has fields such as emptyDir, gcePersistentDisk, awsElasticBlockStore and other 20 fields. Only one of these fields can be specified.
We should add discriminators to union / oneof APIs, since it has several advantages.
Original issue is described in kubernetes/kubernetes#35345
Advantages
Adding discriminators to all unions/oneof cases would have multiple advantages:
-
Clients could effectively implement a switch instead of if-else trees to inspect the resource -- look at discriminator and lookup the corresponding field in a map (though differences in capitalization of the first letter in the API convention currently prevents the discriminator value from exactly matching the field name).
-
The API server could automatically clear non-selected fields, which would be convenient for kubectl apply
and other cases.
Analysis
List of Impacted APIs
In pkg/api/v1/types.go
:
In pkg/authorization/types.go
:
In pkg/apis/extensions/v1beta1/types.go
:
Behavior
If the discriminator were set, we'd require that the field corresponding to its value were set and the APIServer (registry) could automatically clear the other fields.
If the discriminator were unset, behavior would be as before -- exactly one of the fields in the union/oneof would be required to be set and the operation would otherwise fail validation.
We should set discriminators by default. This means we need to change it accordingly when the corresponding union/oneof fields were set and unset. If so, clients can rely on this for purpose (1).
Proposed Changes
API
Add a discriminator field in all unions/oneof APIs.
The discriminator should be optional for backward compatibility. There is an example below, the field Type
works as a discriminator.
type PersistentVolumeSource struct {
// +optional
GCEPersistentDisk *GCEPersistentDiskVolumeSource `json:"gcePersistentDisk,omitempty" protobuf:"bytes,1,opt,name=gcePersistentDisk"`
// +optional
AWSElasticBlockStore *AWSElasticBlockStoreVolumeSource `json:"awsElasticBlockStore,omitempty" protobuf:"bytes,2,opt,name=awsElasticBlockStore"`
...
// Discriminator for PersistentVolumeSource, it can be "gcePersistentDisk", "awsElasticBlockStore" and etc.
// +optional
Type *string `json:"type,omitempty" protobuf:"bytes,24,opt,name=type"`
}
API Server
We need to add defaulting logic described in the Behavior section.
kubectl
No change required on kubectl.
Example Discriminators
Discriminator are Set by Default
Assume we have added a field as discussed in section API changes
We first use kubectl apply -f
create a PersistentVolume
using the following config:
apiVersion: v1
kind: PersistentVolume
metadata:
name: pv0001
annotations:
volume.beta.kubernetes.io/storage-class: "slow"
spec:
capacity:
storage: 5Gi
accessModes:
- ReadWriteOnce
persistentVolumeReclaimPolicy: Recycle
hostPath:
path: /tmp
The subsequent kubectl get
should have the discriminator field set by API server:
apiVersion: v1
kind: PersistentVolume
metadata:
annotations:
volume.beta.kubernetes.io/storage-class: slow
creationTimestamp: 2016-12-22T00:56:31Z
name: pv0001
namespace: ""
resourceVersion: "1059564"
selfLink: /api/v1/persistentvolumespv0001
uid: 7a57e42b-c7e1-11e6-aa89-42010a800002
spec:
accessModes:
- ReadWriteOnce
capacity:
storage: 5Gi
hostPath:
path: /tmp
persistentVolumeReclaimPolicy: Recycle
# Discriminator showing the type is "hostPath"
type: hostPath
status:
phase: Available
Automatically Clear Unselected Fields
Issue kubernetes/kubernetes#34292 will be fixed if spec.strategy.type
is treated as a discriminator.
Create a deployment using kubectl apply-f
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx
ports:
- containerPort: 80
Get the deployment back. Fields spec.strategy.type
and spec.strategy.rollingUpdate
have been defaulted.
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
annotations:
deployment.kubernetes.io/revision: "1"
kubectl.kubernetes.io/last-applied-configuration: |
{"kind":"Deployment","apiVersion":"extensions/v1beta1","metadata":{"name":"nginx-deployment","creationTimestamp":null},"spec":{"replicas":1,"template":{"metadata":{"creationTimestamp":null,"labels":{"app":"nginx"}},"spec":{"containers":[{"name":"nginx","image":"nginx","ports":[{"containerPort":80}],"resources":{}}]}},"strategy":{}},"status":{}}
creationTimestamp: 2016-12-22T01:23:34Z
generation: 1
labels:
app: nginx
name: nginx-deployment
namespace: default
resourceVersion: "1062013"
selfLink: /apis/extensions/v1beta1/namespaces/default/deployments/nginx-deployment
uid: 416ef165-c7e5-11e6-aa89-42010a800002
spec:
replicas: 1
selector:
matchLabels:
app: nginx
strategy:
# Defaulted by API server
rollingUpdate:
maxSurge: 1
maxUnavailable: 1
# Defaulted by API server
type: RollingUpdate
...
Then we update the config file by adding
Apply the new config by kubectl apply -f
.
The operation should succeed now, because now the API server knows to clear field spec.strategy.rollingUpdate
after updating spec.strategy.type
.
@kubernetes/sig-api-machinery-misc @kubernetes/api-reviewers @kubernetes/kubectl