pantheon-systems / docker-build-tools-ci

Dockerfile source for docker image pantheon-public/build-tools-ci on quay.io
34 stars 39 forks source link

BT-43: Don't create multidevs when commits are made to master #23

Closed ataylorme closed 5 years ago

ataylorme commented 5 years ago

Set TERMINUS_ENV to dev if CI_BRANCH is master.

ataylorme commented 5 years ago

Tested and confirmed working as expected (build log).

It might also be useful to have an environment variable that enabled / disabled this behavior. We could make it disabled by default in the 4.x branch, and enabled by default in the 5.x branch.

What if we went about this differently and changed DEFAULT_ENV. Something like:

if [[ -z ${DEFAULT_ENV} ]] ; then
    DEFAULT_ENV=ci-$CI_BUILD_NUMBER
fi

We could then have build:project:create set DEFAULT_ENV to dev. The next conditional for overwriting DEFAULT_ENV if PR_NUMBER is set could remain.

The TERMINUS_ENV declaration would pick up DEFAULT_ENV, which will be dev unless on a pull request.

greg-1-anderson commented 5 years ago

The implementation for that is nice, but the outcome is wrong, as the end result would be that the dev environment would be used for any branch that is not a PR. We should only use the dev environment for the master branch.

Another thing I just thought of: we need to make sure that the "merge" operation does nothing if TERMINUS_ENV is dev.

ataylorme commented 5 years ago

Okay, so something like this perhaps:

DEFAULT_BRANCH=${DEFAULT_BRANCH:-master}

if [[ ${CI_BRANCH} == ${DEFAULT_BRANCH} ]] ; then
    DEFAULT_ENV=${DEFAULT_ENV:-dev}
else
    DEFAULT_ENV=ci-$CI_BUILD_NUMBER
fi
ataylorme commented 5 years ago

Another thing I just thought of: we need to make sure that the "merge" operation does nothing if TERMINUS_ENV is dev.

I had to update 02-init-site-under-test-clone-existing to use

if [[ $TERMINUS_ENV == "dev" ]] ; then
    terminus -n build:env:push "$TERMINUS_SITE.dev"
else
    terminus -n build:env:create "$TERMINUS_SITE.dev" "$TERMINUS_ENV" --yes --clone-content
fi
greg-1-anderson commented 5 years ago

That change should be unnecessary, I would think; build:env:create should do just a push if the multidev already exists.

The part I am concerned about is 05-merge-master. Not sure if this does the right thing if the source is "dev".

greg-1-anderson commented 5 years ago

Looks like that is probably already handled.

ataylorme commented 5 years ago

That change should be unnecessary, I would think; build:env:create should do just a push if the multidev already exists.

What about the case when we are pushing to dev because the branch is master? Does build:env:create "$TERMINUS_SITE.dev" "$TERMINUS_ENV" translate to build:env:push when TERMINUS_ENV is dev?

greg-1-anderson commented 5 years ago

Yeah, I think so; I just reviewed the code and it looks like it should be okay. Did you encounter any problems when running it?

ataylorme commented 5 years ago

@greg-1-anderson just tested and build:env:create worked to push code to dev when TERMINUS_ENV is dev (build log).

Can you review this PR again? I am okay with making this a 5.x branch.

ataylorme commented 5 years ago

@greg-1-anderson so would you prefer:

1) This PR be tweaked to be compatible with 4.x (contents below) 1) Create a new 5.x branch from the current state of this PR 1) Both! Start 5.x and port back to 4.x

# If a default branch is defined and we are on the default branch.
if [[ -z ${DEFAULT_BRANCH} && ${CI_BRANCH} == ${DEFAULT_BRANCH} ]] ; then
  # Use dev as the environment.
    DEFAULT_ENV=${DEFAULT_ENV:-dev}
else
  # Otherwise, name the environment after the CI build number.
    DEFAULT_ENV=ci-$CI_BUILD_NUMBER
fi
ataylorme commented 5 years ago

I would prefer options 2 or 3 and have the example- repos default to always building using the dev environment when on the master branch.

greg-1-anderson commented 5 years ago

I think 2 is fine. If you want to make the latest example- repos use 5.x by default, then there are no users who need 1

ataylorme commented 5 years ago

5.x is up. PRs for the example-* repos incoming. After that, we can make 5.x the default branch here.