cloudfoundry / cf-deployment-concourse-tasks

Apache License 2.0
23 stars 76 forks source link

Request: BBL_STATE_DIR calculates full(er) path #25

Closed evanfarrar closed 6 years ago

evanfarrar commented 7 years ago

BBL is considering adding a BBL_STATE_DIR flag to mirror the functionality --state-dir flag. We actually tried shipping it, and then noticed in pipelines that this broke pipelines because of the process you follow, simplified like so:

cd $BBL_STATE_DIR
bbl up

Sure: we could pick a different name, or you could pick a different name, but I think in the long run it's a little more intuitive for people looking at the concourse task if all BBL_ are environment variables that BBL understands. For instance, I can check out my bbl-states repo, cd to that repo, set the vars I use in my concourse YML, and just bbl up. So my team has proposed this change as an example of a way that you can retain the name BBL_STATE_DIR and have a process that would be compatible with BBL before the change and after the change. Then long after the change you can elect at some time in the future to just cd bbl-states and allow BBL_STATE_DIR to be implicitly passed to bbl.

  local root_dir
  root_dir="${1}"
  export BBL_STATE_DIR=${root_dir}/bbl-state/${BBL_STATE_DIR}

...

+  mkdir -p "${BBL_STATE_DIR}"
+  pushd "${BBL_STATE_DIR}"
-  mkdir -p "bbl-state/${BBL_STATE_DIR}"
-  pushd "bbl-state/${BBL_STATE_DIR}"
  bbl up
cf-gitbot commented 7 years ago

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

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

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

dsabeti commented 7 years ago

It looks like the critical line here is:

  export BBL_STATE_DIR=${root_dir}/bbl-state/${BBL_STATE_DIR}

Essentially, our Concourse task would allow users to set their BBL_STATE_DIR to be relative to the bbl-state input, but we'd expand the relative path to the full path (based on the root_dir of the concourse build).

In that case, what does bbl expect BBL_STATE_DIR to look like -- a relative path or an absolute one? In the workflow you described, where a user copies the variables in their pipeline yaml, would bbl still be able to respect the relative path defined there?

dsabeti commented 7 years ago

@evanfarrar Hey, any updates on this?

evanfarrar commented 7 years ago

Hey, sorry I missed this:

The BBL_STATE_DIR in bbl is fine being relative or absolute, we only expand it in the concourse task to incorporate the "bbl-state" parent directory scheme that cf-deployment-concourse follows. It could be expanded less fully in the task, if you prefer.

dsabeti commented 7 years ago

Hey @evanfarrar. This sounds reasonable. I've updated the story:

Title: As a Concourse user, I want the bbl-up task to expand BBL_STATE_DIR to a full path so that, eventually, bbl can use the value natively.

Acceptance Criteria:

How does that look?

evanfarrar commented 6 years ago

I have not answered this in forever but, that looks great.

dsabeti commented 6 years ago

Yo @evanfarrar, do we still need to do this since the release of bbl 5? If so, I'll prioritize it.

dsabeti commented 6 years ago

Hey, I'm going to close this out since I don't think we need to do this after all. Feel free to re-open it if you think this would still be valuable.