Volume Mounter trait creates PVC with "ephemeral: true" configuration

[abhirockzz]

Output of helm version:

version.BuildInfo{Version:"v3.0.1", GitCommit:"7c22ef9ce89e0ebeb7125ba2ebf7d421f3e82ffa", GitTreeState:"clean", GoVersion:"go1.13.4"}

Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.3", GitCommit:"2d3c76f9091b6bec110a5e63777c332469e0cba2", GitTreeState:"clean", BuildDate:"2019-08-19T11:13:54Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.7", GitCommit:"6c143d35bb11d74970e7bc0b6c45b6bfdffc0bd4", GitTreeState:"clean", BuildDate:"2019-12-13T18:46:24Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.):

Azure Kubernetes Service

Describe the bug

As per Volume mounter trait doc, PVC should not be created for ephemeral (ephemeral: true)

OAM yaml files used

Component

apiVersion: core.oam.dev/v1alpha1
kind: ComponentSchematic
metadata:
  name: emphemeral-vol-example
spec:
  workloadType: core.oam.dev/v1alpha1.Server
  containers:
    - name: server
      image: nginx:latest
      resources:
        volumes:
          - name: emphemeral-vol
            mountPath: /emphemeral-vol
            disk:
              required: "50M"
              ephemeral: true

App config

apiVersion: core.oam.dev/v1alpha1
kind: ApplicationConfiguration
metadata:
  name: example-server-with-volume
spec:
  components:
    - componentName: emphemeral-vol-example
      instanceName: emphemeral-vol-example1
      traits:
        - name: volume-mounter
          properties:
            volumeName: emphemeral-vol
            storageClass: default

What happened:

• PV and PVC were created
• Azure Disk was provisioned

What you expected to happen:

PV and PVC should not be created since ephemeral storage was requested in ComponentSchematic

Relevant screenshots:

How to reproduce it (as minimally and precisely as possible):

Create the OAM Component followed by the ApplicationConfiguration (contents pasted above)

Anything else we need to know:

The Deployment uses emptyDir volume type (not PVC)

[zhxu2]

I think if you decide to include vol mounter trait such as:
traits:
- name: volume-mounter
properties:
volumeName: emphemeral-vol
storageClass: default
into your app cfg, a PVC is created whatsoever, however only by setting "ephemeral: false" should it actually mount that PVC(created by trait). It finds the right PVC by matching names

not a real bug but needs more docs

[ryanzhang-oss]

I think the bug is that a trait acts without any context in the current rudr implementation. Each trait's action is pre determined regardless of the component it applies to. The volume-mounter just creates a PVC no matter what. We need to fix this in the new OAM go implementation. We may also need to add a policy field into the component in the spec too so that the trait will behave just as the application developer expected.

[ryanzhang-oss]

Another thing is that the the spec didn't say what happens when there are multiple containers/volumes in the component so it's probably not a good example. We should remove the semantics of the ephemeral.

[abhirockzz]

into your app cfg,

I think if you decide to include vol mounter trait such as:
traits:

• name: volume-mounter
  properties:
  volumeName: emphemeral-vol
  storageClass: default
  into your app cfg, a PVC is created whatsoever, however only by setting "ephemeral: false" should it actually mount that PVC(created by trait). It finds the right PVC by matching names

not a real bug but needs more docs

The problem is that when the PVC/PV are created, the cloud provider also created resources behind the scenes - as mentioned in the bug details, in this case it was an Azure Disk. This is not desirable. If all that's required is an emptyDir, why create a PV/PVC?

[abhirockzz]

I think the bug is that a trait acts without any context in the current rudr implementation. Each trait's action is pre determined regardless of the component it applies to. The volume-mounter just creates a PVC no matter what. We need to fix this in the new OAM go implementation. We may also need to add a policy field into the component in the spec too so that the trait will behave just as the application developer expected.

^ agree. Just out of curiosity, what is the "new OAM go implementation" you referred to? Is it the same as the oam-go-sdk?

[ryanzhang-oss]

^ agree. Just out of curiosity, what is the "new OAM go implementation" you referred to? Is it the same as the oam-go-sdk?

It's not the same but it will probably use oam-go-sdk. It's still in the planning phase.

[hongchaodeng]

I agree with @zhxu2

The volume-mounter trait (better named as PV trait?) should be set in AppConfig if ephemeral=true.

[hongchaodeng]

The ideal solution to this problem is to intercept and reject invalid AppConfigs in Admission webhook: https://github.com/oam-dev/admission-controller/