Prevent deadlocks trying to setup IAP. #924
Conversation
* We want a single process to update the backend service and IAP.
Right now we do this in the envoy side car which is replicated.
* The script gets a lock to prevent multiple services from trying to update
IAP simultaneously.
* But until the process acquires the lock it can't update the local copy
of the envoy config to do proper JWT validation.
* So we move updating the backend into a separate deployment that has a single
replica 1 (although we still use a lock).
* envoy sidecars now do not modify the backend but just get relevant info
and update the envoy config.
* To support getting the ksonnet app of the bootstrapper add source repository
IAM role to the admin service account and mount the service account
into the bootstrapper.
This will allow use to push the ksonnet app from the bootstrapper to
a cloud source repository.
Related to kubeflow#903
|
/assign @danisla |
| LOCK=$(jq -r ".metadata.annotations.iaplock" service.json) | ||
|
|
||
| NOW=$(date -u +'%s') | ||
| if [[ -z "${LOCK}" || "${LOCK}" == "null" ]]; then |
There was a problem hiding this comment.
why we still need lock here if replicas=1?
There was a problem hiding this comment.
Because there could still be more than 1 pod at a given time; K8s doesn't guarantee that there is a single pod all the time; it just guarantees that in steady state there will be 1 pod.
|
/assign @ankushagarwal |
kubeflow/core/setup_iap.sh
Outdated
|
|
||
| # TODO(jlewi): We should publish the modified envoy config as a config map | ||
| # and use that in the envoy sidecars. | ||
| kubectl get configmap -n ${NAMESPACE} envoy-config -o jsonpath='{.data.envoy-config\.json}' | \ |
There was a problem hiding this comment.
No need to write out the envoy-config.json to the shared directory since there is no envoy sidecar in the "iap-enabler" deployment.
|
/lgtm /hold Feel free to run |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ankushagarwal The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all |
|
/lgtm |
|
/hold cancel |
* Prevent deadlocks trying to setup IAP.
* We want a single process to update the backend service and IAP.
Right now we do this in the envoy side car which is replicated.
* The script gets a lock to prevent multiple services from trying to update
IAP simultaneously.
* But until the process acquires the lock it can't update the local copy
of the envoy config to do proper JWT validation.
* So we move updating the backend into a separate deployment that has a single
replica 1 (although we still use a lock).
* envoy sidecars now do not modify the backend but just get relevant info
and update the envoy config.
* To support getting the ksonnet app of the bootstrapper add source repository
IAM role to the admin service account and mount the service account
into the bootstrapper.
This will allow use to push the ksonnet app from the bootstrapper to
a cloud source repository.
Related to kubeflow#903
* format jsonnet files.
* Address comments; no need to modify the envoy config.
We want a single process to update the backend service and IAP.
Right now we do this in the envoy side car which is replicated.
The script gets a lock to prevent multiple services from trying to update
IAP simultaneously.
But until the process acquires the lock it can't update the local copy
of the envoy config to do proper JWT validation.
So we move updating the backend into a separate deployment that has a single
replica 1 (although we still use a lock).
envoy sidecars now do not modify the backend but just get relevant info
and update the envoy config.
To support getting the ksonnet app of the bootstrapper add source repository
IAM role to the admin service account and mount the service account
into the bootstrapper.
This will allow use to push the ksonnet app from the bootstrapper to
a cloud source repository.
Related to #903
This change is