Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make IAP config robust to updating the Ingress #550

Closed
jlewi opened this issue Mar 31, 2018 · 4 comments
Closed

Make IAP config robust to updating the Ingress #550

jlewi opened this issue Mar 31, 2018 · 4 comments

Comments

@jlewi
Copy link
Contributor

jlewi commented Mar 31, 2018

See #545 for examples of how certain changes to the config (e.g. ingress) can cause the loadbalancer to end up in a misconfigured state e.g.

  • Loadbalancer uses the wrong path for the health check
  • Timeout on the backend service reverts to the default of 30 seconds
  • IAP gets disabled

We'd like to find a way to properly detect this and run the corrective steps.
The script used in the envoy container as an init script has most of the commands needed to correct the setup; we just need to figure out how to trigger it when things break.

There are some ideas in #545.

@danisla
Copy link
Contributor

danisla commented Mar 31, 2018

Any change in the nodePort or backend service ID breaks IAP.

It looks like the service nodePort changes every time you run ks apply ${ENV} -c iap-ingress. I would actually expect this to not modify the service nodePort if if the service already exists but for some reason it's being updated.

#552 is a working fix that converts the initContainer into a sidecar. It periodically verifies that the backend service ID has not changed. If it does change, it exits with status 1 causing K8S to restart the container and reconfigure IAP. It also restarts envoy to pull the new config.

@danisla
Copy link
Contributor

danisla commented Apr 2, 2018

ksonnet/ksonnet#200 is related to the issue regarding the nodeport changing whenever ks apply is run.

@jlewi
Copy link
Contributor Author

jlewi commented Apr 2, 2018

Thanks

/assign @danisla

@jlewi
Copy link
Contributor Author

jlewi commented Apr 9, 2018

@danisla looks like #552 has been committed. Should we close this issue or is there more work to be done to make it robust?

@jlewi jlewi closed this as completed Apr 13, 2018
yanniszark pushed a commit to arrikto/kubeflow that referenced this issue Feb 15, 2021
* Fix default metricsController wrong args

* Add command for metricsConllector deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants