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

Add `dotenv` command #50

Merged
merged 2 commits into from Aug 12, 2019
Merged

Add `dotenv` command #50

merged 2 commits into from Aug 12, 2019

Conversation

@TangRufus
Copy link
Collaborator

TangRufus commented Aug 3, 2019

Ported from https://github.com/ItinerisLtd/enveigle/tree/8f1c803b7125694516e9de15c95874cf80b99917

Related: #7 (comment)

FileIO and anisble playbook (dotenv.yml) are not tested. Help wanted!

CircleCI is not building. Please check https://discuss.circleci.com/t/pull-requests-not-triggering-build/1213/5

@TangRufus TangRufus closed this Aug 3, 2019
@TangRufus TangRufus reopened this Aug 3, 2019

switch len(args) {
case 0:
environment = "development"

This comment has been minimized.

Copy link
@swalkinshaw

swalkinshaw Aug 6, 2019

Member

Oh good idea. There's probably other commands where this can be done too 👍

runErr := dotEnv.Run()

// Remove playbook file no matter command success or not
removeFileErr := os.Remove(playbookPath)

This comment has been minimized.

Copy link
@swalkinshaw

swalkinshaw Aug 6, 2019

Member

I think we'd want this to be done via defer for more guarantees:

defer deletePlaybook(playbookPath) # function can do error handling

This comment has been minimized.

Copy link
@TangRufus

TangRufus Aug 7, 2019

Author Collaborator

done.


// runErr gets priority because it is more important than removeFileErr
if runErr != nil {
log.Fatal(runErr)

This comment has been minimized.

Copy link
@swalkinshaw

swalkinshaw Aug 6, 2019

Member

The other log.Fatals are probably fine to stay since they should be uncommon, but this one should use c.UI.Error I think

This comment has been minimized.

Copy link
@TangRufus

TangRufus Aug 7, 2019

Author Collaborator

done.
Also added return 1 when ansible-playbook fails

Note the command output and the actual exit status:

$ trellis dotenv
// ...
failed: [192.168.50.5] (item=insecure.com) => {"changed": false, "item": {"key": "insecure.com", "value": {"local_path": "../bedrock-insecure", "site_hosts": [{"canonical": "insecure.test"}], "ssl": {"enabled": false}}}}

PLAY RECAP *********************************************************************
192.168.50.5               : ok=0    changed=0    unreachable=0    failed=1

Error running ansible-playbook: exit status 2

$ echo $?
1
@TangRufus TangRufus force-pushed the TangRufus:dotenv branch from 11d199c to 5f8f2e4 Aug 7, 2019
@swalkinshaw

This comment has been minimized.

Copy link
Member

swalkinshaw commented Aug 7, 2019

I'm confused about the CI issue since there was a CI run and it passed?

I'm going to test this out later but PR looks good to me.

@TangRufus

This comment has been minimized.

Copy link
Collaborator Author

TangRufus commented Aug 7, 2019

Weird, my first commit didn't triggered CircleCI but the last 2 did.
Ignore me.

@TangRufus

This comment has been minimized.

Copy link
Collaborator Author

TangRufus commented Aug 8, 2019

Added integration tests.

@TangRufus TangRufus force-pushed the TangRufus:dotenv branch from 9960dde to 9a67c75 Aug 8, 2019
@swalkinshaw

This comment has been minimized.

Copy link
Member

swalkinshaw commented Aug 9, 2019

That's awesome. My only complaint is having the submodules by default. What if part of CI (and manual instructions) just said to clone them? Maybe I'm concerned over nothing though

@TangRufus

This comment has been minimized.

Copy link
Collaborator Author

TangRufus commented Aug 10, 2019

Using docker to run integration tests instead of git submodule.

Todo:

  • Push rootsdev/trellis-cli-dev to docker hub
  • Change docker image in .circleci/config.yml
@TangRufus TangRufus force-pushed the TangRufus:dotenv branch from 9152b7a to ef669fc Aug 10, 2019
TangRufus added 2 commits Aug 3, 2019
@TangRufus TangRufus force-pushed the TangRufus:dotenv branch from 74a3e92 to b5856b3 Aug 12, 2019
@TangRufus TangRufus requested a review from swalkinshaw Aug 12, 2019
apt-get install -q -y --no-install-recommends ansible && \
apt-get clean && apt-get -y autoremove && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

# Trellis

This comment has been minimized.

Copy link
@swalkinshaw

swalkinshaw Aug 12, 2019

Member

Maybe in the future we could use trellis-cli itself to create a new project which takes care of this (and would test trellis-cli itself)!

Copy link
Member

swalkinshaw left a comment

👏 thanks for the integration tests

@swalkinshaw swalkinshaw merged commit 6a44fe5 into roots:master Aug 12, 2019
2 checks passed
2 checks passed
ci/circleci: integration-test Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
TangRufus added a commit to ItinerisLtd/enveigle that referenced this pull request Aug 12, 2019
@TangRufus TangRufus mentioned this pull request Aug 12, 2019
TangRufus added a commit to ItinerisLtd/enveigle that referenced this pull request Aug 12, 2019
TangRufus added a commit to ItinerisLtd/enveigle that referenced this pull request Aug 12, 2019
@TangRufus TangRufus deleted the TangRufus:dotenv branch Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.