Serving: Enable the ability to customize the mounted volume for user-container

[lvjing2]

Expected Behavior

We can define the volumeMounts in user-container

Actual Behavior

We can't define the volumeMounts in user-container

Additional Info

As the comment said, disable such field is to get rid of confliction with the auto generated by knative controller. https://github.com/knative/serving/blob/master/pkg/apis/serving/v1alpha1/revision_validation.go#L104-L106

But I check the source code about the controller to generate the default mounted volume, only varlog:/var/log is mounted. I am wondering why should we disabled this feature if the controller only define one volume. we can just check to not define the volumeMount with the same name as varlog. So I recommand to enable customize volumeMounts defination, and only check not to define volumeMount with name varlog.

[lichuqiang]

duplication to #2025

[lvjing2]

@lichuqiang OK, I will following this issue, and check the progess. Thanks

[mattmoor]

This is a superset of the linked issue, and something we've discussed before.

I suspect that there are certain volume types that this makes sense for (e.g. configmap, secrets), but even those were mildly contentious due to the fact that they could be mutated and become inconsistent across Pods of a Revision.

Are there specific volume types you are interested in? Want to elaborate more on the target use cases and bring it to the API WG?

[lvjing2]

@mattmoor
Acctually, for my scenario, I need to allow the container to storage some resource(logs, cache, generated files) on the host, and we would create a agent on each host to upload those resource to remote storage platform regular.
and I think @saturnism 's scenario in #2025 is also useful and also a common case for many developers..
At least, I think the /tmp folder should be free for users, we just need to define the capacity limitation of storage usage like 500M.

[saturnism]

In my use case to share pre-compiled class data cache, it really just need a shared file system on the host/node. I.e., the class data cache is only shared w/ containers on the same host.

I also see a use case where I want to share read-only volume across my container instances.

[mattmoor]

Generally, I do think we need volume support, but again the set of volumes we allow should necessarily be limited.

_Some thoughts on the volume types we're discussing..._

PersistentVolumes are the hallmark of stateful workloads, and while they could legitimately be used in ReadMany mode, we lack a way to validate that the claimed persistent volume has that mode. Moreover, typically mounting these volumes adds startup latency that could be prohibitive for cold starts.

HostPath volumes are an Operator's nightmare (effectively equivalent to root on host) and a very nodeful construct that feels poorly placed in the user-facing API. For self-operated clusters, I could see folks opting to extend the runtime contract with this kind of shared data cache, but a writeable hostpath sounds like a nightmare for multi-tenant scenarios (what's to stop me from poisoning the cache?).

This is mostly my $0.02, we can discuss this at an upcoming working group and see what folks think. Feel free to add it to the agenda and I'll try to make sure all the interested parties (I know of) show up.

[crypto-apple]

I would like to mount gcp service account JSON credential key file like firebase-admin-sdk in minikube environment. Is it possible?

e.g.

---
apiVersion: v1
kind: Secret
metadata:
  name: service-account
type: Opaque
data:
  key.json: {{ .Values.gcp.key | b64enc }}
---
apiVersion: serving.knative.dev/v1alpha1
kind: Service
metadata:
  name: some-service
spec:
  runLatest:
    configuration:
      revisionTemplate:
        spec:
          container:
            image: gcr.io/my-project/some-service
            args: [
              "--key_file",
              "/etc/service-account/key.json"
            ]
            volumeMounts:
            - name: service-account
              readOnly: true
              mountPath: "/etc/service-account"
          volumes:
          - name: service-account
            secret:
              secretName: service-account

[mattmoor]

Not currently, but I feel like we should consider relaxing this in 0.4 (for secrets and configmaps)...

[evankanderson]

Looks like this might be resolved by #3061

[duglin]

Would it make sense to reopen this so we can discuss the generic PV usecase? While I understand not all volume types would work, this sounds like a generic Kube issue and not Knative. In that I mean, if there are certain PVs that can't work with K8s Deployments then since K8s would have the same constraints as Kn, I'm not sure why Kn would remove all generic PV support instead of just saying "Kn has the same restrictions as Kube - be warned".