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

[V2] Improve Change Set behaviour #520

Open
janrotter opened this issue Nov 25, 2018 · 15 comments
Open

[V2] Improve Change Set behaviour #520

janrotter opened this issue Nov 25, 2018 · 15 comments

Comments

@janrotter
Copy link
Contributor

Since all cli commands use SceptrePlan in sceptre 2.0 to modify the stacks, each time an action is taken on a stack, all stacks that it depends on take that action as well. This makes directly translating the sceptre 1.0 commands like: generate-template and update-stack to their 2.0 equivalents impossible.

Please consider the following example:

├── config
│   ├── config.yaml
│   ├── A
│   │   └── dummy.yaml
│   └── B
│       └── dummy.yaml

The stack B/dummy.yaml has dependencies: ["A/dummy.yaml"] in the stack configuration.

  • sceptre generate B/dummy.yaml returns dict_values([template_for_A/dummy.yaml, template_for_B/dummy.yaml])
  • sceptre launch B/dummy.yaml asks for confirmation of stack B launch:
    Do you want to launch 'B/dummy.yaml' [y/N]: y
    and begins to update the A:
    [2018-11-25 00:58:21] - sceptre.plan.actions - A/dummy - Stack is in the CREATE_COMPLETE state
    [2018-11-25 00:58:21] - sceptre.plan.actions - A/dummy - Updating Stack
    ...
    
  • sceptre update -c B/dummy.yaml doesn't work at all (because the status here is a dict with statuses for all relevant changesets):
    [2018-11-25 01:12:57] - sceptre.plan.actions - A/dummy - Successfully initiated creation of 
    Change Set 'change-set-3e653fe8f04f11e897d202adf7329286'
    [2018-11-25 01:12:58] - sceptre.plan.actions - B/dummy - Successfully initiated creation of 
    Change Set 'change-set-3e653fe8f04f11e897d202adf7329286'
    [2018-11-25 01:12:58] - sceptre.plan.actions - A/dummy - Successfully deleted Change Set 
    'change-set-3e653fe8f04f11e897d202adf7329286'
    [2018-11-25 01:12:58] - sceptre.plan.actions - B/dummy - Successfully deleted Change Set 
    'change-set-3e653fe8f04f11e897d202adf7329286'
    

The last example is particularly interesting. The stacks depend on each other, so generating the change sets for all of them at the beginning as it is implemented now is not correct (I assume we would be using the !stack_output resolver here).

Is that behavior intentional? I would rather like to be notified that there are some other stacks that need updating as well instead of being surprised with the scope of changes.

I would gladly submit a pull request for the change set update, but I am not sure what is the expected behavior after the cli redesign. I have implemented something that behaves more or less like this:

$ sceptre update -c B/dummy.yaml
[2018-11-25 01:36:31] - sceptre.plan.actions - A/dummy - Successfully initiated creation of Change Set 'change-set-892759faf05211e897d202adf7329286'
ChangeSetName: change-set-892759faf05211e897d202adf7329286
Changes:
...
CreationTime: 2018-11-25 01:39:21.894000+00:00
ExecutionStatus: AVAILABLE
StackName: sceptre-A-dummy
Status: CREATE_COMPLETE

Proceed with stack update of ['A/dummy']? [y/N]: y
[2018-11-25 01:36:41] - sceptre.plan.actions - 2018-11-25T01:39:31+00:00 A/dummy sceptre-A-dummy AWS::CloudFormation::Stack UPDATE_IN_PROGRESS User Initiated
[2018-11-25 01:36:50] - sceptre.plan.actions - 2018-11-25T01:39:36+00:00 A/dummy sceptre-A-dummy AWS::CloudFormation::Stack UPDATE_COMPLETE_CLEANUP_IN_PROGRESS 
[2018-11-25 01:36:50] - sceptre.plan.actions - 2018-11-25T01:39:37+00:00 A/dummy sceptre-A-dummy AWS::CloudFormation::Stack UPDATE_COMPLETE 
[2018-11-25 01:36:54] - sceptre.plan.actions - A/dummy - Successfully deleted Change Set 'change-set-892759faf05211e897d202adf7329286'
[2018-11-25 01:36:54] - sceptre.plan.actions - B/dummy - Successfully initiated creation of Change Set 'change-set-96c73288f05211e897d202adf7329286'

B/dummy                                            NO CHANGES

[2018-11-25 01:36:54] - sceptre.plan.actions - B/dummy - Successfully deleted Change Set 'change-set-96c73288f05211e897d202adf7329286'
$

Is that how it should work in 2.0 cli or do you have a different idea for that?

@ngfgrant
Copy link
Contributor

Hey @janrotter

By default we want to make sure that dependencies are considered when acting on a stack.

I can definitely see the case you’ve highlighted is an issue and I think what we will do is add something like a ‘-- no-dependencies’ flag, that can be applied to any command as an override so that you can act on a single stack.

I know for the ‘delete’ command we print all the stacks that will be deleted during the confirmation stage; I don’t see any issue adding this for other command so that they are no surprises.

Good suggestion. I’ll look to get a fix up for this on Monday and maybe if you have time once a branch is up you could help test it?

Cheers,
Niall

@ngfgrant
Copy link
Contributor

Hey @janrotter

Got ahead of myself and pushed a WIP branch add-ignore-dependencies-option to try and resolve this.

Single Stack/StackGroup commands should be possible on this branch if you do:

sceptre --ignore-dependencies ...

Could you just confirm this is the sort of behaviour you would be after/expect based on the comments above? If so, I will tidy things up a bit and update the docs and add some tests around this behaviour. Hopefully, get an RC3 at some point on Monday or early Tuesday.

I will deal with printing to the cli the stacks that will be operated on in a separate branch/issue.

@janrotter
Copy link
Contributor Author

That's exactly it! Excellent work! I guess you could use something like this test from my pull request to ensure, that the --ignore-dependencies flag is propagated correctly.

What do you think about the change set update proposal I described previously? Would you be willing to pull something like that into the v2? I'm not sure how to handle the stacks that haven't been launched yet, but one solution could be to launch empty stacks with the relevant names and then apply change-sets to them (so that you can easily see the list of resources to create). (Related issue: #65)

@ngfgrant
Copy link
Contributor

ngfgrant commented Nov 25, 2018

Thanks @janrotter - I will need to think about how best to deal with Change Sets. I like the idea in #65 whereby the user could be "stepped through" each change set. Although, with all the refactoring we have done we will be able to generate a full print out about what will happen ahead of time. In terms of updating whole StackGroup, I think this problem is probably solved with v2. Again not 100% best how to handle situation where Stack doesn't exist yet. Let me speak to a few folk tomorrow and I'll have a better idea about how we should implement.

Maybe the flow should be something like you suggested:

Print summary of Change Sets about to be executed

Ask user for confirmation [y/N]

for each change-set in StackGroup to update:
    - Attempt update stack with change-set 
    - If Stack does not exist ask user if they want to create Stack.
    - If yes: create-stack, apply empty change-set, if no move on. 

Print summary of Change Sets applied vs not applied/created.

I think safety is key, the user will want to be confident. I am happy to get the barebones of this working without having perfect output if it means we can get it into v2, due for release this upcoming week. We can then improve the output/messaging after v2 is out.

@janrotter
Copy link
Contributor Author

I would propose something like:

Resolve the dependencies in the plan
If there are any non-existing stacks in the plan:
    - Ask user if he wants to create them
    - If not: abort

For each batch in plan.launch_order:
    - If user wants to create non-existing stacks:
          create empty stacks with the proper names for non-existing stacks in the batch
    - create change sets for stacks in the batch and wait for completion
    - remove all stacks with empty change sets from the batch
    - print summaries for change sets of stacks in the batch
    - ask user for confirmation of the bulk apply
    - if yes: apply all change sets in the batch
    - in case of errors, abort
    finally: remove all changesets

Print summary of Change Sets applied vs not applied/created.

If there are multiple batches [A,B,C], you can't tell if the changes in A affect the change sets for B (e.g. stacks from B may rely on outputs from stacks in A), so you can't compute the change sets for the whole plan in advance. That's why you need to create change sets and ask for confirmation before each batch.

Launching empty stacks and applying the change sets would make the output consistent for the user - you will see exactly what will be created/updated/removed for both existing and non-existing stacks.

@ngfgrant
Copy link
Contributor

Thanks for the suggestion - will be sure to include this in the discussion.

Just as an FYI more than anything else, what are Plan and Graph do is create batches where by each batch is executed in serial but each stack in the batch are executed concurrently. We can guarantee that by the time the launch_order is generated that there no stacks that will be generated without first having all of its dependencies resolved. So in the case you mentioned above, if we have a batch set(A,B,C) we can guarantee that none of the stacks in that batch depend on one another.

Take the case where the Stack dependencies are as follows (-> = depends on):

H -> D
D -> A
E -> A
F -> B & C
G -> A & C

A, B, C have no dependencies. 

The StackGraph is generated with edges that represent a "depends on" relationship and then generate_launch_order() reads the graph and converts this into the following:

launch order = [
    set(A,B,C)
    set(D,E,F,G)
    set{H}
]

In the above example, Sceptre has understood that set(A,B,C) need to be launched first but can be launched in any order, each stack needs to resolve before set(D,E,F,G) can start.

If I understand your suggestion correctly then it might make things a bit simpler?

@janrotter
Copy link
Contributor Author

janrotter commented Nov 26, 2018

I'd like to clarify what would be the expected behavior.

Let's consider the sample sceptre v2 project:
https://github.com/janrotter/sceptre_devops_meetup/tree/master/v2/1_dependencies
The stacks there just rewrite the Nonce parameter from the input to the output. Their dependencies are as follows (-> = depends on):

C/dummy.yaml -> B/dummy.yaml -> A/dummy.yaml
B/dummyB.yaml -> A/dummyB.yaml

After the launch (sceptre launch /), all dummy.yaml stacks have the output Nonce = A:

$ sceptre list outputs C
- - OutputKey: Nonce
    OutputValue: A
- - OutputKey: Nonce
    OutputValue: A
- - OutputKey: Nonce
    OutputValue: A

Now we update the stack A/dummy.yaml parameter:

$ sed -i 's/A/Amodified/' config/A/dummy.yaml

and update the stack group (not working correctly at the moment as the test doesn't expect a dict with stacks as keys):

$ sceptre update -c /
...

My expected output values would be now:

$ sceptre list outputs C
- - OutputKey: Nonce
    OutputValue: Amodified
- - OutputKey: Nonce
    OutputValue: Amodified
- - OutputKey: Nonce
    OutputValue: Amodified

I'm afraid that the current implementation would end up with the following results as initially only the first change set is non-empty:

$ sceptre list outputs C
- - OutputKey: Nonce
    OutputValue: Amodified
- - OutputKey: Nonce
    OutputValue: A
- - OutputKey: Nonce
    OutputValue: A

The current implementation executes the change-sets in the correct order, but the problem is that the change sets are created before the parameters can be properly resolved.

Creating the change sets separately right before each batch (set) in the launch order is executed should fix that.

@nbjcn1
Copy link

nbjcn1 commented Nov 26, 2018

Create changesets right before each batch set, reduces the value in change sets though.

I think the ideal UX depends on the workflow
For update -c "Create changesets right before each batch set" makes the most sense for now, but it should probably end up being the below as using -c signifies you want to validate the changes prior to execution.

For create-change-sets, describe-change-sets, execute-change-sets:

  • on execute-change-sets, prior to each change set execution (for stacks that have dependencies) create a new change-set (--no-check can skip)
    • if new-change-set == change-set-to-execute silently execute change-set & delete new-change-set
    • if new-change-set != change-set-to-execute prompt for user input (-y can skip )

note that this would mean, if you run update -c it could prompt you multiple times, which isn't ideal but is the only way to be sure about what you are changing.

  1. update -c
  2. generates plain change sets
  3. describes full expected changes
  4. executes parent stack change sets
  5. prompts for dependency change sets if they are different
  6. executes dependency change sets
  7. potentially go to 5

Thinking about it, without explicit prompting, there is always a change of getting stuck in a loop (and potentially one scepter can't be aware of if ssm parameters are used)

Given that you can do nice stuff with hooks and change sets (e.g backup your current template and create a roll-back after the deployment (and cascading these if needed)) I hope the above workflows make it into the 2.x release, but guess they aren't needed for 2.0 as 1.0 doesn't have that functionality.

@ngfgrant ngfgrant changed the title [RC2] Single stack CLI operations are not possible [RC2] Fix Change Set behaviour Nov 26, 2018
@janrotter
Copy link
Contributor Author

After a short discussion with my coworker, we agreed that actually the launching of non existing stacks could be not required for the update -c command. It should simply fail and ask the user to launch the stack manually. The update or create if not exists behavior could be implemented in launch -c.

Assuming that the above is true, I have prepared a working proof of concept for the update with change-sets for multiple stacks:
janrotter@a3033ff

If you think that the general approach there is correct, I'd be happy to improve the implementation and submit a PR.

@ngfgrant
Copy link
Contributor

Hey @janrotter

Yeh launch has always been used to create or update. Where are update/create were more aimed at individual stacks. I think I would still like to propose a nice way, similar to something you suggested to handle change-sets. It is a best practice to use change-sets when updating CloudFormation and I think it would be useful to have Sceptre automatically manage Change Sets for the user.

After speaking to some colleagues somethings came up:

If we use Change Sets by default it would also give us a nice output to present to the user too?

@janrotter
Copy link
Contributor Author

I didn't know that you can create a change set for a non existing stack. That's great news!

The change set name is already generated for the update -c.

I don't know what the error handling should be for the change set update. I assumed it should just finish the current batch, report the result to the user and abort next batches. Then the user should resolve it (maybe by updating the stacks one by one with --no-dependencies). Do you have any certain scenarios in mind?

I think that the major use cases for multi-stack change set update/launch is to:

  • deploy / update all the stacks safely (with no surprises, knowing if the resources can be modified in place or have to be recreated)
  • actually figure out what is the difference between the state in the AWS and the one described by sceptre project without changing anything (I use the v1 update-stack-cs like that quite often)

What do you think about extracting the second one as a sceptre status command? One could even draw a graph of stacks with dependencies and use colors to highlight non existing, in-sync and out-of-sync ones (see http://lisperati.com/vijual/). For now a simple list of the above states would be great :)

@ngfgrant
Copy link
Contributor

Yeh just needs to be clear what the behaviour will be for certain.

I think for the latter point we will add a feature to support Drift Detection - which is a recent addition to the CloudFormation offering. Won’t be until after 2.0 release. That will be the most robust way to see what the diff is between local and actual.

@ngfgrant
Copy link
Contributor

Proposal

From the documentation the below image shows how AWS sees the interaction with Change Sets. I don't think Sceptre should stray from this flow.

update-stack-changesets-diagram

We have currently have 6 CLI command groups that handle Change Sets.

sceptre/cli/create.py
sceptre/cli/update.py
sceptre/cli/execute.py
sceptre/cli/delete.py
sceptre/cli/describe.py
sceptre/cli/list.py

I think we then have the following user use cases:

  1. Create a Change Set for a single Stack
  2. Create a Change Set for a StackGroup
  3. Update an individual Stack using a Change Set
  4. Update a StackGroup using a Change Set(s)
  5. Execute a Change Set for a single Stack
  6. Execute a Change Set for a StackGroup
  7. Delete a single Stack using a Change Set
  8. Delete a StackGroup using a Change Set.
  9. Describe a Change Set for a single Stack
  10. Describe a Change Set for a StackGroup
  11. List a a Change Set for a single Stack
  12. List a Change Set for a Group of Stacks.

As it stands, with the ongoing work for --ignore_dependencies (#523) I think each of these use cases can be met given the right combination of flags and the appropriate command. Each of the existing actions for the CLI command groups listed above will remain so the user can chose to use a combination of them or just a single command like execute.

My understanding is that there is appetite for a final use-case which is more of a guided/step-through process that merges a few of these commands together and produces a nicer output along the way. I believe that if we make this part seamless enough we can use Change Sets behind the scenes by default, without the user necessarily being aware/caring, thus enabling our users to easily follow an AWS best practice without thinking.

The above vision of a Change Set world by default might take a bit of time to get right.

A pseudocode example where we don't utilise AWS change-set-type parameter:

# ... command has been submitted, SceptreContext and Plan is created....

# First figure out what needs created

change_set_name = # generated or user specified
to_change = set()
to_create = set()

for each stack in plan.command_stacks:
    if stack_exists:
        to_change.add(stack_path)
    else:
        to_create(stack_path)

write_to_cli("You are about to create the following stacks: {}".format(launch_order))

# User input: y/N - continue if yes abort if no.

write_to_cli("Creating Stacks")

# Create the Stacks that don't exist

for stack_path in to_create:
    result = plan.create(stack_path)
    # log will capture and print out put of create() as it goes
    if result == SUCCESS:
        to_create.remove(stack_path)
        to_change.add(stack_path)
    elif result == FAILED:
        write_to_cli("{} failed to create".format(stack_path))
    # Probably need way to re-try of get user input to retry/abort

# Now create Change Sets 

write_to_cli("You are about to create the following Change Sets: {}".format(to_change))

# User input: y/N - continue if yes abort if no.

write_to_cli("Creating Change Sets")

while to_change:
    stack = to_change.pop()
    plan.create_change_set(change_set_name) 
    # plan.create_change_set will print description of Change Set from AWS

write_to_cli("The following changes will be applied:")

for stack in plan.command_stacks:
    plan.describe_change_set(change_set_name)
    # plan.describe_change_set will print description of Change Set from AWS

# User input- Do you want to execute these Change Sets: y/N 

succeeded = {}
failed = {}
for stack in plan.command_stacks:
     result = plan.execute_change_set(change_set_name)
     result == SUCCESS:
         succeeded.append(stack: result)
     result == FAILED:
         failed.append(stack: result)

# print summary of succeeded and failed results

END

There is probably a simpler solution to this but it shows the process of joining up the dots with the existing command into an existing flow. I could see this being useful as the default behaviour for something like update or create?

Thoughts?

@ngfgrant ngfgrant changed the title [RC2] Fix Change Set behaviour [RC2] Improve Change Set behaviour Nov 27, 2018
@ngfgrant ngfgrant mentioned this issue Dec 8, 2018
@ngfgrant ngfgrant changed the title [RC2] Improve Change Set behaviour [V2] Improve Change Set behaviour Jan 8, 2019
michaetto referenced this issue in michaetto/sceptre Apr 5, 2019
michaetto referenced this issue in michaetto/sceptre Apr 5, 2019
@jfhamlin
Copy link

jfhamlin commented Apr 8, 2019

Hi folks, I'm curious about the status of this work. I'm working on moving from a self-maintained tool to sceptre, and change set support is one of the big missing features. I'd be happy to contribute if there's a clear plan of record. There are a few points of confusion for me right now (not all related to change sets, but good change set support would resolve all):

  1. sceptre update -c ... simply creates and then deletes change sets. Change set details aren't shown, nor are they executed.
  2. sceptre update won't update a stack with changes if its dependencies don't have changes. sceptre launch will. Is this intentional? To be honest I'm a bit confused by the number of commands — the intended behaviors of create, update, and launch don't seem sufficiently documented, and it's not clear why more than a single command for stack changes would be needed. Don't we just want to bring CFN state up-to-date with local state?
  3. None of these operations show a diff before execution. If I had a dollar for every time inspecting a diff before update has saved me from disaster... Change sets or no, diffs (of the stack templates and parameters) seem to be a fundamental feature, and I'd advocate including them in the roadmap. In my experience, the information in the change set isn't sufficient.

I realize not all of these comments and questions are specific to change set support. I'm happy to open new issues for each if I'm not misunderstanding anything.

@ngfgrant
Copy link
Contributor

@jfhamlin I kno we are discussing the points above in more detail in #683 but just wanted to clarify for any future readers that sceptre has change-set support. This issue is just whether/how we should enable change-sets by default for the user.

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

4 participants