search

cloudfoundry / cf-deployment-concourse-tasks

Apache License 2.0
23 stars 76 forks source link

Allow `bbl up --credhub` in CI pipelines #28

Closed rowanjacobs closed 7 years ago

rowanjacobs commented 7 years ago

This PR contains a couple changes designed to enable using bbl up --credhub in CI pipelines.

The first is that, in order to prevent containers from hanging for a long time after jobs which run eval "$(bbl print-env)" (which opens an ssh connection when credhub is enabled), the setup_bosh_env_vars task in shared_functions kills ssh connections on exit.

The second change is due to bbl no longer persisting IAAS credentials in bbl-state.json when credhub is enabled. As a result, when credhub is enabled, we must pass in the IAAS credentials manually to every task which runs a bbl command (including bbl print-env).

/cc @mcwumbly

cfdreddbot commented 7 years ago

Hey rowanjacobs!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

cf-gitbot commented 7 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/150618506

The labels on this github issue will be updated when the story is started.

staylor14 commented 7 years ago

Hi @rowanjacobs! The RelInt team has multiple traps on EXIT in the cf-deployment-concourse-tasks. We ran into a problem with updating our vars store because your newly-introduced trap is overriding ours on the EXIT signal. We removed EXIT, and kept your INT and TERM signals.

Could we simulate your trapping on EXIT by explicitly calling pkill ssh || true everywhere where bbl print-env is shelling out? We're not sure how else to address this, tbh.

rowanjacobs commented 7 years ago

You'd have to call pkill ssh || true at the end of every task that calls setup_bosh_env_vars (since the ssh connection from bbl print-env has to stay alive long enough for bosh commands to be executed).

staylor14 commented 7 years ago

Yes, I see! We'll do that to work around the issue. We're not sure how to test this, though. If you have an environment where you could vet the approach, would you mind filing another pull request sans EXIT trap?

We don't yet have a credhub'd environment, so we'd just be guessing on the implementation.

rowanjacobs commented 7 years ago

Sure, I can test this in our pipeline and create a new PR.

rowanjacobs commented 7 years ago

New PR, #30, has been created.