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

feature request: apply on merge #162

Closed
robertlabrie opened this issue Oct 19, 2017 · 13 comments
Closed

feature request: apply on merge #162

robertlabrie opened this issue Oct 19, 2017 · 13 comments

Comments

@robertlabrie
Copy link

Would be great if merging a PR triggered a terraform apply

@robertlabrie
Copy link
Author

piling on my own request, plan on push would be dandy as well :)

@lkysow
Copy link
Collaborator

lkysow commented Oct 23, 2017

Thanks for the ticket! The main reason we do apply on comment is that often there are errors when you run apply. If this happens then it's easier to push a change to an open pull request and run plan/apply again.

That being said, I don't think there's a reason we couldn't also support a final apply on merge for any plans that are outstanding. What do you think @anubhavmishra?

For plan on push, we definitely agree this is needed. I created #165 to track this separately.

@robertlabrie
Copy link
Author

Hi @lkysow ... hrm, you're right, there are often errors when you apply. Can a closed PR still be updated by Atlantis? That final run should still be pushed back to the PR I think so there is some record of it.

I guess for us, there are some projects that are trivial, or some changes that are trivial and unlikely to fail, and it'd be nice to "one-step" it.

Maybe make it configurable per-project in the atlantis.yaml file?

We've built a workflow in chef and kubernetes around pushing merge being the act of actually deploying the work, maybe that doen't fit for terraform.

@lkysow
Copy link
Collaborator

lkysow commented Oct 24, 2017

We've built a workflow in chef and kubernetes around pushing merge being the act of actually deploying the work, maybe that doesn't fit for terraform.

I love that workflow and I think it totally fits for terraform. We just need to figure out the workflow if an apply fails. We can basically treat it as a broken build I think.

Here's what we could do:

  • PR is merged
  • we run apply for all "workspaces" that had a plan in the pull request
  • we comment back on the pull request with the apply output
  • if apply fails, we hold the lock on the workspaces and pull request and allow users to run plan/apply again on the closed pull
  • if they need to open a new pull request to fix any issues, they can discard the lock and do that too

Questions I have:

  • will it be confusing for users if we auto-run apply for them on merge? It's a potentially disruptive command so we'd need to make sure everyone knew that this was going to happen
  • should it be configurable in atlantis.yaml? Personally I don't think so because I would like most teams to never need an atlantis.yaml.
  • we'd need a way to discard a plan that you didn't want apply'd on merge
  • with auto-planning this could be super magical but we need to think carefully about the workflow so it works as users expect and isn't surprising

@anubhavmishra thoughts?

@robertlabrie
Copy link
Author

FWIW seems sane to me. We're already used to "Merge pull request" being 'dangerous' on some repos (ie: your work will go live right now).

If you really want to get ambitious, you could fire a webhook at a user configurable URL in addition to GH so I could intercept and publish updates to Cisco Spark (our collaborative chat tool) -- but that's becoming feature creep

@anubhavmishra
Copy link
Collaborator

As an operator I like the idea of planning for a environment and applying that to the environment and then seeing a result back in the same place. But I like the idea of apply on merge to master. If you have planned for multiple environments in the PR we will need to figure out a way to track each one of things that needs to applied and handle errors after the fact.

@majormoses
Copy link

Can a closed PR still be updated by Atlantis?

Well it depends, if you force push (after squashing,rebasing, etc) no you will need a new PR.

@majormoses
Copy link

I like the concept of apply on merge but I don't think it is practical because bugs exist in code and are often break on the simplist of things (like adding a new key to an array in json and not putting the comma). I think you should apply it and if it succeeds you merge it, until it succeeds it should stay in a feature branch imho. I realize that as I also follow the mantra of master is the source of truth and deployable anywhere is guiding me here. The nature of terraform sorta conflicts with this mantra to a certain extent but is less of a concern if you use remote state files.

@grobie
Copy link

grobie commented Feb 14, 2018

How would you deal with multiple PRs merged at the same time?

@lkysow
Copy link
Collaborator

lkysow commented Feb 14, 2018

@grobie atlantis will "lock" a directory and workspace to prevent other plans from other PR's when there's an unapplied plan pending so you wouldn't have the issue of two PR's attempting to be planned.

You are correct that there's still an issue however. If there's a pending plan (in say PR 10) and I open up another pull request (say PR 11), then we won't be able to autoplan in 11. If 10 gets merged, then the user would have to comment on the pull request to get the plan to appear.

I'm thinking about using the following config in atlantis.yaml to toggle this behaviour:

...
pipelines:
  ...
  apply:
    on_merge: true # will apply any unapplied plans on merge

@grobie
Copy link

grobie commented Feb 15, 2018

The lock wouldn't prevent some overzealous engineer to merge their PR, right? I'm currently playing around with Github's merge protection features etc. to figure out how to set up enough safe guards to prevent that (might be useful to document that in Atlantis somehwere).

@lkysow
Copy link
Collaborator

lkysow commented Feb 15, 2018

Correct. We could have the plan build status in GitHub be pending until there's a successful apply which then you could block on with the merge protection features.

@majormoses
Copy link

One problem here is that the status checks run one time unless there is some change such as a comment or commit triggers it to run again. This is not a reliable mechanism to check lock status. There will always be race conditions.

@hootsuite hootsuite locked and limited conversation to collaborators Mar 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants