[Apache Airflow addon] Dependency added for EFS #719
Conversation
youngjeong46
requested review from
aws-ia-ci,
a team,
parkand1,
shapirov103 and
elamaran11
as code owners
youngjeong46
changed the title
lib/addons/apache-airflow/index.ts
Outdated
| @@ -217,6 +217,8 @@ function populateValues(clusterInfo: ClusterInfo, ns: KubernetesManifest, helmOp | |||
| overwrite: true, | |||
| }); | |||
|
|
|||
| efsResources.node.addDependency(ns); | |||
There was a problem hiding this comment.
efs storage class is not a namespace resource - created at the global. So adding dependency to the namespace is irrelevant for it but it is revelant for the PVC.
The error that we saw was: either chart or PVC or both are created before the storage class is created.
We need the following dependencies:
efs storage class depends efs csi driver - it is a global resource so no dependencies on ns, pvc, chart
PVC depends on storage class and ns
chart depends on PVC/storage class/ns
There was a problem hiding this comment.
Am I interpreting this correctly:
For the chart, we would only need to add Dependency for the PVC - if I implement PVC node dependency on sc and ns, by transitivity, sc and ns would provision before the chart.
There was a problem hiding this comment.
Yeah, that seems right. The storage class may require dependency on the EFS CSI driver most likely. I am curious if CSI driver dependency is critical for this case, this will have to be tested.
There was a problem hiding this comment.
Is there a way to add CSI driver as dependency? addons are Construct promises, which do not use IDependable interface.
There was a problem hiding this comment.
I am a bit confused with this question since we had this conversation in the past, I attribute it to the case of Mondays. I believe this is your code based on the discussion we had.
There was a problem hiding this comment.
Case of the Mondays it is.
…ovided helm value
|
/do-e2e-tests |
Issue #, if available:
Description of changes: Dependency for EFS Kubernetes resources (storageclass and PVC) added so that they are provisioned after namespace creation.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.