Closed brightzheng100 closed 6 years ago
Thanks for blazing trail on this one.
We [ not maintainers here ] deployed this PR and had some feedback for you:
the dependency on static-web is a bit of a gotcha... seems like atc & friends should talk to credhub on 127.0.0.1
, no? exposing credhub to the internet seems far from ideal and we had to modify this in all the places where it used web_ip, and to get tls working over localhost we had to modify common names in tls-vars.yml.
to get this to deploy on a large gcp VM, we had to add the following to the credhub-colocated.yml ops file because credhub doesn't seem to come up in time to meet the default manifest's deploy timeline. (specifically it takes about 4.5 minutes to come up on a large VM.)
handling credhub and uaa versions inside of versions.yml
seems wrong as well. If I to use dev versions, I'll have to use my own ops files, and if I'm not deploying credhub or uaa I should not specify versions for them.
type: replace path: /update/canary_watch_time? value: 1000-600000
type: replace path: /update/update_watch_time? value: 1000-600000
Thanks for the feedback @cwlbraa
For feedback 1, I personally think that CredHub should be reachable at some point as we still need to credhub set
some stuff for Concourse pipelines. The way I provided here is to colocate CredHub with the web VM which is reasonable in most of the cases, especially for those resource sensitive organizations. You may refer to credhub.yml for an isolated CredHub if you want, or you can have to control in load balancer level that you only expose the Concourse ATC port, not CredHub port, if it's really Internet facing.
I agree with you about tls-vars.yml
which is required to patch for the common name like I always do as below:
- type: replace
path: /variables/-
value:
name: atc_tls
type: certificate
options:
ca: atc_ca
common_name: ((concourse_domain_name))
alternative_names: ((web_ip))
Note: as what I focus here was colocated CredHub and UAA, I didn't mention this.
For feedback 2, I totally agree with you to have longer watch time for CredHub otherwise the BOSH deployment would sound failed but it will be recovered after a while. I did the exactly the same in my local environment but again, it's not this PR's focus.
For feedback 3, putting versions in that file is just a way to follow what this repo is doing, as per the doc. You can do without the -l ../versions.yml
and set whatever versions you want -- it's totally up to you.
So I totally understand that the main goal here is colocated credhubs on web vms. That's what we want and that's also why we deployed this. I can even get behind making credhub publically addressable in the same way the web api is. That doesn't change the fact that it's strange to force the the user to use static IPs for their web VMs when there's many ways (vm_extensions/target_pool) to make them externally routable. The advantage of having deployment-internal stuff find credhub on localhost is that it doesn't make any assumptions about a concourse operator's desired network topology.
it's not this PR's focus.
It seems like a working credhub+concourse colocated deployment is this PRs focus, and if you don't extend the update times, your PR will always fail to deploy.
as per the doc
Not sure what doc you're referring to. I just think this'd be much more amenable to merging if it was self-contained. Say I'm a concourse newbie who wanders in to versions.yml and sees credhub and uaa in there. Am I not going to assume that those deploy with concourse by default? If they were in the same ops file that adds them to the deployment, we'd have all the information we needed in one place.
Closing this PR and please refer to #85 instead for Concourse v4
Ops files for colocated UAA and CredHub integration:
db
To try it out? Refer to my workable example, as below: