[v2] Use an apply configuration for the workspace template #713
Conversation
| PodTemplate: &apply.EmbeddedPodTemplateSpecApplyConfiguration{ | ||
| Metadata: &apply.EmbeddedObjectMetaApplyConfiguration{ |
There was a problem hiding this comment.
If we're happy with the direction of this I can take it further and also use an apply config for our pod template -- it should be very minor.
There was a problem hiding this comment.
That would be pretty wonderful.
There was a problem hiding this comment.
I spoke too soon -- this is actually quite involved. I'll put up a WIP with what I have so far.
| WorkspaceTemplate: &autov1alpha1.EmbeddedWorkspaceTemplateSpec{ | ||
| Metadata: autov1alpha1.EmbeddedObjectMeta{ | ||
| WorkspaceTemplate: &shared.WorkspaceApplyConfiguration{ | ||
| ObjectMetaApplyConfiguration: &metav1apply.ObjectMetaApplyConfiguration{ |
There was a problem hiding this comment.
Worth calling out that ObjectMetaApplyConfiguration differs from EmbeddedWorkspaceTemplateSpec in that it does expose all of the underlying fields (like generation) even if they can't actually be set. I think it makes for a more bloated/confusing schema but I don't see a good way around it.
There was a problem hiding this comment.
This approach is similar to PodTemplateSpec so it shouldn't add confusion to K8s users.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #713 +/- ##
==========================================
- Coverage 51.20% 49.93% -1.27%
==========================================
Files 29 30 +1
Lines 3199 4031 +832
==========================================
+ Hits 1638 2013 +375
- Misses 1379 1836 +457
Partials 182 182 ☔ View full report in Codecov by Sentry. |
blampe
added
impact/no-changelog-required
| Metadata EmbeddedObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` | ||
| Spec *WorkspaceSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` | ||
| } | ||
|
|
||
| // EmbeddedObjectMeta contains a subset of the fields included in k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta | ||
| // Only fields which are relevant to embedded resources are included. | ||
| type EmbeddedObjectMeta struct { |
There was a problem hiding this comment.
Is EmbeddedObjectMeta obsolete?
| PodTemplate: &apply.EmbeddedPodTemplateSpecApplyConfiguration{ | ||
| Metadata: &apply.EmbeddedObjectMetaApplyConfiguration{ |
There was a problem hiding this comment.
That would be pretty wonderful.
This adds
applyconfiguration-gento generate apply configurations for our APIs. The workspace template is updated to use it.It's a moderately uphill battle to get apply configurations working with controllers due to various pieces not including built-in support for them. In practice this means we need to cut a couple small corners -- in particular implementing our own
DeepCopyand tweaking the generatedGroupVersioncode.Fixes #704.