community
community copied to clipboard
[emr-containers] Add support to "configuration-overrides" in JobRun CRD
Describe the bug ACK runtime doesn't support cyclic structure. This impacts adding API attributes under "configuration-overrides" in JobRun CRD. "configuration-overrides" has key features from EMR such as logging, monitoring, pod template features.
Steps to reproduce code-generator goes into memory leak if "configuration-overrides" is enabled
Expected outcome After cyclic structure is supported, we can add complete featureset of JobRun API
Environment EMR on EKS (emr-containers)
- Kubernetes version
- Using EKS (yes/no), if so version? Yes, all versions
- AWS service targeted (S3, RDS, etc.) EMR on EKS (emr-containers)
Hey Peter. Sadly, it's just not possible to support cyclic references for anything within a K8s CRD. By their design, a CRD must be describable using an OpenAPIv3 schema. That means, each spec can be broken down into its constituent primitive types. There is no way to (finitely) decompose any cyclic properties.
Ignoring all ACK limitations, kubebuilder (the CLI package we use to transform Go code to K8s YAML) doesn't break when feeding it a cyclic property. Instead, it converts the first child reference of the property into a generic object type (schema-less). I'm not sure how any of the JSON (un-)marshalling works with this in Go, but essentially customers would see that ConfigurationOverrides accepts an "object".
That being said, I did make an attempt to remove the ACK assumption that all schemas must be DAGs and it did not go well. It's fundamental to most of our designs that we can represent all properties in a DAG tree. So, we'll just need to find a clever way to work ConfigurationOverrides (and any other cyclic referencing property) in that paradigm.
I understand @RedbackThomson . I think we need clever solution for sure to mitigate the issue. ConfigurationOverrides is an important part of JobRun API and we can't use complete features of EMR on EKS without this capability
We can close this issue bcoz workaround fix is deployed via PR