Deterministically resolve the containers from env vars and volume mounts values

[baijum]

I am facing some difficulty implementing .spec.versions[*].envs and .spec.versions[*].volumeMounts of ClusterApplicationResourceMapping resource. To persist the env vars and volume mounts, first those should be attributes should be attached to a container. But the spec is not setting a mechanism to deterministically derive the containers from env vars and volume mounts values. At the same time spec says ...for all available containers based on those attributes:

If a ClusterApplicationResourceMapping defines .envs and .volumeMounts, the reconciler MUST first resolve a set of candidate locations in the application resource addressed by the ServiceBinding for all available containers and then filter that collection by the ServiceBinding .spec.application.containers filter before applying the appropriate modification.

(highlighted for all available containers part)

If env vars and volume mounts are expected to be part of containers, why there is a special provision to set those attributes directly?

I think a typical Non-PodSpec-able Custom Pod definition would be like this:

// CustomPodSpec defines the desired state of CustomPod
type CustomPodSpec struct {
    InitContainers []corev1.Container `json:"initContainers,omitempty"`
    Containers     []corev1.Container `json:"containers,omitempty"`
    Volumes        []corev1.Volume    `json:"volumes,omitempty"`
}

(full example)

[scothis]

Good question, it's not clear to me what "for all available containers" is trying to convey here. It looks like a holdover from the previous paragraph that should be updated.

The only time you would use env and volumeMount mappings are when the application type doesn't support containers. Otherwise, you'd just use containers. I wouldn't expect the ServiceBinding .spec.application.containers field to have an effect since there is no container name to filter on. Then again, the RuntimeComponent resource doesn't have a container for it's primary "container", but does have containers for init and sidecar containers.

[pedjak]

The only time you would use env and volumeMount mappings are when the application type doesn't support containers.

If the application resource does not support containers, how certain is that it would support env/volumeMount - a custom application CRD can model these aspects in a fully arbitrary way.

IMHO, we should not have ability to set location of individual fields in PodSpec - instead we should set only the location to structure that might have some of containers, initContainers or volumes fields (duck typed):

type PodBindableSpec struct {
  Volumes []Volume
  InitContainers []Container
  Containers []Container
  EphemeralContainers []EphemeralContainer
}

all fields are optional, but for those that exist in the application, relevant data will be injected.

[scothis]

If the application resource does not support containers, how certain is that it would support env/volumeMount - a custom application CRD can model these aspects in a fully arbitrary way.

The two resources that drove this feature were CronJob and RuntimeComponent. In the case of CronJob, it has a PodTemplateSpec, but in an unusual location (.spec.jobTemplate.spec.template). RuntimeComponent defines volumes, volume mounts and env, but not a container (unless your talking about init containers or sidecars)

[baijum]

Updated PR #165 based on yesterday's discussion.

[nebhale]

@baijum Do you think this is completely superseded by #173 and can be closed?