Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

[Proposal] Add a 'podSpecPath' field in WorkloadDefinition #233

Closed
wonderflow opened this issue Oct 8, 2020 · 5 comments · Fixed by #240
Closed

[Proposal] Add a 'podSpecPath' field in WorkloadDefinition #233

wonderflow opened this issue Oct 8, 2020 · 5 comments · Fixed by #240

Comments

@wonderflow
Copy link
Member

wonderflow commented Oct 8, 2020

We have discussed a lot and finally conclude to add labels like workload.oam.dev/podspecable to indicate the workload will contain 'podSpec' in it's spec.

But it's hard to know which field is podSpec, so I propose we add a 'podSpecPath' field in WorkloadDefinition.

By defualt, if workload.oam.dev/podspecable: true label is set but podSpecPath not, we may go to spec.template to find 'podTemplate', and get 'podSpec' in spec.template.spec.

If this field is set like below, we will directly regard path specified in podSpecPath as podSpec path, in this example, it's spec.podSpec

apiVersion: core.oam.dev/v1alpha2
kind: WorkloadDefinition
metadata:
  name: webservice
  labels:
     workload.oam.dev/podspecable: true
  annotations:
    definition.oam.dev/apiVersion: "standard.oam.dev/v1alpha1"
    definition.oam.dev/kind: "PodSpecWorkload"
spec:
  podSpecPath: "spec.podSpec"
  definitionRef:
    name: podspecworkloads.standard.oam.dev
  childResourceKinds:
    - apiVersion: apps/v1
      kind: Deployment
    - apiVersion: v1
      kind: Service

This is also needed to fix kubevela/kubevela#354

@resouer
Copy link
Contributor

resouer commented Oct 8, 2020

+1, I also noticed this issue for a while: podspecable doesn't mean spec.template.spec exists.

@hongchaodeng
Copy link
Member

+1.
Shouldn't it be podSpecFieldPath ?

@ryanzhang-oss
Copy link
Collaborator

ryanzhang-oss commented Oct 8, 2020

This reminds me of the workloadRefPath which caused some problems when it is not presented (#232). I know this will be an optional field too, I wonder what's the exact behavior when this field is not present?

@resouer
Copy link
Contributor

resouer commented Oct 9, 2020

@ryanzhang-oss This field doesn't need to present if:

  1. the workload is not podspecable,
  2. the workload is podspecable but assume 'template.spec' as pod spec field path.

A workload is podspecable if the podspecable label present on its definition.

@wonderflow
Copy link
Member Author

@hongchaodeng @resouer podSpecPath is better as we already have workloadRefPath.

@ryanzhang-oss Yes, it's an optional field as I said in the proposal, it could have following behaviors:

No label and No podSpecPath set

apiVersion: core.oam.dev/v1alpha2
kind: WorkloadDefinition
metadata:
  name: webservice
spec:
  definitionRef:
    name: podspecworkloads.standard.oam.dev
   ...

In this case, we will not do any podSpec specific thing.

Has label with No podSpecPath set

apiVersion: core.oam.dev/v1alpha2
kind: WorkloadDefinition
metadata:
  name: webservice
  labels:
     workload.oam.dev/podspecable: true
spec:
  definitionRef:
    name: podspecworkloads.standard.oam.dev
   ...

In this case, we will always regard spec.template.spec as the podSpecPath. This will fit with K8s Deployment/StatefulSet and many other CRD Objects like OpenKruise CloneSet/Knative Service.

Has podSpecPath set

apiVersion: core.oam.dev/v1alpha2
kind: WorkloadDefinition
metadata:
  name: webservice
  labels:
     workload.oam.dev/podspecable: true
spec:
  podSpecPath: "spec.podSpec"
  definitionRef:
    name: podspecworkloads.standard.oam.dev
   ...

In this case, whether there's label or not, we will always make the workload as podspecable=true. We will find podSpecPath to get the field of podSpec.

wonderflow added a commit to wonderflow/oam-kubernetes-runtime that referenced this issue Oct 9, 2020
wonderflow added a commit to wonderflow/oam-kubernetes-runtime that referenced this issue Oct 9, 2020
wonderflow added a commit to wonderflow/oam-kubernetes-runtime that referenced this issue Oct 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants