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: add option to discard plan or support multiple independent plans #118

Open
cludden opened this issue Apr 30, 2020 · 3 comments

Comments

@cludden
Copy link

cludden commented Apr 30, 2020

in addition to or instead of #117 . would like to support various PR flows that are currently difficult with the existing implementation, mostly around validation and/or plans that don't persist or overwrite other plans

@ljfranklin
Copy link
Owner

@cludden could you give a few more details about your PR workflow? Do you have a single environment with multiple PRs submitted against it?

@cludden
Copy link
Author

cludden commented May 8, 2020

@ljfranklin a common flow for us is to validate infra changes at PR time, and we'd like for these plans to not interfere with actual infra deployments, today we do this as a task, but there is a lot of overlap with this resource. quick example: if one engineer is iterating on larger infra changes, we don't want these plans applied the next time the resource is run, and my understanding of the current implementation is that they're can only be a single plan per workspace at any given moment

an alternative to a discard_plan option, would be the ability to assign a unique id to a plan, and specify that in the apply step.. im open to other ideas as well, I'm also happy to submit a PR if any of these suggestions sound reasonable to you

@ljfranklin
Copy link
Owner

current implementation is that they're can only be a single plan per workspace at any given moment

That is correct. The current plan implementation creates a new temporary workspace named $ORIGINAL_ENV-plan to hold the contents of the plan file. The original idea with using a fixed name instead of a random name is to avoid leaking plan files if a plan was created but never applied.

add option to discard plan

This makes me a little nervous long-term. The Concourse team has put out multiple proposals to refine the resource interface. Last I checked this proposal would require that check can retrieve metadata for a version produced by put. If there's no persisted planfile I don't see how the resource could satisfy that requirement in that flow.

would be the ability to assign a unique id to a plan

This sounds reasonable but we'd have to be careful about implementing it. The two main issues are deleting the unused planfiles and not breaking people on upgrade. I think the following implementation would work:

  • On plan put:
    • Create a plan workspace named $ORIGINAL_ENV-plan$UUID
    • Store plan_id field in version struct
  • On apply put:
    • if version contains plan_id, download $ORIGINAL_ENV-plan$UUID planfile
    • else download $ORIGINAL_ENV-plan
    • on success, delete all plan workspaces for that env
      • run terraform workspace list and use regex to look for -plan$UUID suffix

This implementation is a little more involved but does have the benefit of requiring no new pipeline config options and doesn't leak planfiles. Open to suggestions if you see a better solution. I'll mark this as open for a PR, appreciate the contribution if you have the time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants